Re: [PATCH v3 2/3] iommu/amd: Do not convert IVMD exclusion range to unity range
From: Sairaj Kodilkar
Date: Mon Nov 09 2026 - 01:36:10 EST
On 11/6/2026 3:32 PM, Jindal, Akshay wrote:
>
>
> 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()?
Sorry I made a mistake here,
Here's summary of what I wanted to do,
1. Reject UNITY mapping if IR and IW flags are not present
2. Reject exclusion mapping if either UNITY / IR / IW flags are present
Thanks for pointing it out. I'll update it in the next version and move
all the firmware related checks inside init_unity_map_range(). I'll also
reject this condition as it does not make sense to add the region with
invalid flag combination (a region cannot be both UNITY and EXCL).
Thanks
Sairaj
>>
>> region = iommu_alloc_resv_region(entry->address_start,
>> length, prot, type,
>> --
>> 2.34.1
>>
>>