Hi Marc,
On 4/27/25 18:24, Marc Zyngier wrote:
On Mon, 14 Apr 2025 13:40:58 +0100,
Ben Horgan <[email protected]> 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
