Hi Zenghui, On 11/17/20 4:16 PM, Zenghui Yu wrote: > It was recently reported that if GICR_TYPER is accessed before the RD base > address is set, we'll suffer from the unset @rdreg dereferencing. Oops... > > gpa_t last_rdist_typer = rdreg->base + GICR_TYPER + > (rdreg->free_index - 1) * KVM_VGIC_V3_REDIST_SIZE; > > It's "expected" that users will access registers in the redistributor if > the RD has been properly configured (e.g., the RD base address is set). But > it hasn't yet been covered by the existing documentation. > > Per discussion on the list [1], the reporting of the GICR_TYPER.Last bit > for userspace never actually worked. And it's difficult for us to emulate > it correctly given that userspace has the flexibility to access it any > time. Let's just drop the reporting of the Last bit for userspace for now > (userspace should have full knowledge about it anyway) and it at least > prevents kernel from panic ;-) > > [1] https://lore.kernel.org/kvmarm/c20865a267e44d1e2c0d52ce4e012263@xxxxxxxxxx/ > > Fixes: ba7b3f1275fd ("KVM: arm/arm64: Revisit Redistributor TYPER last bit computation") > Reported-by: Keqian Zhu <zhukeqian1@xxxxxxxxxx> > Signed-off-by: Zenghui Yu <yuzenghui@xxxxxxxxxx> Given the state of last bit, it looks sensible atm. Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx> Thanks Eric > --- > > This may be the easiest way to fix the issue and to get the fix backported > to stable tree. There is still some work can be done since (at least) we > have code duplicates between the MMIO and uaccess callbacks. > > arch/arm64/kvm/vgic/vgic-mmio-v3.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c > index 52d6f24f65dc..15a6c98ee92f 100644 > --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c > +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c > @@ -273,6 +273,23 @@ static unsigned long vgic_mmio_read_v3r_typer(struct kvm_vcpu *vcpu, > return extract_bytes(value, addr & 7, len); > } > > +static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, > + gpa_t addr, unsigned int len) > +{ > + unsigned long mpidr = kvm_vcpu_get_mpidr_aff(vcpu); > + int target_vcpu_id = vcpu->vcpu_id; > + u64 value; > + > + value = (u64)(mpidr & GENMASK(23, 0)) << 32; > + value |= ((target_vcpu_id & 0xffff) << 8); > + > + if (vgic_has_its(vcpu->kvm)) > + value |= GICR_TYPER_PLPIS; > + > + /* reporting of the Last bit is not supported for userspace */ > + return extract_bytes(value, addr & 7, len); > +} > + > static unsigned long vgic_mmio_read_v3r_iidr(struct kvm_vcpu *vcpu, > gpa_t addr, unsigned int len) > { > @@ -593,8 +610,9 @@ static const struct vgic_register_region vgic_v3_rd_registers[] = { > REGISTER_DESC_WITH_LENGTH(GICR_IIDR, > vgic_mmio_read_v3r_iidr, vgic_mmio_write_wi, 4, > VGIC_ACCESS_32bit), > - REGISTER_DESC_WITH_LENGTH(GICR_TYPER, > - vgic_mmio_read_v3r_typer, vgic_mmio_write_wi, 8, > + REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_TYPER, > + vgic_mmio_read_v3r_typer, vgic_mmio_write_wi, > + vgic_uaccess_read_v3r_typer, vgic_mmio_uaccess_write_wi, 8, > VGIC_ACCESS_64bit | VGIC_ACCESS_32bit), > REGISTER_DESC_WITH_LENGTH(GICR_WAKER, > vgic_mmio_read_raz, vgic_mmio_write_wi, 4, > _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm