| Message ID | 1414872625-2961-1-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Sat, 1 Nov 2014, Julien Grall wrote: > The vGIC will emulate the same version as the hardware. The toolstack has > to retrieve the version of the vGIC in order to be able to create the > corresponding device tree node. > > A new DOMCTL has been introduced for ARM to configure the domain. For now > it only allow the toolstack to retrieve the version of vGIC. > This DOMCTL will be extend later to let the user choose the version of the > emulated GIC. > > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> > Signed-off-by: Julien Grall <julien.grall@linaro.org> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Cc: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Changes in v2: > - Use memcpu in xc_domain_configure > - Rename the DOMCTL into domctl_arm_configuredomain > - Drop arch_domain_create_pre. Will be reintroduced in Xen 4.6 > and fold the code in arch_domain_init_hw_description > - Return -EOPNOTSUPP when the value of gic_version is not > supported > > This patch is based on Vijay's GICv3 guest support patch [1] and my patch to > defer the initialization of the vGIC [2]. > > Xen 4.5 has already support for the hardware GICv3. While it's possible to > boot and use DOM0, there is no guest support. The first version of this patch > has been sent a couple of months ago, but has never reached unstable for > various reasons (based on deferred series, lake of review at that time...). > Without it, platform with GICv3 support won't be able to boot any guest. > > The patch has been reworked to make it acceptable for Xen 4.5. Except the new > DOMCTL to retrieve the GIC version and one check. The code is purely GICv3. > > Features such as adding a new config option to let the user choose the GIC > version are deferred to Xen 4.6. > > It has been tested on both GICv2/GICv3 platform. And build tested for x86. > > [1] http://lists.xen.org/archives/html/xen-devel/2014-10/msg00583.html > [2] https://patches.linaro.org/34664/ > --- > tools/flask/policy/policy/modules/xen/xen.if | 2 +- > tools/libxc/include/xenctrl.h | 6 ++ > tools/libxc/xc_domain.c | 20 ++++++ > tools/libxl/libxl_arm.c | 97 ++++++++++++++++++++++++-- > xen/arch/arm/domctl.c | 35 ++++++++++ > xen/arch/arm/gic-v3.c | 16 ++++- > xen/include/public/arch-arm.h | 16 +++++ > xen/include/public/domctl.h | 17 +++++ > xen/xsm/flask/hooks.c | 3 + > xen/xsm/flask/policy/access_vectors | 2 + > 10 files changed, 206 insertions(+), 8 deletions(-) > > diff --git a/tools/flask/policy/policy/modules/xen/xen.if b/tools/flask/policy/policy/modules/xen/xen.if > index 641c797..fa69c9d 100644 > --- a/tools/flask/policy/policy/modules/xen/xen.if > +++ b/tools/flask/policy/policy/modules/xen/xen.if > @@ -49,7 +49,7 @@ define(`create_domain_common', ` > getdomaininfo hypercall setvcpucontext setextvcpucontext > getscheduler getvcpuinfo getvcpuextstate getaddrsize > getaffinity setaffinity }; > - allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim set_max_evtchn set_vnumainfo get_vnumainfo psr_cmt_op }; > + allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim set_max_evtchn set_vnumainfo get_vnumainfo psr_cmt_op configure_domain }; > allow $1 $2:security check_context; > allow $1 $2:shadow enable; > allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage mmuext_op }; > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h > index 564e187..45e282c 100644 > --- a/tools/libxc/include/xenctrl.h > +++ b/tools/libxc/include/xenctrl.h > @@ -483,6 +483,12 @@ int xc_domain_create(xc_interface *xch, > uint32_t flags, > uint32_t *pdomid); > > +#if defined(__arm__) || defined(__aarch64__) > +typedef xen_domctl_arm_configuredomain_t xc_domain_configuration_t; > + > +int xc_domain_configure(xc_interface *xch, uint32_t domid, > + xc_domain_configuration_t *config); > +#endif > > /* Functions to produce a dump of a given domain > * xc_domain_dumpcore - produces a dump to a specified file > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > index a9bcd4a..1aa1f45 100644 > --- a/tools/libxc/xc_domain.c > +++ b/tools/libxc/xc_domain.c > @@ -48,6 +48,26 @@ int xc_domain_create(xc_interface *xch, > return 0; > } > > +#if defined(__arm__) || defined(__aarch64__) > +int xc_domain_configure(xc_interface *xch, uint32_t domid, > + xc_domain_configuration_t *config) > +{ > + int rc; > + DECLARE_DOMCTL; > + > + domctl.cmd = XEN_DOMCTL_arm_configure_domain; > + domctl.domain = (domid_t)domid; > + /* xc_domain_configure_t is an alias to xen_domctl_arm_configuredomain */ > + memcpy(&domctl.u.configuredomain, config, sizeof(*config)); > + > + rc = do_domctl(xch, &domctl); > + if ( !rc ) > + memcpy(config, &domctl.u.configuredomain, sizeof(*config)); > + > + return rc; > +} > +#endif > + > int xc_domain_cacheflush(xc_interface *xch, uint32_t domid, > xen_pfn_t start_pfn, xen_pfn_t nr_pfns) > { > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c > index a122e4a..3dd6e7c 100644 > --- a/tools/libxl/libxl_arm.c > +++ b/tools/libxl/libxl_arm.c > @@ -23,6 +23,18 @@ > #define DT_IRQ_TYPE_LEVEL_HIGH 0x00000004 > #define DT_IRQ_TYPE_LEVEL_LOW 0x00000008 > > +static const char *gicv_to_string(uint8_t gic_version) > +{ > + switch (gic_version) { > + case XEN_DOMCTL_CONFIG_GIC_V2: > + return "V2"; > + case XEN_DOMCTL_CONFIG_GIC_V3: > + return "V3"; > + default: > + return "unknown"; > + } > +} > + > int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, > uint32_t domid) > { > @@ -307,9 +319,9 @@ static int make_memory_nodes(libxl__gc *gc, void *fdt, > return 0; > } > > -static int make_intc_node(libxl__gc *gc, void *fdt, > - uint64_t gicd_base, uint64_t gicd_size, > - uint64_t gicc_base, uint64_t gicc_size) > +static int make_gicv2_node(libxl__gc *gc, void *fdt, > + uint64_t gicd_base, uint64_t gicd_size, > + uint64_t gicc_base, uint64_t gicc_size) > { > int res; > const char *name = GCSPRINTF("interrupt-controller@%"PRIx64, gicd_base); > @@ -350,6 +362,57 @@ static int make_intc_node(libxl__gc *gc, void *fdt, > return 0; > } > > +static int make_gicv3_node(libxl__gc *gc, void *fdt) > +{ > + int res; > + const uint64_t gicd_base = GUEST_GICV3_GICD_BASE; > + const uint64_t gicd_size = GUEST_GICV3_GICD_SIZE; > + const uint64_t gicr0_base = GUEST_GICV3_GICR0_BASE; > + const uint64_t gicr0_size = GUEST_GICV3_GICR0_SIZE; > + const char *name = GCSPRINTF("interrupt-controller@%"PRIx64, gicd_base); > + > + res = fdt_begin_node(fdt, name); > + if (res) return res; > + > + res = fdt_property_compat(gc, fdt, 1, > + "arm,gic-v3"); > + if (res) return res; > + > + res = fdt_property_cell(fdt, "#interrupt-cells", 3); > + if (res) return res; > + > + res = fdt_property_cell(fdt, "#address-cells", 0); > + if (res) return res; > + > + res = fdt_property(fdt, "interrupt-controller", NULL, 0); > + if (res) return res; > + > + res = fdt_property_cell(fdt, "redistributor-stride", > + GUEST_GICV3_RDIST_STRIDE); > + if (res) return res; > + > + res = fdt_property_cell(fdt, "#redistributor-regions", > + GUEST_GICV3_RDIST_REGIONS); > + if (res) return res; > + > + res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS, > + 2, > + gicd_base, gicd_size, > + gicr0_base, gicr0_size); > + if (res) return res; > + > + res = fdt_property_cell(fdt, "linux,phandle", PHANDLE_GIC); > + if (res) return res; > + > + res = fdt_property_cell(fdt, "phandle", PHANDLE_GIC); > + if (res) return res; > + > + res = fdt_end_node(fdt); > + if (res) return res; > + > + return 0; > +} > + > static int make_timer_node(libxl__gc *gc, void *fdt, const struct arch_info *ainfo) > { > int res; > @@ -456,6 +519,7 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, > libxl_domain_build_info *info, > struct xc_dom_image *dom) > { > + xc_domain_configuration_t config; > void *fdt = NULL; > int rc, res; > size_t fdt_size = 0; > @@ -471,8 +535,16 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, > ainfo = get_arch_info(gc, dom); > if (ainfo == NULL) return ERROR_FAIL; > > + LOG(DEBUG, "configure the domain"); > + config.gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT; > + if (xc_domain_configure(CTX->xch, dom->guest_domid, &config) != 0) { > + LOG(ERROR, "counldn't configure the domain"); > + return ERROR_FAIL; > + } > + > LOG(DEBUG, "constructing DTB for Xen version %d.%d guest", > vers->xen_version_major, vers->xen_version_minor); > + LOG(DEBUG, " - vGIC version: %s", gicv_to_string(config.gic_version)); > > /* > * Call "call" handling FDR_ERR_*. Will either: > @@ -520,9 +592,22 @@ next_resize: > FDT( make_psci_node(gc, fdt) ); > > FDT( make_memory_nodes(gc, fdt, dom) ); > - FDT( make_intc_node(gc, fdt, > - GUEST_GICD_BASE, GUEST_GICD_SIZE, > - GUEST_GICC_BASE, GUEST_GICD_SIZE) ); > + > + switch (config.gic_version) { > + case XEN_DOMCTL_CONFIG_GIC_V2: > + FDT( make_gicv2_node(gc, fdt, > + GUEST_GICD_BASE, GUEST_GICD_SIZE, > + GUEST_GICC_BASE, GUEST_GICC_SIZE) ); > + break; > + case XEN_DOMCTL_CONFIG_GIC_V3: > + FDT( make_gicv3_node(gc, fdt) ); > + break; > + default: > + LOG(ERROR, "Unknown how to create the DT node for gic_version = %d", > + config.gic_version); > + rc = ERROR_FAIL; > + goto out; > + } > > FDT( make_timer_node(gc, fdt, ainfo) ); > FDT( make_hypervisor_node(gc, fdt, vers) ); > diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c > index 45974e7..5b8ff57 100644 > --- a/xen/arch/arm/domctl.c > +++ b/xen/arch/arm/domctl.c > @@ -10,6 +10,8 @@ > #include <xen/errno.h> > #include <xen/sched.h> > #include <xen/hypercall.h> > +#include <asm/gic.h> > +#include <xen/guest_access.h> > #include <public/domctl.h> > > long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, > @@ -30,6 +32,39 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, > > return p2m_cache_flush(d, s, e); > } > + case XEN_DOMCTL_arm_configure_domain: > + { > + uint8_t gic_version; > + > + /* > + * Xen 4.5: The vGIC is emulating the same version of the > + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT > + * is allowed. The DOMCTL will return the actual version of the > + * GIC. > + */ > + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) > + return -EOPNOTSUPP; > + > + switch ( gic_hw_version() ) > + { > + case GIC_V3: > + gic_version = XEN_DOMCTL_CONFIG_GIC_V3; > + break; > + case GIC_V2: > + gic_version = XEN_DOMCTL_CONFIG_GIC_V2; > + break; > + default: > + BUG(); > + } > + > + domctl->u.configuredomain.gic_version = gic_version; > + > + /* TODO: Make the copy generic for all ARCH domctl */ > + if ( __copy_to_guest(u_domctl, domctl, 1) ) > + return -EFAULT; > + > + return 0; > + } > > default: > return subarch_do_domctl(domctl, d, u_domctl); > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 91161a2..076aa62 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -906,7 +906,21 @@ static int gicv_v3_init(struct domain *d) > d->arch.vgic.rdist_count = gicv3.rdist_count; > } > else > - d->arch.vgic.dbase = GUEST_GICD_BASE; > + { > + d->arch.vgic.dbase = GUEST_GICV3_GICD_BASE; > + d->arch.vgic.dbase_size = GUEST_GICV3_GICD_SIZE; > + > + /* XXX: Only one Re-distributor region mapped for the guest */ > + BUILD_BUG_ON(GUEST_GICV3_RDIST_REGIONS != 1); > + > + d->arch.vgic.rdist_count = GUEST_GICV3_RDIST_REGIONS; > + d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE; > + > + /* The first redistributor should contain enough space for all CPUs */ > + BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < MAX_VIRT_CPUS); > + d->arch.vgic.rbase[0] = GUEST_GICV3_GICR0_BASE; > + d->arch.vgic.rbase_size[0] = GUEST_GICV3_GICR0_SIZE; > + } > > d->arch.vgic.nr_lines = 0; > > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > index eecc561..d7df274 100644 > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -373,11 +373,27 @@ typedef uint64_t xen_callback_t; > */ > > /* Physical Address Space */ > + > +/* vGIC mappings: Only one set of mapping is used by the guest. > + * Therefore they can overlap. > + */ > + > +/* vGIC v2 mappings */ > #define GUEST_GICD_BASE 0x03001000ULL > #define GUEST_GICD_SIZE 0x00001000ULL > #define GUEST_GICC_BASE 0x03002000ULL > #define GUEST_GICC_SIZE 0x00000100ULL > > +/* vGIC v3 mappings */ > +#define GUEST_GICV3_GICD_BASE 0x03001000ULL > +#define GUEST_GICV3_GICD_SIZE 0x00010000ULL > + > +#define GUEST_GICV3_RDIST_STRIDE 0x20000ULL > +#define GUEST_GICV3_RDIST_REGIONS 1 > + > +#define GUEST_GICV3_GICR0_BASE 0x03020000ULL /* vCPU0 - vCPU15 */ > +#define GUEST_GICV3_GICR0_SIZE 0x00200000ULL > + > /* 16MB == 4096 pages reserved for guest to use as a region to map its > * grant table in. > */ > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > index 58b19e7..8da204e 100644 > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -68,6 +68,19 @@ struct xen_domctl_createdomain { > typedef struct xen_domctl_createdomain xen_domctl_createdomain_t; > DEFINE_XEN_GUEST_HANDLE(xen_domctl_createdomain_t); > > +#if defined(__arm__) || defined(__aarch64__) > +#define XEN_DOMCTL_CONFIG_GIC_DEFAULT 0 > +#define XEN_DOMCTL_CONFIG_GIC_V2 1 > +#define XEN_DOMCTL_CONFIG_GIC_V3 2 > +/* XEN_DOMCTL_configure_domain */ > +struct xen_domctl_arm_configuredomain { > + /* IN/OUT parameters */ > + uint8_t gic_version; > +}; > +typedef struct xen_domctl_arm_configuredomain xen_domctl_arm_configuredomain_t; > +DEFINE_XEN_GUEST_HANDLE(xen_domctl_arm_configuredomain_t); > +#endif > + > /* XEN_DOMCTL_getdomaininfo */ > struct xen_domctl_getdomaininfo { > /* OUT variables. */ > @@ -1056,6 +1069,7 @@ struct xen_domctl { > #define XEN_DOMCTL_set_vcpu_msrs 73 > #define XEN_DOMCTL_setvnumainfo 74 > #define XEN_DOMCTL_psr_cmt_op 75 > +#define XEN_DOMCTL_arm_configure_domain 76 > #define XEN_DOMCTL_gdbsx_guestmemio 1000 > #define XEN_DOMCTL_gdbsx_pausevcpu 1001 > #define XEN_DOMCTL_gdbsx_unpausevcpu 1002 > @@ -1064,6 +1078,9 @@ struct xen_domctl { > domid_t domain; > union { > struct xen_domctl_createdomain createdomain; > +#if defined(__arm__) || defined(__aarch64__) > + struct xen_domctl_arm_configuredomain configuredomain; > +#endif > struct xen_domctl_getdomaininfo getdomaininfo; > struct xen_domctl_getmemlist getmemlist; > struct xen_domctl_getpageframeinfo getpageframeinfo; > diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c > index 6d0fe72..846cf88 100644 > --- a/xen/xsm/flask/hooks.c > +++ b/xen/xsm/flask/hooks.c > @@ -727,6 +727,9 @@ static int flask_domctl(struct domain *d, int cmd) > case XEN_DOMCTL_psr_cmt_op: > return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_CMT_OP); > > + case XEN_DOMCTL_configure_domain: > + return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__CONFIGURE_DOMAIN); > + > default: > printk("flask_domctl: Unknown op %d\n", cmd); > return -EPERM; > diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors > index de0c707..bfe2fa5 100644 > --- a/xen/xsm/flask/policy/access_vectors > +++ b/xen/xsm/flask/policy/access_vectors > @@ -216,6 +216,8 @@ class domain2 > get_vnumainfo > # XEN_DOMCTL_psr_cmt_op > psr_cmt_op > +# XEN_DOMCTL_configure_domain > + configure_domain > } > > # Similar to class domain, but primarily contains domctls related to HVM domains > -- > 1.7.10.4 >
On Mon, Nov 3, 2014 at 4:31 PM, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > On Sat, 1 Nov 2014, Julien Grall wrote: >> The vGIC will emulate the same version as the hardware. The toolstack has >> to retrieve the version of the vGIC in order to be able to create the >> corresponding device tree node. >> >> A new DOMCTL has been introduced for ARM to configure the domain. For now >> it only allow the toolstack to retrieve the version of vGIC. >> This DOMCTL will be extend later to let the user choose the version of the >> emulated GIC. >> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >> Cc: Jan Beulich <jbeulich@suse.com> >> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> >> Cc: Wei Liu <wei.liu2@citrix.com> > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Tested with GICv3 platform. > > >> Changes in v2: >> - Use memcpu in xc_domain_configure >> - Rename the DOMCTL into domctl_arm_configuredomain >> - Drop arch_domain_create_pre. Will be reintroduced in Xen 4.6 >> and fold the code in arch_domain_init_hw_description >> - Return -EOPNOTSUPP when the value of gic_version is not >> supported >> >> This patch is based on Vijay's GICv3 guest support patch [1] and my patch to >> defer the initialization of the vGIC [2]. >> >> Xen 4.5 has already support for the hardware GICv3. While it's possible to >> boot and use DOM0, there is no guest support. The first version of this patch >> has been sent a couple of months ago, but has never reached unstable for >> various reasons (based on deferred series, lake of review at that time...). >> Without it, platform with GICv3 support won't be able to boot any guest. >> >> The patch has been reworked to make it acceptable for Xen 4.5. Except the new >> DOMCTL to retrieve the GIC version and one check. The code is purely GICv3. >> >> Features such as adding a new config option to let the user choose the GIC >> version are deferred to Xen 4.6. >> >> It has been tested on both GICv2/GICv3 platform. And build tested for x86. >> >> [1] http://lists.xen.org/archives/html/xen-devel/2014-10/msg00583.html >> [2] https://patches.linaro.org/34664/ >> --- >> tools/flask/policy/policy/modules/xen/xen.if | 2 +- >> tools/libxc/include/xenctrl.h | 6 ++ >> tools/libxc/xc_domain.c | 20 ++++++ >> tools/libxl/libxl_arm.c | 97 ++++++++++++++++++++++++-- >> xen/arch/arm/domctl.c | 35 ++++++++++ >> xen/arch/arm/gic-v3.c | 16 ++++- >> xen/include/public/arch-arm.h | 16 +++++ >> xen/include/public/domctl.h | 17 +++++ >> xen/xsm/flask/hooks.c | 3 + >> xen/xsm/flask/policy/access_vectors | 2 + >> 10 files changed, 206 insertions(+), 8 deletions(-) >> >> diff --git a/tools/flask/policy/policy/modules/xen/xen.if b/tools/flask/policy/policy/modules/xen/xen.if >> index 641c797..fa69c9d 100644 >> --- a/tools/flask/policy/policy/modules/xen/xen.if >> +++ b/tools/flask/policy/policy/modules/xen/xen.if >> @@ -49,7 +49,7 @@ define(`create_domain_common', ` >> getdomaininfo hypercall setvcpucontext setextvcpucontext >> getscheduler getvcpuinfo getvcpuextstate getaddrsize >> getaffinity setaffinity }; >> - allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim set_max_evtchn set_vnumainfo get_vnumainfo psr_cmt_op }; >> + allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim set_max_evtchn set_vnumainfo get_vnumainfo psr_cmt_op configure_domain }; >> allow $1 $2:security check_context; >> allow $1 $2:shadow enable; >> allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage mmuext_op }; >> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h >> index 564e187..45e282c 100644 >> --- a/tools/libxc/include/xenctrl.h >> +++ b/tools/libxc/include/xenctrl.h >> @@ -483,6 +483,12 @@ int xc_domain_create(xc_interface *xch, >> uint32_t flags, >> uint32_t *pdomid); >> >> +#if defined(__arm__) || defined(__aarch64__) >> +typedef xen_domctl_arm_configuredomain_t xc_domain_configuration_t; >> + >> +int xc_domain_configure(xc_interface *xch, uint32_t domid, >> + xc_domain_configuration_t *config); >> +#endif >> >> /* Functions to produce a dump of a given domain >> * xc_domain_dumpcore - produces a dump to a specified file >> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c >> index a9bcd4a..1aa1f45 100644 >> --- a/tools/libxc/xc_domain.c >> +++ b/tools/libxc/xc_domain.c >> @@ -48,6 +48,26 @@ int xc_domain_create(xc_interface *xch, >> return 0; >> } >> >> +#if defined(__arm__) || defined(__aarch64__) >> +int xc_domain_configure(xc_interface *xch, uint32_t domid, >> + xc_domain_configuration_t *config) >> +{ >> + int rc; >> + DECLARE_DOMCTL; >> + >> + domctl.cmd = XEN_DOMCTL_arm_configure_domain; >> + domctl.domain = (domid_t)domid; >> + /* xc_domain_configure_t is an alias to xen_domctl_arm_configuredomain */ >> + memcpy(&domctl.u.configuredomain, config, sizeof(*config)); >> + >> + rc = do_domctl(xch, &domctl); >> + if ( !rc ) >> + memcpy(config, &domctl.u.configuredomain, sizeof(*config)); >> + >> + return rc; >> +} >> +#endif >> + >> int xc_domain_cacheflush(xc_interface *xch, uint32_t domid, >> xen_pfn_t start_pfn, xen_pfn_t nr_pfns) >> { >> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c >> index a122e4a..3dd6e7c 100644 >> --- a/tools/libxl/libxl_arm.c >> +++ b/tools/libxl/libxl_arm.c >> @@ -23,6 +23,18 @@ >> #define DT_IRQ_TYPE_LEVEL_HIGH 0x00000004 >> #define DT_IRQ_TYPE_LEVEL_LOW 0x00000008 >> >> +static const char *gicv_to_string(uint8_t gic_version) >> +{ >> + switch (gic_version) { >> + case XEN_DOMCTL_CONFIG_GIC_V2: >> + return "V2"; >> + case XEN_DOMCTL_CONFIG_GIC_V3: >> + return "V3"; >> + default: >> + return "unknown"; >> + } >> +} >> + >> int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, >> uint32_t domid) >> { >> @@ -307,9 +319,9 @@ static int make_memory_nodes(libxl__gc *gc, void *fdt, >> return 0; >> } >> >> -static int make_intc_node(libxl__gc *gc, void *fdt, >> - uint64_t gicd_base, uint64_t gicd_size, >> - uint64_t gicc_base, uint64_t gicc_size) >> +static int make_gicv2_node(libxl__gc *gc, void *fdt, >> + uint64_t gicd_base, uint64_t gicd_size, >> + uint64_t gicc_base, uint64_t gicc_size) >> { >> int res; >> const char *name = GCSPRINTF("interrupt-controller@%"PRIx64, gicd_base); >> @@ -350,6 +362,57 @@ static int make_intc_node(libxl__gc *gc, void *fdt, >> return 0; >> } >> >> +static int make_gicv3_node(libxl__gc *gc, void *fdt) >> +{ >> + int res; >> + const uint64_t gicd_base = GUEST_GICV3_GICD_BASE; >> + const uint64_t gicd_size = GUEST_GICV3_GICD_SIZE; >> + const uint64_t gicr0_base = GUEST_GICV3_GICR0_BASE; >> + const uint64_t gicr0_size = GUEST_GICV3_GICR0_SIZE; >> + const char *name = GCSPRINTF("interrupt-controller@%"PRIx64, gicd_base); >> + >> + res = fdt_begin_node(fdt, name); >> + if (res) return res; >> + >> + res = fdt_property_compat(gc, fdt, 1, >> + "arm,gic-v3"); >> + if (res) return res; >> + >> + res = fdt_property_cell(fdt, "#interrupt-cells", 3); >> + if (res) return res; >> + >> + res = fdt_property_cell(fdt, "#address-cells", 0); >> + if (res) return res; >> + >> + res = fdt_property(fdt, "interrupt-controller", NULL, 0); >> + if (res) return res; >> + >> + res = fdt_property_cell(fdt, "redistributor-stride", >> + GUEST_GICV3_RDIST_STRIDE); >> + if (res) return res; >> + >> + res = fdt_property_cell(fdt, "#redistributor-regions", >> + GUEST_GICV3_RDIST_REGIONS); >> + if (res) return res; >> + >> + res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS, >> + 2, >> + gicd_base, gicd_size, >> + gicr0_base, gicr0_size); >> + if (res) return res; >> + >> + res = fdt_property_cell(fdt, "linux,phandle", PHANDLE_GIC); >> + if (res) return res; >> + >> + res = fdt_property_cell(fdt, "phandle", PHANDLE_GIC); >> + if (res) return res; >> + >> + res = fdt_end_node(fdt); >> + if (res) return res; >> + >> + return 0; >> +} >> + >> static int make_timer_node(libxl__gc *gc, void *fdt, const struct arch_info *ainfo) >> { >> int res; >> @@ -456,6 +519,7 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, >> libxl_domain_build_info *info, >> struct xc_dom_image *dom) >> { >> + xc_domain_configuration_t config; >> void *fdt = NULL; >> int rc, res; >> size_t fdt_size = 0; >> @@ -471,8 +535,16 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, >> ainfo = get_arch_info(gc, dom); >> if (ainfo == NULL) return ERROR_FAIL; >> >> + LOG(DEBUG, "configure the domain"); >> + config.gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT; >> + if (xc_domain_configure(CTX->xch, dom->guest_domid, &config) != 0) { >> + LOG(ERROR, "counldn't configure the domain"); >> + return ERROR_FAIL; >> + } >> + >> LOG(DEBUG, "constructing DTB for Xen version %d.%d guest", >> vers->xen_version_major, vers->xen_version_minor); >> + LOG(DEBUG, " - vGIC version: %s", gicv_to_string(config.gic_version)); >> >> /* >> * Call "call" handling FDR_ERR_*. Will either: >> @@ -520,9 +592,22 @@ next_resize: >> FDT( make_psci_node(gc, fdt) ); >> >> FDT( make_memory_nodes(gc, fdt, dom) ); >> - FDT( make_intc_node(gc, fdt, >> - GUEST_GICD_BASE, GUEST_GICD_SIZE, >> - GUEST_GICC_BASE, GUEST_GICD_SIZE) ); >> + >> + switch (config.gic_version) { >> + case XEN_DOMCTL_CONFIG_GIC_V2: >> + FDT( make_gicv2_node(gc, fdt, >> + GUEST_GICD_BASE, GUEST_GICD_SIZE, >> + GUEST_GICC_BASE, GUEST_GICC_SIZE) ); >> + break; >> + case XEN_DOMCTL_CONFIG_GIC_V3: >> + FDT( make_gicv3_node(gc, fdt) ); >> + break; >> + default: >> + LOG(ERROR, "Unknown how to create the DT node for gic_version = %d", >> + config.gic_version); >> + rc = ERROR_FAIL; >> + goto out; >> + } >> >> FDT( make_timer_node(gc, fdt, ainfo) ); >> FDT( make_hypervisor_node(gc, fdt, vers) ); >> diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c >> index 45974e7..5b8ff57 100644 >> --- a/xen/arch/arm/domctl.c >> +++ b/xen/arch/arm/domctl.c >> @@ -10,6 +10,8 @@ >> #include <xen/errno.h> >> #include <xen/sched.h> >> #include <xen/hypercall.h> >> +#include <asm/gic.h> >> +#include <xen/guest_access.h> >> #include <public/domctl.h> >> >> long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, >> @@ -30,6 +32,39 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, >> >> return p2m_cache_flush(d, s, e); >> } >> + case XEN_DOMCTL_arm_configure_domain: >> + { >> + uint8_t gic_version; >> + >> + /* >> + * Xen 4.5: The vGIC is emulating the same version of the >> + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT >> + * is allowed. The DOMCTL will return the actual version of the >> + * GIC. >> + */ >> + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) >> + return -EOPNOTSUPP; >> + >> + switch ( gic_hw_version() ) >> + { >> + case GIC_V3: >> + gic_version = XEN_DOMCTL_CONFIG_GIC_V3; >> + break; >> + case GIC_V2: >> + gic_version = XEN_DOMCTL_CONFIG_GIC_V2; >> + break; >> + default: >> + BUG(); >> + } >> + >> + domctl->u.configuredomain.gic_version = gic_version; >> + >> + /* TODO: Make the copy generic for all ARCH domctl */ >> + if ( __copy_to_guest(u_domctl, domctl, 1) ) >> + return -EFAULT; >> + >> + return 0; >> + } >> >> default: >> return subarch_do_domctl(domctl, d, u_domctl); >> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c >> index 91161a2..076aa62 100644 >> --- a/xen/arch/arm/gic-v3.c >> +++ b/xen/arch/arm/gic-v3.c >> @@ -906,7 +906,21 @@ static int gicv_v3_init(struct domain *d) >> d->arch.vgic.rdist_count = gicv3.rdist_count; >> } >> else >> - d->arch.vgic.dbase = GUEST_GICD_BASE; >> + { >> + d->arch.vgic.dbase = GUEST_GICV3_GICD_BASE; >> + d->arch.vgic.dbase_size = GUEST_GICV3_GICD_SIZE; >> + >> + /* XXX: Only one Re-distributor region mapped for the guest */ >> + BUILD_BUG_ON(GUEST_GICV3_RDIST_REGIONS != 1); >> + >> + d->arch.vgic.rdist_count = GUEST_GICV3_RDIST_REGIONS; >> + d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE; >> + >> + /* The first redistributor should contain enough space for all CPUs */ >> + BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < MAX_VIRT_CPUS); >> + d->arch.vgic.rbase[0] = GUEST_GICV3_GICR0_BASE; >> + d->arch.vgic.rbase_size[0] = GUEST_GICV3_GICR0_SIZE; >> + } >> >> d->arch.vgic.nr_lines = 0; >> >> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h >> index eecc561..d7df274 100644 >> --- a/xen/include/public/arch-arm.h >> +++ b/xen/include/public/arch-arm.h >> @@ -373,11 +373,27 @@ typedef uint64_t xen_callback_t; >> */ >> >> /* Physical Address Space */ >> + >> +/* vGIC mappings: Only one set of mapping is used by the guest. >> + * Therefore they can overlap. >> + */ >> + >> +/* vGIC v2 mappings */ >> #define GUEST_GICD_BASE 0x03001000ULL >> #define GUEST_GICD_SIZE 0x00001000ULL >> #define GUEST_GICC_BASE 0x03002000ULL >> #define GUEST_GICC_SIZE 0x00000100ULL >> >> +/* vGIC v3 mappings */ >> +#define GUEST_GICV3_GICD_BASE 0x03001000ULL >> +#define GUEST_GICV3_GICD_SIZE 0x00010000ULL >> + >> +#define GUEST_GICV3_RDIST_STRIDE 0x20000ULL >> +#define GUEST_GICV3_RDIST_REGIONS 1 >> + >> +#define GUEST_GICV3_GICR0_BASE 0x03020000ULL /* vCPU0 - vCPU15 */ >> +#define GUEST_GICV3_GICR0_SIZE 0x00200000ULL >> + >> /* 16MB == 4096 pages reserved for guest to use as a region to map its >> * grant table in. >> */ >> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h >> index 58b19e7..8da204e 100644 >> --- a/xen/include/public/domctl.h >> +++ b/xen/include/public/domctl.h >> @@ -68,6 +68,19 @@ struct xen_domctl_createdomain { >> typedef struct xen_domctl_createdomain xen_domctl_createdomain_t; >> DEFINE_XEN_GUEST_HANDLE(xen_domctl_createdomain_t); >> >> +#if defined(__arm__) || defined(__aarch64__) >> +#define XEN_DOMCTL_CONFIG_GIC_DEFAULT 0 >> +#define XEN_DOMCTL_CONFIG_GIC_V2 1 >> +#define XEN_DOMCTL_CONFIG_GIC_V3 2 >> +/* XEN_DOMCTL_configure_domain */ >> +struct xen_domctl_arm_configuredomain { >> + /* IN/OUT parameters */ >> + uint8_t gic_version; >> +}; >> +typedef struct xen_domctl_arm_configuredomain xen_domctl_arm_configuredomain_t; >> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_arm_configuredomain_t); >> +#endif >> + >> /* XEN_DOMCTL_getdomaininfo */ >> struct xen_domctl_getdomaininfo { >> /* OUT variables. */ >> @@ -1056,6 +1069,7 @@ struct xen_domctl { >> #define XEN_DOMCTL_set_vcpu_msrs 73 >> #define XEN_DOMCTL_setvnumainfo 74 >> #define XEN_DOMCTL_psr_cmt_op 75 >> +#define XEN_DOMCTL_arm_configure_domain 76 >> #define XEN_DOMCTL_gdbsx_guestmemio 1000 >> #define XEN_DOMCTL_gdbsx_pausevcpu 1001 >> #define XEN_DOMCTL_gdbsx_unpausevcpu 1002 >> @@ -1064,6 +1078,9 @@ struct xen_domctl { >> domid_t domain; >> union { >> struct xen_domctl_createdomain createdomain; >> +#if defined(__arm__) || defined(__aarch64__) >> + struct xen_domctl_arm_configuredomain configuredomain; >> +#endif >> struct xen_domctl_getdomaininfo getdomaininfo; >> struct xen_domctl_getmemlist getmemlist; >> struct xen_domctl_getpageframeinfo getpageframeinfo; >> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c >> index 6d0fe72..846cf88 100644 >> --- a/xen/xsm/flask/hooks.c >> +++ b/xen/xsm/flask/hooks.c >> @@ -727,6 +727,9 @@ static int flask_domctl(struct domain *d, int cmd) >> case XEN_DOMCTL_psr_cmt_op: >> return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_CMT_OP); >> >> + case XEN_DOMCTL_configure_domain: >> + return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__CONFIGURE_DOMAIN); >> + >> default: >> printk("flask_domctl: Unknown op %d\n", cmd); >> return -EPERM; >> diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors >> index de0c707..bfe2fa5 100644 >> --- a/xen/xsm/flask/policy/access_vectors >> +++ b/xen/xsm/flask/policy/access_vectors >> @@ -216,6 +216,8 @@ class domain2 >> get_vnumainfo >> # XEN_DOMCTL_psr_cmt_op >> psr_cmt_op >> +# XEN_DOMCTL_configure_domain >> + configure_domain >> } >> >> # Similar to class domain, but primarily contains domctls related to HVM domains >> -- >> 1.7.10.4 >> > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On Mon, Nov 03, 2014 at 06:07:33PM +0530, Vijay Kilari wrote: > On Mon, Nov 3, 2014 at 4:31 PM, Stefano Stabellini > <stefano.stabellini@eu.citrix.com> wrote: > > On Sat, 1 Nov 2014, Julien Grall wrote: > >> The vGIC will emulate the same version as the hardware. The toolstack has > >> to retrieve the version of the vGIC in order to be able to create the > >> corresponding device tree node. > >> > >> A new DOMCTL has been introduced for ARM to configure the domain. For now > >> it only allow the toolstack to retrieve the version of vGIC. > >> This DOMCTL will be extend later to let the user choose the version of the > >> emulated GIC. > >> > >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> > >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > >> Cc: Ian Jackson <ian.jackson@eu.citrix.com> > >> Cc: Jan Beulich <jbeulich@suse.com> > >> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> > >> Cc: Wei Liu <wei.liu2@citrix.com> > > > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > Tested with GICv3 platform. Thank you for doing that. It also needs Acks from Daniel and Jan. > > > > > > >> Changes in v2: > >> - Use memcpu in xc_domain_configure > >> - Rename the DOMCTL into domctl_arm_configuredomain > >> - Drop arch_domain_create_pre. Will be reintroduced in Xen 4.6 > >> and fold the code in arch_domain_init_hw_description > >> - Return -EOPNOTSUPP when the value of gic_version is not > >> supported > >> > >> This patch is based on Vijay's GICv3 guest support patch [1] and my patch to > >> defer the initialization of the vGIC [2]. > >> > >> Xen 4.5 has already support for the hardware GICv3. While it's possible to > >> boot and use DOM0, there is no guest support. The first version of this patch > >> has been sent a couple of months ago, but has never reached unstable for > >> various reasons (based on deferred series, lake of review at that time...). > >> Without it, platform with GICv3 support won't be able to boot any guest. > >> > >> The patch has been reworked to make it acceptable for Xen 4.5. Except the new > >> DOMCTL to retrieve the GIC version and one check. The code is purely GICv3. > >> > >> Features such as adding a new config option to let the user choose the GIC > >> version are deferred to Xen 4.6. > >> > >> It has been tested on both GICv2/GICv3 platform. And build tested for x86. > >> > >> [1] http://lists.xen.org/archives/html/xen-devel/2014-10/msg00583.html > >> [2] https://patches.linaro.org/34664/ > >> --- > >> tools/flask/policy/policy/modules/xen/xen.if | 2 +- > >> tools/libxc/include/xenctrl.h | 6 ++ > >> tools/libxc/xc_domain.c | 20 ++++++ > >> tools/libxl/libxl_arm.c | 97 ++++++++++++++++++++++++-- > >> xen/arch/arm/domctl.c | 35 ++++++++++ > >> xen/arch/arm/gic-v3.c | 16 ++++- > >> xen/include/public/arch-arm.h | 16 +++++ > >> xen/include/public/domctl.h | 17 +++++ > >> xen/xsm/flask/hooks.c | 3 + > >> xen/xsm/flask/policy/access_vectors | 2 + > >> 10 files changed, 206 insertions(+), 8 deletions(-) > >> > >> diff --git a/tools/flask/policy/policy/modules/xen/xen.if b/tools/flask/policy/policy/modules/xen/xen.if > >> index 641c797..fa69c9d 100644 > >> --- a/tools/flask/policy/policy/modules/xen/xen.if > >> +++ b/tools/flask/policy/policy/modules/xen/xen.if > >> @@ -49,7 +49,7 @@ define(`create_domain_common', ` > >> getdomaininfo hypercall setvcpucontext setextvcpucontext > >> getscheduler getvcpuinfo getvcpuextstate getaddrsize > >> getaffinity setaffinity }; > >> - allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim set_max_evtchn set_vnumainfo get_vnumainfo psr_cmt_op }; > >> + allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim set_max_evtchn set_vnumainfo get_vnumainfo psr_cmt_op configure_domain }; > >> allow $1 $2:security check_context; > >> allow $1 $2:shadow enable; > >> allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage mmuext_op }; > >> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h > >> index 564e187..45e282c 100644 > >> --- a/tools/libxc/include/xenctrl.h > >> +++ b/tools/libxc/include/xenctrl.h > >> @@ -483,6 +483,12 @@ int xc_domain_create(xc_interface *xch, > >> uint32_t flags, > >> uint32_t *pdomid); > >> > >> +#if defined(__arm__) || defined(__aarch64__) > >> +typedef xen_domctl_arm_configuredomain_t xc_domain_configuration_t; > >> + > >> +int xc_domain_configure(xc_interface *xch, uint32_t domid, > >> + xc_domain_configuration_t *config); > >> +#endif > >> > >> /* Functions to produce a dump of a given domain > >> * xc_domain_dumpcore - produces a dump to a specified file > >> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > >> index a9bcd4a..1aa1f45 100644 > >> --- a/tools/libxc/xc_domain.c > >> +++ b/tools/libxc/xc_domain.c > >> @@ -48,6 +48,26 @@ int xc_domain_create(xc_interface *xch, > >> return 0; > >> } > >> > >> +#if defined(__arm__) || defined(__aarch64__) > >> +int xc_domain_configure(xc_interface *xch, uint32_t domid, > >> + xc_domain_configuration_t *config) > >> +{ > >> + int rc; > >> + DECLARE_DOMCTL; > >> + > >> + domctl.cmd = XEN_DOMCTL_arm_configure_domain; > >> + domctl.domain = (domid_t)domid; > >> + /* xc_domain_configure_t is an alias to xen_domctl_arm_configuredomain */ > >> + memcpy(&domctl.u.configuredomain, config, sizeof(*config)); > >> + > >> + rc = do_domctl(xch, &domctl); > >> + if ( !rc ) > >> + memcpy(config, &domctl.u.configuredomain, sizeof(*config)); > >> + > >> + return rc; > >> +} > >> +#endif > >> + > >> int xc_domain_cacheflush(xc_interface *xch, uint32_t domid, > >> xen_pfn_t start_pfn, xen_pfn_t nr_pfns) > >> { > >> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c > >> index a122e4a..3dd6e7c 100644 > >> --- a/tools/libxl/libxl_arm.c > >> +++ b/tools/libxl/libxl_arm.c > >> @@ -23,6 +23,18 @@ > >> #define DT_IRQ_TYPE_LEVEL_HIGH 0x00000004 > >> #define DT_IRQ_TYPE_LEVEL_LOW 0x00000008 > >> > >> +static const char *gicv_to_string(uint8_t gic_version) > >> +{ > >> + switch (gic_version) { > >> + case XEN_DOMCTL_CONFIG_GIC_V2: > >> + return "V2"; > >> + case XEN_DOMCTL_CONFIG_GIC_V3: > >> + return "V3"; > >> + default: > >> + return "unknown"; > >> + } > >> +} > >> + > >> int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, > >> uint32_t domid) > >> { > >> @@ -307,9 +319,9 @@ static int make_memory_nodes(libxl__gc *gc, void *fdt, > >> return 0; > >> } > >> > >> -static int make_intc_node(libxl__gc *gc, void *fdt, > >> - uint64_t gicd_base, uint64_t gicd_size, > >> - uint64_t gicc_base, uint64_t gicc_size) > >> +static int make_gicv2_node(libxl__gc *gc, void *fdt, > >> + uint64_t gicd_base, uint64_t gicd_size, > >> + uint64_t gicc_base, uint64_t gicc_size) > >> { > >> int res; > >> const char *name = GCSPRINTF("interrupt-controller@%"PRIx64, gicd_base); > >> @@ -350,6 +362,57 @@ static int make_intc_node(libxl__gc *gc, void *fdt, > >> return 0; > >> } > >> > >> +static int make_gicv3_node(libxl__gc *gc, void *fdt) > >> +{ > >> + int res; > >> + const uint64_t gicd_base = GUEST_GICV3_GICD_BASE; > >> + const uint64_t gicd_size = GUEST_GICV3_GICD_SIZE; > >> + const uint64_t gicr0_base = GUEST_GICV3_GICR0_BASE; > >> + const uint64_t gicr0_size = GUEST_GICV3_GICR0_SIZE; > >> + const char *name = GCSPRINTF("interrupt-controller@%"PRIx64, gicd_base); > >> + > >> + res = fdt_begin_node(fdt, name); > >> + if (res) return res; > >> + > >> + res = fdt_property_compat(gc, fdt, 1, > >> + "arm,gic-v3"); > >> + if (res) return res; > >> + > >> + res = fdt_property_cell(fdt, "#interrupt-cells", 3); > >> + if (res) return res; > >> + > >> + res = fdt_property_cell(fdt, "#address-cells", 0); > >> + if (res) return res; > >> + > >> + res = fdt_property(fdt, "interrupt-controller", NULL, 0); > >> + if (res) return res; > >> + > >> + res = fdt_property_cell(fdt, "redistributor-stride", > >> + GUEST_GICV3_RDIST_STRIDE); > >> + if (res) return res; > >> + > >> + res = fdt_property_cell(fdt, "#redistributor-regions", > >> + GUEST_GICV3_RDIST_REGIONS); > >> + if (res) return res; > >> + > >> + res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS, > >> + 2, > >> + gicd_base, gicd_size, > >> + gicr0_base, gicr0_size); > >> + if (res) return res; > >> + > >> + res = fdt_property_cell(fdt, "linux,phandle", PHANDLE_GIC); > >> + if (res) return res; > >> + > >> + res = fdt_property_cell(fdt, "phandle", PHANDLE_GIC); > >> + if (res) return res; > >> + > >> + res = fdt_end_node(fdt); > >> + if (res) return res; > >> + > >> + return 0; > >> +} > >> + > >> static int make_timer_node(libxl__gc *gc, void *fdt, const struct arch_info *ainfo) > >> { > >> int res; > >> @@ -456,6 +519,7 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, > >> libxl_domain_build_info *info, > >> struct xc_dom_image *dom) > >> { > >> + xc_domain_configuration_t config; > >> void *fdt = NULL; > >> int rc, res; > >> size_t fdt_size = 0; > >> @@ -471,8 +535,16 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, > >> ainfo = get_arch_info(gc, dom); > >> if (ainfo == NULL) return ERROR_FAIL; > >> > >> + LOG(DEBUG, "configure the domain"); > >> + config.gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT; > >> + if (xc_domain_configure(CTX->xch, dom->guest_domid, &config) != 0) { > >> + LOG(ERROR, "counldn't configure the domain"); > >> + return ERROR_FAIL; > >> + } > >> + > >> LOG(DEBUG, "constructing DTB for Xen version %d.%d guest", > >> vers->xen_version_major, vers->xen_version_minor); > >> + LOG(DEBUG, " - vGIC version: %s", gicv_to_string(config.gic_version)); > >> > >> /* > >> * Call "call" handling FDR_ERR_*. Will either: > >> @@ -520,9 +592,22 @@ next_resize: > >> FDT( make_psci_node(gc, fdt) ); > >> > >> FDT( make_memory_nodes(gc, fdt, dom) ); > >> - FDT( make_intc_node(gc, fdt, > >> - GUEST_GICD_BASE, GUEST_GICD_SIZE, > >> - GUEST_GICC_BASE, GUEST_GICD_SIZE) ); > >> + > >> + switch (config.gic_version) { > >> + case XEN_DOMCTL_CONFIG_GIC_V2: > >> + FDT( make_gicv2_node(gc, fdt, > >> + GUEST_GICD_BASE, GUEST_GICD_SIZE, > >> + GUEST_GICC_BASE, GUEST_GICC_SIZE) ); > >> + break; > >> + case XEN_DOMCTL_CONFIG_GIC_V3: > >> + FDT( make_gicv3_node(gc, fdt) ); > >> + break; > >> + default: > >> + LOG(ERROR, "Unknown how to create the DT node for gic_version = %d", > >> + config.gic_version); > >> + rc = ERROR_FAIL; > >> + goto out; > >> + } > >> > >> FDT( make_timer_node(gc, fdt, ainfo) ); > >> FDT( make_hypervisor_node(gc, fdt, vers) ); > >> diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c > >> index 45974e7..5b8ff57 100644 > >> --- a/xen/arch/arm/domctl.c > >> +++ b/xen/arch/arm/domctl.c > >> @@ -10,6 +10,8 @@ > >> #include <xen/errno.h> > >> #include <xen/sched.h> > >> #include <xen/hypercall.h> > >> +#include <asm/gic.h> > >> +#include <xen/guest_access.h> > >> #include <public/domctl.h> > >> > >> long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, > >> @@ -30,6 +32,39 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, > >> > >> return p2m_cache_flush(d, s, e); > >> } > >> + case XEN_DOMCTL_arm_configure_domain: > >> + { > >> + uint8_t gic_version; > >> + > >> + /* > >> + * Xen 4.5: The vGIC is emulating the same version of the > >> + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT > >> + * is allowed. The DOMCTL will return the actual version of the > >> + * GIC. > >> + */ > >> + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) > >> + return -EOPNOTSUPP; > >> + > >> + switch ( gic_hw_version() ) > >> + { > >> + case GIC_V3: > >> + gic_version = XEN_DOMCTL_CONFIG_GIC_V3; > >> + break; > >> + case GIC_V2: > >> + gic_version = XEN_DOMCTL_CONFIG_GIC_V2; > >> + break; > >> + default: > >> + BUG(); > >> + } > >> + > >> + domctl->u.configuredomain.gic_version = gic_version; > >> + > >> + /* TODO: Make the copy generic for all ARCH domctl */ > >> + if ( __copy_to_guest(u_domctl, domctl, 1) ) > >> + return -EFAULT; > >> + > >> + return 0; > >> + } > >> > >> default: > >> return subarch_do_domctl(domctl, d, u_domctl); > >> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > >> index 91161a2..076aa62 100644 > >> --- a/xen/arch/arm/gic-v3.c > >> +++ b/xen/arch/arm/gic-v3.c > >> @@ -906,7 +906,21 @@ static int gicv_v3_init(struct domain *d) > >> d->arch.vgic.rdist_count = gicv3.rdist_count; > >> } > >> else > >> - d->arch.vgic.dbase = GUEST_GICD_BASE; > >> + { > >> + d->arch.vgic.dbase = GUEST_GICV3_GICD_BASE; > >> + d->arch.vgic.dbase_size = GUEST_GICV3_GICD_SIZE; > >> + > >> + /* XXX: Only one Re-distributor region mapped for the guest */ > >> + BUILD_BUG_ON(GUEST_GICV3_RDIST_REGIONS != 1); > >> + > >> + d->arch.vgic.rdist_count = GUEST_GICV3_RDIST_REGIONS; > >> + d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE; > >> + > >> + /* The first redistributor should contain enough space for all CPUs */ > >> + BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < MAX_VIRT_CPUS); > >> + d->arch.vgic.rbase[0] = GUEST_GICV3_GICR0_BASE; > >> + d->arch.vgic.rbase_size[0] = GUEST_GICV3_GICR0_SIZE; > >> + } > >> > >> d->arch.vgic.nr_lines = 0; > >> > >> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > >> index eecc561..d7df274 100644 > >> --- a/xen/include/public/arch-arm.h > >> +++ b/xen/include/public/arch-arm.h > >> @@ -373,11 +373,27 @@ typedef uint64_t xen_callback_t; > >> */ > >> > >> /* Physical Address Space */ > >> + > >> +/* vGIC mappings: Only one set of mapping is used by the guest. > >> + * Therefore they can overlap. > >> + */ > >> + > >> +/* vGIC v2 mappings */ > >> #define GUEST_GICD_BASE 0x03001000ULL > >> #define GUEST_GICD_SIZE 0x00001000ULL > >> #define GUEST_GICC_BASE 0x03002000ULL > >> #define GUEST_GICC_SIZE 0x00000100ULL > >> > >> +/* vGIC v3 mappings */ > >> +#define GUEST_GICV3_GICD_BASE 0x03001000ULL > >> +#define GUEST_GICV3_GICD_SIZE 0x00010000ULL > >> + > >> +#define GUEST_GICV3_RDIST_STRIDE 0x20000ULL > >> +#define GUEST_GICV3_RDIST_REGIONS 1 > >> + > >> +#define GUEST_GICV3_GICR0_BASE 0x03020000ULL /* vCPU0 - vCPU15 */ > >> +#define GUEST_GICV3_GICR0_SIZE 0x00200000ULL > >> + > >> /* 16MB == 4096 pages reserved for guest to use as a region to map its > >> * grant table in. > >> */ > >> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > >> index 58b19e7..8da204e 100644 > >> --- a/xen/include/public/domctl.h > >> +++ b/xen/include/public/domctl.h > >> @@ -68,6 +68,19 @@ struct xen_domctl_createdomain { > >> typedef struct xen_domctl_createdomain xen_domctl_createdomain_t; > >> DEFINE_XEN_GUEST_HANDLE(xen_domctl_createdomain_t); > >> > >> +#if defined(__arm__) || defined(__aarch64__) > >> +#define XEN_DOMCTL_CONFIG_GIC_DEFAULT 0 > >> +#define XEN_DOMCTL_CONFIG_GIC_V2 1 > >> +#define XEN_DOMCTL_CONFIG_GIC_V3 2 > >> +/* XEN_DOMCTL_configure_domain */ > >> +struct xen_domctl_arm_configuredomain { > >> + /* IN/OUT parameters */ > >> + uint8_t gic_version; > >> +}; > >> +typedef struct xen_domctl_arm_configuredomain xen_domctl_arm_configuredomain_t; > >> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_arm_configuredomain_t); > >> +#endif > >> + > >> /* XEN_DOMCTL_getdomaininfo */ > >> struct xen_domctl_getdomaininfo { > >> /* OUT variables. */ > >> @@ -1056,6 +1069,7 @@ struct xen_domctl { > >> #define XEN_DOMCTL_set_vcpu_msrs 73 > >> #define XEN_DOMCTL_setvnumainfo 74 > >> #define XEN_DOMCTL_psr_cmt_op 75 > >> +#define XEN_DOMCTL_arm_configure_domain 76 > >> #define XEN_DOMCTL_gdbsx_guestmemio 1000 > >> #define XEN_DOMCTL_gdbsx_pausevcpu 1001 > >> #define XEN_DOMCTL_gdbsx_unpausevcpu 1002 > >> @@ -1064,6 +1078,9 @@ struct xen_domctl { > >> domid_t domain; > >> union { > >> struct xen_domctl_createdomain createdomain; > >> +#if defined(__arm__) || defined(__aarch64__) > >> + struct xen_domctl_arm_configuredomain configuredomain; > >> +#endif > >> struct xen_domctl_getdomaininfo getdomaininfo; > >> struct xen_domctl_getmemlist getmemlist; > >> struct xen_domctl_getpageframeinfo getpageframeinfo; > >> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c > >> index 6d0fe72..846cf88 100644 > >> --- a/xen/xsm/flask/hooks.c > >> +++ b/xen/xsm/flask/hooks.c > >> @@ -727,6 +727,9 @@ static int flask_domctl(struct domain *d, int cmd) > >> case XEN_DOMCTL_psr_cmt_op: > >> return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_CMT_OP); > >> > >> + case XEN_DOMCTL_configure_domain: > >> + return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__CONFIGURE_DOMAIN); > >> + > >> default: > >> printk("flask_domctl: Unknown op %d\n", cmd); > >> return -EPERM; > >> diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors > >> index de0c707..bfe2fa5 100644 > >> --- a/xen/xsm/flask/policy/access_vectors > >> +++ b/xen/xsm/flask/policy/access_vectors > >> @@ -216,6 +216,8 @@ class domain2 > >> get_vnumainfo > >> # XEN_DOMCTL_psr_cmt_op > >> psr_cmt_op > >> +# XEN_DOMCTL_configure_domain > >> + configure_domain > >> } > >> > >> # Similar to class domain, but primarily contains domctls related to HVM domains > >> -- > >> 1.7.10.4 > >> > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On 11/01/2014 04:10 PM, Julien Grall wrote: > The vGIC will emulate the same version as the hardware. The toolstack has > to retrieve the version of the vGIC in order to be able to create the > corresponding device tree node. > > A new DOMCTL has been introduced for ARM to configure the domain. For now > it only allow the toolstack to retrieve the version of vGIC. > This DOMCTL will be extend later to let the user choose the version of the > emulated GIC. > > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> > Signed-off-by: Julien Grall <julien.grall@linaro.org> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Cc: Wei Liu <wei.liu2@citrix.com> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Hi Konrad, On 11/03/2014 04:39 PM, Konrad Rzeszutek Wilk wrote: > On Mon, Nov 03, 2014 at 06:07:33PM +0530, Vijay Kilari wrote: >> On Mon, Nov 3, 2014 at 4:31 PM, Stefano Stabellini >> <stefano.stabellini@eu.citrix.com> wrote: >>> On Sat, 1 Nov 2014, Julien Grall wrote: >>>> The vGIC will emulate the same version as the hardware. The toolstack has >>>> to retrieve the version of the vGIC in order to be able to create the >>>> corresponding device tree node. >>>> >>>> A new DOMCTL has been introduced for ARM to configure the domain. For now >>>> it only allow the toolstack to retrieve the version of vGIC. >>>> This DOMCTL will be extend later to let the user choose the version of the >>>> emulated GIC. >>>> >>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> >>>> Signed-off-by: Julien Grall <julien.grall@linaro.org> >>>> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >>>> Cc: Jan Beulich <jbeulich@suse.com> >>>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> >>>> Cc: Wei Liu <wei.liu2@citrix.com> >>> >>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> >> Tested with GICv3 platform. > > Thank you for doing that. > > It also needs Acks from Daniel and Jan. This patch doesn't modify the x86 part. So I'm not sure if Jan ack is required. Would Ian C. ack be enough? Anyway, Jan do you have any objection on this patch? Regards,
On Sat, 2014-11-01 at 20:10 +0000, Julien Grall wrote: > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > index a9bcd4a..1aa1f45 100644 > --- a/tools/libxc/xc_domain.c > +++ b/tools/libxc/xc_domain.c > @@ -48,6 +48,26 @@ int xc_domain_create(xc_interface *xch, > return 0; > } > > +#if defined(__arm__) || defined(__aarch64__) > +int xc_domain_configure(xc_interface *xch, uint32_t domid, > + xc_domain_configuration_t *config) > +{ > + int rc; > + DECLARE_DOMCTL; > + > + domctl.cmd = XEN_DOMCTL_arm_configure_domain; > + domctl.domain = (domid_t)domid; > + /* xc_domain_configure_t is an alias to xen_domctl_arm_configuredomain */ "is an alias of". > + > + res = fdt_property_compat(gc, fdt, 1, > + "arm,gic-v3"); Doesn't need wrapping I expect. > static int make_timer_node(libxl__gc *gc, void *fdt, const struct arch_info *ainfo) > { > int res; > @@ -456,6 +519,7 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, > libxl_domain_build_info *info, > struct xc_dom_image *dom) > { > + xc_domain_configuration_t config; > void *fdt = NULL; > int rc, res; > size_t fdt_size = 0; > @@ -471,8 +535,16 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, > ainfo = get_arch_info(gc, dom); > if (ainfo == NULL) return ERROR_FAIL; > > + LOG(DEBUG, "configure the domain"); > + config.gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT; > + if (xc_domain_configure(CTX->xch, dom->guest_domid, &config) != 0) { > + LOG(ERROR, "counldn't configure the domain"); "couldn't" > @@ -520,9 +592,22 @@ next_resize: > FDT( make_psci_node(gc, fdt) ); > > FDT( make_memory_nodes(gc, fdt, dom) ); > - FDT( make_intc_node(gc, fdt, > - GUEST_GICD_BASE, GUEST_GICD_SIZE, > - GUEST_GICC_BASE, GUEST_GICD_SIZE) ); > + > + switch (config.gic_version) { > + case XEN_DOMCTL_CONFIG_GIC_V2: > + FDT( make_gicv2_node(gc, fdt, > + GUEST_GICD_BASE, GUEST_GICD_SIZE, > + GUEST_GICC_BASE, GUEST_GICC_SIZE) ); > + break; > + case XEN_DOMCTL_CONFIG_GIC_V3: > + FDT( make_gicv3_node(gc, fdt) ); > + break; > + default: > + LOG(ERROR, "Unknown how to create the DT node for gic_version = %d", > + config.gic_version); "Don't know how..." or even just "Unknown GIC version %d". > @@ -30,6 +32,39 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, > > return p2m_cache_flush(d, s, e); > } > + case XEN_DOMCTL_arm_configure_domain: > + { > + uint8_t gic_version; > + > + /* > + * Xen 4.5: The vGIC is emulating the same version of the No need to say "Xen 4.5" here, this comment is true until it is removed. You could say "Currently the vGIC is..." or something if you wanted to indicate that this is temporary. > + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT > + * is allowed. The DOMCTL will return the actual version of the > + * GIC. > + */ > + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) > + return -EOPNOTSUPP; EOPNOTSUPP doesn't seem quite right. EPARM or EINVAL perhaps? I'm also tempted to suggest that we should accept gic_version == the hw value, i.e. by moping the switch below up and including && domctl->u.configuredomain.gic_version != gic_version in the condition. > +#define GUEST_GICV3_GICR0_BASE 0x03020000ULL /* vCPU0 - vCPU15 */ Don't we only support 8 cpus today? I don't mind reserving space for more, and I'd be pleasantly surprised if 16 actually worked ;-) Ian.
On Tue, 2014-11-04 at 17:26 +0000, Julien Grall wrote: > Hi Konrad, > > On 11/03/2014 04:39 PM, Konrad Rzeszutek Wilk wrote: > > On Mon, Nov 03, 2014 at 06:07:33PM +0530, Vijay Kilari wrote: > >> On Mon, Nov 3, 2014 at 4:31 PM, Stefano Stabellini > >> <stefano.stabellini@eu.citrix.com> wrote: > >>> On Sat, 1 Nov 2014, Julien Grall wrote: > >>>> The vGIC will emulate the same version as the hardware. The toolstack has > >>>> to retrieve the version of the vGIC in order to be able to create the > >>>> corresponding device tree node. > >>>> > >>>> A new DOMCTL has been introduced for ARM to configure the domain. For now > >>>> it only allow the toolstack to retrieve the version of vGIC. > >>>> This DOMCTL will be extend later to let the user choose the version of the > >>>> emulated GIC. > >>>> > >>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> > >>>> Signed-off-by: Julien Grall <julien.grall@linaro.org> > >>>> Cc: Ian Jackson <ian.jackson@eu.citrix.com> > >>>> Cc: Jan Beulich <jbeulich@suse.com> > >>>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> > >>>> Cc: Wei Liu <wei.liu2@citrix.com> > >>> > >>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >> > >> Tested with GICv3 platform. > > > > Thank you for doing that. > > > > It also needs Acks from Daniel and Jan. > > This patch doesn't modify the x86 part. So I'm not sure if Jan ack is > required. Would Ian C. ack be enough? Jan had comments on a previous iteration and adding a new domctl (or hypercalls generally) could do with eyes from all arches even if they are currently arch specific, so it would be good to know that he at least doesn't object. > Anyway, Jan do you have any objection on this patch? > > Regards, >
On Wed, 5 Nov 2014, Ian Campbell wrote: > > + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT > > + * is allowed. The DOMCTL will return the actual version of the > > + * GIC. > > + */ > > + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) > > + return -EOPNOTSUPP; > > EOPNOTSUPP doesn't seem quite right. EPARM or EINVAL perhaps? > > I'm also tempted to suggest that we should accept gic_version == the hw > value, i.e. by moping the switch below up and including > && domctl->u.configuredomain.gic_version != gic_version > in the condition. I suggested to use -EOPNOTSUPP because one day we want to be able to use this hypercall to choose a specific gic_version for the guest domain, including gicv2 on gicv3 hardware for example.
On Wed, 2014-11-05 at 10:31 +0000, Stefano Stabellini wrote: > On Wed, 5 Nov 2014, Ian Campbell wrote: > > > + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT > > > + * is allowed. The DOMCTL will return the actual version of the > > > + * GIC. > > > + */ > > > + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) > > > + return -EOPNOTSUPP; > > > > EOPNOTSUPP doesn't seem quite right. EPARM or EINVAL perhaps? > > > > I'm also tempted to suggest that we should accept gic_version == the hw > > value, i.e. by moping the switch below up and including > > && domctl->u.configuredomain.gic_version != gic_version > > in the condition. > > I suggested to use -EOPNOTSUPP because one day we want to be able to use > this hypercall to choose a specific gic_version for the guest domain, > including gicv2 on gicv3 hardware for example. Why is EOPNOTSUPP an appropriate error code for that though? Ian.
On Wed, 5 Nov 2014, Ian Campbell wrote: > On Wed, 2014-11-05 at 10:31 +0000, Stefano Stabellini wrote: > > On Wed, 5 Nov 2014, Ian Campbell wrote: > > > > + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT > > > > + * is allowed. The DOMCTL will return the actual version of the > > > > + * GIC. > > > > + */ > > > > + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) > > > > + return -EOPNOTSUPP; > > > > > > EOPNOTSUPP doesn't seem quite right. EPARM or EINVAL perhaps? > > > > > > I'm also tempted to suggest that we should accept gic_version == the hw > > > value, i.e. by moping the switch below up and including > > > && domctl->u.configuredomain.gic_version != gic_version > > > in the condition. > > > > I suggested to use -EOPNOTSUPP because one day we want to be able to use > > this hypercall to choose a specific gic_version for the guest domain, > > including gicv2 on gicv3 hardware for example. > > Why is EOPNOTSUPP an appropriate error code for that though? Because at the moment we don't support this use case? If we did we would be OK with the user passing gic_version = 2 for example.
Hi Ian, On 05/11/2014 10:24, Ian Campbell wrote: >> @@ -30,6 +32,39 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, >> >> return p2m_cache_flush(d, s, e); >> } >> + case XEN_DOMCTL_arm_configure_domain: >> + { >> + uint8_t gic_version; >> + >> + /* >> + * Xen 4.5: The vGIC is emulating the same version of the > > No need to say "Xen 4.5" here, this comment is true until it is removed. > You could say "Currently the vGIC is..." or something if you wanted to > indicate that this is temporary. Just saying temporary doesn't say anything about when this has been added and will be removed. "Xen 4.5" gives explicitly the time and show that we want to remove soon. > >> + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT >> + * is allowed. The DOMCTL will return the actual version of the >> + * GIC. >> + */ >> + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) >> + return -EOPNOTSUPP; > I'm also tempted to suggest that we should accept gic_version == the hw > value, i.e. by moping the switch below up and including > && domctl->u.configuredomain.gic_version != gic_version > in the condition. I consider that explicitly asking for a version of the GIC in Xen 4.5 is invalid. The user should only be able to use the default GIC. As the DOMCTL is not set in stone, the developer should not start to consider that XEN_DOMCTL_CONFIG_GIC_V{2,3} will be valid in the future. >> +#define GUEST_GICV3_GICR0_BASE 0x03020000ULL /* vCPU0 - vCPU15 */ > > Don't we only support 8 cpus today? I don't mind reserving space for > more, and I'd be pleasantly surprised if 16 actually worked ;-) I just copied used the code of Vijay. I suspect we could easily bump the number supported VCPUs to 16. But I didn't test it. Regards,
On Wed, 2014-11-05 at 11:02 +0000, Stefano Stabellini wrote: > On Wed, 5 Nov 2014, Ian Campbell wrote: > > On Wed, 2014-11-05 at 10:31 +0000, Stefano Stabellini wrote: > > > On Wed, 5 Nov 2014, Ian Campbell wrote: > > > > > + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT > > > > > + * is allowed. The DOMCTL will return the actual version of the > > > > > + * GIC. > > > > > + */ > > > > > + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) > > > > > + return -EOPNOTSUPP; > > > > > > > > EOPNOTSUPP doesn't seem quite right. EPARM or EINVAL perhaps? > > > > > > > > I'm also tempted to suggest that we should accept gic_version == the hw > > > > value, i.e. by moping the switch below up and including > > > > && domctl->u.configuredomain.gic_version != gic_version > > > > in the condition. > > > > > > I suggested to use -EOPNOTSUPP because one day we want to be able to use > > > this hypercall to choose a specific gic_version for the guest domain, > > > including gicv2 on gicv3 hardware for example. > > > > Why is EOPNOTSUPP an appropriate error code for that though? > > Because at the moment we don't support this use case? If we did we would > be OK with the user passing gic_version = 2 for example. Ah, I suggested separately in the review we should just accept the g/w version. Ian.
On Wed, 2014-11-05 at 11:10 +0000, Julien Grall wrote: > Hi Ian, > > On 05/11/2014 10:24, Ian Campbell wrote: > >> @@ -30,6 +32,39 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, > >> > >> return p2m_cache_flush(d, s, e); > >> } > >> + case XEN_DOMCTL_arm_configure_domain: > >> + { > >> + uint8_t gic_version; > >> + > >> + /* > >> + * Xen 4.5: The vGIC is emulating the same version of the > > > > No need to say "Xen 4.5" here, this comment is true until it is removed. > > You could say "Currently the vGIC is..." or something if you wanted to > > indicate that this is temporary. > > Just saying temporary doesn't say anything about when this has been > added and will be removed. > > "Xen 4.5" gives explicitly the time and show that we want to remove soon. Neither of which things belong in the code comment IMHO. Either this happens soon so it is moot, or it doesn't and we have a strange misleading comment... > > > >> + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT > >> + * is allowed. The DOMCTL will return the actual version of the > >> + * GIC. > >> + */ > >> + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) > >> + return -EOPNOTSUPP; > > > I'm also tempted to suggest that we should accept gic_version == the hw > > value, i.e. by moping the switch below up and including > > && domctl->u.configuredomain.gic_version != gic_version > > in the condition. > > I consider that explicitly asking for a version of the GIC in Xen 4.5 is > invalid. The user should only be able to use the default GIC. > > As the DOMCTL is not set in stone, I disagree with your reasoning, but given this I'm not inclined to keep arguing about it. Ian.
On 05/11/2014 11:22, Ian Campbell wrote: >>> >>>> + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT >>>> + * is allowed. The DOMCTL will return the actual version of the >>>> + * GIC. >>>> + */ >>>> + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) >>>> + return -EOPNOTSUPP; >> >>> I'm also tempted to suggest that we should accept gic_version == the hw >>> value, i.e. by moping the switch below up and including >>> && domctl->u.configuredomain.gic_version != gic_version >>> in the condition. >> >> I consider that explicitly asking for a version of the GIC in Xen 4.5 is >> invalid. The user should only be able to use the default GIC. >> >> As the DOMCTL is not set in stone, > > I disagree with your reasoning, but given this I'm not inclined to keep > arguing about it. TBH, I should just ignore the value for Xen 4.5 and let the hypervisor return the version of the vGIC. The check was just here to explicitly show that we don't support anything else in DOM0. Regards,
>>> Julien Grall <julien.grall@linaro.org> 11/04/14 6:27 PM >>> >On 11/03/2014 04:39 PM, Konrad Rzeszutek Wilk wrote: >> It also needs Acks from Daniel and Jan. > >This patch doesn't modify the x86 part. So I'm not sure if Jan ack is >required. Would Ian C. ack be enough? Yes, it would. >Anyway, Jan do you have any objection on this patch? As said previously, I'm not particularly happy about it, but I also don't strongly mind it going in in the current shape. Jan
Hi Jan, On 05/11/2014 17:15, Jan Beulich wrote: >>>> Julien Grall <julien.grall@linaro.org> 11/04/14 6:27 PM >>> >> On 11/03/2014 04:39 PM, Konrad Rzeszutek Wilk wrote: >>> It also needs Acks from Daniel and Jan. >> >> This patch doesn't modify the x86 part. So I'm not sure if Jan ack is >> required. Would Ian C. ack be enough? > > Yes, it would. > >> Anyway, Jan do you have any objection on this patch? > > As said previously, I'm not particularly happy about it, but I also don't strongly > mind it going in in the current shape. May I ask what is wrong with the new approach to the a DOMCTL in this patch? The DOMCTL has been clearly identify as arm specific (there is "arm" in the name). Therefore it doesn't seem necessary to expose it for other architecture than ARM32 and ARM64. Regards,
>>> On 06.11.14 at 10:45, <julien.grall@linaro.org> wrote: > Hi Jan, > > On 05/11/2014 17:15, Jan Beulich wrote: >>>>> Julien Grall <julien.grall@linaro.org> 11/04/14 6:27 PM >>> >>> On 11/03/2014 04:39 PM, Konrad Rzeszutek Wilk wrote: >>>> It also needs Acks from Daniel and Jan. >>> >>> This patch doesn't modify the x86 part. So I'm not sure if Jan ack is >>> required. Would Ian C. ack be enough? >> >> Yes, it would. >> >>> Anyway, Jan do you have any objection on this patch? >> >> As said previously, I'm not particularly happy about it, but I also don't > strongly >> mind it going in in the current shape. > > May I ask what is wrong with the new approach to the a DOMCTL in this patch? > > The DOMCTL has been clearly identify as arm specific (there is "arm" in > the name). Therefore it doesn't seem necessary to expose it for other > architecture than ARM32 and ARM64. I didn't say there's anything actively wrong with it, all I said is that I'm not particularly happy about it: Irrespective of its name it doesn't look to be really arch-specific in the long run, plus it feels like the data being set here should rather be specified right at domain creation, or via a mechanism similar to x86'es HVM parameters (iirc the value set here can't be changed once the domain got first unpaused). Jan
On 06/11/2014 10:11, Jan Beulich wrote: >>>> On 06.11.14 at 10:45, <julien.grall@linaro.org> wrote: >> Hi Jan, >> >> On 05/11/2014 17:15, Jan Beulich wrote: >>>>>> Julien Grall <julien.grall@linaro.org> 11/04/14 6:27 PM >>> >>>> On 11/03/2014 04:39 PM, Konrad Rzeszutek Wilk wrote: >>>>> It also needs Acks from Daniel and Jan. >>>> >>>> This patch doesn't modify the x86 part. So I'm not sure if Jan ack is >>>> required. Would Ian C. ack be enough? >>> >>> Yes, it would. >>> >>>> Anyway, Jan do you have any objection on this patch? >>> >>> As said previously, I'm not particularly happy about it, but I also don't >> strongly >>> mind it going in in the current shape. >> >> May I ask what is wrong with the new approach to the a DOMCTL in this patch? >> >> The DOMCTL has been clearly identify as arm specific (there is "arm" in >> the name). Therefore it doesn't seem necessary to expose it for other >> architecture than ARM32 and ARM64. > > I didn't say there's anything actively wrong with it, all I said is that > I'm not particularly happy about it: Irrespective of its name it doesn't > look to be really arch-specific in the long run, plus it feels like the > data being set here should rather be specified right at domain > creation, or via a mechanism similar to x86'es HVM parameters (iirc > the value set here can't be changed once the domain got first > unpaused). TBH I choose this solution because I though you would be disagree with extending the domain create hypercall. In long run, there will be more parameters such as the number of spis. All will be required at the same time. So the HVM parameters solution won't really help here. However, I could give a look to extending the domain creation hypercall (xen_domctl_createdomain) to add arch specific field. But I don't think it's Xen 4.5 material. So I would prefer to stay on this approach for this release because we'd like to have GICv3 guest support on Xen. Though I could rename the DOMCTL to DOMCTL_get_gic_version. Regards,
On Thu, Nov 06, 2014 at 10:27:41AM +0000, Julien Grall wrote: > > > On 06/11/2014 10:11, Jan Beulich wrote: > >>>>On 06.11.14 at 10:45, <julien.grall@linaro.org> wrote: > >>Hi Jan, > >> > >>On 05/11/2014 17:15, Jan Beulich wrote: > >>>>>>Julien Grall <julien.grall@linaro.org> 11/04/14 6:27 PM >>> > >>>>On 11/03/2014 04:39 PM, Konrad Rzeszutek Wilk wrote: > >>>>>It also needs Acks from Daniel and Jan. > >>>> > >>>>This patch doesn't modify the x86 part. So I'm not sure if Jan ack is > >>>>required. Would Ian C. ack be enough? > >>> > >>>Yes, it would. > >>> > >>>>Anyway, Jan do you have any objection on this patch? > >>> > >>>As said previously, I'm not particularly happy about it, but I also don't > >>strongly > >>>mind it going in in the current shape. > >> > >>May I ask what is wrong with the new approach to the a DOMCTL in this patch? > >> > >>The DOMCTL has been clearly identify as arm specific (there is "arm" in > >>the name). Therefore it doesn't seem necessary to expose it for other > >>architecture than ARM32 and ARM64. > > > >I didn't say there's anything actively wrong with it, all I said is that > >I'm not particularly happy about it: Irrespective of its name it doesn't > >look to be really arch-specific in the long run, plus it feels like the > >data being set here should rather be specified right at domain > >creation, or via a mechanism similar to x86'es HVM parameters (iirc > >the value set here can't be changed once the domain got first > >unpaused). > > > TBH I choose this solution because I though you would be disagree with > extending the domain create hypercall. > > In long run, there will be more parameters such as the number of spis. All > will be required at the same time. So the HVM parameters solution won't > really help here. > > However, I could give a look to extending the domain creation hypercall > (xen_domctl_createdomain) to add arch specific field. > > But I don't think it's Xen 4.5 material. So I would prefer to stay on this > approach for this release because we'd like to have GICv3 guest support on > Xen. Though I could rename the DOMCTL to DOMCTL_get_gic_version. That is a bit unfortunate as it sounds like that for Xen 4.6 you will then ditch this hypercall and focus on the new one? Won't this one then be bitrotten? > > Regards, > > -- > Julien Grall
Hi Konrad, On 07/11/2014 15:45, Konrad Rzeszutek Wilk wrote: > On Thu, Nov 06, 2014 at 10:27:41AM +0000, Julien Grall wrote: >> >> >> On 06/11/2014 10:11, Jan Beulich wrote: >>>>>> On 06.11.14 at 10:45, <julien.grall@linaro.org> wrote: >>>> Hi Jan, >>>> >>>> On 05/11/2014 17:15, Jan Beulich wrote: >>>>>>>> Julien Grall <julien.grall@linaro.org> 11/04/14 6:27 PM >>> >>>>>> On 11/03/2014 04:39 PM, Konrad Rzeszutek Wilk wrote: >>>>>>> It also needs Acks from Daniel and Jan. >>>>>> >>>>>> This patch doesn't modify the x86 part. So I'm not sure if Jan ack is >>>>>> required. Would Ian C. ack be enough? >>>>> >>>>> Yes, it would. >>>>> >>>>>> Anyway, Jan do you have any objection on this patch? >>>>> >>>>> As said previously, I'm not particularly happy about it, but I also don't >>>> strongly >>>>> mind it going in in the current shape. >>>> >>>> May I ask what is wrong with the new approach to the a DOMCTL in this patch? >>>> >>>> The DOMCTL has been clearly identify as arm specific (there is "arm" in >>>> the name). Therefore it doesn't seem necessary to expose it for other >>>> architecture than ARM32 and ARM64. >>> >>> I didn't say there's anything actively wrong with it, all I said is that >>> I'm not particularly happy about it: Irrespective of its name it doesn't >>> look to be really arch-specific in the long run, plus it feels like the >>> data being set here should rather be specified right at domain >>> creation, or via a mechanism similar to x86'es HVM parameters (iirc >>> the value set here can't be changed once the domain got first >>> unpaused). >> >> >> TBH I choose this solution because I though you would be disagree with >> extending the domain create hypercall. >> >> In long run, there will be more parameters such as the number of spis. All >> will be required at the same time. So the HVM parameters solution won't >> really help here. >> >> However, I could give a look to extending the domain creation hypercall >> (xen_domctl_createdomain) to add arch specific field. >> >> But I don't think it's Xen 4.5 material. So I would prefer to stay on this >> approach for this release because we'd like to have GICv3 guest support on >> Xen. Though I could rename the DOMCTL to DOMCTL_get_gic_version. > > That is a bit unfortunate as it sounds like that for Xen 4.6 you will > then ditch this hypercall and focus on the new one? Won't this one then > be bitrotten? Unfortunately yes. Modifying the xen_domctl_createdomain hypercall at this time of the release is not safe. Even though I like challenge, I don't want do be blamed because of a bug that would slip the release date. But, the DOMCTL interface is not considered as a stable ABI. I guess we could kill this hypercall in Xen 4.6. Regards,
On November 8, 2014 1:01:10 PM EST, Julien Grall <julien.grall@linaro.org> wrote: >Hi Konrad, > >On 07/11/2014 15:45, Konrad Rzeszutek Wilk wrote: >> On Thu, Nov 06, 2014 at 10:27:41AM +0000, Julien Grall wrote: >>> >>> >>> On 06/11/2014 10:11, Jan Beulich wrote: >>>>>>> On 06.11.14 at 10:45, <julien.grall@linaro.org> wrote: >>>>> Hi Jan, >>>>> >>>>> On 05/11/2014 17:15, Jan Beulich wrote: >>>>>>>>> Julien Grall <julien.grall@linaro.org> 11/04/14 6:27 PM >>> >>>>>>> On 11/03/2014 04:39 PM, Konrad Rzeszutek Wilk wrote: >>>>>>>> It also needs Acks from Daniel and Jan. >>>>>>> >>>>>>> This patch doesn't modify the x86 part. So I'm not sure if Jan >ack is >>>>>>> required. Would Ian C. ack be enough? >>>>>> >>>>>> Yes, it would. >>>>>> >>>>>>> Anyway, Jan do you have any objection on this patch? >>>>>> >>>>>> As said previously, I'm not particularly happy about it, but I >also don't >>>>> strongly >>>>>> mind it going in in the current shape. >>>>> >>>>> May I ask what is wrong with the new approach to the a DOMCTL in >this patch? >>>>> >>>>> The DOMCTL has been clearly identify as arm specific (there is >"arm" in >>>>> the name). Therefore it doesn't seem necessary to expose it for >other >>>>> architecture than ARM32 and ARM64. >>>> >>>> I didn't say there's anything actively wrong with it, all I said is >that >>>> I'm not particularly happy about it: Irrespective of its name it >doesn't >>>> look to be really arch-specific in the long run, plus it feels like >the >>>> data being set here should rather be specified right at domain >>>> creation, or via a mechanism similar to x86'es HVM parameters (iirc >>>> the value set here can't be changed once the domain got first >>>> unpaused). >>> >>> >>> TBH I choose this solution because I though you would be disagree >with >>> extending the domain create hypercall. >>> >>> In long run, there will be more parameters such as the number of >spis. All >>> will be required at the same time. So the HVM parameters solution >won't >>> really help here. >>> >>> However, I could give a look to extending the domain creation >hypercall >>> (xen_domctl_createdomain) to add arch specific field. >>> >>> But I don't think it's Xen 4.5 material. So I would prefer to stay >on this >>> approach for this release because we'd like to have GICv3 guest >support on >>> Xen. Though I could rename the DOMCTL to DOMCTL_get_gic_version. >> >> That is a bit unfortunate as it sounds like that for Xen 4.6 you will >> then ditch this hypercall and focus on the new one? Won't this one >then >> be bitrotten? > >Unfortunately yes. Modifying the xen_domctl_createdomain hypercall at >this time of the release is not safe. Even though I like challenge, I >don't want do be blamed because of a bug that would slip the release >date. > <nods> >But, the DOMCTL interface is not considered as a stable ABI. I guess we >could kill this hypercall in Xen 4.6. If that is the intent we should really wrap all of the functionality (in userspace) with Xen v4.5 version check such that it is not exposed to earlier or newer versions of tools. That is to guard against somebody building against libxc or libxl and then becoming dependent on this and then complaining that it is not in Xen 4.6. Perhaps even add in headers a big warning that this is an band-aid! > >Regards,
On Sat, 2014-11-08 at 15:26 -0500, Konrad Rzeszutek Wilk wrote: > That is to guard against somebody building against libxc or libxl and > then becoming dependent on this and then complaining that it is not in > Xen 4.6. libxc does not have a stable API and libxl doesn't expose this interface at all. At the hypercall level this is a domctl which simliarly has no stable interface. So I don't think there is any need to wrap anything or guard against anything.
On Mon, 10 Nov 2014, Ian Campbell wrote: > On Sat, 2014-11-08 at 15:26 -0500, Konrad Rzeszutek Wilk wrote: > > That is to guard against somebody building against libxc or libxl and > > then becoming dependent on this and then complaining that it is not in > > Xen 4.6. > > libxc does not have a stable API and libxl doesn't expose this interface > at all. At the hypercall level this is a domctl which simliarly has no > stable interface. > > So I don't think there is any need to wrap anything or guard against > anything. That's true. A comment in the header file wouldn't hurt though.
Hi Stefano, On 10/11/2014 12:38, Stefano Stabellini wrote: > On Mon, 10 Nov 2014, Ian Campbell wrote: >> On Sat, 2014-11-08 at 15:26 -0500, Konrad Rzeszutek Wilk wrote: >>> That is to guard against somebody building against libxc or libxl and >>> then becoming dependent on this and then complaining that it is not in >>> Xen 4.6. >> >> libxc does not have a stable API and libxl doesn't expose this interface >> at all. At the hypercall level this is a domctl which simliarly has no >> stable interface. >> >> So I don't think there is any need to wrap anything or guard against >> anything. > > That's true. A comment in the header file wouldn't hurt though. It looks like that Ian has already pushed the patch. Do I need to send a follow-up? Regards,
On Tue, 11 Nov 2014, Julien Grall wrote: > Hi Stefano, > > On 10/11/2014 12:38, Stefano Stabellini wrote: > > On Mon, 10 Nov 2014, Ian Campbell wrote: > > > On Sat, 2014-11-08 at 15:26 -0500, Konrad Rzeszutek Wilk wrote: > > > > That is to guard against somebody building against libxc or libxl and > > > > then becoming dependent on this and then complaining that it is not in > > > > Xen 4.6. > > > > > > libxc does not have a stable API and libxl doesn't expose this interface > > > at all. At the hypercall level this is a domctl which simliarly has no > > > stable interface. > > > > > > So I don't think there is any need to wrap anything or guard against > > > anything. > > > > That's true. A comment in the header file wouldn't hurt though. > > It looks like that Ian has already pushed the patch. Do I need to send a > follow-up? No need for just a comment
On Tue, Nov 11, 2014 at 05:41:50PM +0100, Julien Grall wrote: > Hi Stefano, > > On 10/11/2014 12:38, Stefano Stabellini wrote: > >On Mon, 10 Nov 2014, Ian Campbell wrote: > >>On Sat, 2014-11-08 at 15:26 -0500, Konrad Rzeszutek Wilk wrote: > >>>That is to guard against somebody building against libxc or libxl and > >>>then becoming dependent on this and then complaining that it is not in > >>>Xen 4.6. > >> > >>libxc does not have a stable API and libxl doesn't expose this interface > >>at all. At the hypercall level this is a domctl which simliarly has no > >>stable interface. > >> > >>So I don't think there is any need to wrap anything or guard against > >>anything. > > > >That's true. A comment in the header file wouldn't hurt though. > > It looks like that Ian has already pushed the patch. Do I need to send a > follow-up? Nah, I just need to put it the Xen 4.6 todo list with your name on it :-) .. Done. > > Regards, > > -- > Julien Grall
diff --git a/tools/flask/policy/policy/modules/xen/xen.if b/tools/flask/policy/policy/modules/xen/xen.if index 641c797..fa69c9d 100644 --- a/tools/flask/policy/policy/modules/xen/xen.if +++ b/tools/flask/policy/policy/modules/xen/xen.if @@ -49,7 +49,7 @@ define(`create_domain_common', ` getdomaininfo hypercall setvcpucontext setextvcpucontext getscheduler getvcpuinfo getvcpuextstate getaddrsize getaffinity setaffinity }; - allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim set_max_evtchn set_vnumainfo get_vnumainfo psr_cmt_op }; + allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim set_max_evtchn set_vnumainfo get_vnumainfo psr_cmt_op configure_domain }; allow $1 $2:security check_context; allow $1 $2:shadow enable; allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage mmuext_op }; diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 564e187..45e282c 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -483,6 +483,12 @@ int xc_domain_create(xc_interface *xch, uint32_t flags, uint32_t *pdomid); +#if defined(__arm__) || defined(__aarch64__) +typedef xen_domctl_arm_configuredomain_t xc_domain_configuration_t; + +int xc_domain_configure(xc_interface *xch, uint32_t domid, + xc_domain_configuration_t *config); +#endif /* Functions to produce a dump of a given domain * xc_domain_dumpcore - produces a dump to a specified file diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index a9bcd4a..1aa1f45 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -48,6 +48,26 @@ int xc_domain_create(xc_interface *xch, return 0; } +#if defined(__arm__) || defined(__aarch64__) +int xc_domain_configure(xc_interface *xch, uint32_t domid, + xc_domain_configuration_t *config) +{ + int rc; + DECLARE_DOMCTL; + + domctl.cmd = XEN_DOMCTL_arm_configure_domain; + domctl.domain = (domid_t)domid; + /* xc_domain_configure_t is an alias to xen_domctl_arm_configuredomain */ + memcpy(&domctl.u.configuredomain, config, sizeof(*config)); + + rc = do_domctl(xch, &domctl); + if ( !rc ) + memcpy(config, &domctl.u.configuredomain, sizeof(*config)); + + return rc; +} +#endif + int xc_domain_cacheflush(xc_interface *xch, uint32_t domid, xen_pfn_t start_pfn, xen_pfn_t nr_pfns) { diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c index a122e4a..3dd6e7c 100644 --- a/tools/libxl/libxl_arm.c +++ b/tools/libxl/libxl_arm.c @@ -23,6 +23,18 @@ #define DT_IRQ_TYPE_LEVEL_HIGH 0x00000004 #define DT_IRQ_TYPE_LEVEL_LOW 0x00000008 +static const char *gicv_to_string(uint8_t gic_version) +{ + switch (gic_version) { + case XEN_DOMCTL_CONFIG_GIC_V2: + return "V2"; + case XEN_DOMCTL_CONFIG_GIC_V3: + return "V3"; + default: + return "unknown"; + } +} + int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, uint32_t domid) { @@ -307,9 +319,9 @@ static int make_memory_nodes(libxl__gc *gc, void *fdt, return 0; } -static int make_intc_node(libxl__gc *gc, void *fdt, - uint64_t gicd_base, uint64_t gicd_size, - uint64_t gicc_base, uint64_t gicc_size) +static int make_gicv2_node(libxl__gc *gc, void *fdt, + uint64_t gicd_base, uint64_t gicd_size, + uint64_t gicc_base, uint64_t gicc_size) { int res; const char *name = GCSPRINTF("interrupt-controller@%"PRIx64, gicd_base); @@ -350,6 +362,57 @@ static int make_intc_node(libxl__gc *gc, void *fdt, return 0; } +static int make_gicv3_node(libxl__gc *gc, void *fdt) +{ + int res; + const uint64_t gicd_base = GUEST_GICV3_GICD_BASE; + const uint64_t gicd_size = GUEST_GICV3_GICD_SIZE; + const uint64_t gicr0_base = GUEST_GICV3_GICR0_BASE; + const uint64_t gicr0_size = GUEST_GICV3_GICR0_SIZE; + const char *name = GCSPRINTF("interrupt-controller@%"PRIx64, gicd_base); + + res = fdt_begin_node(fdt, name); + if (res) return res; + + res = fdt_property_compat(gc, fdt, 1, + "arm,gic-v3"); + if (res) return res; + + res = fdt_property_cell(fdt, "#interrupt-cells", 3); + if (res) return res; + + res = fdt_property_cell(fdt, "#address-cells", 0); + if (res) return res; + + res = fdt_property(fdt, "interrupt-controller", NULL, 0); + if (res) return res; + + res = fdt_property_cell(fdt, "redistributor-stride", + GUEST_GICV3_RDIST_STRIDE); + if (res) return res; + + res = fdt_property_cell(fdt, "#redistributor-regions", + GUEST_GICV3_RDIST_REGIONS); + if (res) return res; + + res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS, + 2, + gicd_base, gicd_size, + gicr0_base, gicr0_size); + if (res) return res; + + res = fdt_property_cell(fdt, "linux,phandle", PHANDLE_GIC); + if (res) return res; + + res = fdt_property_cell(fdt, "phandle", PHANDLE_GIC); + if (res) return res; + + res = fdt_end_node(fdt); + if (res) return res; + + return 0; +} + static int make_timer_node(libxl__gc *gc, void *fdt, const struct arch_info *ainfo) { int res; @@ -456,6 +519,7 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, libxl_domain_build_info *info, struct xc_dom_image *dom) { + xc_domain_configuration_t config; void *fdt = NULL; int rc, res; size_t fdt_size = 0; @@ -471,8 +535,16 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, ainfo = get_arch_info(gc, dom); if (ainfo == NULL) return ERROR_FAIL; + LOG(DEBUG, "configure the domain"); + config.gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT; + if (xc_domain_configure(CTX->xch, dom->guest_domid, &config) != 0) { + LOG(ERROR, "counldn't configure the domain"); + return ERROR_FAIL; + } + LOG(DEBUG, "constructing DTB for Xen version %d.%d guest", vers->xen_version_major, vers->xen_version_minor); + LOG(DEBUG, " - vGIC version: %s", gicv_to_string(config.gic_version)); /* * Call "call" handling FDR_ERR_*. Will either: @@ -520,9 +592,22 @@ next_resize: FDT( make_psci_node(gc, fdt) ); FDT( make_memory_nodes(gc, fdt, dom) ); - FDT( make_intc_node(gc, fdt, - GUEST_GICD_BASE, GUEST_GICD_SIZE, - GUEST_GICC_BASE, GUEST_GICD_SIZE) ); + + switch (config.gic_version) { + case XEN_DOMCTL_CONFIG_GIC_V2: + FDT( make_gicv2_node(gc, fdt, + GUEST_GICD_BASE, GUEST_GICD_SIZE, + GUEST_GICC_BASE, GUEST_GICC_SIZE) ); + break; + case XEN_DOMCTL_CONFIG_GIC_V3: + FDT( make_gicv3_node(gc, fdt) ); + break; + default: + LOG(ERROR, "Unknown how to create the DT node for gic_version = %d", + config.gic_version); + rc = ERROR_FAIL; + goto out; + } FDT( make_timer_node(gc, fdt, ainfo) ); FDT( make_hypervisor_node(gc, fdt, vers) ); diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c index 45974e7..5b8ff57 100644 --- a/xen/arch/arm/domctl.c +++ b/xen/arch/arm/domctl.c @@ -10,6 +10,8 @@ #include <xen/errno.h> #include <xen/sched.h> #include <xen/hypercall.h> +#include <asm/gic.h> +#include <xen/guest_access.h> #include <public/domctl.h> long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, @@ -30,6 +32,39 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, return p2m_cache_flush(d, s, e); } + case XEN_DOMCTL_arm_configure_domain: + { + uint8_t gic_version; + + /* + * Xen 4.5: The vGIC is emulating the same version of the + * hardware GIC. Only the value XEN_DOMCTL_CONFIG_GIC_DEFAULT + * is allowed. The DOMCTL will return the actual version of the + * GIC. + */ + if ( domctl->u.configuredomain.gic_version != XEN_DOMCTL_CONFIG_GIC_DEFAULT ) + return -EOPNOTSUPP; + + switch ( gic_hw_version() ) + { + case GIC_V3: + gic_version = XEN_DOMCTL_CONFIG_GIC_V3; + break; + case GIC_V2: + gic_version = XEN_DOMCTL_CONFIG_GIC_V2; + break; + default: + BUG(); + } + + domctl->u.configuredomain.gic_version = gic_version; + + /* TODO: Make the copy generic for all ARCH domctl */ + if ( __copy_to_guest(u_domctl, domctl, 1) ) + return -EFAULT; + + return 0; + } default: return subarch_do_domctl(domctl, d, u_domctl); diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index 91161a2..076aa62 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -906,7 +906,21 @@ static int gicv_v3_init(struct domain *d) d->arch.vgic.rdist_count = gicv3.rdist_count; } else - d->arch.vgic.dbase = GUEST_GICD_BASE; + { + d->arch.vgic.dbase = GUEST_GICV3_GICD_BASE; + d->arch.vgic.dbase_size = GUEST_GICV3_GICD_SIZE; + + /* XXX: Only one Re-distributor region mapped for the guest */ + BUILD_BUG_ON(GUEST_GICV3_RDIST_REGIONS != 1); + + d->arch.vgic.rdist_count = GUEST_GICV3_RDIST_REGIONS; + d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE; + + /* The first redistributor should contain enough space for all CPUs */ + BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < MAX_VIRT_CPUS); + d->arch.vgic.rbase[0] = GUEST_GICV3_GICR0_BASE; + d->arch.vgic.rbase_size[0] = GUEST_GICV3_GICR0_SIZE; + } d->arch.vgic.nr_lines = 0; diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h index eecc561..d7df274 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -373,11 +373,27 @@ typedef uint64_t xen_callback_t; */ /* Physical Address Space */ + +/* vGIC mappings: Only one set of mapping is used by the guest. + * Therefore they can overlap. + */ + +/* vGIC v2 mappings */ #define GUEST_GICD_BASE 0x03001000ULL #define GUEST_GICD_SIZE 0x00001000ULL #define GUEST_GICC_BASE 0x03002000ULL #define GUEST_GICC_SIZE 0x00000100ULL +/* vGIC v3 mappings */ +#define GUEST_GICV3_GICD_BASE 0x03001000ULL +#define GUEST_GICV3_GICD_SIZE 0x00010000ULL + +#define GUEST_GICV3_RDIST_STRIDE 0x20000ULL +#define GUEST_GICV3_RDIST_REGIONS 1 + +#define GUEST_GICV3_GICR0_BASE 0x03020000ULL /* vCPU0 - vCPU15 */ +#define GUEST_GICV3_GICR0_SIZE 0x00200000ULL + /* 16MB == 4096 pages reserved for guest to use as a region to map its * grant table in. */ diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 58b19e7..8da204e 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -68,6 +68,19 @@ struct xen_domctl_createdomain { typedef struct xen_domctl_createdomain xen_domctl_createdomain_t; DEFINE_XEN_GUEST_HANDLE(xen_domctl_createdomain_t); +#if defined(__arm__) || defined(__aarch64__) +#define XEN_DOMCTL_CONFIG_GIC_DEFAULT 0 +#define XEN_DOMCTL_CONFIG_GIC_V2 1 +#define XEN_DOMCTL_CONFIG_GIC_V3 2 +/* XEN_DOMCTL_configure_domain */ +struct xen_domctl_arm_configuredomain { + /* IN/OUT parameters */ + uint8_t gic_version; +}; +typedef struct xen_domctl_arm_configuredomain xen_domctl_arm_configuredomain_t; +DEFINE_XEN_GUEST_HANDLE(xen_domctl_arm_configuredomain_t); +#endif + /* XEN_DOMCTL_getdomaininfo */ struct xen_domctl_getdomaininfo { /* OUT variables. */ @@ -1056,6 +1069,7 @@ struct xen_domctl { #define XEN_DOMCTL_set_vcpu_msrs 73 #define XEN_DOMCTL_setvnumainfo 74 #define XEN_DOMCTL_psr_cmt_op 75 +#define XEN_DOMCTL_arm_configure_domain 76 #define XEN_DOMCTL_gdbsx_guestmemio 1000 #define XEN_DOMCTL_gdbsx_pausevcpu 1001 #define XEN_DOMCTL_gdbsx_unpausevcpu 1002 @@ -1064,6 +1078,9 @@ struct xen_domctl { domid_t domain; union { struct xen_domctl_createdomain createdomain; +#if defined(__arm__) || defined(__aarch64__) + struct xen_domctl_arm_configuredomain configuredomain; +#endif struct xen_domctl_getdomaininfo getdomaininfo; struct xen_domctl_getmemlist getmemlist; struct xen_domctl_getpageframeinfo getpageframeinfo; diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index 6d0fe72..846cf88 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -727,6 +727,9 @@ static int flask_domctl(struct domain *d, int cmd) case XEN_DOMCTL_psr_cmt_op: return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_CMT_OP); + case XEN_DOMCTL_configure_domain: + return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__CONFIGURE_DOMAIN); + default: printk("flask_domctl: Unknown op %d\n", cmd); return -EPERM; diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors index de0c707..bfe2fa5 100644 --- a/xen/xsm/flask/policy/access_vectors +++ b/xen/xsm/flask/policy/access_vectors @@ -216,6 +216,8 @@ class domain2 get_vnumainfo # XEN_DOMCTL_psr_cmt_op psr_cmt_op +# XEN_DOMCTL_configure_domain + configure_domain } # Similar to class domain, but primarily contains domctls related to HVM domains