Message ID | 1459332494-18964-21-git-send-email-zhaoshenglong@huawei.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
>>> On 30.03.16 at 12:08, <zhaoshenglong@huawei.com> wrote: > From: Shannon Zhao <shannon.zhao@linaro.org> > > It needs to map platform or amba device mmio to Dom0 on ARM. But when > booting with ACPI, it can't get the mmio region in Xen due to lack of > AML interpreter to parse DSDT table. Therefore, let Dom0 call a > hypercall to map mmio region when it adds the devices. > > Here we add a new map space like the XEN_DOMCTL_memory_mapping to map > mmio region for Dom0. Also add a helper to combine the > xsm_add_to_physmap and XENMAPSPACE_dev_mmio space check together. > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> Acked-by: Jan Beulich <jbeulich@suse.com>
On Wed, Mar 30, 2016 at 06:08:13PM +0800, Shannon Zhao wrote: > From: Shannon Zhao <shannon.zhao@linaro.org> > > It needs to map platform or amba device mmio to Dom0 on ARM. But when > booting with ACPI, it can't get the mmio region in Xen due to lack of > AML interpreter to parse DSDT table. Therefore, let Dom0 call a > hypercall to map mmio region when it adds the devices. > > Here we add a new map space like the XEN_DOMCTL_memory_mapping to map > mmio region for Dom0. Also add a helper to combine the > xsm_add_to_physmap and XENMAPSPACE_dev_mmio space check together. > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Keir Fraser <keir@xen.org> > Cc: Tim Deegan <tim@xen.org> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> > --- > v8: add a helper to combine xsm_add_to_physmap and XENMAPSPACE_dev_mmio > space check together I was by accident reviewing the earlier ersion. So let me give comments here as well. .. snipp. > +int map_dev_mmio_region(struct domain *d, > + unsigned long start_gfn, > + unsigned long nr, > + unsigned long mfn) > +{ > + int res; > + > + if ( !(nr && iomem_access_permitted(d, start_gfn, start_gfn + nr - 1)) ) > + return 0; > + > + res = map_mmio_regions(d, start_gfn, nr, mfn); > + if ( res < 0 ) > + { > + printk(XENLOG_ERR "Unable to map [%#lx - %#lx] in Dom%d\n", Should this be printk ratelimited? > + start_gfn, start_gfn + nr - 1, d->domain_id); > + return res; > + } > + > + return 0; > +} > + > int guest_physmap_add_entry(struct domain *d, > unsigned long gpfn, > unsigned long mfn, > diff --git a/xen/common/memory.c b/xen/common/memory.c > index c7fca96..644f81a 100644 > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -834,6 +834,19 @@ static int get_reserved_device_memory(xen_pfn_t start, xen_ulong_t nr, > } > #endif > > +static long xatp_permission_check(struct domain *d, unsigned int space) > +{ > + /* > + * XENMAPSPACE_dev_mmio mapping is only supported for hardware Domain s/Domain/domain/ > + * to map this kind of space to itself. > + */ > + if ( (space == XENMAPSPACE_dev_mmio) && > + (!is_hardware_domain(current->domain) || (d != current->domain)) ) > + return -EACCES; Why not -EPERM? > + > + return xsm_add_to_physmap(XSM_TARGET, current->domain, d); > +} > + > long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > { > struct domain *d; > @@ -980,7 +993,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > if ( d == NULL ) > return -ESRCH; > > - rc = xsm_add_to_physmap(XSM_TARGET, current->domain, d); > + rc = xatp_permission_check(d, xatp.space); Ah, which nicely takes care of rcu_unlock_domain. > if ( rc ) > { > rcu_unlock_domain(d); > @@ -1024,7 +1037,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > if ( d == NULL ) > return -ESRCH; > > - rc = xsm_add_to_physmap(XSM_TARGET, current->domain, d); > + rc = xatp_permission_check(d, xatpb.space); > if ( rc ) > { > rcu_unlock_domain(d); > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h > index 55626b4..d240d1e 100644 > --- a/xen/include/asm-arm/p2m.h > +++ b/xen/include/asm-arm/p2m.h > @@ -154,6 +154,11 @@ int unmap_regions_rw_cache(struct domain *d, > unsigned long nr_mfns, > unsigned long mfn); > > +int map_dev_mmio_region(struct domain *d, > + unsigned long start_gfn, > + unsigned long nr, > + unsigned long mfn); > + > int guest_physmap_add_entry(struct domain *d, > unsigned long gfn, > unsigned long mfn, > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > index f69e92f..fe52ee1 100644 > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -220,6 +220,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t); > #define XENMAPSPACE_gmfn_range 3 /* GMFN range, XENMEM_add_to_physmap only. */ > #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom, > * XENMEM_add_to_physmap_batch only. */ > +#define XENMAPSPACE_dev_mmio 5 /* device mmio region */ s/device/Device/ And maybe an full stop at the end? > /* ` } */ > > /* > -- > 2.0.4 > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Hi, On 30/03/16 17:11, Konrad Rzeszutek Wilk wrote: > On Wed, Mar 30, 2016 at 06:08:13PM +0800, Shannon Zhao wrote: >> From: Shannon Zhao <shannon.zhao@linaro.org> >> >> It needs to map platform or amba device mmio to Dom0 on ARM. But when >> booting with ACPI, it can't get the mmio region in Xen due to lack of >> AML interpreter to parse DSDT table. Therefore, let Dom0 call a >> hypercall to map mmio region when it adds the devices. >> >> Here we add a new map space like the XEN_DOMCTL_memory_mapping to map >> mmio region for Dom0. Also add a helper to combine the >> xsm_add_to_physmap and XENMAPSPACE_dev_mmio space check together. >> >> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >> Cc: Jan Beulich <jbeulich@suse.com> >> Cc: Keir Fraser <keir@xen.org> >> Cc: Tim Deegan <tim@xen.org> >> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> >> --- >> v8: add a helper to combine xsm_add_to_physmap and XENMAPSPACE_dev_mmio >> space check together > > I was by accident reviewing the earlier ersion. So let me give comments > here as well. > .. snipp. Jan already applied the patch series to xengit/staging. Shannon, can you send a follow-up patch to fix at least the printk? >> +int map_dev_mmio_region(struct domain *d, >> + unsigned long start_gfn, >> + unsigned long nr, >> + unsigned long mfn) >> +{ >> + int res; >> + >> + if ( !(nr && iomem_access_permitted(d, start_gfn, start_gfn + nr - 1)) ) >> + return 0; >> + >> + res = map_mmio_regions(d, start_gfn, nr, mfn); >> + if ( res < 0 ) >> + { >> + printk(XENLOG_ERR "Unable to map [%#lx - %#lx] in Dom%d\n", > > Should this be printk ratelimited? I think so. Today the domain can only be the hardware domain but it may change in the future. >> + start_gfn, start_gfn + nr - 1, d->domain_id); >> + return res; >> + } >> + >> + return 0; >> +} >> + >> int guest_physmap_add_entry(struct domain *d, >> unsigned long gpfn, >> unsigned long mfn, Regards,
On 2016/3/31 0:47, Julien Grall wrote: > Hi, > > On 30/03/16 17:11, Konrad Rzeszutek Wilk wrote: >> On Wed, Mar 30, 2016 at 06:08:13PM +0800, Shannon Zhao wrote: >>> From: Shannon Zhao <shannon.zhao@linaro.org> >>> >>> It needs to map platform or amba device mmio to Dom0 on ARM. But when >>> booting with ACPI, it can't get the mmio region in Xen due to lack of >>> AML interpreter to parse DSDT table. Therefore, let Dom0 call a >>> hypercall to map mmio region when it adds the devices. >>> >>> Here we add a new map space like the XEN_DOMCTL_memory_mapping to map >>> mmio region for Dom0. Also add a helper to combine the >>> xsm_add_to_physmap and XENMAPSPACE_dev_mmio space check together. >>> >>> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >>> Cc: Jan Beulich <jbeulich@suse.com> >>> Cc: Keir Fraser <keir@xen.org> >>> Cc: Tim Deegan <tim@xen.org> >>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> >>> --- >>> v8: add a helper to combine xsm_add_to_physmap and XENMAPSPACE_dev_mmio >>> space check together >> >> I was by accident reviewing the earlier ersion. So let me give comments >> here as well. >> .. snipp. > > Jan already applied the patch series to xengit/staging. Shannon, can you > send a follow-up patch to fix at least the printk? > Ok, will send a patch to fix the printk. Thanks,
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 81f9e2e..0aae6c5 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1138,6 +1138,9 @@ int xenmem_add_to_physmap_one( rcu_unlock_domain(od); break; } + case XENMAPSPACE_dev_mmio: + rc = map_dev_mmio_region(d, gpfn, 1, idx); + return rc; default: return -ENOSYS; diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 7e5f5d1..0011708 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -7,6 +7,7 @@ #include <xen/bitops.h> #include <xen/vm_event.h> #include <xen/mem_access.h> +#include <xen/iocap.h> #include <public/vm_event.h> #include <asm/flushtlb.h> #include <asm/gic.h> @@ -1270,6 +1271,27 @@ int unmap_mmio_regions(struct domain *d, d->arch.p2m.default_access); } +int map_dev_mmio_region(struct domain *d, + unsigned long start_gfn, + unsigned long nr, + unsigned long mfn) +{ + int res; + + if ( !(nr && iomem_access_permitted(d, start_gfn, start_gfn + nr - 1)) ) + return 0; + + res = map_mmio_regions(d, start_gfn, nr, mfn); + if ( res < 0 ) + { + printk(XENLOG_ERR "Unable to map [%#lx - %#lx] in Dom%d\n", + start_gfn, start_gfn + nr - 1, d->domain_id); + return res; + } + + return 0; +} + int guest_physmap_add_entry(struct domain *d, unsigned long gpfn, unsigned long mfn, diff --git a/xen/common/memory.c b/xen/common/memory.c index c7fca96..644f81a 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -834,6 +834,19 @@ static int get_reserved_device_memory(xen_pfn_t start, xen_ulong_t nr, } #endif +static long xatp_permission_check(struct domain *d, unsigned int space) +{ + /* + * XENMAPSPACE_dev_mmio mapping is only supported for hardware Domain + * to map this kind of space to itself. + */ + if ( (space == XENMAPSPACE_dev_mmio) && + (!is_hardware_domain(current->domain) || (d != current->domain)) ) + return -EACCES; + + return xsm_add_to_physmap(XSM_TARGET, current->domain, d); +} + long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) { struct domain *d; @@ -980,7 +993,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) if ( d == NULL ) return -ESRCH; - rc = xsm_add_to_physmap(XSM_TARGET, current->domain, d); + rc = xatp_permission_check(d, xatp.space); if ( rc ) { rcu_unlock_domain(d); @@ -1024,7 +1037,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) if ( d == NULL ) return -ESRCH; - rc = xsm_add_to_physmap(XSM_TARGET, current->domain, d); + rc = xatp_permission_check(d, xatpb.space); if ( rc ) { rcu_unlock_domain(d); diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index 55626b4..d240d1e 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -154,6 +154,11 @@ int unmap_regions_rw_cache(struct domain *d, unsigned long nr_mfns, unsigned long mfn); +int map_dev_mmio_region(struct domain *d, + unsigned long start_gfn, + unsigned long nr, + unsigned long mfn); + int guest_physmap_add_entry(struct domain *d, unsigned long gfn, unsigned long mfn, diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h index f69e92f..fe52ee1 100644 --- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -220,6 +220,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t); #define XENMAPSPACE_gmfn_range 3 /* GMFN range, XENMEM_add_to_physmap only. */ #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom, * XENMEM_add_to_physmap_batch only. */ +#define XENMAPSPACE_dev_mmio 5 /* device mmio region */ /* ` } */ /*