Re: [PATCH 33/55] KVM: arm64: vgic: Handle ITS related GICv3 redistributor registers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 01/08/16 19:20, Christoffer Dall wrote:
> On Fri, Jul 22, 2016 at 06:28:50PM +0100, Marc Zyngier wrote:
>> From: Andre Przywara <andre.przywara@xxxxxxx>
>>
>> In the GICv3 redistributor there are the PENDBASER and PROPBASER
>> registers which we did not emulate so far, as they only make sense
>> when having an ITS. In preparation for that emulate those MMIO
>> accesses by storing the 64-bit data written into it into a variable
>> which we later read in the ITS emulation.
>> We also sanitise the registers, making sure RES0 regions are respected
>> and checking for valid memory attributes.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
>> Reviewed-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>> Tested-by: Eric Auger <eric.auger@xxxxxxxxxx>
>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>> ---
>>  include/kvm/arm_vgic.h           |  13 ++++
>>  virt/kvm/arm/vgic/vgic-mmio-v3.c | 153 ++++++++++++++++++++++++++++++++++++++-
>>  virt/kvm/arm/vgic/vgic-mmio.h    |   8 ++
>>  virt/kvm/arm/vgic/vgic-v3.c      |  11 ++-
>>  4 files changed, 181 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>> index 450b4da..df2dec5 100644
>> --- a/include/kvm/arm_vgic.h
>> +++ b/include/kvm/arm_vgic.h
>> @@ -146,6 +146,14 @@ struct vgic_dist {
>>  	struct vgic_irq		*spis;
>>  
>>  	struct vgic_io_device	dist_iodev;
>> +
>> +	/*
>> +	 * Contains the attributes and gpa of the LPI configuration table.
>> +	 * Since we report GICR_TYPER.CommonLPIAff as 0b00, we can share
>> +	 * one address across all redistributors.
>> +	 * GICv3 spec: 6.1.2 "LPI Configuration tables"
>> +	 */
>> +	u64			propbaser;
>>  };
>>  
>>  struct vgic_v2_cpu_if {
>> @@ -200,6 +208,11 @@ struct vgic_cpu {
>>  	 */
>>  	struct vgic_io_device	rd_iodev;
>>  	struct vgic_io_device	sgi_iodev;
>> +
>> +	/* Contains the attributes and gpa of the LPI pending tables. */
>> +	u64 pendbaser;
>> +
>> +	bool lpis_enabled;
>>  };
>>  
>>  int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write);
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> index bfcafbd..278bfbb 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> @@ -29,6 +29,19 @@ static unsigned long extract_bytes(unsigned long data, unsigned int offset,
>>  	return (data >> (offset * 8)) & GENMASK_ULL(num * 8 - 1, 0);
>>  }
>>  
>> +/* allows updates of any half of a 64-bit register (or the whole thing) */
>> +static u64 update_64bit_reg(u64 reg, unsigned int offset, unsigned int len,
>> +			    unsigned long val)
>> +{
>> +	int lower = (offset & 4) * 8;
>> +	int upper = lower + 8 * len - 1;
>> +
>> +	reg &= ~GENMASK_ULL(upper, lower);
>> +	val &= GENMASK_ULL(len * 8 - 1, 0);
>> +
>> +	return reg | ((u64)val << lower);
>> +}
>> +
>>  static unsined long vgic_mmio_read_v3_misc(struct kvm_vcpu *vcpu,
>>  					    gpa_t addr, unsigned int len)
>>  {
>> @@ -152,6 +165,142 @@ static unsigned long vgic_mmio_read_v3_idregs(struct kvm_vcpu *vcpu,
>>  	return 0;
>>  }
>>  
>> +/* We want to avoid outer shareable. */
>> +u64 vgic_sanitise_shareability(u64 field)
>> +{
>> +	switch (field) {
>> +	case GIC_BASER_OuterShareable:
>> +		return GIC_BASER_InnerShareable;
>> +	default:
>> +		return field;
>> +	}
>> +}
>> +
>> +/* Avoid any inner non-cacheable mapping. */
>> +u64 vgic_sanitise_inner_cacheability(u64 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. */
>> +u64 vgic_sanitise_outer_cacheability(u64 field)
>> +{
>> +	switch (field) {
>> +	case GIC_BASER_CACHE_SameAsInner:
>> +	case GIC_BASER_CACHE_nC:
>> +		return field;
>> +	default:
>> +		return GIC_BASER_CACHE_nC;
>> +	}
>> +}
>> +
>> +u64 vgic_sanitise_field(u64 reg, u64 field_mask, int field_shift,
>> +			u64 (*sanitise_fn)(u64))
>> +{
>> +	u64 field = (reg & field_mask) >> field_shift;
>> +
>> +	field = sanitise_fn(field) << field_shift;
>> +	return (reg & ~field_mask) | field;
>> +}
>> +
>> +#define PROPBASER_RES0_MASK						\
>> +	(GENMASK_ULL(63, 59) | GENMASK_ULL(55, 52) | GENMASK_ULL(6, 5))
>> +#define PENDBASER_RES0_MASK						\
>> +	(BIT_ULL(63) | GENMASK_ULL(61, 59) | GENMASK_ULL(55, 52) |	\
>> +	 GENMASK_ULL(15, 12) | GENMASK_ULL(6, 0))
>> +
>> +static u64 vgic_sanitise_pendbaser(u64 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 &= ~PENDBASER_RES0_MASK;
>> +	reg &= ~GENMASK_ULL(51, 48);
>> +
>> +	return reg;
>> +}
>> +
>> +static u64 vgic_sanitise_propbaser(u64 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 &= ~PROPBASER_RES0_MASK;
>> +	reg &= ~GENMASK_ULL(51, 48);
>> +	return reg;
>> +}
>> +
>> +static unsigned long vgic_mmio_read_propbase(struct kvm_vcpu *vcpu,
>> +					     gpa_t addr, unsigned int len)
>> +{
>> +	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>> +
>> +	return extract_bytes(dist->propbaser, addr & 7, len);
>> +}
>> +
>> +static void vgic_mmio_write_propbase(struct kvm_vcpu *vcpu,
>> +				     gpa_t addr, unsigned int len,
>> +				     unsigned long val)
>> +{
>> +	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>> +	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>> +	u64 propbaser = dist->propbaser;
>> +
>> +	/* Storing a value with LPIs already enabled is undefined */
>> +	if (vgic_cpu->lpis_enabled)
>> +		return;
>> +
>> +	propbaser = update_64bit_reg(propbaser, addr & 4, len, val);
>> +	propbaser = vgic_sanitise_propbaser(propbaser);
>> +
>> +	dist->propbaser = propbaser;
> 
> Which guarantees do we have that this will always be a single atomic
> write?

At least on arm64 - which is the only architecture this code is
compiling for at the moment - I don't see why this shouldn't be a single
write:
	str     x19, [x22,#3544]
is what my setup here creates.

Do we need something stronger? Do we want to postpone this to the point
when we get arm(32) support?

> 
>> +}
>> +
>> +static unsigned long vgic_mmio_read_pendbase(struct kvm_vcpu *vcpu,
>> +					     gpa_t addr, unsigned int len)
>> +{
>> +	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>> +
>> +	return extract_bytes(vgic_cpu->pendbaser, addr & 7, len);
>> +}
>> +
>> +static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
>> +				     gpa_t addr, unsigned int len,
>> +				     unsigned long val)
>> +{
>> +	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>> +	u64 pendbaser = vgic_cpu->pendbaser;
>> +
>> +	/* Storing a value with LPIs already enabled is undefined */
>> +	if (vgic_cpu->lpis_enabled)
>> +		return;
>> +
>> +	pendbaser = update_64bit_reg(pendbaser, addr & 4, len, val);
>> +	pendbaser = vgic_sanitise_pendbaser(pendbaser);
>> +
>> +	vgic_cpu->pendbaser = pendbaser;
> 
> same (assuming multiple VCPUs could be accessing this redistributor at
> the same time?)

same answer ;-)

Cheers,
Andre.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux