Re: [PATCH 3/6] amd_iommu: Update bitmasks representing DTE reserved fields

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

Hi Alejandro,

On 3/11/2025 8:54 PM, Alejandro Jimenez wrote:
> The DTE validation method verifies that all bits in reserved DTE fields are
> unset. Update them according to the latest definition available in AMD I/O
> Virtualization Technology (IOMMU) Specification - Section 2.2.2.1 Device
> Table Entry Format. Remove the magic numbers and use a macro helper to
> generate bitmasks covering the specified ranges for better legibility.
> 
> Note that some reserved fields specify that events are generated when they
> contain non-zero bits, or checks are skipped under certain configurations.
> This change only updates the reserved masks, checks for special conditions
> are not yet implemented.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> ---
>   hw/i386/amd_iommu.c | 7 ++++---
>   hw/i386/amd_iommu.h | 9 ++++++---
>   2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 068eeb0cae..8b97abe28c 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -848,9 +848,10 @@ static inline uint64_t amdvi_get_perms(uint64_t entry)
>   static bool amdvi_validate_dte(AMDVIState *s, uint16_t devid,
>                                  uint64_t *dte)
>   {
> -    if ((dte[0] & AMDVI_DTE_LOWER_QUAD_RESERVED)
> -        || (dte[1] & AMDVI_DTE_MIDDLE_QUAD_RESERVED)
> -        || (dte[2] & AMDVI_DTE_UPPER_QUAD_RESERVED) || dte[3]) {
> +    if ((dte[0] & AMDVI_DTE_QUAD0_RESERVED) ||
> +        (dte[1] & AMDVI_DTE_QUAD1_RESERVED) ||
> +        (dte[2] & AMDVI_DTE_QUAD2_RESERVED) ||
> +        (dte[3] & AMDVI_DTE_QUAD3_RESERVED)) {
>           amdvi_log_illegaldevtab_error(s, devid,
>                                         s->devtab +
>                                         devid * AMDVI_DEVTAB_ENTRY_SIZE, 0);
> diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
> index 4c708f8d74..8d5d882a06 100644
> --- a/hw/i386/amd_iommu.h
> +++ b/hw/i386/amd_iommu.h
> @@ -25,6 +25,8 @@
>   #include "hw/i386/x86-iommu.h"
>   #include "qom/object.h"
>   
> +#define GENMASK64(h, l)  (((~0ULL) >> (63 - (h) + (l))) << (l))
> +

qemu provides the similar macro 'MAKE_64BIT_MASK' in file
'include/qemu/bitops.h', you can use this existing macro
instead of redefining.

>   /* Capability registers */
>   #define AMDVI_CAPAB_BAR_LOW           0x04
>   #define AMDVI_CAPAB_BAR_HIGH          0x08
> @@ -162,9 +164,10 @@
>   #define AMDVI_FEATURE_PC                  (1ULL << 9) /* Perf counters       */
>   
>   /* reserved DTE bits */
> -#define AMDVI_DTE_LOWER_QUAD_RESERVED  0x80300000000000fc
> -#define AMDVI_DTE_MIDDLE_QUAD_RESERVED 0x0000000000000100
> -#define AMDVI_DTE_UPPER_QUAD_RESERVED  0x08f0000000000000
> +#define AMDVI_DTE_QUAD0_RESERVED        (GENMASK64(6, 2) | GENMASK64(63, 63))
> +#define AMDVI_DTE_QUAD1_RESERVED        0
> +#define AMDVI_DTE_QUAD2_RESERVED        GENMASK64(53, 52)
> +#define AMDVI_DTE_QUAD3_RESERVED        (GENMASK64(14, 0) | GENMASK64(53, 48))
>   
>   /* AMDVI paging mode */
>   #define AMDVI_GATS_MODE                 (2ULL <<  12)

Regards
Sairaj Kodilkar