Xen devel - Dec 2012 - [PATCH] xen/arm: flush dcache after memcpy'ing the kernel image

After memcpy''ing the kernel in guest memory we need to flush the dcache
to make sure that the data actually reaches the memory before we start
executing guest code with caches disabled.

This fixes a boot time bug on the Cortex A15 Versatile Express that
usually shows up as follow:

(XEN) Hypervisor Trap. HSR=0x80000006 EC=0x20 IL=0 Syndrome=6
(XEN) Unexpected Trap: Hypervisor

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/kernel.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index b4a823d..81818b1 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -53,6 +53,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long
len, int attrindx)
 
         set_fixmap(FIXMAP_MISC, p, attrindx);
         memcpy(dst, src + s, l);
+        flush_xen_dcache_va_range(dst, l);
 
         paddr += l;
         dst += l;
@@ -82,6 +83,7 @@ static void kernel_zimage_load(struct kernel_info *info)
 
         set_fixmap(FIXMAP_MISC, (flash+offs) >> PAGE_SHIFT, DEV_SHARED);
         memcpy(dst, src, PAGE_SIZE);
+        flush_xen_dcache_va_range(dst, PAGE_SIZE);
         clear_fixmap(FIXMAP_MISC);
 
         unmap_domain_page(dst);
-- 
1.7.2.5

On Wed, 2012-12-05 at 18:37 +0000, Stefano Stabellini
wrote:
> After memcpy''ing the kernel in guest memory we need to flush the
dcache
> to make sure that the data actually reaches the memory before we start
> executing guest code with caches disabled.
> 
> This fixes a boot time bug on the Cortex A15 Versatile Express that
> usually shows up as follow:
> 
> (XEN) Hypervisor Trap. HSR=0x80000006 EC=0x20 IL=0 Syndrome=6
> (XEN) Unexpected Trap: Hypervisor
That''s a symptom of a thousand different problems though, since
it''s
just a generic Instruction Abort from guest mode caused by a translation
fault at stage 2. 

Anyhow this won''t apply in top of "arm: support for initial
modules
(e.g. dom0) and DTB supplied in RAM", could you rebase on that please?
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/arch/arm/kernel.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index b4a823d..81818b1 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -53,6 +53,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned
long len, int attrindx)
>  
>          set_fixmap(FIXMAP_MISC, p, attrindx);
>          memcpy(dst, src + s, l);
> +        flush_xen_dcache_va_range(dst, l);
>  
>          paddr += l;
>          dst += l;
> @@ -82,6 +83,7 @@ static void kernel_zimage_load(struct kernel_info *info)
>  
>          set_fixmap(FIXMAP_MISC, (flash+offs) >> PAGE_SHIFT,
DEV_SHARED);
>          memcpy(dst, src, PAGE_SIZE);
> +        flush_xen_dcache_va_range(dst, PAGE_SIZE);
>          clear_fixmap(FIXMAP_MISC);
>  
>          unmap_domain_page(dst);

On Thu, 6 Dec 2012, Ian Campbell wrote:
> On Wed, 2012-12-05 at 18:37 +0000, Stefano Stabellini wrote:
> > After memcpy''ing the kernel in guest memory we need to flush
the dcache
> > to make sure that the data actually reaches the memory before we start
> > executing guest code with caches disabled.
> > 
> > This fixes a boot time bug on the Cortex A15 Versatile Express that
> > usually shows up as follow:
> > 
> > (XEN) Hypervisor Trap. HSR=0x80000006 EC=0x20 IL=0 Syndrome=6
> > (XEN) Unexpected Trap: Hypervisor
> 
> That''s a symptom of a thousand different problems though, since
it''s
> just a generic Instruction Abort from guest mode caused by a translation
> fault at stage 2. 
> 
> Anyhow this won''t apply in top of "arm: support for initial
modules
> (e.g. dom0) and DTB supplied in RAM", could you rebase on that please?
The patch applies fine on both xen/master and xen/staging.

Do you have a branch with "arm: support for initial modules (e.g. dom0)
and DTB supplied in RAM" somewhere?



> 
> > Signed-off-by: Stefano Stabellini
<stefano.stabellini@eu.citrix.com>
> > ---
> >  xen/arch/arm/kernel.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> > index b4a823d..81818b1 100644
> > --- a/xen/arch/arm/kernel.c
> > +++ b/xen/arch/arm/kernel.c
> > @@ -53,6 +53,7 @@ void copy_from_paddr(void *dst, paddr_t paddr,
unsigned long len, int attrindx)
> >  
> >          set_fixmap(FIXMAP_MISC, p, attrindx);
> >          memcpy(dst, src + s, l);
> > +        flush_xen_dcache_va_range(dst, l);
> >  
> >          paddr += l;
> >          dst += l;
> > @@ -82,6 +83,7 @@ static void kernel_zimage_load(struct kernel_info
*info)
> >  
> >          set_fixmap(FIXMAP_MISC, (flash+offs) >> PAGE_SHIFT,
DEV_SHARED);
> >          memcpy(dst, src, PAGE_SIZE);
> > +        flush_xen_dcache_va_range(dst, PAGE_SIZE);
> >          clear_fixmap(FIXMAP_MISC);
> >  
> >          unmap_domain_page(dst);
> 
> 
>

On Tue, 2012-12-18 at 18:51 +0000, Stefano Stabellini
wrote:
> On Thu, 6 Dec 2012, Ian Campbell wrote:
> > On Wed, 2012-12-05 at 18:37 +0000, Stefano Stabellini wrote:
> > > After memcpy''ing the kernel in guest memory we need to
flush the dcache
> > > to make sure that the data actually reaches the memory before we
start
> > > executing guest code with caches disabled.
> > > 
> > > This fixes a boot time bug on the Cortex A15 Versatile Express
that
> > > usually shows up as follow:
> > > 
> > > (XEN) Hypervisor Trap. HSR=0x80000006 EC=0x20 IL=0 Syndrome=6
> > > (XEN) Unexpected Trap: Hypervisor
> > 
> > That''s a symptom of a thousand different problems though,
since it''s
> > just a generic Instruction Abort from guest mode caused by a
translation
> > fault at stage 2. 
> > 
> > Anyhow this won''t apply in top of "arm: support for
initial modules
> > (e.g. dom0) and DTB supplied in RAM", could you rebase on that
please?
> 
> The patch applies fine on both xen/master and xen/staging.
> 
> Do you have a branch with "arm: support for initial modules (e.g.
dom0)
> and DTB supplied in RAM" somewhere?
I do now:
        git://xenbits.xen.org/people/ianc/xen-unstable.git boot-wrapper
        
You had some comments on the DTB parsing/filtering which I replied to.

Perhaps I should reorder the series to put the useful infrastructure
first (so it can be pushed) but leave the actual concrete DTB interface
until the end. The DTB stuff is really a PoC pending a real bootloader
protocol and if no modules are given (which is what it will look like if
we omit that bit) then Xen should fallback to the current method of
looking in the flash for the kernel.

I think that would mean ordering this series as:

[PATCH 1/9] xen: arm: mark early_panic as a noreturn function
<bits of 2/9 which define the datastructures, but not the parsing stuff>
[PATCH 3/9] arm: avoid placing Xen over any modules.
<patches 4..8>
[PATCH 2/9] xen: arm: parse modules from DT during early boot.
[PATCH 9/9] xen: strip /chosen/modules/module@<N>/* from dom0 device tree


Ian.

On Wed, 19 Dec 2012, Ian Campbell wrote:
> On Tue, 2012-12-18 at 18:51 +0000, Stefano Stabellini wrote:
> > On Thu, 6 Dec 2012, Ian Campbell wrote:
> > > On Wed, 2012-12-05 at 18:37 +0000, Stefano Stabellini wrote:
> > > > After memcpy''ing the kernel in guest memory we need
to flush the dcache
> > > > to make sure that the data actually reaches the memory
before we start
> > > > executing guest code with caches disabled.
> > > > 
> > > > This fixes a boot time bug on the Cortex A15 Versatile
Express that
> > > > usually shows up as follow:
> > > > 
> > > > (XEN) Hypervisor Trap. HSR=0x80000006 EC=0x20 IL=0
Syndrome=6
> > > > (XEN) Unexpected Trap: Hypervisor
> > > 
> > > That''s a symptom of a thousand different problems
though, since it''s
> > > just a generic Instruction Abort from guest mode caused by a
translation
> > > fault at stage 2. 
> > > 
> > > Anyhow this won''t apply in top of "arm: support for
initial modules
> > > (e.g. dom0) and DTB supplied in RAM", could you rebase on
that please?
> > 
> > The patch applies fine on both xen/master and xen/staging.
> > 
> > Do you have a branch with "arm: support for initial modules (e.g.
dom0)
> > and DTB supplied in RAM" somewhere?
> 
> I do now:
>         git://xenbits.xen.org/people/ianc/xen-unstable.git boot-wrapper
>         
> You had some comments on the DTB parsing/filtering which I replied to.
> 
> Perhaps I should reorder the series to put the useful infrastructure
> first (so it can be pushed) but leave the actual concrete DTB interface
> until the end. The DTB stuff is really a PoC pending a real bootloader
> protocol and if no modules are given (which is what it will look like if
> we omit that bit) then Xen should fallback to the current method of
> looking in the flash for the kernel.
> 
> I think that would mean ordering this series as:
> 
> [PATCH 1/9] xen: arm: mark early_panic as a noreturn function
> <bits of 2/9 which define the datastructures, but not the parsing
stuff>
> [PATCH 3/9] arm: avoid placing Xen over any modules.
> <patches 4..8>
> [PATCH 2/9] xen: arm: parse modules from DT during early boot.
> [PATCH 9/9] xen: strip /chosen/modules/module@<N>/* from dom0 device
tree
 
That seems like a good idea.


This is the rebased patch:

----8<----

xen/arm: flush dcache after memcpy''ing the kernel image

After memcpy''ing the kernel in guest memory we need to flush the dcache
to make sure that the data actually reaches the memory before we start
executing guest code with caches disabled.

copy_from_paddr is the function that does the copy, so add a
flush_xen_dcache_va_range there.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index c9265d7..2f7a9ff 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -54,6 +54,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long
len, int attrindx)
 
         set_fixmap(FIXMAP_MISC, p, attrindx);
         memcpy(dst, src + s, l);
+        flush_xen_dcache_va_range(dst, l);
 
         paddr += l;
         dst += l;