Re: [PATCH v3 2/3] iommu/amd: Do not convert IVMD exclusion range to unity range

[Jindal, Akshay]

On 06-11-2026 09:46, Sairaj Kodilkar wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> 
> Original problem was there could be multiple IVMD exclusion entries
> conflicting over a single exclusion range supported in older Linux
> kernel. The fix was first introduced in commit 387caf0b759a ("iommu/amd:
> Treat per-device exclusion ranges as r/w unity-mapped regions") and in
> commit 0bbe4ced53e3 ("iommu/amd: Fix the overwritten field in IVMD header")
> , where IVMD exclusions are converted to separate unity_map_entry objects
> in pci_seg->unity_map so amd_iommu_get_resv_regions() treated every such
> entry as IOMMU_RESV_DIRECT with R/W.
> 
> The driver no longer programs the exclusion base/limit registers and
> relies on the kernel to keep the IOVA ranges named by exclusion IVMDs
> unmapped. Stop forcing exclusion entries to RW-unity. Store the raw
> IVMD flags in struct unity_map_entry instead of a pre-shifted
> protection value, and derive the reserved region from those flags.
> Exclusion entries are now reported as IOMMU_RESV_RESERVED with no
> read/write permission.
> 
> Reject unity entries that grant neither IR nor IW with a FW_BUG
> message, and warn about invalid flag combinations and unknown flag bits
> 
> Fixes: 0bbe4ced53e3 ("iommu/amd: Fix the overwritten field in IVMD header")
> Cc: Baoquan He <bhe@xxxxxxxxxx>
> Cc: Adrian Huang <ahuang12@xxxxxxxxxx>
> Cc: Jerry Snitselaar <jsnitsel@xxxxxxxxxx>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> Signed-off-by: Sairaj Kodilkar <sarunkod@xxxxxxx>
> Reviewed-by: Adrian Huang <ahuang12@xxxxxxxxxx>
> Tested-by: Adam Pelka <adampelka26@xxxxxxxxx>
> ---
>  drivers/iommu/amd/amd_iommu_types.h | 12 +++++++-----
>  drivers/iommu/amd/init.c            | 26 +++++++++-----------------
>  drivers/iommu/amd/iommu.c           | 21 +++++++++++++++++++--
>  3 files changed, 35 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 678ceddd711c..bcb88b476167 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -368,9 +368,11 @@
>  /* DTE[128:179] | DTE[184:191] */
>  #define DTE_DATA2_INTR_MASK    ~GENMASK_ULL(55, 52)
> 
> -#define IOMMU_PROT_MASK 0x03
> -#define IOMMU_PROT_IR 0x01
> -#define IOMMU_PROT_IW 0x02
> +#define IVMD_FLAG_UNITY                BIT(0> +#define IVMD_FLAG_IR           BIT(1)
> +#define IVMD_FLAG_IW           BIT(2)
> +#define IVMD_FLAG_EXCL         BIT(3)
> +#define IVMD_FLAG_MASK         GENMASK_U32(3, 0)
> 
>  /* IOMMU capabilities */
>  #define IOMMU_CAP_IOTLB   24
> @@ -880,8 +882,8 @@ struct unity_map_entry {
>         /* end address to unity map (including) */
>         u64 address_end;
> 
> -       /* required protection */
> -       int prot;
> +       /* IVMD flags for the entry */
> +       u8 flags;
>  };
> 
>  /*
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 13c96feed64f..1f0ca675df1c 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -70,11 +70,6 @@
>  #define IVHD_FLAG_RESPASSPW_EN_MASK     0x04
>  #define IVHD_FLAG_ISOC_EN_MASK          0x08
> 
> -#define IVMD_FLAG_EXCL_RANGE            0x08
> -#define IVMD_FLAG_IW                    0x04
> -#define IVMD_FLAG_IR                    0x02
> -#define IVMD_FLAG_UNITY_MAP             0x01
> -
>  #define ACPI_DEVFLAG_INITPASS           0x01
>  #define ACPI_DEVFLAG_EXTINT             0x02
>  #define ACPI_DEVFLAG_NMI                0x04
> @@ -2636,6 +2631,13 @@ static int __init init_unity_map_range(struct ivmd_header *m,
>         struct amd_iommu_pci_seg *pci_seg;
>         char *s;
> 
> +       if ((m->flags & IVMD_FLAG_UNITY) &&
> +           !(m->flags & (IVMD_FLAG_IR | IVMD_FLAG_IW | IVMD_FLAG_EXCL))) {
> +               pr_err(FW_BUG "IVMD type %#x devid %#x range %#llx+%#llx: Unity set without IR/IW, ignoring\n",
> +                      m->type, m->devid, m->range_start, m->range_length);
> +               return -EINVAL;
> +       }
> +
>         pci_seg = get_pci_segment(m->pci_seg, ivrs_base);
>         if (pci_seg == NULL)
>                 return -ENOMEM;
> @@ -2665,17 +2667,7 @@ static int __init init_unity_map_range(struct ivmd_header *m,
>         }
>         e->address_start = PAGE_ALIGN(m->range_start);
>         e->address_end = e->address_start + PAGE_ALIGN(m->range_length);
> -       e->prot = m->flags >> 1;
> -
> -       /*
> -        * Treat per-device exclusion ranges as r/w unity-mapped regions
> -        * since some buggy BIOSes might lead to the overwritten exclusion
> -        * range (exclusion_start and exclusion_length members). This
> -        * happens when there are multiple exclusion ranges (IVMD entries)
> -        * defined in ACPI table.
> -        */
> -       if (m->flags & IVMD_FLAG_EXCL_RANGE)
> -               e->prot = (IVMD_FLAG_IW | IVMD_FLAG_IR) >> 1;
> +       e->flags = m->flags;
> 
>         DUMP_printk("%s devid_start: %04x:%02x:%02x.%x devid_end: "
>                     "%04x:%02x:%02x.%x range_start: %016llx range_end: %016llx"
> @@ -2702,7 +2694,7 @@ static int __init init_memory_definitions(struct acpi_table_header *table)
> 
>         while (p < end) {
>                 m = (struct ivmd_header *)p;
> -               if (m->flags & (IVMD_FLAG_UNITY_MAP | IVMD_FLAG_EXCL_RANGE))
> +               if (m->flags & (IVMD_FLAG_UNITY | IVMD_FLAG_EXCL))
the original names were more descriptive, suggest keeping
IVMD_FLAG_UNITY_MAP/IVMD_FLAG_EXCL_RANGE rather than shortening them.
if you insist on rename, may be send in seperate patch?

>                         init_unity_map_range(m, table);
> 
>                 p += m->length;
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 56262f6b1f70..699b5f530f11 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3120,12 +3120,29 @@ static void amd_iommu_get_resv_regions(struct device *dev,
>                 if (devid < entry->devid_start || devid > entry->devid_end)
>                         continue;
> 
> +               /*
> +                * IVMD_FLAG_UNITY, IVMD_FLAG_IR, IVMD_FLAG_IW are ignored if
> +                * IVMD_FLAG_EXCL is set.
> +                */
> +               if ((entry->flags & IVMD_FLAG_EXCL) &&
> +                   (entry->flags & (IVMD_FLAG_UNITY | IVMD_FLAG_IR | IVMD_FLAG_IW)))
Here EXCL & UNITY are flagged as invalid combination
Above UNITY & !EXCL are also flagged as invalid.
Suggesting revisiting these conditions.
> +                       pr_err(FW_BUG "%s: Invalid IVMD flags combination: %#02x for sbdf=%#x. Ignore IW/IR/Unity flags\n",
> +                              __func__, entry->flags, sbdf);
> +
> +               if (entry->flags & ~IVMD_FLAG_MASK)
> +                       pr_err("%s: Unknown flags: %#02x for sbdf=%#x\n",
> +                              __func__, entry->flags & ~IVMD_FLAG_MASK, sbdf);
> +
>                 type   = IOMMU_RESV_DIRECT;
>                 length = entry->address_end - entry->address_start;
> -               if (entry->prot & IOMMU_PROT_IR)
> +               if (entry->flags & IVMD_FLAG_IR)
>                         prot |= IOMMU_READ;
> -               if (entry->prot & IOMMU_PROT_IW)
> +               if (entry->flags & IVMD_FLAG_IW)
>                         prot |= IOMMU_WRITE;
> +               if (entry->flags & IVMD_FLAG_EXCL) {
> +                       type = IOMMU_RESV_RESERVED;
> +                       prot = 0;
> +               }
Since in case of invalid flag combinations, we are going ahead with region allocation,
will it be beneficial to add a pr_debug emitting relevant details passed on to the
iommu_alloc_resv_region()?
> 
>                 region = iommu_alloc_resv_region(entry->address_start,
>                                                  length, prot, type,
> --
> 2.34.1
> 
>