Oleksandr Dmytryshyn
2013-Nov-26 10:54 UTC
[PATCH v2 0/2] xen: arm: introduce a function and flush dcache while preparing the device tree for Dom0
Currently we use OMAP5 ES2.0 Panda5 board to work with the hypervisor. Without flushing dcache the hypervisor couldn''t copy the device tree correctly when booting the kernel dom0 Image (memory with device tree is corrupted). As the result - when we try to load the kernel dom0 Image - dom0 hungs frequently. This issue is not reproduced with the kernel dom0 zImage because the zImage decompressor code flushes all dcache before starting the decompressed kernel Image. When the hypervisor loads the kernel image or initrd, this memory region isn''t corrupted because the hypervisor code flushes the dcache. Oleksandr Dmytryshyn (2): xen: arm: introduce raw_copy_to_guest_flush_dcache() function xen: arm: flush dcache while preparing the device tree for Dom0 xen/arch/arm/domain_build.c | 3 ++- xen/arch/arm/guestcopy.c | 16 +++++++++++++++- xen/include/asm-arm/guest_access.h | 2 ++ 3 files changed, 19 insertions(+), 2 deletions(-) -- 1.8.2.rc2
Oleksandr Dmytryshyn
2013-Nov-26 10:54 UTC
[PATCH v2 1/2] xen: arm: introduce raw_copy_to_guest_flush_dcache() function
This function flushes the dcache while copying the data.
Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
---
xen/arch/arm/guestcopy.c | 16 +++++++++++++++-
xen/include/asm-arm/guest_access.h | 2 ++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index d146cd6..4875b1d 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -5,7 +5,8 @@
#include <asm/mm.h>
#include <asm/guest_access.h>
-unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len)
+unsigned long raw_copy_to_guest_helper(void *to, const void *from, unsigned
len,
+ unsigned flush_dcache)
{
/* XXX needs to handle faults */
unsigned offset = (vaddr_t)to & ~PAGE_MASK;
@@ -24,6 +25,8 @@ unsigned long raw_copy_to_guest(void *to, const void *from,
unsigned len)
p = map_domain_page(g>>PAGE_SHIFT);
p += offset;
memcpy(p, from, size);
+ if (flush_dcache)
+ flush_xen_dcache_va_range(p, size);
unmap_domain_page(p - offset);
len -= size;
@@ -35,6 +38,17 @@ unsigned long raw_copy_to_guest(void *to, const void *from,
unsigned len)
return 0;
}
+unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len)
+{
+ return raw_copy_to_guest_helper(to, from, len, 0);
+}
+
+unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
+ unsigned len)
+{
+ return raw_copy_to_guest_helper(to, from, len, 1);
+}
+
unsigned long raw_clear_guest(void *to, unsigned len)
{
/* XXX needs to handle faults */
diff --git a/xen/include/asm-arm/guest_access.h
b/xen/include/asm-arm/guest_access.h
index 8ff088f..5876988 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -11,6 +11,8 @@
(likely(count < (~0UL/size)) && access_ok(addr,count*size))
unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len);
+unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
+ unsigned len);
unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len);
unsigned long raw_clear_guest(void *to, unsigned len);
--
1.8.2.rc2
Oleksandr Dmytryshyn
2013-Nov-26 10:54 UTC
[PATCH v2 2/2] xen: arm: flush dcache while preparing the device tree for Dom0
Without flushing dcache the hypervisor couldn''t copy the device tree
correctly when booting the kernel dom0 Image (memory with device tree
is corrupted). As the result - when we try to load the kernel dom0
Image - dom0 hungs frequently. This issue is not reproduced with the
kernel dom0 zImage because the zImage decompressor code flushes all
dcache before starting the decompressed kernel Image. When the
hypervisor loads the kernel image or initrd, this memory region
isn''t corrupted because the hypervisor code flushes the dcache.
Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
---
xen/arch/arm/domain_build.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index e9bb01f..99e785a 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -906,7 +906,8 @@ static void dtb_load(struct kernel_info *kinfo)
printk("Loading dom0 DTB to
0x%"PRIpaddr"-0x%"PRIpaddr"\n",
kinfo->dtb_paddr, kinfo->dtb_paddr +
fdt_totalsize(kinfo->fdt));
- rc = raw_copy_to_guest(dtb_virt, kinfo->fdt,
fdt_totalsize(kinfo->fdt));
+ rc = raw_copy_to_guest_flush_dcache(dtb_virt, kinfo->fdt,
+ fdt_totalsize(kinfo->fdt));
if ( rc != 0 )
panic("Unable to copy the DTB to dom0 memory (rc = %lu)\n",
rc);
xfree(kinfo->fdt);
--
1.8.2.rc2
Ian Campbell
2013-Nov-27 13:30 UTC
Re: [PATCH v2 0/2] xen: arm: introduce a function and flush dcache while preparing the device tree for Dom0
On Tue, 2013-11-26 at 12:54 +0200, Oleksandr Dmytryshyn wrote:> Currently we use OMAP5 ES2.0 Panda5 board to work with the hypervisor. > > Without flushing dcache the hypervisor couldn''t copy the device tree > correctly when booting the kernel dom0 Image (memory with device tree > is corrupted). As the result - when we try to load the kernel dom0 > Image - dom0 hungs frequently. This issue is not reproduced with the > kernel dom0 zImage because the zImage decompressor code flushes all > dcache before starting the decompressed kernel Image. When the > hypervisor loads the kernel image or initrd, this memory region > isn''t corrupted because the hypervisor code flushes the dcache. > > Oleksandr Dmytryshyn (2): > xen: arm: introduce raw_copy_to_guest_flush_dcache() function > xen: arm: flush dcache while preparing the device tree for Dom0Both of these look good to me as a fix for the dom0 case: Acked-by: Ian Campbell <ian.campbell@citrix.com>> > xen/arch/arm/domain_build.c | 3 ++- > xen/arch/arm/guestcopy.c | 16 +++++++++++++++- > xen/include/asm-arm/guest_access.h | 2 ++ > 3 files changed, 19 insertions(+), 2 deletions(-) >
Julien Grall
2013-Nov-27 13:50 UTC
Re: [PATCH v2 0/2] xen: arm: introduce a function and flush dcache while preparing the device tree for Dom0
On 11/27/2013 01:30 PM, Ian Campbell wrote:> On Tue, 2013-11-26 at 12:54 +0200, Oleksandr Dmytryshyn wrote: >> Currently we use OMAP5 ES2.0 Panda5 board to work with the hypervisor. >> >> Without flushing dcache the hypervisor couldn''t copy the device tree >> correctly when booting the kernel dom0 Image (memory with device tree >> is corrupted). As the result - when we try to load the kernel dom0 >> Image - dom0 hungs frequently. This issue is not reproduced with the >> kernel dom0 zImage because the zImage decompressor code flushes all >> dcache before starting the decompressed kernel Image. When the >> hypervisor loads the kernel image or initrd, this memory region >> isn''t corrupted because the hypervisor code flushes the dcache. >> >> Oleksandr Dmytryshyn (2): >> xen: arm: introduce raw_copy_to_guest_flush_dcache() function >> xen: arm: flush dcache while preparing the device tree for Dom0 > > Both of these look good to me as a fix for the dom0 case: > Acked-by: Ian Campbell <ian.campbell@citrix.com>I''m not convince that is enough to fix dom0 data cache issue. What about the initrd? For the zImage, I took a look to the decompressor code (arch/arm/boot/compressed/head.S) and I didn''t see any data cache flush on the first instruction. So the issue can also unlikely happen with the zImage. Instead of introduce a new helper I would prefer modify the existing raw_copy_* helpers to check if we are currently building the domain. -- Julien Grall
Ian Campbell
2013-Nov-27 13:54 UTC
Re: [PATCH v2 0/2] xen: arm: introduce a function and flush dcache while preparing the device tree for Dom0
On Wed, 2013-11-27 at 13:50 +0000, Julien Grall wrote:> > On 11/27/2013 01:30 PM, Ian Campbell wrote: > > On Tue, 2013-11-26 at 12:54 +0200, Oleksandr Dmytryshyn wrote: > >> Currently we use OMAP5 ES2.0 Panda5 board to work with the hypervisor. > >> > >> Without flushing dcache the hypervisor couldn''t copy the device tree > >> correctly when booting the kernel dom0 Image (memory with device tree > >> is corrupted). As the result - when we try to load the kernel dom0 > >> Image - dom0 hungs frequently. This issue is not reproduced with the > >> kernel dom0 zImage because the zImage decompressor code flushes all > >> dcache before starting the decompressed kernel Image. When the > >> hypervisor loads the kernel image or initrd, this memory region > >> isn''t corrupted because the hypervisor code flushes the dcache. > >> > >> Oleksandr Dmytryshyn (2): > >> xen: arm: introduce raw_copy_to_guest_flush_dcache() function > >> xen: arm: flush dcache while preparing the device tree for Dom0 > > > > Both of these look good to me as a fix for the dom0 case: > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > I''m not convince that is enough to fix dom0 data cache issue. What about > the initrd? > For the zImage, I took a look to the decompressor code > (arch/arm/boot/compressed/head.S) and I didn''t see any data cache flush > on the first instruction. So the issue can also unlikely happen with the > zImage.The zImage and initrd are both loaded with copy_from_paddr, which does the appropriate flushing.> Instead of introduce a new helper I would prefer modify the existing > raw_copy_* helpers to check if we are currently building the domain.Given the above I don''t think that is necessary or correct. Ian.
Julien Grall
2013-Nov-27 14:49 UTC
Re: [PATCH v2 1/2] xen: arm: introduce raw_copy_to_guest_flush_dcache() function
On 11/26/2013 10:54 AM, Oleksandr Dmytryshyn wrote:> This function flushes the dcache while copying the data. > > Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com> > --- > xen/arch/arm/guestcopy.c | 16 +++++++++++++++- > xen/include/asm-arm/guest_access.h | 2 ++ > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c > index d146cd6..4875b1d 100644 > --- a/xen/arch/arm/guestcopy.c > +++ b/xen/arch/arm/guestcopy.c > @@ -5,7 +5,8 @@ > #include <asm/mm.h> > #include <asm/guest_access.h> > > -unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len) > +unsigned long raw_copy_to_guest_helper(void *to, const void *from, unsigned len,This function should be static. Except that: Acked-by: Julien Grall<julien.grall@linaro.org>> + unsigned flush_dcache) > { > /* XXX needs to handle faults */ > unsigned offset = (vaddr_t)to & ~PAGE_MASK; > @@ -24,6 +25,8 @@ unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len) > p = map_domain_page(g>>PAGE_SHIFT); > p += offset; > memcpy(p, from, size); > + if (flush_dcache) > + flush_xen_dcache_va_range(p, size); > > unmap_domain_page(p - offset); > len -= size; > @@ -35,6 +38,17 @@ unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len) > return 0; > } > > +unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len) > +{ > + return raw_copy_to_guest_helper(to, from, len, 0); > +} > + > +unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from, > + unsigned len) > +{ > + return raw_copy_to_guest_helper(to, from, len, 1); > +} > + > unsigned long raw_clear_guest(void *to, unsigned len) > { > /* XXX needs to handle faults */ > diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h > index 8ff088f..5876988 100644 > --- a/xen/include/asm-arm/guest_access.h > +++ b/xen/include/asm-arm/guest_access.h > @@ -11,6 +11,8 @@ > (likely(count < (~0UL/size)) && access_ok(addr,count*size)) > > unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len); > +unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from, > + unsigned len); > unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len); > unsigned long raw_clear_guest(void *to, unsigned len); > >-- Julien Grall
Julien Grall
2013-Nov-27 14:51 UTC
Re: [PATCH v2 0/2] xen: arm: introduce a function and flush dcache while preparing the device tree for Dom0
On 11/27/2013 01:54 PM, Ian Campbell wrote:> On Wed, 2013-11-27 at 13:50 +0000, Julien Grall wrote: >> >> On 11/27/2013 01:30 PM, Ian Campbell wrote: >>> On Tue, 2013-11-26 at 12:54 +0200, Oleksandr Dmytryshyn wrote: >>>> Currently we use OMAP5 ES2.0 Panda5 board to work with the hypervisor. >>>> >>>> Without flushing dcache the hypervisor couldn''t copy the device tree >>>> correctly when booting the kernel dom0 Image (memory with device tree >>>> is corrupted). As the result - when we try to load the kernel dom0 >>>> Image - dom0 hungs frequently. This issue is not reproduced with the >>>> kernel dom0 zImage because the zImage decompressor code flushes all >>>> dcache before starting the decompressed kernel Image. When the >>>> hypervisor loads the kernel image or initrd, this memory region >>>> isn''t corrupted because the hypervisor code flushes the dcache. >>>> >>>> Oleksandr Dmytryshyn (2): >>>> xen: arm: introduce raw_copy_to_guest_flush_dcache() function >>>> xen: arm: flush dcache while preparing the device tree for Dom0 >>> >>> Both of these look good to me as a fix for the dom0 case: >>> Acked-by: Ian Campbell <ian.campbell@citrix.com> >> >> I''m not convince that is enough to fix dom0 data cache issue. What about >> the initrd? >> For the zImage, I took a look to the decompressor code >> (arch/arm/boot/compressed/head.S) and I didn''t see any data cache flush >> on the first instruction. So the issue can also unlikely happen with the >> zImage. > > The zImage and initrd are both loaded with copy_from_paddr, which does > the appropriate flushing.Oh right, I didn''t pay attention that we are using different function for initrd and zImage. I have acked all the patch series. -- Julien Grall
Oleksandr Dmytryshyn
2013-Nov-27 16:37 UTC
Re: [PATCH v2 1/2] xen: arm: introduce raw_copy_to_guest_flush_dcache() function
Hi, Julien. I''ll fix this in the next patch-set. Oleksandr Dmytryshyn | Product Engineering and Development GlobalLogic P x3657 M +38.067.382.2525 www.globallogic.com http://www.globallogic.com/email_disclaimer.txt On Wed, Nov 27, 2013 at 4:49 PM, Julien Grall <julien.grall@linaro.org> wrote:> On 11/26/2013 10:54 AM, Oleksandr Dmytryshyn wrote: >> This function flushes the dcache while copying the data. >> >> Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com> >> --- >> xen/arch/arm/guestcopy.c | 16 +++++++++++++++- >> xen/include/asm-arm/guest_access.h | 2 ++ >> 2 files changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c >> index d146cd6..4875b1d 100644 >> --- a/xen/arch/arm/guestcopy.c >> +++ b/xen/arch/arm/guestcopy.c >> @@ -5,7 +5,8 @@ >> #include <asm/mm.h> >> #include <asm/guest_access.h> >> >> -unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len) >> +unsigned long raw_copy_to_guest_helper(void *to, const void *from, unsigned len, > This function should be static. > > Except that: > > Acked-by: Julien Grall<julien.grall@linaro.org> > >> + unsigned flush_dcache) >> { >> /* XXX needs to handle faults */ >> unsigned offset = (vaddr_t)to & ~PAGE_MASK; >> @@ -24,6 +25,8 @@ unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len) >> p = map_domain_page(g>>PAGE_SHIFT); >> p += offset; >> memcpy(p, from, size); >> + if (flush_dcache) >> + flush_xen_dcache_va_range(p, size); >> >> unmap_domain_page(p - offset); >> len -= size; >> @@ -35,6 +38,17 @@ unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len) >> return 0; >> } >> >> +unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len) >> +{ >> + return raw_copy_to_guest_helper(to, from, len, 0); >> +} >> + >> +unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from, >> + unsigned len) >> +{ >> + return raw_copy_to_guest_helper(to, from, len, 1); >> +} >> + >> unsigned long raw_clear_guest(void *to, unsigned len) >> { >> /* XXX needs to handle faults */ >> diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h >> index 8ff088f..5876988 100644 >> --- a/xen/include/asm-arm/guest_access.h >> +++ b/xen/include/asm-arm/guest_access.h >> @@ -11,6 +11,8 @@ >> (likely(count < (~0UL/size)) && access_ok(addr,count*size)) >> >> unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len); >> +unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from, >> + unsigned len); >> unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len); >> unsigned long raw_clear_guest(void *to, unsigned len); >> >> > > > -- > Julien Grall
Ian Campbell
2013-Nov-29 10:35 UTC
Re: [PATCH v2 0/2] xen: arm: introduce a function and flush dcache while preparing the device tree for Dom0
On Wed, 2013-11-27 at 14:51 +0000, Julien Grall wrote:> I have acked all the patch series.I couldn''t find a mail with your ack to 2/2 but I''ve taken you at your word and applied both patches from v4 of this series with both yours and my acks. Thanks Oleksandr.