linux-kernel - Re: [PATCH rc v1 1/4] iommu/arm-smmu-v3: Add ignored bits to fix STE update sequence

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aTUEikcWXWwKAS/1@nvidia.com>
Date: Sat, 6 Dec 2025 20:37:30 -0800
From: Nicolin Chen <nicolinc@...dia.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: <will@...nel.org>, <robin.murphy@....com>, <joro@...tes.org>,
	<linux-arm-kernel@...ts.infradead.org>, <iommu@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, <skolothumtho@...dia.com>, <praan@...gle.com>
Subject: Re: [PATCH rc v1 1/4] iommu/arm-smmu-v3: Add ignored bits to fix STE
 update sequence

On Sat, Dec 06, 2025 at 03:57:52PM -0400, Jason Gunthorpe wrote:
> I think that supports more that we should do what Shuai suggested and
> keep used as-is.

Yes, that will be probably cleaner.

> Then ignored should be adjusted by the used: Only if both used are 1
> should the bit become ignored. Otherwise we can rely on which ever
> used is 0 to generate the hitless update.

Hmm, not sure why it has to be both used.

The unused_update is computed using cur_used, and the equation for
used_qword_diff is computed using target_used, either of which can
be affected by ignored bits, right?

E.g.
if cur_used[] includes ING bit, target_used doesn't:
    // target must unset IGN bit, last equation isn't affected
    if cur sets IGN bit
        cur_used should exclude IGN bit
    if cur unsets IGN bit
        not affected
if cur_used[] doesn't include ignores, target_used does:
    // cur must unset IGN bit, cur_used isn't affected
    if target sets IGN bit:
        last equation must exclude IGN bit on both sides
    if target unsets IGN bit:
        not affected

> @@ -1109,6 +1118,7 @@ static u8 arm_smmu_entry_qword_diff(struct arm_smmu_entry_writer *writer,
>                 WARN_ON_ONCE(target[i] & ~target_used[i]);
>  
>                 /* Bits can change because they are not currently being used */
> +               cur_used[i] &= ~ignored[i];
>                 unused_update[i] = (entry[i] & cur_used[i]) |
>                                    (target[i] & ~cur_used[i]);

If one of ignored bits is set in entry[i] but unset in target[i],
the unused_update will first mask it away, resulting in an extra
unnecessary update (though it's still hitless).

So, I think this might be better:

-		cur_used[i] &= ~ignored[i];
+		cur_unused[i] = ~cur_used[i] | ignored[i];
 		unused_update[i] = (entry[i] & cur_used[i]) |
-				   (target[i] & ~cur_used[i]);
+				   (target[i] & cur_unused[i]);

Because cur_used includes ignored, the unused_update will retain
the ignored bits from entry. On the other hand, having cur_unused
will also retain the ignored bits from target.

One more change that we need is at the last equation:
-		if ((unused_update[i] & target_used[i]) != target[i])
+		if ((unused_update[i] & target_used[i] & ~ignored[i]) !=
+		    (target[i] & ~ignored[i]))

Either side might have the ignored bits, so we have to suppress
ignored on both sides, which is required in the similar routine
in arm_smmu_entry_differs_in_used_bits() of the kunit code.

With these additional changes, nesting sanity and kunit test are
both passing. I will do a few more tests to make sure things are
okay, before wrapping up the v2. Please let me know if all these
make sense to you.

Thanks
Nicolin