lkml.org 
[lkml]   [2020]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] KVM: arm64: vgic-v3: Drop the reporting of GICR_TYPER.Last for userspace
From
Date
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@kernel.org/
>
> Fixes: ba7b3f1275fd ("KVM: arm/arm64: Revisit Redistributor TYPER last bit computation")
> Reported-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
Given the state of last bit, it looks sensible atm.

Reviewed-by: Eric Auger <eric.auger@redhat.com>

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,
>

\
 
 \ /
  Last update: 2020-11-17 18:32    [W:1.164 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site