On 5/29/2025 3:47 AM, Alejandro Jimenez wrote:
> Address various issues with definitions of the MMIO registers e.g. for the
> Device Table Address Register, the size mask currently encompasses reserved
> bits [11:9], so change it to only extract the bits [8:0] encoding size.
>
> Convert masks to use GENMASK64 for consistency, and make unrelated
> definitions independent.
>
> Cc: qemu-sta...@nongnu.org
> Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
> Signed-off-by: Alejandro Jimenez <alejandro.j.jime...@oracle.com>
This patch makes it easy to read macros! Thanks.
Reviewed-by: Vasant Hegde <vasant.he...@amd.com>
> ---
> hw/i386/amd_iommu.h | 38 +++++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
> index 45a997af861e6..09352672bdcc2 100644
> --- a/hw/i386/amd_iommu.h
> +++ b/hw/i386/amd_iommu.h
> @@ -68,34 +68,34 @@
>
> #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)
> +
> #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_HEAD_MASK (((1ULL << 19) - 1) & ~0x0f)
> -#define AMDVI_MMIO_CMDBUF_TAIL_MASK AMDVI_MMIO_EVTLOG_HEAD_MASK
> +#define AMDVI_MMIO_CMDBUF_BASE_MASK GENMASK64(51, 12)
> +#define AMDVI_MMIO_CMDBUF_HEAD_MASK GENMASK64(18, 4)
> +#define AMDVI_MMIO_CMDBUF_TAIL_MASK GENMASK64(18, 4)
>
> #define AMDVI_MMIO_EVTLOG_SIZE_BYTE (AMDVI_MMIO_EVENT_BASE + 7)
> -#define AMDVI_MMIO_EVTLOG_SIZE_MASK AMDVI_MMIO_CMDBUF_SIZE_MASK
> -#define AMDVI_MMIO_EVTLOG_BASE_MASK AMDVI_MMIO_CMDBUF_BASE_MASK
> -#define AMDVI_MMIO_EVTLOG_HEAD_MASK (((1ULL << 19) - 1) & ~0x0f)
> -#define AMDVI_MMIO_EVTLOG_TAIL_MASK AMDVI_MMIO_EVTLOG_HEAD_MASK
> +#define AMDVI_MMIO_EVTLOG_SIZE_MASK 0x0f
> +#define AMDVI_MMIO_EVTLOG_BASE_MASK GENMASK64(51, 12)
> +#define AMDVI_MMIO_EVTLOG_HEAD_MASK GENMASK64(18, 4)
> +#define AMDVI_MMIO_EVTLOG_TAIL_MASK GENMASK64(18, 4)
>
> -#define AMDVI_MMIO_PPRLOG_SIZE_BYTE (AMDVI_MMIO_EVENT_BASE + 7)
Unrelated to this patch.. I think we should just read the AMDVI_MMIO_EVENT_BASE
register and extract the length. Same for rest of the buffer. We can do that as
improvement later.
-Vasant
