* [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
@ 2019-08-30 2:28 Peng Fan
2019-08-30 8:39 ` Julien Grall
0 siblings, 1 reply; 8+ messages in thread
From: Peng Fan @ 2019-08-30 2:28 UTC (permalink / raw)
To: sstabellini@kernel•org, linux@armlinux•org.uk,
catalin.marinas@arm•com, will@kernel•org, robin.murphy@arm•com
Cc: xen-devel@lists•xenproject.org, Peng Fan, dl-linux-imx,
linux-arm-kernel@lists•infradead.org
From: Peng Fan <peng.fan@nxp•com>
arm64 shares some code under arch/arm/xen, including mm.c.
However ZONE_DMA is removed by commit
ad67f5a6545("arm64: replace ZONE_DMA with ZONE_DMA32").
So introduce xen_set_gfp_dma for arm32/arm64 and using __GFP_DMA
for the former and __GFP_DMA32 for the latter.
Signed-off-by: Peng Fan <peng.fan@nxp•com>
---
V2:
Follow suggestion from Stefano,
introduce static inline void xen_set_gfp_dma(gfp_t *flags) for arm32/arm64, and
for arm64 using __GFP_DMA for the former and __GFP_DMA32 for the latter.
arch/arm/include/asm/xen/page.h | 5 +++++
arch/arm/xen/mm.c | 2 +-
arch/arm64/include/asm/xen/page.h | 5 +++++
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
index 31bbc803cecb..d08309c45e6c 100644
--- a/arch/arm/include/asm/xen/page.h
+++ b/arch/arm/include/asm/xen/page.h
@@ -1 +1,6 @@
#include <xen/arm/page.h>
+
+static inline void xen_set_gfp_dma(gfp_t *flags)
+{
+ *flags |= __GFP_DMA;
+}
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index d33b77e9add3..828f49dc95f9 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -28,7 +28,7 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int order)
for_each_memblock(memory, reg) {
if (reg->base < (phys_addr_t)0xffffffff) {
- flags |= __GFP_DMA;
+ xen_set_gfp_dma(&flags);
break;
}
}
diff --git a/arch/arm64/include/asm/xen/page.h b/arch/arm64/include/asm/xen/page.h
index 31bbc803cecb..5eeabf2c6494 100644
--- a/arch/arm64/include/asm/xen/page.h
+++ b/arch/arm64/include/asm/xen/page.h
@@ -1 +1,6 @@
#include <xen/arm/page.h>
+
+static inline void xen_set_gfp_dma(gfp_t *flags)
+{
+ *flags |= __GFP_DMA32;
+}
--
2.16.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
2019-08-30 2:28 [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64 Peng Fan
@ 2019-08-30 8:39 ` Julien Grall
2019-08-30 8:58 ` Christoph Hellwig
0 siblings, 1 reply; 8+ messages in thread
From: Julien Grall @ 2019-08-30 8:39 UTC (permalink / raw)
To: Peng Fan, sstabellini@kernel•org, linux@armlinux•org.uk,
Catalin Marinas, will@kernel•org, Robin Murphy
Cc: xen-devel@lists•xenproject.org, nd, dl-linux-imx,
linux-arm-kernel@lists•infradead.org
Hi Peng,
On 30/08/2019 04:28, Peng Fan wrote:
> From: Peng Fan <peng.fan@nxp•com>
>
> arm64 shares some code under arch/arm/xen, including mm.c.
> However ZONE_DMA is removed by commit
> ad67f5a6545("arm64: replace ZONE_DMA with ZONE_DMA32").
> So introduce xen_set_gfp_dma for arm32/arm64 and using __GFP_DMA
> for the former and __GFP_DMA32 for the latter.
>
> Signed-off-by: Peng Fan <peng.fan@nxp•com>
> ---
>
> V2:
> Follow suggestion from Stefano,
> introduce static inline void xen_set_gfp_dma(gfp_t *flags) for arm32/arm64, and
> for arm64 using __GFP_DMA for the former and __GFP_DMA32 for the latter.
>
> arch/arm/include/asm/xen/page.h | 5 +++++
> arch/arm/xen/mm.c | 2 +-
> arch/arm64/include/asm/xen/page.h | 5 +++++
> 3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
> index 31bbc803cecb..d08309c45e6c 100644
> --- a/arch/arm/include/asm/xen/page.h
> +++ b/arch/arm/include/asm/xen/page.h
> @@ -1 +1,6 @@
> #include <xen/arm/page.h>
> +
> +static inline void xen_set_gfp_dma(gfp_t *flags)
> +{
> + *flags |= __GFP_DMA;
> +}
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index d33b77e9add3..828f49dc95f9 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -28,7 +28,7 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int order)
>
> for_each_memblock(memory, reg) {
> if (reg->base < (phys_addr_t)0xffffffff) {
> - flags |= __GFP_DMA;
> + xen_set_gfp_dma(&flags);
The name of the helper is quite misleading, this is specific to swiotlb
yet it gives the impression it can be used everywhere. The helper will
actually set the flags in order to allocate memory below 4GB.
Also, I saw an e-mail suggesting that __GFP_DMA may be used on Arm64. So
a user may think using xen_set_gfp_dma() will set _GFP_DMA and not
_GFP_DMA32.
I know duplication is not great but it feels that duplicating the full
function (or only the allocation part) would be the best. This would
require to introduce a new file mm{32,64}.c in the respective arch
directory.
Cheers,
--
Julien Grall
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
2019-08-30 8:39 ` Julien Grall
@ 2019-08-30 8:58 ` Christoph Hellwig
2019-08-31 2:40 ` Stefano Stabellini
0 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2019-08-30 8:58 UTC (permalink / raw)
To: Julien Grall
Cc: Peng Fan, sstabellini@kernel•org, Catalin Marinas,
linux@armlinux•org.uk, dl-linux-imx,
xen-devel@lists•xenproject.org, nd, will@kernel•org,
linux-arm-kernel@lists•infradead.org, Robin Murphy
Can we take a step back and figure out what we want to do here?
AFAICS this function allocates memory for the swiotlb-xen buffer,
and that means it must be <= 32-bit addressable to satisfy the DMA API
guarantees. That means we generally want to use GFP_DMA32 everywhere
that exists, but on systems with odd zones we might want to dip into
GFP_DMA. This also means swiotlb-xen doesn't actually do the right
thing on x86 at the moment. So shouldn't we just have one common
routine in swiotlb-xen.c that checks if we have CONFIG_ZONE_DMA32
set, then try GFP_DMA32, and if not check if CONFIG_ZONE_DMA is set
and then try that, else default to GFP_KERNEL?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
2019-08-30 8:58 ` Christoph Hellwig
@ 2019-08-31 2:40 ` Stefano Stabellini
2019-09-02 15:57 ` Christoph Hellwig
2019-09-11 2:06 ` Peng Fan
0 siblings, 2 replies; 8+ messages in thread
From: Stefano Stabellini @ 2019-08-31 2:40 UTC (permalink / raw)
To: Christoph Hellwig
Cc: jgross, Peng Fan, sstabellini@kernel•org, Catalin Marinas,
linux@armlinux•org.uk, Julien Grall, dl-linux-imx,
xen-devel@lists•xenproject.org, boris.ostrovsky, nd,
will@kernel•org, linux-arm-kernel@lists•infradead.org,
Robin Murphy
+ Juergen, Boris
On Fri, 30 Aug 2019, Christoph Hellwig wrote:
> Can we take a step back and figure out what we want to do here?
>
> AFAICS this function allocates memory for the swiotlb-xen buffer,
> and that means it must be <= 32-bit addressable to satisfy the DMA API
> guarantees. That means we generally want to use GFP_DMA32 everywhere
> that exists, but on systems with odd zones we might want to dip into
> GFP_DMA. This also means swiotlb-xen doesn't actually do the right
> thing on x86 at the moment. So shouldn't we just have one common
> routine in swiotlb-xen.c that checks if we have CONFIG_ZONE_DMA32
> set, then try GFP_DMA32, and if not check if CONFIG_ZONE_DMA is set
> and then try that, else default to GFP_KERNEL?
Yes, for ARM/ARM64 it makes a lot of sense given that dom0 is 1:1 mapped
(pseudo-physical == physical). I'll let Juergen and Boris comment on
the x86 side of things, but on x86 PV Dom0 is not 1:1 mapped so
GFP_DMA32 is probably not meaningful.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
2019-08-31 2:40 ` Stefano Stabellini
@ 2019-09-02 15:57 ` Christoph Hellwig
2019-09-03 11:48 ` Juergen Gross
2019-09-11 2:06 ` Peng Fan
1 sibling, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2019-09-02 15:57 UTC (permalink / raw)
To: Stefano Stabellini
Cc: jgross, Peng Fan, Catalin Marinas, linux@armlinux•org.uk,
Christoph Hellwig, Julien Grall, dl-linux-imx,
xen-devel@lists•xenproject.org, boris.ostrovsky, nd,
will@kernel•org, linux-arm-kernel@lists•infradead.org,
Robin Murphy
On Fri, Aug 30, 2019 at 07:40:42PM -0700, Stefano Stabellini wrote:
> + Juergen, Boris
>
> On Fri, 30 Aug 2019, Christoph Hellwig wrote:
> > Can we take a step back and figure out what we want to do here?
> >
> > AFAICS this function allocates memory for the swiotlb-xen buffer,
> > and that means it must be <= 32-bit addressable to satisfy the DMA API
> > guarantees. That means we generally want to use GFP_DMA32 everywhere
> > that exists, but on systems with odd zones we might want to dip into
> > GFP_DMA. This also means swiotlb-xen doesn't actually do the right
> > thing on x86 at the moment. So shouldn't we just have one common
> > routine in swiotlb-xen.c that checks if we have CONFIG_ZONE_DMA32
> > set, then try GFP_DMA32, and if not check if CONFIG_ZONE_DMA is set
> > and then try that, else default to GFP_KERNEL?
>
> Yes, for ARM/ARM64 it makes a lot of sense given that dom0 is 1:1 mapped
> (pseudo-physical == physical). I'll let Juergen and Boris comment on
> the x86 side of things, but on x86 PV Dom0 is not 1:1 mapped so
> GFP_DMA32 is probably not meaningful.
But is it actually harmful? If the GFP_DMA32 doesn't hurt we could
just use it there. Or if that seems to ugly we can make the dma
flags dependents on a XEN_1TO1_MAPPED config option set by arm/arm64.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
2019-09-02 15:57 ` Christoph Hellwig
@ 2019-09-03 11:48 ` Juergen Gross
0 siblings, 0 replies; 8+ messages in thread
From: Juergen Gross @ 2019-09-03 11:48 UTC (permalink / raw)
To: Christoph Hellwig, Stefano Stabellini
Cc: Peng Fan, will@kernel•org, Catalin Marinas, linux@armlinux•org.uk,
Julien Grall, dl-linux-imx, xen-devel@lists•xenproject.org,
boris.ostrovsky, nd, Robin Murphy,
linux-arm-kernel@lists•infradead.org
On 02.09.19 17:57, Christoph Hellwig wrote:
> On Fri, Aug 30, 2019 at 07:40:42PM -0700, Stefano Stabellini wrote:
>> + Juergen, Boris
>>
>> On Fri, 30 Aug 2019, Christoph Hellwig wrote:
>>> Can we take a step back and figure out what we want to do here?
>>>
>>> AFAICS this function allocates memory for the swiotlb-xen buffer,
>>> and that means it must be <= 32-bit addressable to satisfy the DMA API
>>> guarantees. That means we generally want to use GFP_DMA32 everywhere
>>> that exists, but on systems with odd zones we might want to dip into
>>> GFP_DMA. This also means swiotlb-xen doesn't actually do the right
>>> thing on x86 at the moment. So shouldn't we just have one common
>>> routine in swiotlb-xen.c that checks if we have CONFIG_ZONE_DMA32
>>> set, then try GFP_DMA32, and if not check if CONFIG_ZONE_DMA is set
>>> and then try that, else default to GFP_KERNEL?
>>
>> Yes, for ARM/ARM64 it makes a lot of sense given that dom0 is 1:1 mapped
>> (pseudo-physical == physical). I'll let Juergen and Boris comment on
>> the x86 side of things, but on x86 PV Dom0 is not 1:1 mapped so
>> GFP_DMA32 is probably not meaningful.
>
> But is it actually harmful? If the GFP_DMA32 doesn't hurt we could
> just use it there. Or if that seems to ugly we can make the dma
> flags dependents on a XEN_1TO1_MAPPED config option set by arm/arm64.
>
I'd rather have it only if needed. Especially on X86 PV dom0 I'd like to
avoid GFP_DMA32 as memory below 4GB (guest physical) might be rather
scarce in some configurations.
I think X86 PVH dom0 should need GFP_DMA32, too, as the limit is related
to the address as communicated to the device (before being translated by
the IOMMU), right? This would mean on a X86 kernel configured to support
PV and PVH the test for setting GFP_DMA32 can't depend on a config
option alone, it needs to be dynamic.
BTW, for PV guests the DMA address width is handled via
xen_create_contiguous_region().
Juergen
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
2019-08-31 2:40 ` Stefano Stabellini
2019-09-02 15:57 ` Christoph Hellwig
@ 2019-09-11 2:06 ` Peng Fan
2019-09-11 19:12 ` Stefano Stabellini
1 sibling, 1 reply; 8+ messages in thread
From: Peng Fan @ 2019-09-11 2:06 UTC (permalink / raw)
To: Stefano Stabellini, Christoph Hellwig
Cc: jgross@suse•com, Catalin Marinas, linux@armlinux•org.uk,
Julien Grall, dl-linux-imx, xen-devel@lists•xenproject.org,
boris.ostrovsky@oracle•com, nd, will@kernel•org,
linux-arm-kernel@lists•infradead.org, Robin Murphy
> Subject: Re: [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
>
> + Juergen, Boris
>
> On Fri, 30 Aug 2019, Christoph Hellwig wrote:
> > Can we take a step back and figure out what we want to do here?
> >
> > AFAICS this function allocates memory for the swiotlb-xen buffer, and
> > that means it must be <= 32-bit addressable to satisfy the DMA API
> > guarantees. That means we generally want to use GFP_DMA32
> everywhere
> > that exists, but on systems with odd zones we might want to dip into
> > GFP_DMA. This also means swiotlb-xen doesn't actually do the right
> > thing on x86 at the moment. So shouldn't we just have one common
> > routine in swiotlb-xen.c that checks if we have CONFIG_ZONE_DMA32 set,
> > then try GFP_DMA32, and if not check if CONFIG_ZONE_DMA is set and
> > then try that, else default to GFP_KERNEL?
>
> Yes, for ARM/ARM64 it makes a lot of sense given that dom0 is 1:1 mapped
> (pseudo-physical == physical). I'll let Juergen and Boris comment on the x86
> side of things, but on x86 PV Dom0 is not 1:1 mapped so
> GFP_DMA32 is probably not meaningful.
If we only take ARM/ARM64, so could the following patch be ok?
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index d33b77e9add3..e5a6a73b2e06 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -28,7 +28,11 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int order)
for_each_memblock(memory, reg) {
if (reg->base < (phys_addr_t)0xffffffff) {
+#ifdef CONFIG_ZONE_DMA32
+ flags |= __GFP_DMA32;
+#else
flags |= __GFP_DMA;
+#endif
break;
}
}
Thanks,
Peng.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread* RE: [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
2019-09-11 2:06 ` Peng Fan
@ 2019-09-11 19:12 ` Stefano Stabellini
0 siblings, 0 replies; 8+ messages in thread
From: Stefano Stabellini @ 2019-09-11 19:12 UTC (permalink / raw)
To: Peng Fan
Cc: jgross@suse•com, Stefano Stabellini, Catalin Marinas,
linux@armlinux•org.uk, Christoph Hellwig, Julien Grall,
dl-linux-imx, xen-devel@lists•xenproject.org,
boris.ostrovsky@oracle•com, nd, will@kernel•org,
linux-arm-kernel@lists•infradead.org, Robin Murphy
On Wed, 11 Sep 2019, Peng Fan wrote:
> > Subject: Re: [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
> >
> > + Juergen, Boris
> >
> > On Fri, 30 Aug 2019, Christoph Hellwig wrote:
> > > Can we take a step back and figure out what we want to do here?
> > >
> > > AFAICS this function allocates memory for the swiotlb-xen buffer, and
> > > that means it must be <= 32-bit addressable to satisfy the DMA API
> > > guarantees. That means we generally want to use GFP_DMA32
> > everywhere
> > > that exists, but on systems with odd zones we might want to dip into
> > > GFP_DMA. This also means swiotlb-xen doesn't actually do the right
> > > thing on x86 at the moment. So shouldn't we just have one common
> > > routine in swiotlb-xen.c that checks if we have CONFIG_ZONE_DMA32 set,
> > > then try GFP_DMA32, and if not check if CONFIG_ZONE_DMA is set and
> > > then try that, else default to GFP_KERNEL?
> >
> > Yes, for ARM/ARM64 it makes a lot of sense given that dom0 is 1:1 mapped
> > (pseudo-physical == physical). I'll let Juergen and Boris comment on the x86
> > side of things, but on x86 PV Dom0 is not 1:1 mapped so
> > GFP_DMA32 is probably not meaningful.
>
> If we only take ARM/ARM64, so could the following patch be ok?
>
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index d33b77e9add3..e5a6a73b2e06 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -28,7 +28,11 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int order)
>
> for_each_memblock(memory, reg) {
> if (reg->base < (phys_addr_t)0xffffffff) {
> +#ifdef CONFIG_ZONE_DMA32
> + flags |= __GFP_DMA32;
> +#else
> flags |= __GFP_DMA;
> +#endif
> break;
> }
> }
I am OK with something like this, but at that point we can use
IS_ENABLED(CONFIG_ZONE_DMA32) for the check.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-09-11 19:12 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-30 2:28 [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64 Peng Fan
2019-08-30 8:39 ` Julien Grall
2019-08-30 8:58 ` Christoph Hellwig
2019-08-31 2:40 ` Stefano Stabellini
2019-09-02 15:57 ` Christoph Hellwig
2019-09-03 11:48 ` Juergen Gross
2019-09-11 2:06 ` Peng Fan
2019-09-11 19:12 ` Stefano Stabellini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox