| Message ID | 20171123183210.12045-9-julien.grall@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Stage-2 handling cleanup | expand |
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 >
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 --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;
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(-)