Re: [PATCH v4 07/36] KVM: arm64: nv: Save/Restore vEL2 sysregs

[Oliver Upton <oliver.upton@linux.dev>]

On Wed, Oct 09, 2024 at 07:59:50PM +0100, Marc Zyngier wrote:
> +static void __sysreg_restore_vel2_state(struct kvm_vcpu *vcpu)
> +{
> +	u64 val;
> +
> +	/* These registers are common with EL1 */
> +	write_sysreg(__vcpu_sys_reg(vcpu, PAR_EL1),	par_el1);
> +	write_sysreg(__vcpu_sys_reg(vcpu, TPIDR_EL1),	tpidr_el1);
> +
> +	write_sysreg(read_cpuid_id(),				vpidr_el2);

I don't think we need to restore VPIDR_EL2 here, so long as we do it on
vcpu_put() when leaving a nested VM context. That seems like the right
place to have it, as we could be running a mix of nested and non-nested
VMs and don't ever poke VPIDR_EL2 for non-NV VMs.

> @@ -89,7 +192,29 @@ void __vcpu_load_switch_sysregs(struct kvm_vcpu *vcpu)
>  	 */
>  	__sysreg32_restore_state(vcpu);
>  	__sysreg_restore_user_state(guest_ctxt);
> -	__sysreg_restore_el1_state(guest_ctxt);
> +
> +	if (unlikely(__is_hyp_ctxt(guest_ctxt))) {
> +		__sysreg_restore_vel2_state(vcpu);
> +	} else {
> +		if (vcpu_has_nv(vcpu)) {
> +			/*
> +			 * Only set VPIDR_EL2 for nested VMs, as this is the
> +			 * only time it changes. We'll restore the MIDR_EL1
> +			 * view on put.
> +			 */

Slightly ambiguous what "VPIDR_EL2" this is referring to (hardware reg
v. guest value). Maybe:

			/*
			 * Use the guest hypervisor's VPIDR_EL2 when in a nested
			 * state. The hardware value of MIDR_EL1 gets restored on
			 * put.
			 */

> +			write_sysreg(ctxt_sys_reg(guest_ctxt, VPIDR_EL2), vpidr_el2);
> +
> +			/*
> +			 * As we're restoring a nested guest, set the value
> +			 * provided by the guest hypervisor.
> +			 */
> +			mpidr = ctxt_sys_reg(guest_ctxt, VMPIDR_EL2);
> +		} else {
> +			mpidr = ctxt_sys_reg(guest_ctxt, MPIDR_EL1);
> +		}
> +
> +		__sysreg_restore_el1_state(guest_ctxt, mpidr);
> +	}
>  
>  	vcpu_set_flag(vcpu, SYSREGS_ON_CPU);
>  }
> @@ -112,12 +237,20 @@ void __vcpu_put_switch_sysregs(struct kvm_vcpu *vcpu)
>  
>  	host_ctxt = host_data_ptr(host_ctxt);
>  
> -	__sysreg_save_el1_state(guest_ctxt);
> +	if (unlikely(__is_hyp_ctxt(guest_ctxt)))
> +		__sysreg_save_vel2_state(vcpu);
> +	else
> +		__sysreg_save_el1_state(guest_ctxt);
> +
>  	__sysreg_save_user_state(guest_ctxt);
>  	__sysreg32_save_state(vcpu);
>  
>  	/* Restore host user state */
>  	__sysreg_restore_user_state(host_ctxt);
>  
> +	/* If leaving a nesting guest, restore MPIDR_EL1 default view */

typo: MIDR_EL1

> +	if (vcpu_has_nv(vcpu))
> +		write_sysreg(read_cpuid_id(),	vpidr_el2);
> +
>  	vcpu_clear_flag(vcpu, SYSREGS_ON_CPU);
>  }
> -- 
> 2.39.2
> 

-- 
Thanks,
Oliver