Re: [Xen-devel] [PATCH 13/28] ARM: vGICv3: handle virtual LPI pending and property tables
Hi Andre, On 30/01/17 18:31, Andre Przywara wrote: Allow a guest to provide the address and size for the memory regions it has reserved for the GICv3 pending and property tables. We sanitise the various fields of the respective redistributor registers and map those pages into Xen's address space to have easy access. Signed-off-by: Andre Przywara --- xen/arch/arm/vgic-v3.c | 220 +++ xen/arch/arm/vgic.c | 4 + xen/include/asm-arm/domain.h | 8 +- xen/include/asm-arm/vgic.h | 24 - 4 files changed, 233 insertions(+), 23 deletions(-) diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index b0653c2..c6db2d7 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/xen/arch/arm/vgic-v3.c @@ -20,12 +20,14 @@ #include #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -229,12 +231,15 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info, goto read_reserved; case VREG64(GICR_PROPBASER): -/* LPI's not implemented */ -goto read_as_zero_64; +if ( !vgic_reg64_check_access(dabt) ) goto bad_width; +*r = vgic_reg64_extract(v->domain->arch.vgic.rdist_propbase, info); +return 1; case VREG64(GICR_PENDBASER): -/* LPI's not implemented */ -goto read_as_zero_64; +if ( !vgic_reg64_check_access(dabt) ) goto bad_width; +*r = vgic_reg64_extract(v->arch.vgic.rdist_pendbase, info); +*r &= ~GICR_PENDBASER_PTZ; /* WO, reads as 0 */ +return 1; case 0x0080: goto read_reserved; @@ -302,11 +307,6 @@ bad_width: domain_crash_synchronous(); return 0; -read_as_zero_64: -if ( !vgic_reg64_check_access(dabt) ) goto bad_width; -*r = 0; -return 1; - read_as_zero_32: if ( dabt.size != DABT_WORD ) goto bad_width; *r = 0; @@ -331,11 +331,179 @@ read_unknown: return 1; } +static uint64_t vgic_sanitise_field(uint64_t reg, uint64_t field_mask, +int field_shift, +uint64_t (*sanitise_fn)(uint64_t)) +{ +uint64_t field = (reg & field_mask) >> field_shift; + +field = sanitise_fn(field) << field_shift; + +return (reg & ~field_mask) | field; +} + +/* We want to avoid outer shareable. */ +static uint64_t vgic_sanitise_shareability(uint64_t field) +{ +switch (field) { Coding style: switch ( field ) { +case GIC_BASER_OuterShareable: +return GIC_BASER_InnerShareable; +default: +return field; +} +} + +/* Avoid any inner non-cacheable mapping. */ +static uint64_t vgic_sanitise_inner_cacheability(uint64_t field) +{ +switch (field) { Ditto +case GIC_BASER_CACHE_nCnB: +case GIC_BASER_CACHE_nC: +return GIC_BASER_CACHE_RaWb; +default: +return field; +} +} + +/* Non-cacheable or same-as-inner are OK. */ +static uint64_t vgic_sanitise_outer_cacheability(uint64_t field) +{ +switch (field) { Ditto +case GIC_BASER_CACHE_SameAsInner: +case GIC_BASER_CACHE_nC: +return field; +default: +return GIC_BASER_CACHE_nC; +} +} + +static uint64_t sanitize_propbaser(uint64_t reg) +{ +reg = vgic_sanitise_field(reg, GICR_PROPBASER_SHAREABILITY_MASK, + GICR_PROPBASER_SHAREABILITY_SHIFT, + vgic_sanitise_shareability); +reg = vgic_sanitise_field(reg, GICR_PROPBASER_INNER_CACHEABILITY_MASK, + GICR_PROPBASER_INNER_CACHEABILITY_SHIFT, + vgic_sanitise_inner_cacheability); +reg = vgic_sanitise_field(reg, GICR_PROPBASER_OUTER_CACHEABILITY_MASK, + GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT, + vgic_sanitise_outer_cacheability); + +reg &= ~GICR_PROPBASER_RES0_MASK; Newline here please. +return reg; +} + +static uint64_t sanitize_pendbaser(uint64_t reg) +{ +reg = vgic_sanitise_field(reg, GICR_PENDBASER_SHAREABILITY_MASK, + GICR_PENDBASER_SHAREABILITY_SHIFT, + vgic_sanitise_shareability); +reg = vgic_sanitise_field(reg, GICR_PENDBASER_INNER_CACHEABILITY_MASK, + GICR_PENDBASER_INNER_CACHEABILITY_SHIFT, + vgic_sanitise_inner_cacheability); +reg = vgic_sanitise_field(reg, GICR_PENDBASER_OUTER_CACHEABILITY_MASK, + GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT, + vgic_sanitise_outer_cacheability); + +reg &= ~GICR_PENDBASER_RES0_MASK; Newline here please. +return reg; +} + +/* + * Mark a given number of guest pages as used (by increasing their refcount), + * starting with the given guest address. This needs to be called once before + * calling (possibly repeatedly) map_guest_pages(). + * Be
Re: [Xen-devel] [PATCH 13/28] ARM: vGICv3: handle virtual LPI pending and property tables
On Mon, 30 Jan 2017, Andre Przywara wrote: > Allow a guest to provide the address and size for the memory regions > it has reserved for the GICv3 pending and property tables. > We sanitise the various fields of the respective redistributor > registers and map those pages into Xen's address space to have easy > access. > > Signed-off-by: Andre Przywara Please give a look at alpine.DEB.2.10.1610281619240.9978@sstabellini-ThinkPad-X260 > xen/arch/arm/vgic-v3.c | 220 > +++ > xen/arch/arm/vgic.c | 4 + > xen/include/asm-arm/domain.h | 8 +- > xen/include/asm-arm/vgic.h | 24 - > 4 files changed, 233 insertions(+), 23 deletions(-) > > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c > index b0653c2..c6db2d7 100644 > --- a/xen/arch/arm/vgic-v3.c > +++ b/xen/arch/arm/vgic-v3.c > @@ -20,12 +20,14 @@ > > #include > #include > +#include > #include > #include > #include > #include > #include > #include > +#include > #include > #include > #include > @@ -229,12 +231,15 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu > *v, mmio_info_t *info, > goto read_reserved; > > case VREG64(GICR_PROPBASER): > -/* LPI's not implemented */ > -goto read_as_zero_64; > +if ( !vgic_reg64_check_access(dabt) ) goto bad_width; > +*r = vgic_reg64_extract(v->domain->arch.vgic.rdist_propbase, info); > +return 1; > > case VREG64(GICR_PENDBASER): > -/* LPI's not implemented */ > -goto read_as_zero_64; > +if ( !vgic_reg64_check_access(dabt) ) goto bad_width; > +*r = vgic_reg64_extract(v->arch.vgic.rdist_pendbase, info); > +*r &= ~GICR_PENDBASER_PTZ; /* WO, reads as 0 */ > +return 1; > > case 0x0080: > goto read_reserved; > @@ -302,11 +307,6 @@ bad_width: > domain_crash_synchronous(); > return 0; > > -read_as_zero_64: > -if ( !vgic_reg64_check_access(dabt) ) goto bad_width; > -*r = 0; > -return 1; > - > read_as_zero_32: > if ( dabt.size != DABT_WORD ) goto bad_width; > *r = 0; > @@ -331,11 +331,179 @@ read_unknown: > return 1; > } > > +static uint64_t vgic_sanitise_field(uint64_t reg, uint64_t field_mask, > +int field_shift, > +uint64_t (*sanitise_fn)(uint64_t)) > +{ > +uint64_t field = (reg & field_mask) >> field_shift; > + > +field = sanitise_fn(field) << field_shift; > + > +return (reg & ~field_mask) | field; > +} > + > +/* We want to avoid outer shareable. */ > +static uint64_t vgic_sanitise_shareability(uint64_t field) > +{ > +switch (field) { > +case GIC_BASER_OuterShareable: > +return GIC_BASER_InnerShareable; > +default: > +return field; > +} > +} > + > +/* Avoid any inner non-cacheable mapping. */ > +static uint64_t vgic_sanitise_inner_cacheability(uint64_t field) > +{ > +switch (field) { > +case GIC_BASER_CACHE_nCnB: > +case GIC_BASER_CACHE_nC: > +return GIC_BASER_CACHE_RaWb; > +default: > +return field; > +} > +} > + > +/* Non-cacheable or same-as-inner are OK. */ > +static uint64_t vgic_sanitise_outer_cacheability(uint64_t field) > +{ > +switch (field) { > +case GIC_BASER_CACHE_SameAsInner: > +case GIC_BASER_CACHE_nC: > +return field; > +default: > +return GIC_BASER_CACHE_nC; > +} > +} > + > +static uint64_t sanitize_propbaser(uint64_t reg) > +{ > +reg = vgic_sanitise_field(reg, GICR_PROPBASER_SHAREABILITY_MASK, > + GICR_PROPBASER_SHAREABILITY_SHIFT, > + vgic_sanitise_shareability); > +reg = vgic_sanitise_field(reg, GICR_PROPBASER_INNER_CACHEABILITY_MASK, > + GICR_PROPBASER_INNER_CACHEABILITY_SHIFT, > + vgic_sanitise_inner_cacheability); > +reg = vgic_sanitise_field(reg, GICR_PROPBASER_OUTER_CACHEABILITY_MASK, > + GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT, > + vgic_sanitise_outer_cacheability); > + > +reg &= ~GICR_PROPBASER_RES0_MASK; > +return reg; > +} > + > +static uint64_t sanitize_pendbaser(uint64_t reg) > +{ > +reg = vgic_sanitise_field(reg, GICR_PENDBASER_SHAREABILITY_MASK, > + GICR_PENDBASER_SHAREABILITY_SHIFT, > + vgic_sanitise_shareability); > +reg = vgic_sanitise_field(reg, GICR_PENDBASER_INNER_CACHEABILITY_MASK, > + GICR_PENDBASER_INNER_CACHEABILITY_SHIFT, > + vgic_sanitise_inner_cacheability); > +reg = vgic_sanitise_field(reg, GICR_PENDBASER_OUTER_CACHEABILITY_MASK, > + GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT, > + vgic_sanitise_outer_cacheability); > + > +reg &= ~GICR_PEND
[Xen-devel] [PATCH 13/28] ARM: vGICv3: handle virtual LPI pending and property tables
Allow a guest to provide the address and size for the memory regions it has reserved for the GICv3 pending and property tables. We sanitise the various fields of the respective redistributor registers and map those pages into Xen's address space to have easy access. Signed-off-by: Andre Przywara --- xen/arch/arm/vgic-v3.c | 220 +++ xen/arch/arm/vgic.c | 4 + xen/include/asm-arm/domain.h | 8 +- xen/include/asm-arm/vgic.h | 24 - 4 files changed, 233 insertions(+), 23 deletions(-) diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index b0653c2..c6db2d7 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/xen/arch/arm/vgic-v3.c @@ -20,12 +20,14 @@ #include #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -229,12 +231,15 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info, goto read_reserved; case VREG64(GICR_PROPBASER): -/* LPI's not implemented */ -goto read_as_zero_64; +if ( !vgic_reg64_check_access(dabt) ) goto bad_width; +*r = vgic_reg64_extract(v->domain->arch.vgic.rdist_propbase, info); +return 1; case VREG64(GICR_PENDBASER): -/* LPI's not implemented */ -goto read_as_zero_64; +if ( !vgic_reg64_check_access(dabt) ) goto bad_width; +*r = vgic_reg64_extract(v->arch.vgic.rdist_pendbase, info); +*r &= ~GICR_PENDBASER_PTZ; /* WO, reads as 0 */ +return 1; case 0x0080: goto read_reserved; @@ -302,11 +307,6 @@ bad_width: domain_crash_synchronous(); return 0; -read_as_zero_64: -if ( !vgic_reg64_check_access(dabt) ) goto bad_width; -*r = 0; -return 1; - read_as_zero_32: if ( dabt.size != DABT_WORD ) goto bad_width; *r = 0; @@ -331,11 +331,179 @@ read_unknown: return 1; } +static uint64_t vgic_sanitise_field(uint64_t reg, uint64_t field_mask, +int field_shift, +uint64_t (*sanitise_fn)(uint64_t)) +{ +uint64_t field = (reg & field_mask) >> field_shift; + +field = sanitise_fn(field) << field_shift; + +return (reg & ~field_mask) | field; +} + +/* We want to avoid outer shareable. */ +static uint64_t vgic_sanitise_shareability(uint64_t field) +{ +switch (field) { +case GIC_BASER_OuterShareable: +return GIC_BASER_InnerShareable; +default: +return field; +} +} + +/* Avoid any inner non-cacheable mapping. */ +static uint64_t vgic_sanitise_inner_cacheability(uint64_t field) +{ +switch (field) { +case GIC_BASER_CACHE_nCnB: +case GIC_BASER_CACHE_nC: +return GIC_BASER_CACHE_RaWb; +default: +return field; +} +} + +/* Non-cacheable or same-as-inner are OK. */ +static uint64_t vgic_sanitise_outer_cacheability(uint64_t field) +{ +switch (field) { +case GIC_BASER_CACHE_SameAsInner: +case GIC_BASER_CACHE_nC: +return field; +default: +return GIC_BASER_CACHE_nC; +} +} + +static uint64_t sanitize_propbaser(uint64_t reg) +{ +reg = vgic_sanitise_field(reg, GICR_PROPBASER_SHAREABILITY_MASK, + GICR_PROPBASER_SHAREABILITY_SHIFT, + vgic_sanitise_shareability); +reg = vgic_sanitise_field(reg, GICR_PROPBASER_INNER_CACHEABILITY_MASK, + GICR_PROPBASER_INNER_CACHEABILITY_SHIFT, + vgic_sanitise_inner_cacheability); +reg = vgic_sanitise_field(reg, GICR_PROPBASER_OUTER_CACHEABILITY_MASK, + GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT, + vgic_sanitise_outer_cacheability); + +reg &= ~GICR_PROPBASER_RES0_MASK; +return reg; +} + +static uint64_t sanitize_pendbaser(uint64_t reg) +{ +reg = vgic_sanitise_field(reg, GICR_PENDBASER_SHAREABILITY_MASK, + GICR_PENDBASER_SHAREABILITY_SHIFT, + vgic_sanitise_shareability); +reg = vgic_sanitise_field(reg, GICR_PENDBASER_INNER_CACHEABILITY_MASK, + GICR_PENDBASER_INNER_CACHEABILITY_SHIFT, + vgic_sanitise_inner_cacheability); +reg = vgic_sanitise_field(reg, GICR_PENDBASER_OUTER_CACHEABILITY_MASK, + GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT, + vgic_sanitise_outer_cacheability); + +reg &= ~GICR_PENDBASER_RES0_MASK; +return reg; +} + +/* + * Mark a given number of guest pages as used (by increasing their refcount), + * starting with the given guest address. This needs to be called once before + * calling (possibly repeatedly) map_guest_pages(). + * Before the domain gets destroyed, call put_guest_pages() to drop the + * reference. + */ +int get_guest_pages(struct domain *d, paddr_t gpa, int nr_pages) +{