Messages in this thread |  | | Subject | Re: [PATCH] KVM: arm/arm64: vgic: Handle GICR PENDBAS ER.PTZ filed as RAZ | | Date | Mon, 23 Dec 2019 14:07:19 +0000 | | From | Marc Zyngier <> |
| |
Hi Zenghui,
On 2019-12-23 13:43, Zenghui Yu wrote: > On 2019/12/20 19:18, Zenghui Yu wrote: >> Although guest will hardly read and use the PTZ (Pending Table Zero) >> bit in GICR_PENDBASER, let us emulate the architecture strictly. >> As per IHI 0069E 9.11.30, PTZ field is WO, and reads as 0. >> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com> >> --- >> Noticed when checking all fields of GICR_PENDBASER register. >> But _not_ sure whether it's worth a fix, as Linux never sets >> the PTZ bit before enabling LPI (set GICR_CTLR_ENABLE_LPIS). >> And I wonder under which scenarios can this bit be written as 1. >> It seems difficult for software to determine whether the pending >> table contains all zeros when writing this bit. >> virt/kvm/arm/vgic/vgic-mmio-v3.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c >> b/virt/kvm/arm/vgic/vgic-mmio-v3.c >> index 7dfd15dbb308..ebc218840fc2 100644 >> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c >> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c >> @@ -414,8 +414,11 @@ 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; >> + u64 value = vgic_cpu->pendbaser; >> - return extract_bytes(vgic_cpu->pendbaser, addr & 7, len); >> + value &= ~GICR_PENDBASER_PTZ; >> + >> + return extract_bytes(value, addr & 7, len); >> } >> static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu, >> > > I noticed there is no userspace access callbacks for GICR_PENDBASER, > so this patch will make the PTZ field also 'Read As Zero' by > userspace. > Should we consider adding a uaccess_read callback for GICR_PENDBASER > which just returns the unchanged vgic_cpu->pendbaser to userspace? > (Though this is really not a big deal. We now always emulate the PTZ > field to guest as RAZ. And 'vgic_cpu->pendbaser & GICR_PENDBASER_PTZ' > only indicates whether KVM will optimize the LPI enabling process, > where Read As Zero indicates never optimize..)
I don't think adding a userspace accessor would help much. All this bit tells userspace is that the guest has programmed a zero filled table. On restore, we'd avoid a rescan of the table if there was no LPI mapped.
And thinking of it, this fixes a bug for non-Linux guests: If you write PTZ=1, we never clear it. Which means that if userspace saves and restores PENDBASER with PTZ set, we'll never restore the pending bits, which is pretty bad (see vgic_enable_lpis()).
This patch on its own fixes more than one bug!
Thanks,
M. -- Jazz is not dead. It just smells funny...
|  |