Re: [PATCH rc v5 1/4] iommu/arm-smmu-v3: Add update_safe bits to fix STE update sequence

[Nicolin Chen <nicolinc@nvidia.com>]

On Mon, Jan 12, 2026 at 12:10:10PM -0400, Jason Gunthorpe wrote:
> Still, it seems easy enough to improve, do not add EATS to the safe
> bits if either the current or new STE has S2S set. That will force a
> V=0 and avoid the illegal STE. Nicolin?

Ack. I made the following changes:
-----------------------------------------------------------------
@@ -1083,7 +1083,8 @@ void arm_smmu_get_ste_used(const __le64 *ent, __le64 *used_bits)
 EXPORT_SYMBOL_IF_KUNIT(arm_smmu_get_ste_used);

 VISIBLE_IF_KUNIT
-void arm_smmu_get_ste_update_safe(__le64 *safe_bits)
+void arm_smmu_get_ste_update_safe(const __le64 *cur, const __le64 *target,
+                                 __le64 *safe_bits)
 {
        /*
         * MEV does not meaningfully impact the operation of the HW, it only
@@ -1097,13 +1098,22 @@ void arm_smmu_get_ste_update_safe(__le64 *safe_bits)
        safe_bits[1] |= cpu_to_le64(STRTAB_STE_1_MEV);

        /*
-        * EATS is used to reject and control the ATS behavior of the device. If
-        * we are changing it away from 0 then we already trust the device to
-        * use ATS properly and we have sequenced the device's ATS enable in PCI
-        * config space to prevent it from issuing ATS while we are changing
-        * EATS.
+        * When a STE comes to change EATS the sequencing code in the attach
+        * logic already will have the PCI cap for ATS disabled. Thus at this
+        * moment we can expect that the device will not generate ATS queries
+        * and so we don't care about the sequencing of EATS. The purpose of
+        * EATS is to protect the system from hostile untrusted devices that
+        * issue ATS when the PCI config space is disabled. However, if EATS
+        * is being changed then we already must be trusting the device since
+        * the EATS security block is being disabled.
+        *
+        *  Note: Since we moved the EATS update to the first phase, changing
+        *  S2S and EATS might transiently set S2S=1 and EATS=1, resulting in
+        *  a bad STE. See "5.2 Stream Table Entry". In such a case, we can't
+        *  do a hitless update.
         */
-       safe_bits[1] |= cpu_to_le64(STRTAB_STE_1_EATS);
+       if (!((cur[2] | target[2]) & cpu_to_le64(STRTAB_STE_2_S2S)))
+               safe_bits[1] |= cpu_to_le64(STRTAB_STE_1_EATS);
 }
 EXPORT_SYMBOL_IF_KUNIT(arm_smmu_get_ste_update_safe);

-----------------------------------------------------------------

I'll send v6 after running some tests.

Thanks
Nicolin