Re: [PATCH 2/8] amd_iommu: update PA, GVA and VA size macros

On 5/11/26 8:39 AM, Sairaj Kodilkar wrote:
> Current implemenation 
s/implemenation/implementation

supports upto 64 bit IOVA and upto 52 bit PA.

s/upto/up to

> Modify related macros so that guest can see right size.
> 
> Signed-off-by: Sairaj Kodilkar <[email protected]>
> ---
>  hw/i386/acpi-build.c | 2 +-
>  hw/i386/amd_iommu.c  | 2 +-
>  hw/i386/amd_iommu.h  | 6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 9446a9f862ca..e4ad01eec037 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1763,7 +1763,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
>       */
>      build_append_int_noprefix(table_data,
>                               (1UL << 0) | /* EFRSup */
> -                             (40UL << 8), /* PASize */
> +                             AMDVI_PA_SIZE_52,
>                               4);
>      /* reserved */
>      build_append_int_noprefix(table_data, 0, 8);
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 789e09d6f2bc..7c52f9ade6e6 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -2450,7 +2450,7 @@ static void amdvi_pci_realize(PCIDevice *pdev, Error **errp)
>                   0xff000000);
>      pci_set_long(pdev->config + s->capab_offset + AMDVI_CAPAB_MISC, 0);
>      pci_set_long(pdev->config + s->capab_offset + AMDVI_CAPAB_MISC,
> -            AMDVI_MAX_PH_ADDR | AMDVI_MAX_GVA_ADDR | AMDVI_MAX_VA_ADDR);
> +            AMDVI_PA_SIZE_52 | AMDVI_GVA_SIZE_48 | AMDVI_VA_SIZE_64);
>  }
>  
>  static void amdvi_sysbus_reset(DeviceState *dev)
> diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
> index 302ccca5121f..fe8f4a6cdc74 100644
> --- a/hw/i386/amd_iommu.h
> +++ b/hw/i386/amd_iommu.h
> @@ -245,9 +245,9 @@
>  #define AMDVI_PAGE_SHIFT_4K 12
>  #define AMDVI_PAGE_MASK_4K      GENMASK64(63, 12)
>  
> -#define AMDVI_MAX_GVA_ADDR      (2UL << 5)
> -#define AMDVI_MAX_PH_ADDR       (40UL << 8)
> -#define AMDVI_MAX_VA_ADDR       (48UL << 15)

Could you add a comment explaining that these constants encode fields from
the AMD IOMMU “Miscellaneous Information Register 0” at IOMMU Capability
Offset 10h. I prefer listing the field names and bit ranges exactly how
they appear in the spec, which helps when searching through the doc, and
also help explain the shifts. i.e. something like:

+/*
+ * IOMMU Capability Offset 10h: Miscellaneous Information Register 0.
+ * VAsize[21:15], PAsize[14:8], and GVAsize[7:5].
+ * These fields use the same layout in the IVRS IVinfo Field.
+ */
> +#define AMDVI_GVA_SIZE_48       (2UL << 5)
> +#define AMDVI_PA_SIZE_52        (52UL << 8)
> +#define AMDVI_VA_SIZE_64        (64UL << 15)
>  
>  /* Completion Wait data size */
>  #define AMDVI_COMPLETION_DATA_SIZE    8

With the above changes:

Reviewed-by: Alejandro Jimenez <[email protected]>