| Message ID | 20221104100741.2176307-3-wei.chen@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: Add Armv8-R64 MPU support to Xen - Part#1 | expand |
(+ Henry) Hi, On 04/11/2022 10:07, Wei Chen wrote: > domain_build use ioremap_wc to map a new non-cacheable virtual s/use/uses/ > address for initrd. After Xen copy initrd from this address to > guest, this new allocated virtual address has not been unmapped. > > So in this patch, we add an iounmap to the end of domain_build, > after Xen loaded initrd to guest memory. > Please a fixes tag. The issue was introduced by commit bb7e6d565d92. > Signed-off-by: Wei Chen <wei.chen@arm.com> > --- > xen/arch/arm/domain_build.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 4fb5c20b13..bd30d3798c 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -3418,6 +3418,8 @@ static void __init initrd_load(struct kernel_info *kinfo) > initrd, len); > if ( res != 0 ) > panic("Unable to copy the initrd in the hwdom memory\n"); > + > + iounmap(initrd); This looks good to me. But I am wondering whether using ioremap_wc() is actually correct because we are reading the region. So it seems strang to map it with write-combine. So I would consider to use ioremap_cache(). That said, this would be a separate patch. I think this wants to be in 4.17. This will avoid Xen to have two mappings with different caching attribute (initrd is part of the RAM and therefore directmap). Cheers,
Hi Julien, > -----Original Message----- > Subject: Re: [PATCH v6 02/11] xen/arm: add iounmap after initrd has been > loaded in domain_build > > (+ Henry) > I think this wants to be in 4.17. This will avoid Xen to have two > mappings with different caching attribute (initrd is part of the RAM and > therefore directmap). Sounds good to me, I am wondering if we need to include also patch #1 though. If this patch wants to be in 4.17: Release-acked-by: Henry Wang <Henry.Wang@arm.com> Kind regards, Henry > > Cheers, > > -- > Julien Grall
On 07/11/2022 01:33, Henry Wang wrote: > Hi Julien, Hi Henry, > >> -----Original Message----- >> Subject: Re: [PATCH v6 02/11] xen/arm: add iounmap after initrd has been >> loaded in domain_build >> >> (+ Henry) >> I think this wants to be in 4.17. This will avoid Xen to have two >> mappings with different caching attribute (initrd is part of the RAM and >> therefore directmap). > > Sounds good to me, I am wondering if we need to include also patch #1 though. If we were earlier in the release, I would have say yes. But now, it is a no as this just a cleanup (having extra definitions is harmless). > > If this patch wants to be in 4.17: > > Release-acked-by: Henry Wang <Henry.Wang@arm.com> Thanks! Cheers,
Hi Julien, > -----Original Message----- > Subject: Re: [PATCH v6 02/11] xen/arm: add iounmap after initrd has been > loaded in domain_build > >> (+ Henry) > >> I think this wants to be in 4.17. This will avoid Xen to have two > >> mappings with different caching attribute (initrd is part of the RAM and > >> therefore directmap). > > > > Sounds good to me, I am wondering if we need to include also patch #1 > though. > > If we were earlier in the release, I would have say yes. But now, it is > a no as this just a cleanup (having extra definitions is harmless). Thanks for your confirmation :) No problem. Kind regards, Henry
On 06/11/2022 18:55, Julien Grall wrote: > (+ Henry) > > Hi, > > On 04/11/2022 10:07, Wei Chen wrote: >> domain_build use ioremap_wc to map a new non-cacheable virtual > > s/use/uses/ > >> address for initrd. After Xen copy initrd from this address to >> guest, this new allocated virtual address has not been unmapped. >> >> So in this patch, we add an iounmap to the end of domain_build, >> after Xen loaded initrd to guest memory. >> > > Please a fixes tag. The issue was introduced by commit bb7e6d565d92. Well I forgot to add it on commit :/. I will try to remember to backport it. Cheers,
Hi Julien, > -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Julien Grall > Sent: 2022年11月7日 2:55 > To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org > Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand > Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk > <Volodymyr_Babchuk@epam.com>; Henry Wang <Henry.Wang@arm.com> > Subject: Re: [PATCH v6 02/11] xen/arm: add iounmap after initrd has been > loaded in domain_build > > (+ Henry) > > Hi, > > On 04/11/2022 10:07, Wei Chen wrote: > > domain_build use ioremap_wc to map a new non-cacheable virtual > > s/use/uses/ > > > address for initrd. After Xen copy initrd from this address to > > guest, this new allocated virtual address has not been unmapped. > > > > So in this patch, we add an iounmap to the end of domain_build, > > after Xen loaded initrd to guest memory. > > > > Please a fixes tag. The issue was introduced by commit bb7e6d565d92. > > > Signed-off-by: Wei Chen <wei.chen@arm.com> > > --- > > xen/arch/arm/domain_build.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > index 4fb5c20b13..bd30d3798c 100644 > > --- a/xen/arch/arm/domain_build.c > > +++ b/xen/arch/arm/domain_build.c > > @@ -3418,6 +3418,8 @@ static void __init initrd_load(struct kernel_info > *kinfo) > > initrd, len); > > if ( res != 0 ) > > panic("Unable to copy the initrd in the hwdom memory\n"); > > + > > + iounmap(initrd); > > This looks good to me. But I am wondering whether using ioremap_wc() is > actually correct because we are reading the region. So it seems strang > to map it with write-combine. > > So I would consider to use ioremap_cache(). That said, this would be a > separate patch. > Ok, we will try to use ioremap_cache and test it. If everything works well we will introduce a separate patch in next version. Cheers, Wei Chen > I think this wants to be in 4.17. This will avoid Xen to have two > mappings with different caching attribute (initrd is part of the RAM and > therefore directmap). > > Cheers, > > -- > Julien Grall
> > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > > index 4fb5c20b13..bd30d3798c 100644 > > > --- a/xen/arch/arm/domain_build.c > > > +++ b/xen/arch/arm/domain_build.c > > > @@ -3418,6 +3418,8 @@ static void __init initrd_load(struct > kernel_info > > *kinfo) > > > initrd, len); > > > if ( res != 0 ) > > > panic("Unable to copy the initrd in the hwdom memory\n"); > > > + > > > + iounmap(initrd); > > > > This looks good to me. But I am wondering whether using ioremap_wc() is > > actually correct because we are reading the region. So it seems strang > > to map it with write-combine. > > > > So I would consider to use ioremap_cache(). That said, this would be a > > separate patch. > > > > Ok, we will try to use ioremap_cache and test it. If everything works > well we will introduce a separate patch in next version. > Or is it better to send a separate patch for this? Because I think we might need something to address the v1 comments. Cheers, Wei Chen > Cheers, > Wei Chen > > > > I think this wants to be in 4.17. This will avoid Xen to have two > > mappings with different caching attribute (initrd is part of the RAM and > > therefore directmap). > > > > Cheers, > > > > -- > > Julien Grall
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 4fb5c20b13..bd30d3798c 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -3418,6 +3418,8 @@ static void __init initrd_load(struct kernel_info *kinfo) initrd, len); if ( res != 0 ) panic("Unable to copy the initrd in the hwdom memory\n"); + + iounmap(initrd); } /*
domain_build use ioremap_wc to map a new non-cacheable virtual address for initrd. After Xen copy initrd from this address to guest, this new allocated virtual address has not been unmapped. So in this patch, we add an iounmap to the end of domain_build, after Xen loaded initrd to guest memory. Signed-off-by: Wei Chen <wei.chen@arm.com> --- xen/arch/arm/domain_build.c | 2 ++ 1 file changed, 2 insertions(+)