Re: [PATCH 5/5] iommu/amd: Control INVALIDATE_IOMMU_PAGES PDE from the gather

Wei Wang
2026-04-01T08:52:06Z

On 3/27/26 11:23 PM, Jason Gunthorpe wrote:
> Now that AMD uses iommupt, it is easy to make use of the PDE bit. If
> the gather has no free list then no page directory entries were
> changed.
>
> Pass GN/PDE through the invalidation call chain in a u32 flags field
> that is OR'd into data[2] and set it properly from the gather.
>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
> drivers/iommu/amd/amd_iommu.h | 2 +-
> drivers/iommu/amd/iommu.c | 61 +++++++++++++++++++----------------
> 2 files changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 67d4daaaf6f0ce..364878a923edd0 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -88,7 +88,7 @@ int amd_iommu_complete_ppr(struct device *dev, u32 pasid, int status, int tag);
> */
> void amd_iommu_flush_all_caches(struct amd_iommu *iommu);
> void amd_iommu_domain_flush_pages(struct protection_domain *domain,
> - u64 address, u64 last);
> + u64 address, u64 last, u32 flags);
> void amd_iommu_dev_flush_pasid_pages(struct iommu_dev_data *dev_data,
> ioasid_t pasid, u64 address, u64 last);
>
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 4e057fbc20021e..95f7dc6cf67eaf 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -1301,8 +1301,8 @@ static inline u64 build_inv_address(u64 address, u64 last)
> }
>
> static void build_inv_iommu_pages(struct iommu_cmd *cmd, u64 address,
> - u64 last, u16 domid,
> - ioasid_t pasid, bool gn)
> + u64 last, u16 domid, ioasid_t pasid,
> + u32 flags)
> {
> u64 inv_address = build_inv_address(address, last);
>
> @@ -1311,12 +1311,9 @@ static void build_inv_iommu_pages(struct iommu_cmd *cmd, u64 address,
> cmd->data[1] |= domid;
> cmd->data[2] = lower_32_bits(inv_address);
> cmd->data[3] = upper_32_bits(inv_address);
> - /* PDE bit - we want to flush everything, not only the PTEs */
> - cmd->data[2] |= CMD_INV_IOMMU_PAGES_PDE_MASK;
> - if (gn) {
> + cmd->data[2] |= flags;
> + if (flags & CMD_INV_IOMMU_PAGES_GN_MASK)
> cmd->data[0] |= pasid;
> - cmd->data[2] |= CMD_INV_IOMMU_PAGES_GN_MASK;
> - }
> CMD_SET_TYPE(cmd, CMD_INV_IOMMU_PAGES);
> }
>
> @@ -1523,7 +1520,8 @@ static void amd_iommu_flush_tlb_all(struct amd_iommu *iommu)
> for (dom_id = 0; dom_id <= last_bdf; ++dom_id) {
> struct iommu_cmd cmd;
> build_inv_iommu_pages(&cmd, 0, U64_MAX,
> - dom_id, IOMMU_NO_PASID, false);
> + dom_id, IOMMU_NO_PASID,
> + CMD_INV_IOMMU_PAGES_PDE_MASK);
> iommu_queue_command(iommu, &cmd);
> }
>
> @@ -1535,13 +1533,15 @@ static void amd_iommu_flush_tlb_domid(struct amd_iommu *iommu, u32 dom_id)
> struct iommu_cmd cmd;
>
> build_inv_iommu_pages(&cmd, 0, U64_MAX,
> - dom_id, IOMMU_NO_PASID, false);
> + dom_id, IOMMU_NO_PASID,
> + CMD_INV_IOMMU_PAGES_PDE_MASK);
> iommu_queue_command(iommu, &cmd);
>
> iommu_completion_wait(iommu);
> }
>
> -static int iommu_flush_pages_v1_hdom_ids(struct protection_domain *pdom, u64 address, u64 last)
> +static int iommu_flush_pages_v1_hdom_ids(struct protection_domain *pdom,
> + u64 address, u64 last, u32 flags)
> {
> int ret = 0;
> struct amd_iommu_viommu *aviommu;
> @@ -1559,7 +1559,7 @@ static int iommu_flush_pages_v1_hdom_ids(struct protection_domain *pdom, u64 add
> pr_debug("%s: iommu=%#x, hdom_id=%#x\n", __func__,
> iommu->devid, gdom_info->hdom_id);
> build_inv_iommu_pages(&cmd, address, last, gdom_info->hdom_id,
> - IOMMU_NO_PASID, false);
> + IOMMU_NO_PASID, flags);
> ret |= iommu_queue_command(iommu, &cmd);
> }
> xa_unlock(&aviommu->gdomid_array);
> @@ -1674,7 +1674,7 @@ static int device_flush_dte(struct iommu_dev_data *dev_data)
> }
>
> static int domain_flush_pages_v2(struct protection_domain *pdom,
> - u64 address, u64 last)
> + u64 address, u64 last, u32 flags)
> {
> struct iommu_dev_data *dev_data;
> struct iommu_cmd cmd;
> @@ -1685,8 +1685,9 @@ static int domain_flush_pages_v2(struct protection_domain *pdom,
> struct amd_iommu *iommu = get_amd_iommu_from_dev(dev_data->dev);
> u16 domid = dev_data->gcr3_info.domid;
>
> - build_inv_iommu_pages(&cmd, address, last,
> - domid, IOMMU_NO_PASID, true);
> + build_inv_iommu_pages(&cmd, address, last, domid,
> + IOMMU_NO_PASID,
> + flags | CMD_INV_IOMMU_PAGES_GN_MASK);
>
> ret |= iommu_queue_command(iommu, &cmd);
> }
> @@ -1695,7 +1696,7 @@ static int domain_flush_pages_v2(struct protection_domain *pdom,
> }
>
> static int domain_flush_pages_v1(struct protection_domain *pdom,
> - u64 address, u64 last)
> + u64 address, u64 last, u32 flags)
> {
> struct pdom_iommu_info *pdom_iommu_info;
> struct iommu_cmd cmd;
> @@ -1705,7 +1706,7 @@ static int domain_flush_pages_v1(struct protection_domain *pdom,
> lockdep_assert_held(&pdom->lock);
>
> build_inv_iommu_pages(&cmd, address, last,
> - pdom->id, IOMMU_NO_PASID, false);
> + pdom->id, IOMMU_NO_PASID, flags);
>
> xa_for_each(&pdom->iommu_array, i, pdom_iommu_info) {
> /*
> @@ -1724,7 +1725,7 @@ static int domain_flush_pages_v1(struct protection_domain *pdom,
> * See drivers/iommu/amd/nested.c: amd_iommu_alloc_domain_nested()
> */
> if (!list_empty(&pdom->viommu_list))
> - ret |= iommu_flush_pages_v1_hdom_ids(pdom, address, last);
> + ret |= iommu_flush_pages_v1_hdom_ids(pdom, address, last, flags);
>
> return ret;
> }
> @@ -1734,7 +1735,7 @@ static int domain_flush_pages_v1(struct protection_domain *pdom,
> * It flushes range of PTEs of the domain.
> */
> static void __domain_flush_pages(struct protection_domain *domain,
> - u64 address, u64 last)
> + u64 address, u64 last, u32 flags)
> {
> struct iommu_dev_data *dev_data;
> int ret = 0;
> @@ -1745,9 +1746,9 @@ static void __domain_flush_pages(struct protection_domain *domain,
>
> if (pdom_is_v2_pgtbl_mode(domain)) {
> gn = true;
> - ret = domain_flush_pages_v2(domain, address, last);
> + ret = domain_flush_pages_v2(domain, address, last, flags);
> } else {
> - ret = domain_flush_pages_v1(domain, address, last);
> + ret = domain_flush_pages_v1(domain, address, last, flags);
> }
>
> list_for_each_entry(dev_data, &domain->dev_list, list) {
> @@ -1762,13 +1763,13 @@ static void __domain_flush_pages(struct protection_domain *domain,
> }
>
> void amd_iommu_domain_flush_pages(struct protection_domain *domain,
> - u64 address, u64 last)
> + u64 address, u64 last, u32 flags)
> {
> lockdep_assert_held(&domain->lock);
>
> if (likely(!amd_iommu_np_cache) ||
> unlikely(address == 0 && last == U64_MAX)) {
> - __domain_flush_pages(domain, address, last);
> + __domain_flush_pages(domain, address, last, flags);
>
> /* Wait until IOMMU TLB and all device IOTLB flushes are complete */
> domain_flush_complete(domain);
> @@ -1794,7 +1795,7 @@ void amd_iommu_domain_flush_pages(struct protection_domain *domain,
> sz_lg2 = min_t(unsigned int, sz_lg2, __ffs64(address));
>
> flush_last = address + (1ULL << sz_lg2) - 1;
> - __domain_flush_pages(domain, address, flush_last);
> + __domain_flush_pages(domain, address, flush_last, flags);
> if (check_add_overflow(flush_last, 1, &address))
> break;
> }
> @@ -1806,7 +1807,8 @@ void amd_iommu_domain_flush_pages(struct protection_domain *domain,
> /* Flush the whole IO/TLB for a given protection domain - including PDE */
> static void amd_iommu_domain_flush_all(struct protection_domain *domain)
> {
> - amd_iommu_domain_flush_pages(domain, 0, U64_MAX);
> + amd_iommu_domain_flush_pages(domain, 0, U64_MAX,
> + CMD_INV_IOMMU_PAGES_PDE_MASK);
> }
>
> void amd_iommu_dev_flush_pasid_pages(struct iommu_dev_data *dev_data,
> @@ -1816,7 +1818,9 @@ void amd_iommu_dev_flush_pasid_pages(struct iommu_dev_data *dev_data,
> struct amd_iommu *iommu = get_amd_iommu_from_dev(dev_data->dev);
>
> build_inv_iommu_pages(&cmd, address, last,
> - dev_data->gcr3_info.domid, pasid, true);
> + dev_data->gcr3_info.domid, pasid,
> + CMD_INV_IOMMU_PAGES_GN_MASK |
> + CMD_INV_IOMMU_PAGES_PDE_MASK);
> iommu_queue_command(iommu, &cmd);
>
> if (dev_data->ats_enabled)
> @@ -2606,7 +2610,8 @@ static int amd_iommu_iotlb_sync_map(struct iommu_domain *dom,
> return 0;
>
> spin_lock_irqsave(&domain->lock, flags);
> - amd_iommu_domain_flush_pages(domain, iova, iova + size - 1);
> + amd_iommu_domain_flush_pages(domain, iova, iova + size - 1,
> + CMD_INV_IOMMU_PAGES_PDE_MASK);
> spin_unlock_irqrestore(&domain->lock, flags);
> return 0;
> }
> @@ -2628,7 +2633,9 @@ static void amd_iommu_iotlb_sync(struct iommu_domain *domain,
> unsigned long flags;
>
> spin_lock_irqsave(&dom->lock, flags);
> - amd_iommu_domain_flush_pages(dom, gather->start, gather->end);
> + amd_iommu_domain_flush_pages(dom, gather->start, gather->end,
> + iommu_pages_list_empty(&gather->freelist) ?
> + 0 : CMD_INV_IOMMU_PAGES_PDE_MASK);
> spin_unlock_irqrestore(&dom->lock, flags);
> iommu_put_pages_list(&gather->freelist);
> }

The logic for checking the gather freelist to conditionally set the PDE
bit is a nice optimization.

Reviewed-by: Wei Wang <wei.w.wang@hotmail.com>