[Qemu-devel] [PATCH 0/3] Some vt-d cleanups

[Qemu-devel] [PATCH 0/3] Some vt-d cleanups

[Peter Xu]

Patches 1-2 are related to endianess fixings. Patch 3 is to fix a
assertion issue.

Thanks,

Peter Xu (3):
  intel_iommu: fix several incorrect endianess and bit fields
  acpi: fix DMAR device scope for IOAPIC
  intel_iommu: fix incorrect assert

 hw/i386/acpi-build.c          | 3 ++-
 hw/i386/intel_iommu.c         | 4 ++--
 include/hw/acpi/acpi-defs.h   | 5 ++++-
 include/hw/i386/intel_iommu.h | 9 ++++-----
 4 files changed, 12 insertions(+), 9 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/3] intel_iommu: fix several incorrect endianess and bit fields

[Peter Xu]

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/intel_iommu.c         | 2 +-
 include/hw/i386/intel_iommu.h | 9 ++++-----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 1655a65..168cd5f 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2180,7 +2180,7 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
     }
 
     addr.data = origin->address & VTD_MSI_ADDR_LO_MASK;
-    if (le16_to_cpu(addr.addr.__head) != 0xfee) {
+    if (addr.addr.__head != 0xfee) {
         VTD_DPRINTF(GENERAL, "error: MSI addr low 32 bits invalid: "
                     "0x%"PRIx32, addr.data);
         return -VTD_FR_IR_REQ_RSVD;
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 1989c1e..405c9d1 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -123,7 +123,6 @@ enum {
 union VTD_IR_TableEntry {
     struct {
 #ifdef HOST_WORDS_BIGENDIAN
-        uint32_t dest_id:32;         /* Destination ID */
         uint32_t __reserved_1:8;     /* Reserved 1 */
         uint32_t vector:8;           /* Interrupt Vector */
         uint32_t irte_mode:1;        /* IRTE Mode */
@@ -147,9 +146,9 @@ union VTD_IR_TableEntry {
         uint32_t irte_mode:1;        /* IRTE Mode */
         uint32_t vector:8;           /* Interrupt Vector */
         uint32_t __reserved_1:8;     /* Reserved 1 */
-        uint32_t dest_id:32;         /* Destination ID */
 #endif
-        uint16_t source_id:16;       /* Source-ID */
+        uint32_t dest_id;            /* Destination ID */
+        uint16_t source_id;          /* Source-ID */
 #ifdef HOST_WORDS_BIGENDIAN
         uint64_t __reserved_2:44;    /* Reserved 2 */
         uint64_t sid_vtype:2;        /* Source-ID Validation Type */
@@ -220,7 +219,7 @@ struct VTD_MSIMessage {
             uint32_t dest:8;
             uint32_t __addr_head:12; /* 0xfee */
 #endif
-            uint32_t __addr_hi:32;
+            uint32_t __addr_hi;
         } QEMU_PACKED;
         uint64_t msi_addr;
     };
@@ -239,7 +238,7 @@ struct VTD_MSIMessage {
             uint16_t level:1;
             uint16_t trigger_mode:1;
 #endif
-            uint16_t __resved1:16;
+            uint16_t __resved1;
         } QEMU_PACKED;
         uint32_t msi_data;
     };
-- 
2.7.4

[Qemu-devel] [PATCH 2/3] acpi: fix DMAR device scope for IOAPIC

[Peter Xu]

We should not use cpu_to_le16() here, instead each of device/function
value is stored in a 8 byte field.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/acpi-build.c        | 3 ++-
 include/hw/acpi/acpi-defs.h | 5 ++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 93be96f..263ed3b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2599,7 +2599,8 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
     scope->length = ioapic_scope_size;
     scope->enumeration_id = ACPI_BUILD_IOAPIC_ID;
     scope->bus = Q35_PSEUDO_BUS_PLATFORM;
-    scope->path[0] = cpu_to_le16(Q35_PSEUDO_DEVFN_IOAPIC);
+    scope->path[0].device = PCI_SLOT(Q35_PSEUDO_DEVFN_IOAPIC);
+    scope->path[0].function = PCI_FUNC(Q35_PSEUDO_DEVFN_IOAPIC);
 
     build_header(linker, table_data, (void *)(table_data->data + dmar_start),
                  "DMAR", table_data->len - dmar_start, 1, NULL, NULL);
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index d1d1d61..154f3b8 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -619,7 +619,10 @@ struct AcpiDmarDeviceScope {
     uint16_t reserved;
     uint8_t enumeration_id;
     uint8_t bus;
-    uint16_t path[0];           /* list of dev:func pairs */
+    struct {
+        uint8_t device;
+        uint8_t function;
+    } path[0];
 } QEMU_PACKED;
 typedef struct AcpiDmarDeviceScope AcpiDmarDeviceScope;
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH 2/3] acpi: fix DMAR device scope for IOAPIC

[Igor Mammedov]

On Mon, 31 Oct 2016 15:34:39 +0800
Peter Xu <peterx@redhat.com> wrote:

> We should not use cpu_to_le16() here, instead each of device/function
> value is stored in a 8 byte field.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
it better matches spec and the same as linux kernel handles 'path' so

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/acpi-build.c        | 3 ++-
>  include/hw/acpi/acpi-defs.h | 5 ++++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 93be96f..263ed3b 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2599,7 +2599,8 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
>      scope->length = ioapic_scope_size;
>      scope->enumeration_id = ACPI_BUILD_IOAPIC_ID;
>      scope->bus = Q35_PSEUDO_BUS_PLATFORM;
> -    scope->path[0] = cpu_to_le16(Q35_PSEUDO_DEVFN_IOAPIC);
> +    scope->path[0].device = PCI_SLOT(Q35_PSEUDO_DEVFN_IOAPIC);
> +    scope->path[0].function = PCI_FUNC(Q35_PSEUDO_DEVFN_IOAPIC);
>  
>      build_header(linker, table_data, (void *)(table_data->data + dmar_start),
>                   "DMAR", table_data->len - dmar_start, 1, NULL, NULL);
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index d1d1d61..154f3b8 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -619,7 +619,10 @@ struct AcpiDmarDeviceScope {
>      uint16_t reserved;
>      uint8_t enumeration_id;
>      uint8_t bus;
> -    uint16_t path[0];           /* list of dev:func pairs */
> +    struct {
> +        uint8_t device;
> +        uint8_t function;
> +    } path[0];
>  } QEMU_PACKED;
>  typedef struct AcpiDmarDeviceScope AcpiDmarDeviceScope;
>  

[Qemu-devel] [PATCH 3/3] intel_iommu: fix incorrect assert

[Peter Xu]

Reported-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/intel_iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 168cd5f..17a3de1 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2463,7 +2463,7 @@ static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
     IntelIOMMUState *s = opaque;
     VTDAddressSpace *vtd_as;
 
-    assert(0 <= devfn && devfn <= X86_IOMMU_PCI_DEVFN_MAX);
+    assert(0 <= devfn && devfn < X86_IOMMU_PCI_DEVFN_MAX);
 
     vtd_as = vtd_find_add_as(s, bus, devfn);
     return &vtd_as->as;
-- 
2.7.4