On 7/28/23 07:37, Daniel P. Berrangé wrote:
On Fri, Jul 28, 2023 at 04:27:46PM +0200, Thomas Huth wrote:
We might want to compile QEMU with Clang on Windows - but it
does not support the __attribute__((gcc_struct)) yet. So we
have to make sure that the structs will stay the same when
the compiler uses the "ms_struct" layout. The VTD_IR_TableEntry
struct is affected - rewrite it a little bit so that it works
fine with both struct layouts.

Reported-by: Daniel P. Berrangé <berra...@redhat.com>
Signed-off-by: Thomas Huth <th...@redhat.com>
---
  include/hw/i386/intel_iommu.h | 14 ++++++++------
  hw/i386/intel_iommu.c         |  2 +-
  2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 89dcbc5e1e..08bf220393 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -204,18 +204,20 @@ union VTD_IR_TableEntry {
  #endif
          uint32_t dest_id;            /* Destination ID */
          uint16_t source_id;          /* Source-ID */
+        uint16_t __reserved_2;       /* Reserved 2 */
  #if HOST_BIG_ENDIAN
-        uint64_t __reserved_2:44;    /* Reserved 2 */
-        uint64_t sid_vtype:2;        /* Source-ID Validation Type */
-        uint64_t sid_q:2;            /* Source-ID Qualifier */
+        uint32_t __reserved_3:28;    /* Reserved 3 */
+        uint32_t sid_vtype:2;        /* Source-ID Validation Type */
+        uint32_t sid_q:2;            /* Source-ID Qualifier */
  #else
-        uint64_t sid_q:2;            /* Source-ID Qualifier */
-        uint64_t sid_vtype:2;        /* Source-ID Validation Type */
-        uint64_t __reserved_2:44;    /* Reserved 2 */
+        uint32_t sid_q:2;            /* Source-ID Qualifier */
+        uint32_t sid_vtype:2;        /* Source-ID Validation Type */
+        uint32_t __reserved_3:28;    /* Reserved 3 */

Hasn't this has changed the struct layout in the else clause

  Old layout:

    source_id : 16
    sid_q : 2
    sid_vtype : 2
    reserved_2 : 44

  New layout

    source_id : 16
    reserved_2 : 16
    sid_q : 2
    sid_vtype : 2
    reserved_3 : 28

Was there something wrong with the change I suggested to
just make source_id be a bitfield too:

        uint64_t source_id: 16;          /* Source-ID */

which could make ms_struct layout avoid padding to the following
bitfields.

  #endif
      } QEMU_PACKED irte;
      uint64_t data[2];
  };

Making the point that we should never use bitfields to match hardware. This should be converted to <hw/registerfields.h>, ARRAY_FIELD_EX64.


r~

Reply via email to