Re: [PATCH 1/5] iommu/amd: Simplify build_inv_address()

Jason Gunthorpe
2026-03-30T12:05:06Z

On Mon, Mar 30, 2026 at 03:39:16PM +0800, Wei Wang wrote:

> > @@ -1267,39 +1267,37 @@ static void build_inv_dte(struct iommu_cmd *cmd, u16 devid)
> > */
> > static inline u64 build_inv_address(u64 address, size_t size)
> > {
> > - u64 pages, end, msb_diff;
> > + u64 last = address + size - 1;
> > + unsigned int sz_lg2;
>
> Would naming it shift (or order) be more conventional?

I find that confusing, shift of what exactly? It is size of the
invalidation encoded in log 2.

> > - end = address + size - 1;
> > + address &= GENMASK_U64(63, 12);
> > + sz_lg2 = fls64(address ^ last);
> > + if (sz_lg2 <= 12)
>
> then could use "if (shift < PAGE_SHIFT)" here.

PAGE_SHIFT defines the kernel mm configuration, "12" is a hardware
constant from the iommu spec. They should not be intermixed.

> > + return address;
> > /*
> > - * msb_diff would hold the index of the most significant bit that
> > - * flipped between the start and end.
> > + * Encode sz_lg2 according to Table 14: Example Page Size Encodings
> > + *
> > + * See "Note *":
> > + * Address bits 51:32 can be used to encode page sizes greater
> > + * that 4 Gbytes.
>
> "than" (seems a spec typo)

Yeah, I'll leave it as is.

> > + * Which we take to mean that the highest page size has bit
> > + * [51]=0, [50:12]=1
> > + * and that coding happens when sz_lg2 is 52. Fall back to full
> > + * invalidation if the size is too big.
> > + *
> > */
> > - msb_diff = fls64(end ^ address) - 1;
> > + if (unlikely(sz_lg2 > 52))
> > + return (CMD_INV_IOMMU_ALL_PAGES_ADDRESS & PAGE_MASK) |
> > + CMD_INV_IOMMU_PAGES_SIZE_MASK;
>
>
> The spec mentions "Address bits 63:52 are zero-extended.", should we enforce
> it:
>
> -#define CMD_INV_IOMMU_ALL_PAGES_ADDRESS 0x7ffffffffffff000ULL
> +#define CMD_INV_IOMMU_ALL_PAGES_ADDRESS GENMASK_ULL(51, 12)
>
> also for the returned "address"

2.4.3 specifically calls out

The IOMMU invalidates all translation
information associated with the DomainID for both nested and guest levels when S=1, PDE=1,
GN=0, and Address[63:12]=7_FFFF_FFFF_FFFFh.

I wouldn't change it.

Jason