Re: [PATCH 4/6] amd_iommu: Fix masks for Device Table Address Register

["Arun Kodilkar, Sairaj" <sarunkod@amd.com>]

On 3/11/2025 8:54 PM, Alejandro Jimenez wrote:
> The size mask currently encompasses reserved bits [11:9]. Extract only the
> corrects bits encoding size (i.e. [8:0]).
> 
> Cc: qemu-stable@nongnu.org
> Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> ---
>   hw/i386/amd_iommu.h | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
> index 8d5d882a06..2c5c8c70f1 100644
> --- a/hw/i386/amd_iommu.h
> +++ b/hw/i386/amd_iommu.h
> @@ -68,16 +68,16 @@
>   
>   #define AMDVI_MMIO_SIZE               0x4000
>   
> -#define AMDVI_MMIO_DEVTAB_SIZE_MASK   ((1ULL << 12) - 1)
> -#define AMDVI_MMIO_DEVTAB_BASE_MASK   (((1ULL << 52) - 1) & ~ \
> -                                       AMDVI_MMIO_DEVTAB_SIZE_MASK)
> +#define AMDVI_MMIO_DEVTAB_SIZE_MASK     GENMASK64(8, 0)
> +#define AMDVI_MMIO_DEVTAB_BASE_MASK     GENMASK64(51, 12)
> +

Use MAKE_64BIT_MASK here as well.

>   #define AMDVI_MMIO_DEVTAB_ENTRY_SIZE  32
>   #define AMDVI_MMIO_DEVTAB_SIZE_UNIT   4096
>   
>   /* some of this are similar but just for readability */
>   #define AMDVI_MMIO_CMDBUF_SIZE_BYTE       (AMDVI_MMIO_COMMAND_BASE + 7)
>   #define AMDVI_MMIO_CMDBUF_SIZE_MASK       0x0f
> -#define AMDVI_MMIO_CMDBUF_BASE_MASK       AMDVI_MMIO_DEVTAB_BASE_MASK
> +#define AMDVI_MMIO_CMDBUF_BASE_MASK       GENMASK64(51, 12)
>   #define AMDVI_MMIO_CMDBUF_HEAD_MASK       (((1ULL << 19) - 1) & ~0x0f)

I think its better to modify remaining mask fields as well (*HEAD_MASK 
and *TAIL_MASK). That way everything is consistent.

>   #define AMDVI_MMIO_CMDBUF_TAIL_MASK       AMDVI_MMIO_EVTLOG_HEAD_MASK
>   
> @@ -95,7 +95,7 @@
>   
>   #define AMDVI_MMIO_EXCL_ENABLED_MASK      (1ULL << 0)
>   #define AMDVI_MMIO_EXCL_ALLOW_MASK        (1ULL << 1)
> -#define AMDVI_MMIO_EXCL_LIMIT_MASK        AMDVI_MMIO_DEVTAB_BASE_MASK
> +#define AMDVI_MMIO_EXCL_LIMIT_MASK        GENMASK64(51, 12)
>   #define AMDVI_MMIO_EXCL_LIMIT_LOW         0xfff
>   
>   /* mmio control register flags */

Regards
Sairaj Kodilkar