On Tue, Aug 20, 2024 at 11:03:43AM +0100, Marc Zyngier wrote:
> In order to be consistent, we shouldn't advertise a GICv3 when none
> is actually usable by the guest.
>
> Wipe the feature when these conditions apply, and allow the field
> to be written from userspace.
>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> ---
> arch/arm64/kvm/sys_regs.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index bc2d54da3827..7d00d7e359e1 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2365,7 +2365,6 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> ID_AA64PFR0_EL1_MPAM |
> ID_AA64PFR0_EL1_SVE |
> ID_AA64PFR0_EL1_RAS |
> - ID_AA64PFR0_EL1_GIC |
> ID_AA64PFR0_EL1_AdvSIMD |
> ID_AA64PFR0_EL1_FP), },
> ID_SANITISED(ID_AA64PFR1_EL1),
> @@ -4634,6 +4633,11 @@ int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu)
>
> guard(mutex)(&kvm->arch.config_lock);
>
> + if (!kvm_has_gicv3(kvm)) {
> + kvm->arch.id_regs[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] &= ~ID_AA64PFR0_EL1_GIC_MASK;
> + kvm->arch.id_regs[IDREG_IDX(SYS_ID_PFR1_EL1)] &= ~ID_PFR1_EL1_GIC_MASK;
> + }
> +
Hmm, should we use the ID register field as the source of truth for
kvm_has_gicv3() at this point?
I think what you have in patch #1 makes good sense for a stable
backport. Using the ID register from this point forward would make the
behavior consistent for a stupid userspace instantiated GICv3 for the VM
but clobbered it from the ID register.
AFAICT all other usage of kvm_has_gicv3() happens after
kvm_finalize_sys_regs(), so it should take this last-minute fixup into
account.
--
Thanks,
Oliver
