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