Re: [PATCH] [RFC] KVM: arm64/vgic: Populate GICR_TYPER with Aff3

[Marc Zyngier]

Hi Saurav,

On Mon, 05 Feb 2024 18:43:26 +0000,
Saurav Sachidanand <sauravsc@xxxxxxxxxx> wrote:
> 
> According to spec, bits [63:56] of the GICR_TYPER register are supposed
> to contain Affinity level 3 (Aff3) bits of the Processing Element (PE)
> associated with its GIC redistributor. Linux guests on boot match PEs
> with their redistributor using all four Affinity level bits from this
> register.

If you read the spec carefully, you will notice that Aff3 support is
*optional*, and that it is on purpose that we don't support Aff3: this
cannot work for 32bit guests.

The other thing is that there is no guarantee that the HW actually
supports that either: ICH_VTR_EL2.A3V tells you whether the CPU
interface actually supports it.

> 
> Currently, vGIC populates GICR_TYPER with just the first three Affinity
> levels of a vCPU's MPIDR. This works fine for a Linux guest that boots
> with KVM's default vCPU MPIDR assignment, which also only populates till
> the first three Affinity levels.
> 
> However, a hypervisor can override KVM's default MPIDR assignment by
> writing directly to a vCPU's MPIDR_EL1 register. If such a hypervisor
> were to populate Aff3 bits for a VM, a Linux guest booting there would
> fail to match vCPUs with their vGIC redistributors, since their virtual
> GICR_TYPER registers would be missing the respective Aff3 bits.
> 
> To change that, let's populate GICR_TYPER using Aff3 bits [39:32] from
> the vCPU's MPIDR.
> 
> Signed-off-by: Saurav Sachidanand <sauravsc@xxxxxxxxxx>
> ---
>  arch/arm64/kvm/vgic/vgic-mmio-v3.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> index c15ee1df036a..26bc838ce14c 100644
> --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> @@ -324,6 +324,7 @@ static unsigned long vgic_mmio_read_v3r_typer(struct kvm_vcpu *vcpu,
>  	u64 value;
>  
>  	value = (u64)(mpidr & GENMASK(23, 0)) << 32;
> +	value |= (u64)((mpidr >> 32) & GENMASK(7, 0)) << 56;
>  	value |= ((target_vcpu_id & 0xffff) << 8);
>  
>  	if (vgic_has_its(vcpu->kvm))

So if you consider the above remarks, this is neither sufficient nor
guaranteed to work. I'm not opposed to having full Aff3 support (most
of it is already there), but this needs to be done in a consistent
way, and consider both the case of 32bit guests (which are not going
away) and the capabilities of the HW (the current code being rather
inconsistent in this regard).

If you are willing to work on this, I'll happily review a more
complete series of patches.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.