On 3/13/2025 7:53 PM, Alejandro Jimenez wrote:
On 3/12/25 12:12 AM, Arun Kodilkar, Sairaj wrote:
Hi Alejandro,
On 3/11/2025 8:54 PM, Alejandro Jimenez wrote:
[...]
--- 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.
Hi Sairaj,
I became aware of MAKE_64BIT_MASK() because you used it in your recent
patch, but as you mentioned they are similar but not the same. I
personally find that using bit indexes is less prone to errors since
that is the same format the spec uses to define the fields.
So translating a RSVD[6:2] field from the spec becomes:
GENMASK64(6, 2);
vs
MAKE_64BIT_MASK(6, 5);
The latter is more prone to off-by-one errors in my opinion, specially
when you are defining lots of masks. Perhaps more importantly, I'd like
to progressively convert the amd_iommu definitions to use GENMASK() and
the code that retrieves bit fields to use FIELD_GET().
I am planning on later porting the generic GENMASK* definitions (from
the kernel into "qemu/bitops.h", since the RISC-V IOMMU is also a
consumer of GENMASK, but I am trying to keep the focus on the AMD vIOMMU
for this series.
Thank you,
Alejandro
Hi Alejandro,
Yes indeed, GENMASK64() is more readable than the MAKE_64BIT_MASK().
and Since you are going to port it to generic layer, its better
to use GENMASK64().
Feel free to replace existing MAKE_64BIT_MASK() in my recent patches.
otherwise, I can modify it during next patch series.
Regards
Sairaj Kodilkar
/* 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
