Re: [PATCH v2 1/4] KVM: arm64: Allow userspace to change MIDR_EL1

[Sebastian Ott]

Hello Oliver,

On Sat, 15 Feb 2025, Oliver Upton wrote:
> On Tue, Feb 11, 2025 at 03:39:07PM +0100, Sebastian Ott wrote:
> > +static int set_id_reg_non_ftr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > +			      u64 val)
> > +{
> > +	u32 id = reg_to_encoding(rd);
> > +	int ret;
> > +
> > +	mutex_lock(&vcpu->kvm->arch.config_lock);
>
> There's quite a few early outs, guard() might be a better fit than
> explicitly dropping the lock.

Yea, I thought about that too but most of the other functions in that file
use the classic lock primitives. But you're right - it looks cleaner.

> > +	/*
> > +	 * Since guest access to MIDR_EL1 is not trapped
> > +	 * set up VPIDR_EL2 to hold the MIDR_EL1 value.
> > +	 */
> > +	if (id == SYS_MIDR_EL1)
> > +		write_sysreg(val, vpidr_el2);
>
> This is problematic for a couple reasons:
>
> - If the kernel isn't running at EL2, VPIDR_EL2 is undefined
>
> - VPIDR_EL2 needs to be handled as part of the vCPU context, not
>   written to without a running vCPU. What would happen if two vCPUs
>   have different MIDR values?

Indeed. Sry, I hadn't thought about that. That makes much more sense now.

> Here's a new diff with some hacks thrown in to handle VPIDR_EL2
> correctly. Very lightly tested :)

Thank you very much! I've integrated that and currently run some tests
with it.

Sebastian