Re: [RFC PATCH 2/3] KVM: arm64: Make MTE_frac masking conditional on MTE capability

[Ben Horgan <ben.horgan@arm.com>]

Hi Marc,

On 4/27/25 18:24, Marc Zyngier wrote:
> On Mon, 14 Apr 2025 13:40:58 +0100,
> Ben Horgan <ben.horgan@arm.com> wrote:
>> [...]
>> +	/*
>> +	 * Previously MTE_frac was hidden from guest. However, if the
>> +	 * hardware supports MTE2 but not MTE_ASYM_FAULT then a value
>> +	 * of 0 for this field indicates that the hardware supports
>> +	 * MTE_ASYNC. Whereas, 0xf indicates MTE_ASYNC is not supported.
>> +	 *
>> +	 * As KVM must accept values from KVM provided by user-space,
>> +	 * when ID_AA64PFR1_EL1.MTE is 2 allow user-space to set
>> +	 * ID_AA64PFR1_EL1.MTE_frac to 0. However, ignore it to avoid
>> +	 * incorrectly claiming hardware support for MTE_ASYNC in the
>> +	 * guest.
>> +	 */
>> +
>> +	if (mte == ID_AA64PFR1_EL1_MTE_MTE2 &&
> 
> The spec says that MTE_frac is valid if ID_AA64PFR1_EL1.MTE >= 0b0010.
> Not strictly equal to 0b0010 (which represents MTE2). Crucially, MTE3
> should receive the same treatment.

This is specific to MTE2 as when MTE3 is supported MTE_ASYM_FAULT is 
also supported and when MTE_ASYM_FAULT is supported the spec says 
MTE_frac is 0.
> 
>> +	    user_mte_frac == ID_AA64PFR1_EL1_MTE_frac_ASYNC) {
>> +		user_val &= ~ID_AA64PFR1_EL1_MTE_frac_MASK;
>> +		user_val |= hw_val & ID_AA64PFR1_EL1_MTE_frac_MASK;
> 
> This means you are unconditionally propagating what the HW supports,
> which feels dodgy, specially considering that we don't know how
> MTE_frac is going to evolve in the future.
> 
> I think you should limit the fix to the exact case we're mitigating
> here, not blindly overwrite the guest's view with the HW's capability.

Sure, better safe than sorry. I can update the if condition to the below.

u8 hw_mte_frac = SYS_FIELD_GET(ID_AA64PFR1_EL1, MTE_frac, hw_val);
...
if (mte == ID_AA64PFR1_EL1_MTE_MTE2 &&
     hw_mte_frac == ID_AA64PFR1_EL1_MTE_frac_NI &&
     user_mte_frac == ID_AA64PFR1_EL1_MTE_frac_ASYNC)

> 
> Thanks,
> 
> 	M.
> 

Thanks,

Ben