diff mbox series

[Xen-devel,for-next,08/16] xen/arm: kernel: Rework kernel_zimage_load to use the generic copy helper

Message ID 20171123183210.12045-9-julien.grall@linaro.org
State Superseded
Headers show
Series xen/arm: Stage-2 handling cleanup | expand

Commit Message

Julien Grall Nov. 23, 2017, 6:32 p.m. UTC
The function kernel_zimage is dealing with IPA but uses gvirt_to_maddr to
do the translation. This is currently working fine because the stage-1 MMU
is disabled.

Furthermore, the function is implementing its own copy to guest resulting
in code duplication and making more difficult to update the logic in
page-tables (such support for Populate On Demand).

The new copy_to_guest_phys_flush_dcache could be used here by
temporarily mapping the full kernel in the virtual space.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/domain_build.c |  1 +
 xen/arch/arm/kernel.c       | 33 ++++++++++++---------------------
 xen/arch/arm/kernel.h       |  2 ++
 3 files changed, 15 insertions(+), 21 deletions(-)

Comments

Stefano Stabellini Dec. 6, 2017, 1:38 a.m. UTC | #1
On Thu, 23 Nov 2017, Julien Grall wrote:
> The function kernel_zimage is dealing with IPA but uses gvirt_to_maddr to
> do the translation. This is currently working fine because the stage-1 MMU
> is disabled.
> 
> Furthermore, the function is implementing its own copy to guest resulting
> in code duplication and making more difficult to update the logic in
> page-tables (such support for Populate On Demand).
> 
> The new copy_to_guest_phys_flush_dcache could be used here by
> temporarily mapping the full kernel in the virtual space.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/domain_build.c |  1 +
>  xen/arch/arm/kernel.c       | 33 ++++++++++++---------------------
>  xen/arch/arm/kernel.h       |  2 ++
>  3 files changed, 15 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index c74f4dd69d..3f87bf2051 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2133,6 +2133,7 @@ int construct_dom0(struct domain *d)
>      d->max_pages = ~0U;
>  
>      kinfo.unassigned_mem = dom0_mem;
> +    kinfo.d = d;
>  
>      rc = kernel_probe(&kinfo);
>      if ( rc < 0 )
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index a6c6413712..2fb0b9684d 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -15,6 +15,8 @@
>  #include <xen/gunzip.h>
>  #include <xen/vmap.h>
>  
> +#include <asm/guest_access.h>
> +
>  #include "kernel.h"
>  
>  #define UIMAGE_MAGIC          0x27051956
> @@ -157,7 +159,8 @@ static void kernel_zimage_load(struct kernel_info *info)
>      paddr_t load_addr = kernel_zimage_place(info);
>      paddr_t paddr = info->zimage.kernel_addr;
>      paddr_t len = info->zimage.len;
> -    unsigned long offs;
> +    void *kernel;
> +    int rc;
>  
>      info->entry = load_addr;
>  
> @@ -165,29 +168,17 @@ static void kernel_zimage_load(struct kernel_info *info)
>  
>      printk("Loading zImage from %"PRIpaddr" to %"PRIpaddr"-%"PRIpaddr"\n",
>             paddr, load_addr, load_addr + len);
> -    for ( offs = 0; offs < len; )
> -    {
> -        uint64_t par;
> -        paddr_t s, l, ma = 0;
> -        void *dst;
> -
> -        s = offs & ~PAGE_MASK;
> -        l = min(PAGE_SIZE - s, len);
> -
> -        par = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
> -        if ( par )
> -        {
> -            panic("Unable to map translate guest address");
> -            return;
> -        }
>  
> -        dst = map_domain_page(maddr_to_mfn(ma));
> +    kernel = ioremap_wc(paddr, len);

Why ioremap_wc?


> +    if ( !kernel )
> +        panic("Unable to map the hwdom kernel");
>  
> -        copy_from_paddr(dst + s, paddr + offs, l);
> +    rc = copy_to_guest_phys_flush_dcache(info->d, load_addr,
> +                                         kernel, len);
> +    if ( rc != 0 )
> +        panic("Unable to copy the kernel in the hwdom memory");
>  
> -        unmap_domain_page(dst);
> -        offs += l;
> -    }
> +    iounmap(kernel);
>  }
>  
>  /*
> diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
> index c1b07d4f7b..6d695097b5 100644
> --- a/xen/arch/arm/kernel.h
> +++ b/xen/arch/arm/kernel.h
> @@ -15,6 +15,8 @@ struct kernel_info {
>      enum domain_type type;
>  #endif
>  
> +    struct domain *d;
> +
>      void *fdt; /* flat device tree */
>      paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
>      struct meminfo mem;
> -- 
> 2.11.0
>
Stefano Stabellini Dec. 6, 2017, 1:42 a.m. UTC | #2
On Tue, 5 Dec 2017, Stefano Stabellini wrote:
> On Thu, 23 Nov 2017, Julien Grall wrote:
> > The function kernel_zimage is dealing with IPA but uses gvirt_to_maddr to
> > do the translation. This is currently working fine because the stage-1 MMU
> > is disabled.
> > 
> > Furthermore, the function is implementing its own copy to guest resulting
> > in code duplication and making more difficult to update the logic in
> > page-tables (such support for Populate On Demand).
> > 
> > The new copy_to_guest_phys_flush_dcache could be used here by
> > temporarily mapping the full kernel in the virtual space.
> > 
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > ---
> >  xen/arch/arm/domain_build.c |  1 +
> >  xen/arch/arm/kernel.c       | 33 ++++++++++++---------------------
> >  xen/arch/arm/kernel.h       |  2 ++
> >  3 files changed, 15 insertions(+), 21 deletions(-)
> > 
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index c74f4dd69d..3f87bf2051 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -2133,6 +2133,7 @@ int construct_dom0(struct domain *d)
> >      d->max_pages = ~0U;
> >  
> >      kinfo.unassigned_mem = dom0_mem;
> > +    kinfo.d = d;
> >  
> >      rc = kernel_probe(&kinfo);
> >      if ( rc < 0 )
> > diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> > index a6c6413712..2fb0b9684d 100644
> > --- a/xen/arch/arm/kernel.c
> > +++ b/xen/arch/arm/kernel.c
> > @@ -15,6 +15,8 @@
> >  #include <xen/gunzip.h>
> >  #include <xen/vmap.h>
> >  
> > +#include <asm/guest_access.h>
> > +
> >  #include "kernel.h"
> >  
> >  #define UIMAGE_MAGIC          0x27051956
> > @@ -157,7 +159,8 @@ static void kernel_zimage_load(struct kernel_info *info)
> >      paddr_t load_addr = kernel_zimage_place(info);
> >      paddr_t paddr = info->zimage.kernel_addr;
> >      paddr_t len = info->zimage.len;
> > -    unsigned long offs;
> > +    void *kernel;
> > +    int rc;
> >  
> >      info->entry = load_addr;
> >  
> > @@ -165,29 +168,17 @@ static void kernel_zimage_load(struct kernel_info *info)
> >  
> >      printk("Loading zImage from %"PRIpaddr" to %"PRIpaddr"-%"PRIpaddr"\n",
> >             paddr, load_addr, load_addr + len);
> > -    for ( offs = 0; offs < len; )
> > -    {
> > -        uint64_t par;
> > -        paddr_t s, l, ma = 0;
> > -        void *dst;
> > -
> > -        s = offs & ~PAGE_MASK;
> > -        l = min(PAGE_SIZE - s, len);
> > -
> > -        par = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
> > -        if ( par )
> > -        {
> > -            panic("Unable to map translate guest address");
> > -            return;
> > -        }
> >  
> > -        dst = map_domain_page(maddr_to_mfn(ma));
> > +    kernel = ioremap_wc(paddr, len);
> 
> Why ioremap_wc?

That is because we kept the same attributes used today by
copy_from_paddr. Makes sense.

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> > +    if ( !kernel )
> > +        panic("Unable to map the hwdom kernel");
> >  
> > -        copy_from_paddr(dst + s, paddr + offs, l);
> > +    rc = copy_to_guest_phys_flush_dcache(info->d, load_addr,
> > +                                         kernel, len);
> > +    if ( rc != 0 )
> > +        panic("Unable to copy the kernel in the hwdom memory");
> >  
> > -        unmap_domain_page(dst);
> > -        offs += l;
> > -    }
> > +    iounmap(kernel);
> >  }
> >  
> >  /*
> > diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
> > index c1b07d4f7b..6d695097b5 100644
> > --- a/xen/arch/arm/kernel.h
> > +++ b/xen/arch/arm/kernel.h
> > @@ -15,6 +15,8 @@ struct kernel_info {
> >      enum domain_type type;
> >  #endif
> >  
> > +    struct domain *d;
> > +
> >      void *fdt; /* flat device tree */
> >      paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
> >      struct meminfo mem;
> > -- 
> > 2.11.0
> > 
>
diff mbox series

Patch

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c74f4dd69d..3f87bf2051 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2133,6 +2133,7 @@  int construct_dom0(struct domain *d)
     d->max_pages = ~0U;
 
     kinfo.unassigned_mem = dom0_mem;
+    kinfo.d = d;
 
     rc = kernel_probe(&kinfo);
     if ( rc < 0 )
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index a6c6413712..2fb0b9684d 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -15,6 +15,8 @@ 
 #include <xen/gunzip.h>
 #include <xen/vmap.h>
 
+#include <asm/guest_access.h>
+
 #include "kernel.h"
 
 #define UIMAGE_MAGIC          0x27051956
@@ -157,7 +159,8 @@  static void kernel_zimage_load(struct kernel_info *info)
     paddr_t load_addr = kernel_zimage_place(info);
     paddr_t paddr = info->zimage.kernel_addr;
     paddr_t len = info->zimage.len;
-    unsigned long offs;
+    void *kernel;
+    int rc;
 
     info->entry = load_addr;
 
@@ -165,29 +168,17 @@  static void kernel_zimage_load(struct kernel_info *info)
 
     printk("Loading zImage from %"PRIpaddr" to %"PRIpaddr"-%"PRIpaddr"\n",
            paddr, load_addr, load_addr + len);
-    for ( offs = 0; offs < len; )
-    {
-        uint64_t par;
-        paddr_t s, l, ma = 0;
-        void *dst;
-
-        s = offs & ~PAGE_MASK;
-        l = min(PAGE_SIZE - s, len);
-
-        par = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
-        if ( par )
-        {
-            panic("Unable to map translate guest address");
-            return;
-        }
 
-        dst = map_domain_page(maddr_to_mfn(ma));
+    kernel = ioremap_wc(paddr, len);
+    if ( !kernel )
+        panic("Unable to map the hwdom kernel");
 
-        copy_from_paddr(dst + s, paddr + offs, l);
+    rc = copy_to_guest_phys_flush_dcache(info->d, load_addr,
+                                         kernel, len);
+    if ( rc != 0 )
+        panic("Unable to copy the kernel in the hwdom memory");
 
-        unmap_domain_page(dst);
-        offs += l;
-    }
+    iounmap(kernel);
 }
 
 /*
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index c1b07d4f7b..6d695097b5 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -15,6 +15,8 @@  struct kernel_info {
     enum domain_type type;
 #endif
 
+    struct domain *d;
+
     void *fdt; /* flat device tree */
     paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
     struct meminfo mem;