[Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Michael S. Tsirkin]

The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' into staging (2018-10-23 17:20:23 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to 6a9fb4e1ba5594cde7739068617ad88e6117db93:

  vhost-scsi: prevent using uninitialized vqs (2018-10-24 20:50:13 -0400)

----------------------------------------------------------------
pci, pc, virtio: fixes, features

AMD IOMMU VAPIC support + fixes all over the place.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Gerd Hoffmann (1):
      pci-testdev: add optional memory bar

Laszlo Ersek (4):
      MAINTAINERS: list "tests/acpi-test-data" files in ACPI/SMBIOS section
      hw/pci-host/x86: extract get_pci_hole64_start_value() helpers
      hw/pci-host/x86: extend the 64-bit PCI hole relative to the fw-assigned base
      tests/bios-tables-test: add 64-bit PCI MMIO aperture round-up test on Q35

Li Qiang (3):
      i440fx: use ARRAY_SIZE for pam_regions
      piix: use TYPE_FOO constants than string constats
      piix_pci: fix i440fx data sheet link

Mao Zhongyi (1):
      pci_bridge: fix typo in comment

Peter Xu (4):
      intel_iommu: introduce vtd_reset_caches()
      intel_iommu: better handling of dmar state switch
      intel_iommu: move ce fetching out when sync shadow
      intel_iommu: handle invalid ce for shadow sync

Philippe Mathieu-Daudé (3):
      hw/pci-bridge/xio3130: Remove unused functions
      hw/pci-bridge/ioh3420: Remove unuseful header
      hw/pci: Add missing include

Singh, Brijesh (9):
      x86_iommu: move the kernel-irqchip check in common code
      x86_iommu: move vtd_generate_msi_message in common file
      x86_iommu/amd: remove V=1 check from amdvi_validate_dte()
      x86_iommu/amd: make the address space naming consistent with intel-iommu
      x86_iommu/amd: Prepare for interrupt remap support
      x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled
      i386: acpi: add IVHD device entry for IOAPIC
      x86_iommu/amd: Add interrupt remap support when VAPIC is enabled
      x86_iommu/amd: Enable Guest virtual APIC support

Yaowei Bai (1):
      virtio-blk: fix comment for virtio_blk_rw_complete

Yongji Xie (1):
      vhost-user-blk: start vhost when guest kicks

yuchenlin (1):
      vhost-scsi: prevent using uninitialized vqs

 docs/specs/pci-testdev.txt           |  15 +-
 hw/i386/amd_iommu.h                  |  96 +++++++-
 hw/pci-bridge/ioh3420.h              |   6 -
 hw/pci-bridge/xio3130_downstream.h   |  11 -
 hw/pci-bridge/xio3130_upstream.h     |  10 -
 include/hw/i386/intel_iommu.h        |  59 -----
 include/hw/i386/x86-iommu.h          |  66 ++++++
 include/hw/pci/pci_bus.h             |   2 +
 hw/block/vhost-user-blk.c            |  25 +++
 hw/block/virtio-blk.c                |   4 +-
 hw/i386/acpi-build.c                 |  31 ++-
 hw/i386/amd_iommu.c                  | 414 ++++++++++++++++++++++++++++++++++-
 hw/i386/intel_iommu.c                | 131 +++++------
 hw/i386/x86-iommu.c                  |  33 +++
 hw/misc/pci-testdev.c                |  19 ++
 hw/pci-bridge/ioh3420.c              |   2 +-
 hw/pci-bridge/xio3130_downstream.c   |  28 +--
 hw/pci-bridge/xio3130_upstream.c     |  24 +-
 hw/pci-host/piix.c                   |  34 ++-
 hw/pci-host/q35.c                    |  17 +-
 hw/pci/pci_bridge.c                  |   2 +-
 hw/scsi/vhost-scsi.c                 |   2 +-
 tests/bios-tables-test.c             |  16 ++
 MAINTAINERS                          |   2 +
 hw/i386/trace-events                 |  14 ++
 tests/acpi-test-data/q35/DSDT.mmio64 | Bin 0 -> 8947 bytes
 tests/acpi-test-data/q35/SRAT.mmio64 | Bin 0 -> 224 bytes
 27 files changed, 811 insertions(+), 252 deletions(-)
 delete mode 100644 hw/pci-bridge/ioh3420.h
 delete mode 100644 hw/pci-bridge/xio3130_downstream.h
 delete mode 100644 hw/pci-bridge/xio3130_upstream.h
 create mode 100644 tests/acpi-test-data/q35/DSDT.mmio64
 create mode 100644 tests/acpi-test-data/q35/SRAT.mmio64

[Qemu-devel] [PULL v2 01/28] virtio-blk: fix comment for virtio_blk_rw_complete

[Michael S. Tsirkin]

From: Yaowei Bai <baiyaowei@cmss.chinamobile.com>

Here should be submit_requests, there is no submit_merged_requests
function.

Signed-off-by: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/block/virtio-blk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 225fe44b7a..83cf5c01f9 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -97,8 +97,8 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
 
         if (req->qiov.nalloc != -1) {
             /* If nalloc is != 1 req->qiov is a local copy of the original
-             * external iovec. It was allocated in submit_merged_requests
-             * to be able to merge requests. */
+             * external iovec. It was allocated in submit_requests to be
+             * able to merge requests. */
             qemu_iovec_destroy(&req->qiov);
         }
 
-- 
MST

[Qemu-devel] [PULL v2 02/28] intel_iommu: introduce vtd_reset_caches()

[Michael S. Tsirkin]

From: Peter Xu <peterx@redhat.com>

Provide the function and use it in vtd_init().  Used to reset both
context entry cache and iotlb cache for the whole IOMMU unit.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/intel_iommu.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3dfada19a6..1137861a9d 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -227,6 +227,14 @@ static void vtd_reset_iotlb(IntelIOMMUState *s)
     vtd_iommu_unlock(s);
 }
 
+static void vtd_reset_caches(IntelIOMMUState *s)
+{
+    vtd_iommu_lock(s);
+    vtd_reset_iotlb_locked(s);
+    vtd_reset_context_cache_locked(s);
+    vtd_iommu_unlock(s);
+}
+
 static uint64_t vtd_get_iotlb_key(uint64_t gfn, uint16_t source_id,
                                   uint32_t level)
 {
@@ -3160,10 +3168,7 @@ static void vtd_init(IntelIOMMUState *s)
         s->cap |= VTD_CAP_CM;
     }
 
-    vtd_iommu_lock(s);
-    vtd_reset_context_cache_locked(s);
-    vtd_reset_iotlb_locked(s);
-    vtd_iommu_unlock(s);
+    vtd_reset_caches(s);
 
     /* Define registers with default values and bit semantics */
     vtd_define_long(s, DMAR_VER_REG, 0x10UL, 0, 0);
-- 
MST

[Qemu-devel] [PULL v2 03/28] intel_iommu: better handling of dmar state switch

[Michael S. Tsirkin]

From: Peter Xu <peterx@redhat.com>

QEMU is not handling the global DMAR switch well, especially when from
"on" to "off".

Let's first take the example of system reset.

Assuming that a guest has IOMMU enabled.  When it reboots, we will drop
all the existing DMAR mappings to handle the system reset, however we'll
still keep the existing memory layouts which has the IOMMU memory region
enabled.  So after the reboot and before the kernel reloads again, there
will be no mapping at all for the host device.  That's problematic since
any software (for example, SeaBIOS) that runs earlier than the kernel
after the reboot will assume the IOMMU is disabled, so any DMA from the
software will fail.

For example, a guest that boots on an assigned NVMe device might fail to
find the boot device after a system reboot/reset and we'll be able to
observe SeaBIOS errors if we capture the debugging log:

  WARNING - Timeout at nvme_wait:144!

Meanwhile, we should see DMAR errors on the host of that NVMe device.
It's the DMA fault that caused a NVMe driver timeout.

The correct fix should be that we do proper switching of device DMA
address spaces when system resets, which will setup correct memory
regions and notify the backend of the devices.  This might not affect
much on non-assigned devices since QEMU VT-d emulation will assume a
default passthrough mapping if DMAR is not enabled in the GCMD
register (please refer to vtd_iommu_translate).  However that's required
for an assigned devices, since that'll rebuild the correct GPA to HPA
mapping that is needed for any DMA operation during guest bootstrap.

Besides the system reset, we have some other places that might change
the global DMAR status and we'd better do the same thing there.  For
example, when we change the state of GCMD register, or the DMAR root
pointer.  Do the same refresh for all these places.  For these two
places we'll also need to explicitly invalidate the context entry cache
and iotlb cache.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1625173
CC: QEMU Stable <qemu-stable@nongnu.org>
Reported-by: Cong Li <coli@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
--
v2:
- do the same for GCMD write, or root pointer update [Alex]
- test is carried out by me this time, by observing the
  vtd_switch_address_space tracepoint after system reboot
v3:
- rewrite commit message as suggested by Alex
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/intel_iommu.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 1137861a9d..306708eb3b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -37,6 +37,8 @@
 #include "kvm_i386.h"
 #include "trace.h"
 
+static void vtd_address_space_refresh_all(IntelIOMMUState *s);
+
 static void vtd_define_quad(IntelIOMMUState *s, hwaddr addr, uint64_t val,
                             uint64_t wmask, uint64_t w1cmask)
 {
@@ -1436,7 +1438,7 @@ static void vtd_context_global_invalidate(IntelIOMMUState *s)
         vtd_reset_context_cache_locked(s);
     }
     vtd_iommu_unlock(s);
-    vtd_switch_address_space_all(s);
+    vtd_address_space_refresh_all(s);
     /*
      * From VT-d spec 6.5.2.1, a global context entry invalidation
      * should be followed by a IOTLB global invalidation, so we should
@@ -1727,6 +1729,8 @@ static void vtd_handle_gcmd_srtp(IntelIOMMUState *s)
     vtd_root_table_setup(s);
     /* Ok - report back to driver */
     vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_RTPS);
+    vtd_reset_caches(s);
+    vtd_address_space_refresh_all(s);
 }
 
 /* Set Interrupt Remap Table Pointer */
@@ -1759,7 +1763,8 @@ static void vtd_handle_gcmd_te(IntelIOMMUState *s, bool en)
         vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_TES, 0);
     }
 
-    vtd_switch_address_space_all(s);
+    vtd_reset_caches(s);
+    vtd_address_space_refresh_all(s);
 }
 
 /* Handle Interrupt Remap Enable/Disable */
@@ -3059,6 +3064,12 @@ static void vtd_address_space_unmap_all(IntelIOMMUState *s)
     }
 }
 
+static void vtd_address_space_refresh_all(IntelIOMMUState *s)
+{
+    vtd_address_space_unmap_all(s);
+    vtd_switch_address_space_all(s);
+}
+
 static int vtd_replay_hook(IOMMUTLBEntry *entry, void *private)
 {
     memory_region_notify_one((IOMMUNotifier *)private, entry);
@@ -3231,11 +3242,7 @@ static void vtd_reset(DeviceState *dev)
     IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
 
     vtd_init(s);
-
-    /*
-     * When device reset, throw away all mappings and external caches
-     */
-    vtd_address_space_unmap_all(s);
+    vtd_address_space_refresh_all(s);
 }
 
 static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
-- 
MST

[Qemu-devel] [PULL v2 04/28] intel_iommu: move ce fetching out when sync shadow

[Michael S. Tsirkin]

From: Peter Xu <peterx@redhat.com>

There are two callers for vtd_sync_shadow_page_table_range(): one
provided a valid context entry and one not.  Move that fetching
operation into the caller vtd_sync_shadow_page_table() where we need to
fetch the context entry.

Meanwhile, remove the error_report_once() directly since we're already
tracing all the error cases in the previous call.  Instead, return error
number back to caller.  This will not change anything functional since
callers are dropping it after all.

We do this move majorly because we want to do something more later in
vtd_sync_shadow_page_table().

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/intel_iommu.c | 41 +++++++++++++----------------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 306708eb3b..25e54671f4 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -1045,7 +1045,6 @@ static int vtd_sync_shadow_page_hook(IOMMUTLBEntry *entry,
     return 0;
 }
 
-/* If context entry is NULL, we'll try to fetch it on our own. */
 static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as,
                                             VTDContextEntry *ce,
                                             hwaddr addr, hwaddr size)
@@ -1057,39 +1056,25 @@ static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as,
         .notify_unmap = true,
         .aw = s->aw_bits,
         .as = vtd_as,
+        .domain_id = VTD_CONTEXT_ENTRY_DID(ce->hi),
     };
-    VTDContextEntry ce_cache;
-    int ret;
 
-    if (ce) {
-        /* If the caller provided context entry, use it */
-        ce_cache = *ce;
-    } else {
-        /* If the caller didn't provide ce, try to fetch */
-        ret = vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
-                                       vtd_as->devfn, &ce_cache);
-        if (ret) {
-            /*
-             * This should not really happen, but in case it happens,
-             * we just skip the sync for this time.  After all we even
-             * don't have the root table pointer!
-             */
-            error_report_once("%s: invalid context entry for bus 0x%x"
-                              " devfn 0x%x",
-                              __func__, pci_bus_num(vtd_as->bus),
-                              vtd_as->devfn);
-            return 0;
-        }
-    }
-
-    info.domain_id = VTD_CONTEXT_ENTRY_DID(ce_cache.hi);
-
-    return vtd_page_walk(&ce_cache, addr, addr + size, &info);
+    return vtd_page_walk(ce, addr, addr + size, &info);
 }
 
 static int vtd_sync_shadow_page_table(VTDAddressSpace *vtd_as)
 {
-    return vtd_sync_shadow_page_table_range(vtd_as, NULL, 0, UINT64_MAX);
+    int ret;
+    VTDContextEntry ce;
+
+    ret = vtd_dev_to_context_entry(vtd_as->iommu_state,
+                                   pci_bus_num(vtd_as->bus),
+                                   vtd_as->devfn, &ce);
+    if (ret) {
+        return ret;
+    }
+
+    return vtd_sync_shadow_page_table_range(vtd_as, &ce, 0, UINT64_MAX);
 }
 
 /*
-- 
MST

[Qemu-devel] [PULL v2 05/28] intel_iommu: handle invalid ce for shadow sync

[Michael S. Tsirkin]

From: Peter Xu <peterx@redhat.com>

We should handle VTD_FR_CONTEXT_ENTRY_P properly when synchronizing
shadow page tables.  Having invalid context entry there is perfectly
valid when we move a device out of an existing domain.  When that
happens, instead of posting an error we invalidate the whole region.

Without this patch, QEMU will crash if we do these steps:

(1) start QEMU with VT-d IOMMU and two 10G NICs (ixgbe)
(2) bind the NICs with vfio-pci in the guest
(3) start testpmd with the NICs applied
(4) stop testpmd
(5) rebind the NIC back to ixgbe kernel driver

The patch should fix it.

Reported-by: Pei Zhang <pezhang@redhat.com>
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1627272
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/intel_iommu.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 25e54671f4..f24ebfca1c 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -38,6 +38,7 @@
 #include "trace.h"
 
 static void vtd_address_space_refresh_all(IntelIOMMUState *s);
+static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n);
 
 static void vtd_define_quad(IntelIOMMUState *s, hwaddr addr, uint64_t val,
                             uint64_t wmask, uint64_t w1cmask)
@@ -1066,11 +1067,27 @@ static int vtd_sync_shadow_page_table(VTDAddressSpace *vtd_as)
 {
     int ret;
     VTDContextEntry ce;
+    IOMMUNotifier *n;
 
     ret = vtd_dev_to_context_entry(vtd_as->iommu_state,
                                    pci_bus_num(vtd_as->bus),
                                    vtd_as->devfn, &ce);
     if (ret) {
+        if (ret == -VTD_FR_CONTEXT_ENTRY_P) {
+            /*
+             * It's a valid scenario to have a context entry that is
+             * not present.  For example, when a device is removed
+             * from an existing domain then the context entry will be
+             * zeroed by the guest before it was put into another
+             * domain.  When this happens, instead of synchronizing
+             * the shadow pages we should invalidate all existing
+             * mappings and notify the backends.
+             */
+            IOMMU_NOTIFIER_FOREACH(n, &vtd_as->iommu) {
+                vtd_address_space_unmap(vtd_as, n);
+            }
+            ret = 0;
+        }
         return ret;
     }
 
-- 
MST

[Qemu-devel] [PULL v2 06/28] vhost-user-blk: start vhost when guest kicks

[Michael S. Tsirkin]

From: Yongji Xie <elohimes@gmail.com>

Some old guests (before commit 7a11370e5: "virtio_blk: enable VQs early")
kick virtqueue before setting VIRTIO_CONFIG_S_DRIVER_OK. This violates
the virtio spec. But virtio 1.0 transitional devices support this behaviour.
So we should start vhost when guest kicks in this case.

Signed-off-by: Yongji Xie <xieyongji@baidu.com>
Signed-off-by: Chai Wen <chaiwen@baidu.com>
Signed-off-by: Ni Xun <nixun@baidu.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/block/vhost-user-blk.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index d755223643..1451940845 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -217,7 +217,32 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev,
 
 static void vhost_user_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
+    VHostUserBlk *s = VHOST_USER_BLK(vdev);
+    int i;
 
+    if (!(virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
+        !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1))) {
+        return;
+    }
+
+    if (s->dev.started) {
+        return;
+    }
+
+    /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
+     * vhost here instead of waiting for .set_status().
+     */
+    vhost_user_blk_start(vdev);
+
+    /* Kick right away to begin processing requests already in vring */
+    for (i = 0; i < s->dev.nvqs; i++) {
+        VirtQueue *kick_vq = virtio_get_queue(vdev, i);
+
+        if (!virtio_queue_get_desc_addr(vdev, i)) {
+            continue;
+        }
+        event_notifier_set(virtio_queue_get_host_notifier(kick_vq));
+    }
 }
 
 static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
-- 
MST

[Qemu-devel] [PULL v2 07/28] x86_iommu: move the kernel-irqchip check in common code

[Michael S. Tsirkin]

From: "Singh, Brijesh" <brijesh.singh@amd.com>

Interrupt remapping needs kernel-irqchip={off|split} on both Intel and AMD
platforms. Move the check in common place.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/intel_iommu.c | 7 -------
 hw/i386/x86-iommu.c   | 9 +++++++++
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index f24ebfca1c..015a6fc492 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3262,13 +3262,6 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
 {
     X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
 
-    /* Currently Intel IOMMU IR only support "kernel-irqchip={off|split}" */
-    if (x86_iommu->intr_supported && kvm_irqchip_in_kernel() &&
-        !kvm_irqchip_is_split()) {
-        error_setg(errp, "Intel Interrupt Remapping cannot work with "
-                         "kernel-irqchip=on, please use 'split|off'.");
-        return false;
-    }
     if (s->intr_eim == ON_OFF_AUTO_ON && !x86_iommu->intr_supported) {
         error_setg(errp, "eim=on cannot be selected without intremap=on");
         return false;
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 8a01a2dd25..7440cb8d60 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -25,6 +25,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "trace.h"
+#include "sysemu/kvm.h"
 
 void x86_iommu_iec_register_notifier(X86IOMMUState *iommu,
                                      iec_notify_fn fn, void *data)
@@ -94,6 +95,14 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    /* Both Intel and AMD IOMMU IR only support "kernel-irqchip={off|split}" */
+    if (x86_iommu->intr_supported && kvm_irqchip_in_kernel() &&
+        !kvm_irqchip_is_split()) {
+        error_setg(errp, "Interrupt Remapping cannot work with "
+                         "kernel-irqchip=on, please use 'split|off'.");
+        return;
+    }
+
     if (x86_class->realize) {
         x86_class->realize(dev, errp);
     }
-- 
MST

[Qemu-devel] [PULL v2 08/28] x86_iommu: move vtd_generate_msi_message in common file

[Michael S. Tsirkin]

From: "Singh, Brijesh" <brijesh.singh@amd.com>

The vtd_generate_msi_message() in intel-iommu is used to construct a MSI
Message from IRQ. A similar function will be needed when we add interrupt
remapping support in amd-iommu. Moving the function in common file to
avoid the code duplication. Rename it to x86_iommu_irq_to_msi_message().
There is no logic changes in the code flow.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Suggested-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/i386/intel_iommu.h | 59 -------------------------------
 include/hw/i386/x86-iommu.h   | 66 +++++++++++++++++++++++++++++++++++
 hw/i386/intel_iommu.c         | 32 +++--------------
 hw/i386/x86-iommu.c           | 24 +++++++++++++
 4 files changed, 94 insertions(+), 87 deletions(-)

diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index fbfedcb1c0..ed4e758273 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -66,8 +66,6 @@ typedef struct VTDIOTLBEntry VTDIOTLBEntry;
 typedef struct VTDBus VTDBus;
 typedef union VTD_IR_TableEntry VTD_IR_TableEntry;
 typedef union VTD_IR_MSIAddress VTD_IR_MSIAddress;
-typedef struct VTDIrq VTDIrq;
-typedef struct VTD_MSIMessage VTD_MSIMessage;
 
 /* Context-Entry */
 struct VTDContextEntry {
@@ -197,63 +195,6 @@ union VTD_IR_MSIAddress {
     uint32_t data;
 };
 
-/* Generic IRQ entry information */
-struct VTDIrq {
-    /* Used by both IOAPIC/MSI interrupt remapping */
-    uint8_t trigger_mode;
-    uint8_t vector;
-    uint8_t delivery_mode;
-    uint32_t dest;
-    uint8_t dest_mode;
-
-    /* only used by MSI interrupt remapping */
-    uint8_t redir_hint;
-    uint8_t msi_addr_last_bits;
-};
-
-struct VTD_MSIMessage {
-    union {
-        struct {
-#ifdef HOST_WORDS_BIGENDIAN
-            uint32_t __addr_head:12; /* 0xfee */
-            uint32_t dest:8;
-            uint32_t __reserved:8;
-            uint32_t redir_hint:1;
-            uint32_t dest_mode:1;
-            uint32_t __not_used:2;
-#else
-            uint32_t __not_used:2;
-            uint32_t dest_mode:1;
-            uint32_t redir_hint:1;
-            uint32_t __reserved:8;
-            uint32_t dest:8;
-            uint32_t __addr_head:12; /* 0xfee */
-#endif
-            uint32_t __addr_hi;
-        } QEMU_PACKED;
-        uint64_t msi_addr;
-    };
-    union {
-        struct {
-#ifdef HOST_WORDS_BIGENDIAN
-            uint16_t trigger_mode:1;
-            uint16_t level:1;
-            uint16_t __resved:3;
-            uint16_t delivery_mode:3;
-            uint16_t vector:8;
-#else
-            uint16_t vector:8;
-            uint16_t delivery_mode:3;
-            uint16_t __resved:3;
-            uint16_t level:1;
-            uint16_t trigger_mode:1;
-#endif
-            uint16_t __resved1;
-        } QEMU_PACKED;
-        uint32_t msi_data;
-    };
-};
-
 /* When IR is enabled, all MSI/MSI-X data bits should be zero */
 #define VTD_IR_MSI_DATA          (0)
 
diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h
index 7c71fc7470..2b22a579a3 100644
--- a/include/hw/i386/x86-iommu.h
+++ b/include/hw/i386/x86-iommu.h
@@ -22,6 +22,7 @@
 
 #include "hw/sysbus.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/msi.h"
 
 #define  TYPE_X86_IOMMU_DEVICE  ("x86-iommu")
 #define  X86_IOMMU_DEVICE(obj) \
@@ -35,6 +36,8 @@
 
 typedef struct X86IOMMUState X86IOMMUState;
 typedef struct X86IOMMUClass X86IOMMUClass;
+typedef struct X86IOMMUIrq X86IOMMUIrq;
+typedef struct X86IOMMU_MSIMessage X86IOMMU_MSIMessage;
 
 typedef enum IommuType {
     TYPE_INTEL,
@@ -78,6 +81,63 @@ struct X86IOMMUState {
     QLIST_HEAD(, IEC_Notifier) iec_notifiers; /* IEC notify list */
 };
 
+/* Generic IRQ entry information when interrupt remapping is enabled */
+struct X86IOMMUIrq {
+    /* Used by both IOAPIC/MSI interrupt remapping */
+    uint8_t trigger_mode;
+    uint8_t vector;
+    uint8_t delivery_mode;
+    uint32_t dest;
+    uint8_t dest_mode;
+
+    /* only used by MSI interrupt remapping */
+    uint8_t redir_hint;
+    uint8_t msi_addr_last_bits;
+};
+
+struct X86IOMMU_MSIMessage {
+    union {
+        struct {
+#ifdef HOST_WORDS_BIGENDIAN
+            uint32_t __addr_head:12; /* 0xfee */
+            uint32_t dest:8;
+            uint32_t __reserved:8;
+            uint32_t redir_hint:1;
+            uint32_t dest_mode:1;
+            uint32_t __not_used:2;
+#else
+            uint32_t __not_used:2;
+            uint32_t dest_mode:1;
+            uint32_t redir_hint:1;
+            uint32_t __reserved:8;
+            uint32_t dest:8;
+            uint32_t __addr_head:12; /* 0xfee */
+#endif
+            uint32_t __addr_hi;
+        } QEMU_PACKED;
+        uint64_t msi_addr;
+    };
+    union {
+        struct {
+#ifdef HOST_WORDS_BIGENDIAN
+            uint16_t trigger_mode:1;
+            uint16_t level:1;
+            uint16_t __resved:3;
+            uint16_t delivery_mode:3;
+            uint16_t vector:8;
+#else
+            uint16_t vector:8;
+            uint16_t delivery_mode:3;
+            uint16_t __resved:3;
+            uint16_t level:1;
+            uint16_t trigger_mode:1;
+#endif
+            uint16_t __resved1;
+        } QEMU_PACKED;
+        uint32_t msi_data;
+    };
+};
+
 /**
  * x86_iommu_get_default - get default IOMMU device
  * @return: pointer to default IOMMU device
@@ -110,4 +170,10 @@ void x86_iommu_iec_register_notifier(X86IOMMUState *iommu,
 void x86_iommu_iec_notify_all(X86IOMMUState *iommu, bool global,
                               uint32_t index, uint32_t mask);
 
+/**
+ * x86_iommu_irq_to_msi_message - Populate one MSIMessage from X86IOMMUIrq
+ * @X86IOMMUIrq: The IRQ information
+ * @out: Output MSI message
+ */
+void x86_iommu_irq_to_msi_message(X86IOMMUIrq *irq, MSIMessage *out);
 #endif
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 015a6fc492..d97bcbc2f7 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2716,7 +2716,7 @@ static int vtd_irte_get(IntelIOMMUState *iommu, uint16_t index,
 
 /* Fetch IRQ information of specific IR index */
 static int vtd_remap_irq_get(IntelIOMMUState *iommu, uint16_t index,
-                             VTDIrq *irq, uint16_t sid)
+                             X86IOMMUIrq *irq, uint16_t sid)
 {
     VTD_IR_TableEntry irte = {};
     int ret = 0;
@@ -2745,30 +2745,6 @@ static int vtd_remap_irq_get(IntelIOMMUState *iommu, uint16_t index,
     return 0;
 }
 
-/* Generate one MSI message from VTDIrq info */
-static void vtd_generate_msi_message(VTDIrq *irq, MSIMessage *msg_out)
-{
-    VTD_MSIMessage msg = {};
-
-    /* Generate address bits */
-    msg.dest_mode = irq->dest_mode;
-    msg.redir_hint = irq->redir_hint;
-    msg.dest = irq->dest;
-    msg.__addr_hi = irq->dest & 0xffffff00;
-    msg.__addr_head = cpu_to_le32(0xfee);
-    /* Keep this from original MSI address bits */
-    msg.__not_used = irq->msi_addr_last_bits;
-
-    /* Generate data bits */
-    msg.vector = irq->vector;
-    msg.delivery_mode = irq->delivery_mode;
-    msg.level = 1;
-    msg.trigger_mode = irq->trigger_mode;
-
-    msg_out->address = msg.msi_addr;
-    msg_out->data = msg.msi_data;
-}
-
 /* Interrupt remapping for MSI/MSI-X entry */
 static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
                                    MSIMessage *origin,
@@ -2778,7 +2754,7 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
     int ret = 0;
     VTD_IR_MSIAddress addr;
     uint16_t index;
-    VTDIrq irq = {};
+    X86IOMMUIrq irq = {};
 
     assert(origin && translated);
 
@@ -2857,8 +2833,8 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
      */
     irq.msi_addr_last_bits = addr.addr.__not_care;
 
-    /* Translate VTDIrq to MSI message */
-    vtd_generate_msi_message(&irq, translated);
+    /* Translate X86IOMMUIrq to MSI message */
+    x86_iommu_irq_to_msi_message(&irq, translated);
 
 out:
     trace_vtd_ir_remap_msi(origin->address, origin->data,
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 7440cb8d60..abc3c03158 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -53,6 +53,30 @@ void x86_iommu_iec_notify_all(X86IOMMUState *iommu, bool global,
     }
 }
 
+/* Generate one MSI message from VTDIrq info */
+void x86_iommu_irq_to_msi_message(X86IOMMUIrq *irq, MSIMessage *msg_out)
+{
+    X86IOMMU_MSIMessage msg = {};
+
+    /* Generate address bits */
+    msg.dest_mode = irq->dest_mode;
+    msg.redir_hint = irq->redir_hint;
+    msg.dest = irq->dest;
+    msg.__addr_hi = irq->dest & 0xffffff00;
+    msg.__addr_head = cpu_to_le32(0xfee);
+    /* Keep this from original MSI address bits */
+    msg.__not_used = irq->msi_addr_last_bits;
+
+    /* Generate data bits */
+    msg.vector = irq->vector;
+    msg.delivery_mode = irq->delivery_mode;
+    msg.level = 1;
+    msg.trigger_mode = irq->trigger_mode;
+
+    msg_out->address = msg.msi_addr;
+    msg_out->data = msg.msi_data;
+}
+
 /* Default X86 IOMMU device */
 static X86IOMMUState *x86_iommu_default = NULL;
 
-- 
MST

[Qemu-devel] [PULL v2 09/28] x86_iommu/amd: remove V=1 check from amdvi_validate_dte()

[Michael S. Tsirkin]

From: "Singh, Brijesh" <brijesh.singh@amd.com>

Currently, the amdvi_validate_dte() assumes that a valid DTE will
always have V=1. This is not true. The V=1 means that bit[127:1] are
valid. A valid DTE can have IV=1 and V=0 (i.e address translation
disabled and interrupt remapping enabled)

Remove the V=1 check from amdvi_validate_dte(), make the caller
responsible to check for V or IV bits.

This also fixes a bug in existing code that when error is
detected during the translation we'll fail the translation
instead of assuming a passthrough mode.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/amd_iommu.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 1fd669fef8..7206bb09c2 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -807,7 +807,7 @@ static inline uint64_t amdvi_get_perms(uint64_t entry)
            AMDVI_DEV_PERM_SHIFT;
 }
 
-/* a valid entry should have V = 1 and reserved bits honoured */
+/* validate that reserved bits are honoured */
 static bool amdvi_validate_dte(AMDVIState *s, uint16_t devid,
                                uint64_t *dte)
 {
@@ -820,7 +820,7 @@ static bool amdvi_validate_dte(AMDVIState *s, uint16_t devid,
         return false;
     }
 
-    return dte[0] & AMDVI_DEV_VALID;
+    return true;
 }
 
 /* get a device table entry given the devid */
@@ -966,8 +966,12 @@ static void amdvi_do_translate(AMDVIAddressSpace *as, hwaddr addr,
         return;
     }
 
-    /* devices with V = 0 are not translated */
     if (!amdvi_get_dte(s, devid, entry)) {
+        return;
+    }
+
+    /* devices with V = 0 are not translated */
+    if (!(entry[0] & AMDVI_DEV_VALID)) {
         goto out;
     }
 
-- 
MST

[Qemu-devel] [PULL v2 10/28] x86_iommu/amd: make the address space naming consistent with intel-iommu

[Michael S. Tsirkin]

From: "Singh, Brijesh" <brijesh.singh@amd.com>

To be consistent with intel-iommu:

- rename the address space to use '_' instead of '-'
- update the memory region relationships

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/amd_iommu.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 7206bb09c2..4bec1c6688 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -55,6 +55,7 @@ struct AMDVIAddressSpace {
     uint8_t bus_num;            /* bus number                           */
     uint8_t devfn;              /* device function                      */
     AMDVIState *iommu_state;    /* AMDVI - one per machine              */
+    MemoryRegion root;          /* AMDVI Root memory map region */
     IOMMUMemoryRegion iommu;    /* Device's address translation region  */
     MemoryRegion iommu_ir;      /* Device's interrupt remapping region  */
     AddressSpace as;            /* device's corresponding address space */
@@ -1032,8 +1033,9 @@ static IOMMUTLBEntry amdvi_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
 
 static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 {
+    char name[128];
     AMDVIState *s = opaque;
-    AMDVIAddressSpace **iommu_as;
+    AMDVIAddressSpace **iommu_as, *amdvi_dev_as;
     int bus_num = pci_bus_num(bus);
 
     iommu_as = s->address_spaces[bus_num];
@@ -1046,19 +1048,37 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 
     /* set up AMD-Vi region */
     if (!iommu_as[devfn]) {
+        snprintf(name, sizeof(name), "amd_iommu_devfn_%d", devfn);
+
         iommu_as[devfn] = g_malloc0(sizeof(AMDVIAddressSpace));
         iommu_as[devfn]->bus_num = (uint8_t)bus_num;
         iommu_as[devfn]->devfn = (uint8_t)devfn;
         iommu_as[devfn]->iommu_state = s;
 
-        memory_region_init_iommu(&iommu_as[devfn]->iommu,
-                                 sizeof(iommu_as[devfn]->iommu),
+        amdvi_dev_as = iommu_as[devfn];
+
+        /*
+         * Memory region relationships looks like (Address range shows
+         * only lower 32 bits to make it short in length...):
+         *
+         * |-----------------+-------------------+----------|
+         * | Name            | Address range     | Priority |
+         * |-----------------+-------------------+----------+
+         * | amdvi_root      | 00000000-ffffffff |        0 |
+         * |  amdvi_iommu    | 00000000-ffffffff |        1 |
+         * |-----------------+-------------------+----------|
+         */
+        memory_region_init_iommu(&amdvi_dev_as->iommu,
+                                 sizeof(amdvi_dev_as->iommu),
                                  TYPE_AMD_IOMMU_MEMORY_REGION,
                                  OBJECT(s),
-                                 "amd-iommu", UINT64_MAX);
-        address_space_init(&iommu_as[devfn]->as,
-                           MEMORY_REGION(&iommu_as[devfn]->iommu),
-                           "amd-iommu");
+                                 "amd_iommu", UINT64_MAX);
+        memory_region_init(&amdvi_dev_as->root, OBJECT(s),
+                           "amdvi_root", UINT64_MAX);
+        address_space_init(&amdvi_dev_as->as, &amdvi_dev_as->root, name);
+        memory_region_add_subregion_overlap(&amdvi_dev_as->root, 0,
+                                            MEMORY_REGION(&amdvi_dev_as->iommu),
+                                            1);
     }
     return &iommu_as[devfn]->as;
 }
-- 
MST

[Qemu-devel] [PULL v2 11/28] x86_iommu/amd: Prepare for interrupt remap support

[Michael S. Tsirkin]

From: "Singh, Brijesh" <brijesh.singh@amd.com>

Register the interrupt remapping callback and read/write ops for the
amd-iommu-ir memory region.

amd-iommu-ir is set to higher priority to ensure that this region won't
be masked out by other memory regions.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/amd_iommu.h  |  14 +++++-
 hw/i386/amd_iommu.c  | 106 +++++++++++++++++++++++++++++++++++++++++++
 hw/i386/trace-events |   5 ++
 3 files changed, 123 insertions(+), 2 deletions(-)

diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 874030582d..4e7cc271c4 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -206,8 +206,18 @@
 
 #define AMDVI_COMMAND_SIZE   16
 
-#define AMDVI_INT_ADDR_FIRST 0xfee00000
-#define AMDVI_INT_ADDR_LAST  0xfeefffff
+#define AMDVI_INT_ADDR_FIRST    0xfee00000
+#define AMDVI_INT_ADDR_LAST     0xfeefffff
+#define AMDVI_INT_ADDR_SIZE     (AMDVI_INT_ADDR_LAST - AMDVI_INT_ADDR_FIRST + 1)
+#define AMDVI_MSI_ADDR_HI_MASK  (0xffffffff00000000ULL)
+#define AMDVI_MSI_ADDR_LO_MASK  (0x00000000ffffffffULL)
+
+/* SB IOAPIC is always on this device in AMD systems */
+#define AMDVI_IOAPIC_SB_DEVID   PCI_BUILD_BDF(0, PCI_DEVFN(0x14, 0))
+
+/* Interrupt remapping errors */
+#define AMDVI_IR_ERR            0x1
+
 
 #define TYPE_AMD_IOMMU_DEVICE "amd-iommu"
 #define AMD_IOMMU_DEVICE(obj)\
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 4bec1c6688..9118a75530 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -26,6 +26,7 @@
 #include "amd_iommu.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
+#include "hw/i386/apic_internal.h"
 #include "trace.h"
 
 /* used AMD-Vi MMIO registers */
@@ -1031,6 +1032,99 @@ static IOMMUTLBEntry amdvi_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
     return ret;
 }
 
+/* Interrupt remapping for MSI/MSI-X entry */
+static int amdvi_int_remap_msi(AMDVIState *iommu,
+                               MSIMessage *origin,
+                               MSIMessage *translated,
+                               uint16_t sid)
+{
+    assert(origin && translated);
+
+    trace_amdvi_ir_remap_msi_req(origin->address, origin->data, sid);
+
+    if (!iommu || !X86_IOMMU_DEVICE(iommu)->intr_supported) {
+        memcpy(translated, origin, sizeof(*origin));
+        goto out;
+    }
+
+    if (origin->address & AMDVI_MSI_ADDR_HI_MASK) {
+        trace_amdvi_err("MSI address high 32 bits non-zero when "
+                        "Interrupt Remapping enabled.");
+        return -AMDVI_IR_ERR;
+    }
+
+    if ((origin->address & AMDVI_MSI_ADDR_LO_MASK) != APIC_DEFAULT_ADDRESS) {
+        trace_amdvi_err("MSI is not from IOAPIC.");
+        return -AMDVI_IR_ERR;
+    }
+
+out:
+    trace_amdvi_ir_remap_msi(origin->address, origin->data,
+                             translated->address, translated->data);
+    return 0;
+}
+
+static int amdvi_int_remap(X86IOMMUState *iommu,
+                           MSIMessage *origin,
+                           MSIMessage *translated,
+                           uint16_t sid)
+{
+    return amdvi_int_remap_msi(AMD_IOMMU_DEVICE(iommu), origin,
+                               translated, sid);
+}
+
+static MemTxResult amdvi_mem_ir_write(void *opaque, hwaddr addr,
+                                      uint64_t value, unsigned size,
+                                      MemTxAttrs attrs)
+{
+    int ret;
+    MSIMessage from = { 0, 0 }, to = { 0, 0 };
+    uint16_t sid = AMDVI_IOAPIC_SB_DEVID;
+
+    from.address = (uint64_t) addr + AMDVI_INT_ADDR_FIRST;
+    from.data = (uint32_t) value;
+
+    trace_amdvi_mem_ir_write_req(addr, value, size);
+
+    if (!attrs.unspecified) {
+        /* We have explicit Source ID */
+        sid = attrs.requester_id;
+    }
+
+    ret = amdvi_int_remap_msi(opaque, &from, &to, sid);
+    if (ret < 0) {
+        /* TODO: log the event using IOMMU log event interface */
+        error_report_once("failed to remap interrupt from devid 0x%x", sid);
+        return MEMTX_ERROR;
+    }
+
+    apic_get_class()->send_msi(&to);
+
+    trace_amdvi_mem_ir_write(to.address, to.data);
+    return MEMTX_OK;
+}
+
+static MemTxResult amdvi_mem_ir_read(void *opaque, hwaddr addr,
+                                     uint64_t *data, unsigned size,
+                                     MemTxAttrs attrs)
+{
+    return MEMTX_OK;
+}
+
+static const MemoryRegionOps amdvi_ir_ops = {
+    .read_with_attrs = amdvi_mem_ir_read,
+    .write_with_attrs = amdvi_mem_ir_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    }
+};
+
 static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 {
     char name[128];
@@ -1066,6 +1160,7 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
          * |-----------------+-------------------+----------+
          * | amdvi_root      | 00000000-ffffffff |        0 |
          * |  amdvi_iommu    | 00000000-ffffffff |        1 |
+         * |  amdvi_iommu_ir | fee00000-feefffff |       64 |
          * |-----------------+-------------------+----------|
          */
         memory_region_init_iommu(&amdvi_dev_as->iommu,
@@ -1076,6 +1171,13 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
         memory_region_init(&amdvi_dev_as->root, OBJECT(s),
                            "amdvi_root", UINT64_MAX);
         address_space_init(&amdvi_dev_as->as, &amdvi_dev_as->root, name);
+        memory_region_init_io(&amdvi_dev_as->iommu_ir, OBJECT(s),
+                              &amdvi_ir_ops, s, "amd_iommu_ir",
+                              AMDVI_INT_ADDR_SIZE);
+        memory_region_add_subregion_overlap(&amdvi_dev_as->root,
+                                            AMDVI_INT_ADDR_FIRST,
+                                            &amdvi_dev_as->iommu_ir,
+                                            64);
         memory_region_add_subregion_overlap(&amdvi_dev_as->root, 0,
                                             MEMORY_REGION(&amdvi_dev_as->iommu),
                                             1);
@@ -1196,6 +1298,9 @@ static void amdvi_realize(DeviceState *dev, Error **err)
         return;
     }
 
+    /* Pseudo address space under root PCI bus. */
+    pcms->ioapic_as = amdvi_host_dma_iommu(bus, s, AMDVI_IOAPIC_SB_DEVID);
+
     /* set up MMIO */
     memory_region_init_io(&s->mmio, OBJECT(s), &mmio_mem_ops, s, "amdvi-mmio",
                           AMDVI_MMIO_SIZE);
@@ -1229,6 +1334,7 @@ static void amdvi_class_init(ObjectClass *klass, void* data)
     dc->vmsd = &vmstate_amdvi;
     dc->hotpluggable = false;
     dc_class->realize = amdvi_realize;
+    dc_class->int_remap = amdvi_int_remap;
     /* Supported by the pc-q35-* machine types */
     dc->user_creatable = true;
 }
diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index 9e6fc4dca9..41d533c250 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -101,6 +101,11 @@ amdvi_mode_invalid(uint8_t level, uint64_t addr)"error: translation level 0x%"PR
 amdvi_page_fault(uint64_t addr) "error: page fault accessing guest physical address 0x%"PRIx64
 amdvi_iotlb_hit(uint8_t bus, uint8_t slot, uint8_t func, uint64_t addr, uint64_t txaddr) "hit iotlb devid %02x:%02x.%x gpa 0x%"PRIx64" hpa 0x%"PRIx64
 amdvi_translation_result(uint8_t bus, uint8_t slot, uint8_t func, uint64_t addr, uint64_t txaddr) "devid: %02x:%02x.%x gpa 0x%"PRIx64" hpa 0x%"PRIx64
+amdvi_mem_ir_write_req(uint64_t addr, uint64_t val, uint32_t size) "addr 0x%"PRIx64" data 0x%"PRIx64" size 0x%"PRIx32
+amdvi_mem_ir_write(uint64_t addr, uint64_t val) "addr 0x%"PRIx64" data 0x%"PRIx64
+amdvi_ir_remap_msi_req(uint64_t addr, uint64_t data, uint8_t devid) "addr 0x%"PRIx64" data 0x%"PRIx64" devid 0x%"PRIx8
+amdvi_ir_remap_msi(uint64_t addr, uint64_t data, uint64_t addr2, uint64_t data2) "(addr 0x%"PRIx64", data 0x%"PRIx64") -> (addr 0x%"PRIx64", data 0x%"PRIx64")"
+amdvi_err(const char *str) "%s"
 
 # hw/i386/vmport.c
 vmport_register(unsigned char command, void *func, void *opaque) "command: 0x%02x func: %p opaque: %p"
-- 
MST

[Qemu-devel] [PULL v2 12/28] x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled

[Michael S. Tsirkin]

From: "Singh, Brijesh" <brijesh.singh@amd.com>

Emulate the interrupt remapping support when guest virtual APIC is
not enabled.

For more info Refer: AMD IOMMU spec Rev 3.0 - section 2.2.5.1

When VAPIC is not enabled, it uses interrupt remapping as defined in
Table 20 and Figure 15 from IOMMU spec.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/amd_iommu.h  |  44 ++++++++++
 hw/i386/amd_iommu.c  | 199 ++++++++++++++++++++++++++++++++++++++++++-
 hw/i386/trace-events |   7 ++
 3 files changed, 249 insertions(+), 1 deletion(-)

diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 4e7cc271c4..f73be48fca 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -217,7 +217,51 @@
 
 /* Interrupt remapping errors */
 #define AMDVI_IR_ERR            0x1
+#define AMDVI_IR_GET_IRTE       0x2
+#define AMDVI_IR_TARGET_ABORT   0x3
 
+/* Interrupt remapping */
+#define AMDVI_IR_REMAP_ENABLE           1ULL
+#define AMDVI_IR_INTCTL_SHIFT           60
+#define AMDVI_IR_INTCTL_ABORT           0
+#define AMDVI_IR_INTCTL_PASS            1
+#define AMDVI_IR_INTCTL_REMAP           2
+
+#define AMDVI_IR_PHYS_ADDR_MASK         (((1ULL << 45) - 1) << 6)
+
+/* MSI data 10:0 bits (section 2.2.5.1 Fig 14) */
+#define AMDVI_IRTE_OFFSET               0x7ff
+
+/* Delivery mode of MSI data (same as IOAPIC deilver mode encoding) */
+#define AMDVI_IOAPIC_INT_TYPE_FIXED          0x0
+#define AMDVI_IOAPIC_INT_TYPE_ARBITRATED     0x1
+#define AMDVI_IOAPIC_INT_TYPE_SMI            0x2
+#define AMDVI_IOAPIC_INT_TYPE_NMI            0x4
+#define AMDVI_IOAPIC_INT_TYPE_INIT           0x5
+#define AMDVI_IOAPIC_INT_TYPE_EINT           0x7
+
+/* Pass through interrupt */
+#define AMDVI_DEV_INT_PASS_MASK         (1UL << 56)
+#define AMDVI_DEV_EINT_PASS_MASK        (1UL << 57)
+#define AMDVI_DEV_NMI_PASS_MASK         (1UL << 58)
+#define AMDVI_DEV_LINT0_PASS_MASK       (1UL << 62)
+#define AMDVI_DEV_LINT1_PASS_MASK       (1UL << 63)
+
+/* Interrupt remapping table fields (Guest VAPIC not enabled) */
+union irte {
+    uint32_t val;
+    struct {
+        uint32_t valid:1,
+                 no_fault:1,
+                 int_type:3,
+                 rq_eoi:1,
+                 dm:1,
+                 guest_mode:1,
+                 destination:8,
+                 vector:8,
+                 rsvd:8;
+    } fields;
+};
 
 #define TYPE_AMD_IOMMU_DEVICE "amd-iommu"
 #define AMD_IOMMU_DEVICE(obj)\
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 9118a75530..8e2f13c029 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -28,6 +28,7 @@
 #include "qemu/error-report.h"
 #include "hw/i386/apic_internal.h"
 #include "trace.h"
+#include "hw/i386/apic-msidef.h"
 
 /* used AMD-Vi MMIO registers */
 const char *amdvi_mmio_low[] = {
@@ -1032,21 +1033,146 @@ static IOMMUTLBEntry amdvi_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
     return ret;
 }
 
+static int amdvi_get_irte(AMDVIState *s, MSIMessage *origin, uint64_t *dte,
+                          union irte *irte, uint16_t devid)
+{
+    uint64_t irte_root, offset;
+
+    irte_root = dte[2] & AMDVI_IR_PHYS_ADDR_MASK;
+    offset = (origin->data & AMDVI_IRTE_OFFSET) << 2;
+
+    trace_amdvi_ir_irte(irte_root, offset);
+
+    if (dma_memory_read(&address_space_memory, irte_root + offset,
+                        irte, sizeof(*irte))) {
+        trace_amdvi_ir_err("failed to get irte");
+        return -AMDVI_IR_GET_IRTE;
+    }
+
+    trace_amdvi_ir_irte_val(irte->val);
+
+    return 0;
+}
+
+static int amdvi_int_remap_legacy(AMDVIState *iommu,
+                                  MSIMessage *origin,
+                                  MSIMessage *translated,
+                                  uint64_t *dte,
+                                  X86IOMMUIrq *irq,
+                                  uint16_t sid)
+{
+    int ret;
+    union irte irte;
+
+    /* get interrupt remapping table */
+    ret = amdvi_get_irte(iommu, origin, dte, &irte, sid);
+    if (ret < 0) {
+        return ret;
+    }
+
+    if (!irte.fields.valid) {
+        trace_amdvi_ir_target_abort("RemapEn is disabled");
+        return -AMDVI_IR_TARGET_ABORT;
+    }
+
+    if (irte.fields.guest_mode) {
+        error_report_once("guest mode is not zero");
+        return -AMDVI_IR_ERR;
+    }
+
+    if (irte.fields.int_type > AMDVI_IOAPIC_INT_TYPE_ARBITRATED) {
+        error_report_once("reserved int_type");
+        return -AMDVI_IR_ERR;
+    }
+
+    irq->delivery_mode = irte.fields.int_type;
+    irq->vector = irte.fields.vector;
+    irq->dest_mode = irte.fields.dm;
+    irq->redir_hint = irte.fields.rq_eoi;
+    irq->dest = irte.fields.destination;
+
+    return 0;
+}
+
+static int __amdvi_int_remap_msi(AMDVIState *iommu,
+                                 MSIMessage *origin,
+                                 MSIMessage *translated,
+                                 uint64_t *dte,
+                                 X86IOMMUIrq *irq,
+                                 uint16_t sid)
+{
+    uint8_t int_ctl;
+
+    int_ctl = (dte[2] >> AMDVI_IR_INTCTL_SHIFT) & 3;
+    trace_amdvi_ir_intctl(int_ctl);
+
+    switch (int_ctl) {
+    case AMDVI_IR_INTCTL_PASS:
+        memcpy(translated, origin, sizeof(*origin));
+        return 0;
+    case AMDVI_IR_INTCTL_REMAP:
+        break;
+    case AMDVI_IR_INTCTL_ABORT:
+        trace_amdvi_ir_target_abort("int_ctl abort");
+        return -AMDVI_IR_TARGET_ABORT;
+    default:
+        trace_amdvi_ir_err("int_ctl reserved");
+        return -AMDVI_IR_ERR;
+    }
+
+    return amdvi_int_remap_legacy(iommu, origin, translated, dte, irq, sid);
+}
+
 /* Interrupt remapping for MSI/MSI-X entry */
 static int amdvi_int_remap_msi(AMDVIState *iommu,
                                MSIMessage *origin,
                                MSIMessage *translated,
                                uint16_t sid)
 {
+    int ret = 0;
+    uint64_t pass = 0;
+    uint64_t dte[4] = { 0 };
+    X86IOMMUIrq irq = { 0 };
+    uint8_t dest_mode, delivery_mode;
+
     assert(origin && translated);
 
+    /*
+     * When IOMMU is enabled, interrupt remap request will come either from
+     * IO-APIC or PCI device. If interrupt is from PCI device then it will
+     * have a valid requester id but if the interrupt is from IO-APIC
+     * then requester id will be invalid.
+     */
+    if (sid == X86_IOMMU_SID_INVALID) {
+        sid = AMDVI_IOAPIC_SB_DEVID;
+    }
+
     trace_amdvi_ir_remap_msi_req(origin->address, origin->data, sid);
 
-    if (!iommu || !X86_IOMMU_DEVICE(iommu)->intr_supported) {
+    /* check if device table entry is set before we go further. */
+    if (!iommu || !iommu->devtab_len) {
         memcpy(translated, origin, sizeof(*origin));
         goto out;
     }
 
+    if (!amdvi_get_dte(iommu, sid, dte)) {
+        return -AMDVI_IR_ERR;
+    }
+
+    /* Check if IR is enabled in DTE */
+    if (!(dte[2] & AMDVI_IR_REMAP_ENABLE)) {
+        memcpy(translated, origin, sizeof(*origin));
+        goto out;
+    }
+
+    /* validate that we are configure with intremap=on */
+    if (!X86_IOMMU_DEVICE(iommu)->intr_supported) {
+        trace_amdvi_err("Interrupt remapping is enabled in the guest but "
+                        "not in the host. Use intremap=on to enable interrupt "
+                        "remapping in amd-iommu.");
+        return -AMDVI_IR_ERR;
+    }
+
     if (origin->address & AMDVI_MSI_ADDR_HI_MASK) {
         trace_amdvi_err("MSI address high 32 bits non-zero when "
                         "Interrupt Remapping enabled.");
@@ -1058,10 +1184,81 @@ static int amdvi_int_remap_msi(AMDVIState *iommu,
         return -AMDVI_IR_ERR;
     }
 
+    /*
+     * The MSI data register [10:8] are used to get the upstream interrupt type.
+     *
+     * See MSI/MSI-X format:
+     * https://pdfs.semanticscholar.org/presentation/9420/c279e942eca568157711ef5c92b800c40a79.pdf
+     * (page 5)
+     */
+    delivery_mode = (origin->data >> MSI_DATA_DELIVERY_MODE_SHIFT) & 7;
+
+    switch (delivery_mode) {
+    case AMDVI_IOAPIC_INT_TYPE_FIXED:
+    case AMDVI_IOAPIC_INT_TYPE_ARBITRATED:
+        trace_amdvi_ir_delivery_mode("fixed/arbitrated");
+        ret = __amdvi_int_remap_msi(iommu, origin, translated, dte, &irq, sid);
+        if (ret < 0) {
+            goto remap_fail;
+        } else {
+            /* Translate IRQ to MSI messages */
+            x86_iommu_irq_to_msi_message(&irq, translated);
+            goto out;
+        }
+        break;
+    case AMDVI_IOAPIC_INT_TYPE_SMI:
+        error_report("SMI is not supported!");
+        ret = -AMDVI_IR_ERR;
+        break;
+    case AMDVI_IOAPIC_INT_TYPE_NMI:
+        pass = dte[3] & AMDVI_DEV_NMI_PASS_MASK;
+        trace_amdvi_ir_delivery_mode("nmi");
+        break;
+    case AMDVI_IOAPIC_INT_TYPE_INIT:
+        pass = dte[3] & AMDVI_DEV_INT_PASS_MASK;
+        trace_amdvi_ir_delivery_mode("init");
+        break;
+    case AMDVI_IOAPIC_INT_TYPE_EINT:
+        pass = dte[3] & AMDVI_DEV_EINT_PASS_MASK;
+        trace_amdvi_ir_delivery_mode("eint");
+        break;
+    default:
+        trace_amdvi_ir_delivery_mode("unsupported delivery_mode");
+        ret = -AMDVI_IR_ERR;
+        break;
+    }
+
+    if (ret < 0) {
+        goto remap_fail;
+    }
+
+    /*
+     * The MSI address register bit[2] is used to get the destination
+     * mode. The dest_mode 1 is valid for fixed and arbitrated interrupts
+     * only.
+     */
+    dest_mode = (origin->address >> MSI_ADDR_DEST_MODE_SHIFT) & 1;
+    if (dest_mode) {
+        trace_amdvi_ir_err("invalid dest_mode");
+        ret = -AMDVI_IR_ERR;
+        goto remap_fail;
+    }
+
+    if (pass) {
+        memcpy(translated, origin, sizeof(*origin));
+    } else {
+        trace_amdvi_ir_err("passthrough is not enabled");
+        ret = -AMDVI_IR_ERR;
+        goto remap_fail;
+    }
+
 out:
     trace_amdvi_ir_remap_msi(origin->address, origin->data,
                              translated->address, translated->data);
     return 0;
+
+remap_fail:
+    return ret;
 }
 
 static int amdvi_int_remap(X86IOMMUState *iommu,
diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index 41d533c250..98150c93db 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -106,6 +106,13 @@ amdvi_mem_ir_write(uint64_t addr, uint64_t val) "addr 0x%"PRIx64" data 0x%"PRIx6
 amdvi_ir_remap_msi_req(uint64_t addr, uint64_t data, uint8_t devid) "addr 0x%"PRIx64" data 0x%"PRIx64" devid 0x%"PRIx8
 amdvi_ir_remap_msi(uint64_t addr, uint64_t data, uint64_t addr2, uint64_t data2) "(addr 0x%"PRIx64", data 0x%"PRIx64") -> (addr 0x%"PRIx64", data 0x%"PRIx64")"
 amdvi_err(const char *str) "%s"
+amdvi_ir_irte(uint64_t addr, uint64_t data) "addr 0x%"PRIx64" offset 0x%"PRIx64
+amdvi_ir_irte_val(uint32_t data) "data 0x%"PRIx32
+amdvi_ir_err(const char *str) "%s"
+amdvi_ir_intctl(uint8_t val) "int_ctl 0x%"PRIx8
+amdvi_ir_target_abort(const char *str) "%s"
+amdvi_ir_delivery_mode(const char *str) "%s"
+amdvi_ir_generate_msi_message(uint8_t vector, uint8_t delivery_mode, uint8_t dest_mode, uint8_t dest, uint8_t rh) "vector %d delivery-mode %d dest-mode %d dest-id %d rh %d"
 
 # hw/i386/vmport.c
 vmport_register(unsigned char command, void *func, void *opaque) "command: 0x%02x func: %p opaque: %p"
-- 
MST

[Qemu-devel] [PULL v2 13/28] i386: acpi: add IVHD device entry for IOAPIC

[Michael S. Tsirkin]

From: "Singh, Brijesh" <brijesh.singh@amd.com>

When interrupt remapping is enabled, add a special IVHD device
(type IOAPIC).

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Acked-by: Peter Xu <peterx@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1599caa7c5..1ef396ddbb 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2467,9 +2467,12 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
  *   IVRS table as specified in AMD IOMMU Specification v2.62, Section 5.2
  *   accessible here http://support.amd.com/TechDocs/48882_IOMMU.pdf
  */
+#define IOAPIC_SB_DEVID   (uint64_t)PCI_BUILD_BDF(0, PCI_DEVFN(0x14, 0))
+
 static void
 build_amd_iommu(GArray *table_data, BIOSLinker *linker)
 {
+    int ivhd_table_len = 28;
     int iommu_start = table_data->len;
     AMDVIState *s = AMD_IOMMU_DEVICE(x86_iommu_get_default());
 
@@ -2491,8 +2494,16 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
                              (1UL << 6) | /* PrefSup      */
                              (1UL << 7),  /* PPRSup       */
                              1);
+
+    /*
+     * When interrupt remapping is supported, we add a special IVHD device
+     * for type IO-APIC.
+     */
+    if (x86_iommu_get_default()->intr_supported) {
+        ivhd_table_len += 8;
+    }
     /* IVHD length */
-    build_append_int_noprefix(table_data, 28, 2);
+    build_append_int_noprefix(table_data, ivhd_table_len, 2);
     /* DeviceID */
     build_append_int_noprefix(table_data, s->devid, 2);
     /* Capability offset */
@@ -2516,6 +2527,21 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
      */
     build_append_int_noprefix(table_data, 0x0000001, 4);
 
+    /*
+     * Add a special IVHD device type.
+     * Refer to spec - Table 95: IVHD device entry type codes
+     *
+     * Linux IOMMU driver checks for the special IVHD device (type IO-APIC).
+     * See Linux kernel commit 'c2ff5cf5294bcbd7fa50f7d860e90a66db7e5059'
+     */
+    if (x86_iommu_get_default()->intr_supported) {
+        build_append_int_noprefix(table_data,
+                                 (0x1ull << 56) |           /* type IOAPIC */
+                                 (IOAPIC_SB_DEVID << 40) |  /* IOAPIC devid */
+                                 0x48,                      /* special device */
+                                 8);
+    }
+
     build_header(linker, table_data, (void *)(table_data->data + iommu_start),
                  "IVRS", table_data->len - iommu_start, 1, NULL, NULL);
 }
-- 
MST

[Qemu-devel] [PULL v2 14/28] x86_iommu/amd: Add interrupt remap support when VAPIC is enabled

[Michael S. Tsirkin]

From: "Singh, Brijesh" <brijesh.singh@amd.com>

Emulate the interrupt remapping support when guest virtual APIC is
enabled.

For more information refer: IOMMU spec rev 3.0 (section 2.2.5.2)

When VAPIC is enabled, it uses interrupt remapping as defined in
Table 22 and Figure 17 from IOMMU spec.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/amd_iommu.h  | 36 +++++++++++++++++++++++
 hw/i386/amd_iommu.c  | 69 +++++++++++++++++++++++++++++++++++++++++++-
 hw/i386/trace-events |  2 ++
 3 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index f73be48fca..8061e9c49c 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -103,6 +103,7 @@
 #define AMDVI_MMIO_CONTROL_EVENTINTEN     (1ULL << 3)
 #define AMDVI_MMIO_CONTROL_COMWAITINTEN   (1ULL << 4)
 #define AMDVI_MMIO_CONTROL_CMDBUFLEN      (1ULL << 12)
+#define AMDVI_MMIO_CONTROL_GAEN           (1ULL << 17)
 
 /* MMIO status register bits */
 #define AMDVI_MMIO_STATUS_CMDBUF_RUN  (1 << 4)
@@ -263,6 +264,38 @@ union irte {
     } fields;
 };
 
+/* Interrupt remapping table fields (Guest VAPIC is enabled) */
+union irte_ga_lo {
+  uint64_t val;
+
+  /* For int remapping */
+  struct {
+      uint64_t  valid:1,
+                no_fault:1,
+                /* ------ */
+                int_type:3,
+                rq_eoi:1,
+                dm:1,
+                /* ------ */
+                guest_mode:1,
+                destination:8,
+                rsvd_1:48;
+  } fields_remap;
+};
+
+union irte_ga_hi {
+  uint64_t val;
+  struct {
+      uint64_t  vector:8,
+                rsvd_2:56;
+  } fields;
+};
+
+struct irte_ga {
+  union irte_ga_lo lo;
+  union irte_ga_hi hi;
+};
+
 #define TYPE_AMD_IOMMU_DEVICE "amd-iommu"
 #define AMD_IOMMU_DEVICE(obj)\
     OBJECT_CHECK(AMDVIState, (obj), TYPE_AMD_IOMMU_DEVICE)
@@ -332,6 +365,9 @@ typedef struct AMDVIState {
 
     /* IOTLB */
     GHashTable *iotlb;
+
+    /* Interrupt remapping */
+    bool ga_enabled;
 } AMDVIState;
 
 #endif
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 8e2f13c029..353a810e6b 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -608,6 +608,7 @@ static void amdvi_handle_control_write(AMDVIState *s)
     s->completion_wait_intr = !!(control & AMDVI_MMIO_CONTROL_COMWAITINTEN);
     s->cmdbuf_enabled = s->enabled && !!(control &
                         AMDVI_MMIO_CONTROL_CMDBUFLEN);
+    s->ga_enabled = !!(control & AMDVI_MMIO_CONTROL_GAEN);
 
     /* update the flags depending on the control register */
     if (s->cmdbuf_enabled) {
@@ -1094,6 +1095,65 @@ static int amdvi_int_remap_legacy(AMDVIState *iommu,
     return 0;
 }
 
+static int amdvi_get_irte_ga(AMDVIState *s, MSIMessage *origin, uint64_t *dte,
+                             struct irte_ga *irte, uint16_t devid)
+{
+    uint64_t irte_root, offset;
+
+    irte_root = dte[2] & AMDVI_IR_PHYS_ADDR_MASK;
+    offset = (origin->data & AMDVI_IRTE_OFFSET) << 4;
+    trace_amdvi_ir_irte(irte_root, offset);
+
+    if (dma_memory_read(&address_space_memory, irte_root + offset,
+                        irte, sizeof(*irte))) {
+        trace_amdvi_ir_err("failed to get irte_ga");
+        return -AMDVI_IR_GET_IRTE;
+    }
+
+    trace_amdvi_ir_irte_ga_val(irte->hi.val, irte->lo.val);
+    return 0;
+}
+
+static int amdvi_int_remap_ga(AMDVIState *iommu,
+                              MSIMessage *origin,
+                              MSIMessage *translated,
+                              uint64_t *dte,
+                              X86IOMMUIrq *irq,
+                              uint16_t sid)
+{
+    int ret;
+    struct irte_ga irte;
+
+    /* get interrupt remapping table */
+    ret = amdvi_get_irte_ga(iommu, origin, dte, &irte, sid);
+    if (ret < 0) {
+        return ret;
+    }
+
+    if (!irte.lo.fields_remap.valid) {
+        trace_amdvi_ir_target_abort("RemapEn is disabled");
+        return -AMDVI_IR_TARGET_ABORT;
+    }
+
+    if (irte.lo.fields_remap.guest_mode) {
+        error_report_once("guest mode is not zero");
+        return -AMDVI_IR_ERR;
+    }
+
+    if (irte.lo.fields_remap.int_type > AMDVI_IOAPIC_INT_TYPE_ARBITRATED) {
+        error_report_once("reserved int_type is set");
+        return -AMDVI_IR_ERR;
+    }
+
+    irq->delivery_mode = irte.lo.fields_remap.int_type;
+    irq->vector = irte.hi.fields.vector;
+    irq->dest_mode = irte.lo.fields_remap.dm;
+    irq->redir_hint = irte.lo.fields_remap.rq_eoi;
+    irq->dest = irte.lo.fields_remap.destination;
+
+    return 0;
+}
+
 static int __amdvi_int_remap_msi(AMDVIState *iommu,
                                  MSIMessage *origin,
                                  MSIMessage *translated,
@@ -1101,6 +1161,7 @@ static int __amdvi_int_remap_msi(AMDVIState *iommu,
                                  X86IOMMUIrq *irq,
                                  uint16_t sid)
 {
+    int ret;
     uint8_t int_ctl;
 
     int_ctl = (dte[2] >> AMDVI_IR_INTCTL_SHIFT) & 3;
@@ -1120,7 +1181,13 @@ static int __amdvi_int_remap_msi(AMDVIState *iommu,
         return -AMDVI_IR_ERR;
     }
 
-    return amdvi_int_remap_legacy(iommu, origin, translated, dte, irq, sid);
+    if (iommu->ga_enabled) {
+        ret = amdvi_int_remap_ga(iommu, origin, translated, dte, irq, sid);
+    } else {
+        ret = amdvi_int_remap_legacy(iommu, origin, translated, dte, irq, sid);
+    }
+
+    return ret;
 }
 
 /* Interrupt remapping for MSI/MSI-X entry */
diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index 98150c93db..6ac347d18c 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -113,6 +113,8 @@ amdvi_ir_intctl(uint8_t val) "int_ctl 0x%"PRIx8
 amdvi_ir_target_abort(const char *str) "%s"
 amdvi_ir_delivery_mode(const char *str) "%s"
 amdvi_ir_generate_msi_message(uint8_t vector, uint8_t delivery_mode, uint8_t dest_mode, uint8_t dest, uint8_t rh) "vector %d delivery-mode %d dest-mode %d dest-id %d rh %d"
+amdvi_ir_irte_ga(uint64_t addr, uint64_t data) "addr 0x%"PRIx64" offset 0x%"PRIx64
+amdvi_ir_irte_ga_val(uint64_t hi, uint64_t lo) "hi 0x%"PRIx64" lo 0x%"PRIx64
 
 # hw/i386/vmport.c
 vmport_register(unsigned char command, void *func, void *opaque) "command: 0x%02x func: %p opaque: %p"
-- 
MST

[Qemu-devel] [PULL v2 15/28] x86_iommu/amd: Enable Guest virtual APIC support

[Michael S. Tsirkin]

From: "Singh, Brijesh" <brijesh.singh@amd.com>

Now that amd-iommu support interrupt remapping, enable the GASup in IVRS
table and GASup in extended feature register to indicate that IOMMU
support guest virtual APIC mode. GASup provides option to guest OS to
make use of 128-bit IRTE.

Note that the GAMSup is set to zero to indicate that amd-iommu does not
support guest virtual APIC mode (aka AVIC) which would be used for the
nested VMs.

See Table 21 from IOMMU spec for interrupt virtualization controls

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/amd_iommu.h  | 2 +-
 hw/i386/acpi-build.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 8061e9c49c..687fcd8521 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -176,7 +176,7 @@
 /* extended feature support */
 #define AMDVI_EXT_FEATURES (AMDVI_FEATURE_PREFETCH | AMDVI_FEATURE_PPR | \
         AMDVI_FEATURE_IA | AMDVI_FEATURE_GT | AMDVI_FEATURE_HE | \
-        AMDVI_GATS_MODE | AMDVI_HATS_MODE)
+        AMDVI_GATS_MODE | AMDVI_HATS_MODE | AMDVI_FEATURE_GA)
 
 /* capabilities header */
 #define AMDVI_CAPAB_FEATURES (AMDVI_CAPAB_FLAT_EXT | \
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1ef396ddbb..236a20eaa8 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2518,7 +2518,8 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
     build_append_int_noprefix(table_data,
                              (48UL << 30) | /* HATS   */
                              (48UL << 28) | /* GATS   */
-                             (1UL << 2),    /* GTSup  */
+                             (1UL << 2)   | /* GTSup  */
+                             (1UL << 6),    /* GASup  */
                              4);
     /*
      *   Type 1 device entry reporting all devices
-- 
MST

[Qemu-devel] [PULL v2 16/28] MAINTAINERS: list "tests/acpi-test-data" files in ACPI/SMBIOS section

[Michael S. Tsirkin]

From: Laszlo Ersek <lersek@redhat.com>

The "tests/acpi-test-data" files are currently not covered by any section
in MAINTAINERS, and "scripts/checkpatch.pl" complains when new data files
are added.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 40672c4eba..b4d4bd4129 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1113,6 +1113,8 @@ F: hw/i386/acpi-build.[hc]
 F: hw/arm/virt-acpi-build.c
 F: tests/bios-tables-test.c
 F: tests/acpi-utils.[hc]
+F: tests/acpi-test-data/*
+F: tests/acpi-test-data/*/*
 
 ppc4xx
 M: Alexander Graf <agraf@suse.de>
-- 
MST

[Qemu-devel] [PULL v2 17/28] pci-testdev: add optional memory bar

[Michael S. Tsirkin]

From: Gerd Hoffmann <kraxel@redhat.com>

Add memory bar to pci-testdev.  Size is configurable using the membar
property.  Setting the size to zero (default) turns it off.  Can be used
to check whether guests handle large pci bars correctly.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 docs/specs/pci-testdev.txt | 15 ++++++++++-----
 hw/misc/pci-testdev.c      | 19 +++++++++++++++++++
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/docs/specs/pci-testdev.txt b/docs/specs/pci-testdev.txt
index 128ae222ef..4280a1e73c 100644
--- a/docs/specs/pci-testdev.txt
+++ b/docs/specs/pci-testdev.txt
@@ -1,11 +1,11 @@
 pci-test is a device used for testing low level IO
 
-device implements up to two BARs: BAR0 and BAR1.
-Each BAR can be memory or IO. Guests must detect
-BAR type and act accordingly.
+device implements up to three BARs: BAR0, BAR1 and BAR2.
+Each of BAR 0+1 can be memory or IO. Guests must detect
+BAR types and act accordingly.
 
-Each BAR size is up to 4K bytes.
-Each BAR starts with the following header:
+BAR 0+1 size is up to 4K bytes each.
+BAR 0+1 starts with the following header:
 
 typedef struct PCITestDevHdr {
     uint8_t test;  <- write-only, starts a given test number
@@ -24,3 +24,8 @@ All registers are little endian.
 device is expected to always implement tests 0 to N on each BAR, and to add new
 tests with higher numbers.  In this way a guest can scan test numbers until it
 detects an access type that it does not support on this BAR, then stop.
+
+BAR2 is a 64bit memory bar, without backing storage.  It is disabled
+by default and can be enabled using the membar=<size> property.  This
+can be used to test whether guests handle pci bars of a specific
+(possibly quite large) size correctly.
diff --git a/hw/misc/pci-testdev.c b/hw/misc/pci-testdev.c
index 32041f535f..a811b2ce20 100644
--- a/hw/misc/pci-testdev.c
+++ b/hw/misc/pci-testdev.c
@@ -85,6 +85,9 @@ typedef struct PCITestDevState {
     MemoryRegion portio;
     IOTest *tests;
     int current;
+
+    size_t membar_size;
+    MemoryRegion membar;
 } PCITestDevState;
 
 #define TYPE_PCI_TEST_DEV "pci-testdev"
@@ -253,6 +256,16 @@ static void pci_testdev_realize(PCIDevice *pci_dev, Error **errp)
     pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
     pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &d->portio);
 
+    if (d->membar_size) {
+        memory_region_init(&d->membar, OBJECT(d), "pci-testdev-membar",
+                           d->membar_size);
+        pci_register_bar(pci_dev, 2,
+                         PCI_BASE_ADDRESS_SPACE_MEMORY |
+                         PCI_BASE_ADDRESS_MEM_PREFETCH |
+                         PCI_BASE_ADDRESS_MEM_TYPE_64,
+                         &d->membar);
+    }
+
     d->current = -1;
     d->tests = g_malloc0(IOTEST_MAX * sizeof *d->tests);
     for (i = 0; i < IOTEST_MAX; ++i) {
@@ -305,6 +318,11 @@ static void qdev_pci_testdev_reset(DeviceState *dev)
     pci_testdev_reset(d);
 }
 
+static Property pci_testdev_properties[] = {
+    DEFINE_PROP_SIZE("membar", PCITestDevState, membar_size, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void pci_testdev_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -319,6 +337,7 @@ static void pci_testdev_class_init(ObjectClass *klass, void *data)
     dc->desc = "PCI Test Device";
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     dc->reset = qdev_pci_testdev_reset;
+    dc->props = pci_testdev_properties;
 }
 
 static const TypeInfo pci_testdev_info = {
-- 
MST

[Qemu-devel] [PULL v2 18/28] hw/pci-host/x86: extract get_pci_hole64_start_value() helpers

[Michael S. Tsirkin]

From: Laszlo Ersek <lersek@redhat.com>

Expose the calculated "hole64 start" GPAs as plain uint64_t values,
extracting the internals of the current property getters.

This patch doesn't change behavior.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/piix.c | 15 +++++++++++----
 hw/pci-host/q35.c  | 15 +++++++++++----
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index da73743fa2..15cc34912e 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -249,9 +249,7 @@ static void i440fx_pcihost_get_pci_hole_end(Object *obj, Visitor *v,
  * the 64bit PCI hole will start after "over 4G RAM" and the
  * reserved space for memory hotplug if any.
  */
-static void i440fx_pcihost_get_pci_hole64_start(Object *obj, Visitor *v,
-                                                const char *name,
-                                                void *opaque, Error **errp)
+static uint64_t i440fx_pcihost_get_pci_hole64_start_value(Object *obj)
 {
     PCIHostState *h = PCI_HOST_BRIDGE(obj);
     I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
@@ -263,7 +261,16 @@ static void i440fx_pcihost_get_pci_hole64_start(Object *obj, Visitor *v,
     if (!value && s->pci_hole64_fix) {
         value = pc_pci_hole64_start();
     }
-    visit_type_uint64(v, name, &value, errp);
+    return value;
+}
+
+static void i440fx_pcihost_get_pci_hole64_start(Object *obj, Visitor *v,
+                                                const char *name,
+                                                void *opaque, Error **errp)
+{
+    uint64_t hole64_start = i440fx_pcihost_get_pci_hole64_start_value(obj);
+
+    visit_type_uint64(v, name, &hole64_start, errp);
 }
 
 /*
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 8ce1e09932..919de104fc 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -113,9 +113,7 @@ static void q35_host_get_pci_hole_end(Object *obj, Visitor *v,
  * the 64bit PCI hole will start after "over 4G RAM" and the
  * reserved space for memory hotplug if any.
  */
-static void q35_host_get_pci_hole64_start(Object *obj, Visitor *v,
-                                          const char *name, void *opaque,
-                                          Error **errp)
+static uint64_t q35_host_get_pci_hole64_start_value(Object *obj)
 {
     PCIHostState *h = PCI_HOST_BRIDGE(obj);
     Q35PCIHost *s = Q35_HOST_DEVICE(obj);
@@ -127,7 +125,16 @@ static void q35_host_get_pci_hole64_start(Object *obj, Visitor *v,
     if (!value && s->pci_hole64_fix) {
         value = pc_pci_hole64_start();
     }
-    visit_type_uint64(v, name, &value, errp);
+    return value;
+}
+
+static void q35_host_get_pci_hole64_start(Object *obj, Visitor *v,
+                                          const char *name, void *opaque,
+                                          Error **errp)
+{
+    uint64_t hole64_start = q35_host_get_pci_hole64_start_value(obj);
+
+    visit_type_uint64(v, name, &hole64_start, errp);
 }
 
 /*
-- 
MST

[Qemu-devel] [PULL v2 19/28] hw/pci-host/x86: extend the 64-bit PCI hole relative to the fw-assigned base

[Michael S. Tsirkin]

From: Laszlo Ersek <lersek@redhat.com>

In commit 9fa99d2519cb ("hw/pci-host: Fix x86 Host Bridges 64bit PCI
hole", 2017-11-16), we meant to expose such a 64-bit PCI MMIO aperture in
the ACPI DSDT that would be at least as large as the new "pci-hole64-size"
property (2GB on i440fx, 32GB on q35). The goal was to offer "enough"
64-bit MMIO aperture to the guest OS for hotplug purposes.

In that commit, we added or modified five functions:

- pc_pci_hole64_start(): shared between i440fx and q35. Provides a default
  64-bit base, which starts beyond the cold-plugged 64-bit RAM, and skips
  the DIMM hotplug area too (if any).

- i440fx_pcihost_get_pci_hole64_start(), q35_host_get_pci_hole64_start():
  board-specific 64-bit base property getters called abstractly by the
  ACPI generator. Both of these fall back to pc_pci_hole64_start() if the
  firmware didn't program any 64-bit hole (i.e. if the firmware didn't
  assign a 64-bit GPA to any MMIO BAR on any device). Otherwise, they
  honor the firmware's BAR assignments (i.e., they treat the lowest 64-bit
  GPA programmed by the firmware as the base address for the aperture).

- i440fx_pcihost_get_pci_hole64_end(), q35_host_get_pci_hole64_end():
  these intended to extend the aperture to our size recommendation,
  calculated relative to the base of the aperture.

Despite the original intent, i440fx_pcihost_get_pci_hole64_end() and
q35_host_get_pci_hole64_end() currently only extend the aperture relative
to the default base (pc_pci_hole64_start()), ignoring any programming done
by the firmware. This means that our size recommendation may not be met.
Fix it by honoring the firmware's address assignments.

The strange extension sizes were spotted by Alex, in the log of a guest
kernel running on top of OVMF (which prefers to assign 64-bit GPAs to
64-bit BARs).

This change only affects DSDT generation, therefore no new compat property
is being introduced.

Using an i440fx OVMF guest with 5GB RAM, an example _CRS change is:

> @@ -881,9 +881,9 @@
>              QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
>                  0x0000000000000000, // Granularity
>                  0x0000000800000000, // Range Minimum
> -                0x000000080001C0FF, // Range Maximum
> +                0x000000087FFFFFFF, // Range Maximum
>                  0x0000000000000000, // Translation Offset
> -                0x000000000001C100, // Length
> +                0x0000000080000000, // Length
>                  ,, , AddressRangeMemory, TypeStatic)
>          })
>          Device (GPE0)

(On i440fx, the low RAM split is at 3GB, in this case. Therefore, with 5GB
guest RAM and no DIMM hotplug range, pc_pci_hole64_start() returns 4 +
(5-3) = 6 GB. Adding the 2GB extension to that yields 8GB, which is below
the firmware-programmed base of 32GB, before the patch. Therefore, before
the patch, the extension is ineffective. After the patch, we add the 2GB
extension to the firmware-programmed base, namely 32GB.)

Using a q35 OVMF guest with 5GB RAM, an example _CRS change is:

> @@ -3162,9 +3162,9 @@
>              QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
>                  0x0000000000000000, // Granularity
>                  0x0000000800000000, // Range Minimum
> -                0x00000009BFFFFFFF, // Range Maximum
> +                0x0000000FFFFFFFFF, // Range Maximum
>                  0x0000000000000000, // Translation Offset
> -                0x00000001C0000000, // Length
> +                0x0000000800000000, // Length
>                  ,, , AddressRangeMemory, TypeStatic)
>          })
>          Device (GPE0)

(On Q35, the low RAM split is at 2GB. Therefore, with 5GB guest RAM and no
DIMM hotplug range, pc_pci_hole64_start() returns 4 + (5-2) = 7 GB. Adding
the 32GB extension to that yields 39GB (0x0000_0009_BFFF_FFFF + 1), before
the patch. After the patch, we add the 32GB extension to the
firmware-programmed base, namely 32GB.)

The ACPI test data for the bios-tables-test case that we added earlier in
this series are corrected too, as follows:

> @@ -3339,9 +3339,9 @@
>              QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
>                  0x0000000000000000, // Granularity
>                  0x0000000200000000, // Range Minimum
> -                0x00000009BFFFFFFF, // Range Maximum
> +                0x00000009FFFFFFFF, // Range Maximum
>                  0x0000000000000000, // Translation Offset
> -                0x00000007C0000000, // Length
> +                0x0000000800000000, // Length
>                  ,, , AddressRangeMemory, TypeStatic)
>          })
>          Device (GPE0)

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Fixes: 9fa99d2519cbf71f871e46871df12cb446dc1c3e
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/piix.c | 2 +-
 hw/pci-host/q35.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 15cc34912e..999db824da 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -285,7 +285,7 @@ static void i440fx_pcihost_get_pci_hole64_end(Object *obj, Visitor *v,
 {
     PCIHostState *h = PCI_HOST_BRIDGE(obj);
     I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
-    uint64_t hole64_start = pc_pci_hole64_start();
+    uint64_t hole64_start = i440fx_pcihost_get_pci_hole64_start_value(obj);
     Range w64;
     uint64_t value, hole64_end;
 
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 919de104fc..382d61f03d 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -149,7 +149,7 @@ static void q35_host_get_pci_hole64_end(Object *obj, Visitor *v,
 {
     PCIHostState *h = PCI_HOST_BRIDGE(obj);
     Q35PCIHost *s = Q35_HOST_DEVICE(obj);
-    uint64_t hole64_start = pc_pci_hole64_start();
+    uint64_t hole64_start = q35_host_get_pci_hole64_start_value(obj);
     Range w64;
     uint64_t value, hole64_end;
 
-- 
MST

[Qemu-devel] [PULL v2 20/28] tests/bios-tables-test: add 64-bit PCI MMIO aperture round-up test on Q35

[Michael S. Tsirkin]

From: Laszlo Ersek <lersek@redhat.com>

In commit 9fa99d2519cb ("hw/pci-host: Fix x86 Host Bridges 64bit PCI
hole", 2017-11-16), we meant to expose such a 64-bit PCI MMIO aperture in
the ACPI DSDT that would be at least as large as the new "pci-hole64-size"
property (2GB on i440fx, 32GB on q35). The goal was to offer "enough"
64-bit MMIO aperture to the guest OS for hotplug purposes.

Currently the aperture is extended relative to a possibly incorrect base.
This may result in an aperture size that is smaller than the intent of
commit 9fa99d2519cb.

We're going to fix the error in a later patch in this series; now we just
add a test case that reproduces and captures the problem. In the fix, the
test data will be updated as well.

In the test case being added:
- use 128 MB initial RAM size,
- ask for one DIMM hotplug slot,
- ask for 2 GB maximum RAM size,
- use a pci-testdev with a 64-bit BAR of 2 GB size.

Consequences:

(1) In pc_memory_init() [hw/i386/pc.c], the DIMM hotplug area size is
    initially set to 2048-128 = 1920 MB. (Maximum RAM size minus initial
    RAM size.)

(2) The DIMM area base is set to 4096 MB (because the initial RAM is only
    128 MB -- there is no initial "high RAM").

(3) Due to commit 085f8e88ba73 ("pc: count in 1Gb hugepage alignment when
    sizing hotplug-memory container", 2014-11-24), we add 1 GB for the one
    DIMM hotplug slot that was specified. This sets the DIMM area size to
    1920+1024 = 2944 MB.

(4) The reserved-memory-end address (exclusive) is set to 4096 + 2944 =
    7040 MB (DIMM area base plus DIMM area size).

(5) The reserved-memory-end address is rounded up to GB alignment,
    yielding 7 GB (7168 MB).

(6) Given the 2 GB BAR size of pci-testdev, SeaBIOS allocates said 64-bit
    BAR in 64-bit address space.

(7) Because reserved-memory-end is at 7 GB, it is unaligned for the 2 GB
    BAR. Therefore SeaBIOS allocates the BAR at 8 GB. QEMU then
    (correctly) assigns the root bridge aperture base this BAR address, to
    be exposed in \_SB.PCI0._CRS.

(8) The intent of commit 9fa99d2519cb dictates that QEMU extend the
    aperture size to 32 GB, implying a 40 GB end address. However, QEMU
    performs the extension relative to reserved-memory-end (7 GB), not
    relative to the bridge aperture base that was correctly deduced from
    SeaBIOS's BAR programming (8 GB). Therefore we see 39 GB as the
    aperture end address in \_SB.PCI0._CRS:

> QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
>     0x0000000000000000, // Granularity
>     0x0000000200000000, // Range Minimum
>     0x00000009BFFFFFFF, // Range Maximum
>     0x0000000000000000, // Translation Offset
>     0x00000007C0000000, // Length
>     ,, , AddressRangeMemory, TypeStatic)

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/bios-tables-test.c             |  16 ++++++++++++++++
 tests/acpi-test-data/q35/DSDT.mmio64 | Bin 0 -> 8947 bytes
 tests/acpi-test-data/q35/SRAT.mmio64 | Bin 0 -> 224 bytes
 3 files changed, 16 insertions(+)
 create mode 100644 tests/acpi-test-data/q35/DSDT.mmio64
 create mode 100644 tests/acpi-test-data/q35/SRAT.mmio64

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 4e24930c4b..9dd88f9d86 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -708,6 +708,21 @@ static void test_acpi_q35_tcg_bridge(void)
     free_test_data(&data);
 }
 
+static void test_acpi_q35_tcg_mmio64(void)
+{
+    test_data data = {
+        .machine = MACHINE_Q35,
+        .variant = ".mmio64",
+        .required_struct_types = base_required_struct_types,
+        .required_struct_types_len = ARRAY_SIZE(base_required_struct_types)
+    };
+
+    test_acpi_one("-m 128M,slots=1,maxmem=2G "
+                  "-device pci-testdev,membar=2G",
+                  &data);
+    free_test_data(&data);
+}
+
 static void test_acpi_piix4_tcg_cphp(void)
 {
     test_data data;
@@ -875,6 +890,7 @@ int main(int argc, char *argv[])
         qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
         qtest_add_func("acpi/q35", test_acpi_q35_tcg);
         qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
+        qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
         qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi);
         qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi);
         qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp);
diff --git a/tests/acpi-test-data/q35/DSDT.mmio64 b/tests/acpi-test-data/q35/DSDT.mmio64
new file mode 100644
index 0000000000000000000000000000000000000000..a058ff2ee31a22a55b5b198bc1531c7f20b243f6
GIT binary patch
literal 8947
zcmb7KTW=f38J*=#t05&VrL`<y5)n$!v}rDs<vK;%1SWSWk`Y&;NIG#q16;dtD<v(m
zP@E!Z5I|OdT>MZVWfQbOALxMpqxlJWYk&d;`r6l^h@YaKZ)QiHSyDi(2XemIbG~oR
z?ChN7uKcdw`*w{n>)+KKuhA`4ueUuPeHLSk+Vt%-GdEcO(6819Rz8)n`gil9jgPAB
zKI2!f)vdqog<pi>osUE7X2foN>3n|cX7u4F;Vnj>cWy<TQ^VQfgKl-uKk|6nYnJUn
zqv;naUvt|Yc(RqY2Nl1=?7_9BZ@I0R!A{r9+<TLG{;TVw?d))`xWt`)@UM;URxe)t
z;?+BquU`D;Z|=RxGXPw|w~g<Oh(5yiL%tf0oIelutq+JUj;)WD{yY?iWuHSwqL6y-
z7i1TuS|7jZ1hyP&&2F@7JvCMfBgEJ+AB}b|U<Jm${yxt#{ksmuORv<n{ZhN*4@&Np
zLAO_75%q)-b=~}9$enOh_-q)mVVFNY9<$N>XTzK9FE<P9KmW^)*_bb2COX|>k<WiL
zi>XI)mSOBY=3Je5j)p_ihUv>SyOn{MDhyy1gK0NAyD1*>LjSOZGnnFsJk`H@j@r!I
z2`Xu3;j4<TR=ek?1ge(eM}#^9RE>F++P39Y@zHAI!?}9)`LwD?)%r)zc6uI)%5#>-
zIiiuS@H2HQVi9vvYs{)!2kd~Qa__PJ(F<Eu{{)^+yR%|GP9zFq$U<g;^*VbU-)ku-
z?H@fA>|8q`E-WR9HqWjxhk!3Z%Ly>U+!Z<FH2!du!z_>4{me;0htJimchm2tZF}ud
z6slz~C+kX4){0kMoQ5Xkv@{RW57NwDb3~d=N^>cZhNskUXiiVflX{Rs9vgY=cGKHr
zZef6LE83&T`O|9_odU=Euee;=onCu!>+QqEd<LtBwYIlvOtvkwH`!u=`gsaLT?J_b
z4YoL!DI6c;VwSi#A{K!2@|fTOszPESzrdyh6WBN+rh=xJU;-+b2#JZ&2y9v~)d*<)
ziL3&emP|lZNK91)>t13-o{V5BXht&C;1TLNvxd&BU@B-<GS%P_>N<0V&YWN>XwKA$
zP}ezQ=$tWh&X_t8>N;&hr)}u8O`QmJo!B9?nVvOt&YC(A>N>fhlN&m@sS}~DGjHh3
z8#?o*PK3Hn>{QzMEEqZqrcQ*q&N)NpoS}2h)QM2nIdABkH+0UMIuYtR7Yv;XhRy|3
zCqiAPW9W1YosOv!p{}!N=qwsKi>6M5y3R#I=c1u=(bS1h*STcqTrzYnnK}{bI$cAj
zYv^=Mod|WEXAGTZ44r38od|WE%ZAQnL+7%o6QQp2tfBL)q4TV%6QQni#n8E8=v*;%
zBGh%B6U<7y6P^>yYP{K<lT7bnNK8M*o;R514d!{1iBM-=Fqjt%<^_|9P-k8=m=_J^
zMU#n8XFg^yA2XPbnM{N_^KrpcF7voxsy+U3$y6IRLSiZ>Sv6{|8Z}qVnh0f08wOg@
z47A>Do){?FC>$Fo0+f|O8i6Vb1C=;oNfz#R1W-9<N<vjgOeGBlDzVByH7FUVgbF4Z
zC_;=z8K}g@5!F;)O9m>TCkBcTLl;;WsKmw*G1cUffl8=gl7S*rI$@v^E0{1)4N3+o
zp@K;Uicsl<fl91k!ay}B8K{H`CK)I~r4t4!v4RN$)u3dc5-ONvpa_*t7^uVwCJa=A
zl7UL7V3L6%R61dx5-XT6Pz_23Dxrc&28vMWgn>$|V8TE(C>f}P3MLsSLZuT1DzSnI
z1J$5ppb{#WWS|I@P8g`f3MLFxgOY(ts9=(TB2+qIpb{&XFi;Ij1}dR~Nd}5g>4bqw
ztYE@GH7FUVgbF4ZC_<$Z1}d?F2?N!jWS|l%m}H;`l};F_#0n-1RD+U%N~mCxfg)5o
zVW1K#m@rTcN(L&Sf=LF7Q0at$N~~bQKs6{CsDuh887M-f69y`=f(ZlFpk$yDDwt%T
z2$fD4sKg2;3{-=Xfl8=gl7S*rI$@v^E0{1)4N3+op@K;Uicsl<fl91k!ay}B8K{H`
zCK)I~r4t4!v4RN$)u3dc5-ONvpa_*t7^uVwCJa=Al7UL7V3L6%R61dxh%^I5q#Gzg
z-9Qm)2C6Y(pc<15RAZ8XYD^fY#)N@tOfpc7Nd~GhVW1il2C6a1Ks6>AsK$hWYD^fY
z#v}vPm}H<D69$S%on06xBAhWXnPi{{aT-)eOf|PKP(*5OVW5cA+>(JJRC7xPiqL27
z`2j4HjS55YV|7eFNbl$9tJ1&wi|5nyE0vxV(9;O3-Px_cad)fq;n9b0GdvsFtx<cE
zKI-(*u-&ERZntNRX5Z(Bdw2*kJLY-!a&y<~%mUCU7w38Y(;S<nhf(;Ye2@Bg7N5=N
z{XM?GsD(jv@MNSlgL{OE`Ym3-;}m*WV`JFe;xb?Qbe_$o;cg83H^WYhZ0>@5E<r=U
zXew@T;5WPM18Va_(X&8LI@p*7B;vD!Zl`PA+l$9Q<*7feUQJZTV}&1Fy{c8Oit1I2
zPp@8er&q7`j{?%}C93n(Bb4{F@}4N~CCYoJl=tHDezH6sm;GS%zE<8B<^4o?|CI86
zT)vhppLvAxHLZM2l&>Yq*G?&4i_32&%V!^<{H9iZQ<UFKl;1q1{AOIfo-Chxgz|N*
zd|i~UC(74PDPNDvH<ION9-(|gE8h_18;SCbQ_43`K0{ksvb-b9tIY$q74(=*Jhe3Y
z+?=@N@kQ5loGy78^P-s!_Zk!FWZ%?u6W8UUnGQD~6X|5%)N~Wq*^-$KcO?_)WZ%?u
z6W5Jvro*kuL^|0wHQmH@aK=oB`<IDyvTtg-iFLhfro&CmL^|0wHQmHIJ!_`Joz6r$
z*=MG6sxQbl1RlTf@L74gQ|qdg$Z-L}f9AaQwNm4)JJIjn`EBXo_S<j0bMWiiZ?nGT
zwOd>9ik#!TZhceureGEDXdAu*L(G3uzytlePf$={C6AuVd(G{x<#mErEU)%**<u;b
z3w*nOcacD8uUl)Cfb=W95*{Ttx0`mvsIz}md*+6CgMhKf@y%7dcC%bwFVhI*+n20c
z^m?1GrAB<X$3F7P<?_DU$a$zf+kfK5^BMI%fHczBkB_6hWH*XMEF7I5CJ)f)Bfb#!
zpPD$*+8T#$kR?TrR>(@GLWZgN-NwAv^|tNPWLV*smHaTon~X4G$)uR!Q9UyuNSp~w
zR}zlNx1x!W0UcT4{jJQXF_}u7&*`andNLQ_$TeMPFzfAfdxg`4PCZ`}gN5Q~e~PY%
zr;@z{eIwb4DVRFC{)aEos=@_^KP*tJEPG7r3u`c_^on_IGyI&V8$EX(@BVsu62tcP
z-@qk$Jz(wyEaG~*HmH>^;WUXCRJ5S!++yF(YS&@VXv<A<uD#tZSISpVg098j_V3?1
zc(1fKaXCt75Z57Y4>?<*>#)%dIu?G1^OgQl<3)J|A+y}g6Pf*H5R_iO{fB+Jxb48-
z?%-AFbK2C<kSJj~=u}%Jd^GUUqE-v7_Vq!jfA^Uw_zJtqgIafB1+?S>S}Z~B<pHuk
zIR%{#XcGu_YOmm<>y_Lkaf3UW9nz5SP7HU~eJf~rTg<KG0?ZtHcns^m{{$U7KK}Ao
zpNGVVLP|JV8p`TerMOJQQ0^~}+@Tx}_0meE=pdxENlC_*`)oivnU*&C+a41M^FyIY
zZV3V%x|NjxH@nya<Pa3r4mwvZ7dLqM-cK1T;FXg3_x-W_Q@wF`jGqcb<ntJLOEwqL
zY{=%1S~b=CG@5bma!|P*W6z`6i0CvCi5e|Rghs1D1ZuP>$pLj!gSZrk!^9rBpHm+l
zKcLtWokh`U-8t!@R*R0cM*hr@y0Lyb8<&c!{Nl&3o`;f2D#;{?Lf*zD&XIlqO=!-8
z8#k9Rpg}~H>2O5J>0m_Tq(ku;BHBsw%4PRSti!-co`8r}r#h*j`(rGuUS<NcPm3;k
zgI1kP>8@d&mQxczv3w<@f3%Ye+GYGN09~Brcd<Dr_beQ$J3v|#bR0456=PacaZJ0y
zn8qyPMcOmcV;UaC*f(5fKt4I9fru`*7+|r+7Dtu$wM}J1Z7Ot))1~3KwB3ji#eRv6
z#HlaHok0E*0|r;T%K6_vnPVwDqocD!A9$_(pzxqz+32_A8T<~19~4gFIdLw()z2`;
znWf*Q^p|4di1^iu@T=qFe|#?vu&-X4yb$n&S*!~2Zvzr0KCzwRZU+A|kbw~SFBge@
zi~6<O68zgkufp@%c38@lau(jWJ3qOBlhfT8Eaoy6hK2U!!J?hvDURxhenv;As`JZ-
z6VyL?Zu3=7-V)C3)`Dv=fJ=l<RB^ISmTrEWo$L+q*1MjS|Lu_Pmdkj}PyZLvKYF@b
o{<#b=mlnf)YIohorbdlRs<A<h{?S5rJthL1Vs5VOZTHy!0aq7uc>n+a

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/q35/SRAT.mmio64 b/tests/acpi-test-data/q35/SRAT.mmio64
new file mode 100644
index 0000000000000000000000000000000000000000..ac35f3dac4f47b86e41c7f35ee40bac14174b37e
GIT binary patch
literal 224
zcmWFzatwLEz`($0?d0$55v<@85#SsQ6k`O6f!H7#gyBE{mCvXFmw__4-~!0{5bA&i
cfWZfLm_qF8V6xb0gn4lH?0~6chB1IN0P8^t0RR91

literal 0
HcmV?d00001

-- 
MST

[Qemu-devel] [PULL v2 21/28] hw/pci-bridge/xio3130: Remove unused functions

[Michael S. Tsirkin]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Introduced in 48ebf2f90f8 and faf1e708d5b, these functions
were never used. Remove them.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-bridge/xio3130_downstream.h | 11 -----------
 hw/pci-bridge/xio3130_upstream.h   | 10 ----------
 hw/pci-bridge/xio3130_downstream.c | 28 +---------------------------
 hw/pci-bridge/xio3130_upstream.c   | 24 +-----------------------
 4 files changed, 2 insertions(+), 71 deletions(-)
 delete mode 100644 hw/pci-bridge/xio3130_downstream.h
 delete mode 100644 hw/pci-bridge/xio3130_upstream.h

diff --git a/hw/pci-bridge/xio3130_downstream.h b/hw/pci-bridge/xio3130_downstream.h
deleted file mode 100644
index 8426d9ffa6..0000000000
--- a/hw/pci-bridge/xio3130_downstream.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#ifndef QEMU_XIO3130_DOWNSTREAM_H
-#define QEMU_XIO3130_DOWNSTREAM_H
-
-#include "hw/pci/pcie_port.h"
-
-PCIESlot *xio3130_downstream_init(PCIBus *bus, int devfn, bool multifunction,
-                                  const char *bus_name, pci_map_irq_fn map_irq,
-                                  uint8_t port, uint8_t chassis,
-                                  uint16_t slot);
-
-#endif /* QEMU_XIO3130_DOWNSTREAM_H */
diff --git a/hw/pci-bridge/xio3130_upstream.h b/hw/pci-bridge/xio3130_upstream.h
deleted file mode 100644
index d0ab7577e2..0000000000
--- a/hw/pci-bridge/xio3130_upstream.h
+++ /dev/null
@@ -1,10 +0,0 @@
-#ifndef QEMU_XIO3130_UPSTREAM_H
-#define QEMU_XIO3130_UPSTREAM_H
-
-#include "hw/pci/pcie_port.h"
-
-PCIEPort *xio3130_upstream_init(PCIBus *bus, int devfn, bool multifunction,
-                                const char *bus_name, pci_map_irq_fn map_irq,
-                                uint8_t port);
-
-#endif /* QEMU_XIO3130_UPSTREAM_H */
diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index b202657954..467bbabe4c 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -23,7 +23,7 @@
 #include "hw/pci/pci_ids.h"
 #include "hw/pci/msi.h"
 #include "hw/pci/pcie.h"
-#include "xio3130_downstream.h"
+#include "hw/pci/pcie_port.h"
 #include "qapi/error.h"
 
 #define PCI_DEVICE_ID_TI_XIO3130D       0x8233  /* downstream port */
@@ -127,32 +127,6 @@ static void xio3130_downstream_exitfn(PCIDevice *d)
     pci_bridge_exitfn(d);
 }
 
-PCIESlot *xio3130_downstream_init(PCIBus *bus, int devfn, bool multifunction,
-                                  const char *bus_name, pci_map_irq_fn map_irq,
-                                  uint8_t port, uint8_t chassis,
-                                  uint16_t slot)
-{
-    PCIDevice *d;
-    PCIBridge *br;
-    DeviceState *qdev;
-
-    d = pci_create_multifunction(bus, devfn, multifunction,
-                                 "xio3130-downstream");
-    if (!d) {
-        return NULL;
-    }
-    br = PCI_BRIDGE(d);
-
-    qdev = DEVICE(d);
-    pci_bridge_map_irq(br, bus_name, map_irq);
-    qdev_prop_set_uint8(qdev, "port", port);
-    qdev_prop_set_uint8(qdev, "chassis", chassis);
-    qdev_prop_set_uint16(qdev, "slot", slot);
-    qdev_init_nofail(qdev);
-
-    return PCIE_SLOT(d);
-}
-
 static Property xio3130_downstream_props[] = {
     DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
                     QEMU_PCIE_SLTCAP_PCP_BITNR, true),
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index bca2f9a5ea..b524908cf1 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -23,7 +23,7 @@
 #include "hw/pci/pci_ids.h"
 #include "hw/pci/msi.h"
 #include "hw/pci/pcie.h"
-#include "xio3130_upstream.h"
+#include "hw/pci/pcie_port.h"
 
 #define PCI_DEVICE_ID_TI_XIO3130U       0x8232  /* upstream port */
 #define XIO3130_REVISION                0x2
@@ -108,28 +108,6 @@ static void xio3130_upstream_exitfn(PCIDevice *d)
     pci_bridge_exitfn(d);
 }
 
-PCIEPort *xio3130_upstream_init(PCIBus *bus, int devfn, bool multifunction,
-                             const char *bus_name, pci_map_irq_fn map_irq,
-                             uint8_t port)
-{
-    PCIDevice *d;
-    PCIBridge *br;
-    DeviceState *qdev;
-
-    d = pci_create_multifunction(bus, devfn, multifunction, "x3130-upstream");
-    if (!d) {
-        return NULL;
-    }
-    br = PCI_BRIDGE(d);
-
-    qdev = DEVICE(d);
-    pci_bridge_map_irq(br, bus_name, map_irq);
-    qdev_prop_set_uint8(qdev, "port", port);
-    qdev_init_nofail(qdev);
-
-    return PCIE_PORT(d);
-}
-
 static const VMStateDescription vmstate_xio3130_upstream = {
     .name = "xio3130-express-upstream-port",
     .priority = MIG_PRI_PCI_BUS,
-- 
MST

[Qemu-devel] [PULL v2 22/28] hw/pci-bridge/ioh3420: Remove unuseful header

[Michael S. Tsirkin]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-bridge/ioh3420.h | 6 ------
 hw/pci-bridge/ioh3420.c | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)
 delete mode 100644 hw/pci-bridge/ioh3420.h

diff --git a/hw/pci-bridge/ioh3420.h b/hw/pci-bridge/ioh3420.h
deleted file mode 100644
index ea423cb991..0000000000
--- a/hw/pci-bridge/ioh3420.h
+++ /dev/null
@@ -1,6 +0,0 @@
-#ifndef QEMU_IOH3420_H
-#define QEMU_IOH3420_H
-
-#include "hw/pci/pcie_port.h"
-
-#endif /* QEMU_IOH3420_H */
diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index a451d74ee6..81f2de6f07 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -24,7 +24,7 @@
 #include "hw/pci/pci_ids.h"
 #include "hw/pci/msi.h"
 #include "hw/pci/pcie.h"
-#include "ioh3420.h"
+#include "hw/pci/pcie_port.h"
 
 #define PCI_DEVICE_ID_IOH_EPORT         0x3420  /* D0:F0 express mode */
 #define PCI_DEVICE_ID_IOH_REV           0x2
-- 
MST

[Qemu-devel] [PULL v2 23/28] hw/pci: Add missing include

[Michael S. Tsirkin]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Noted while refactoring:

      CC      mips-softmmu/hw/mips/gt64xxx_pci.o
    In file included from include/hw/pci-host/gt64xxx.h:2,
                     from hw/mips/gt64xxx_pci.c:30:
    include/hw/pci/pci_bus.h:23:5: error: unknown type name ‘PCIIOMMUFunc’
         PCIIOMMUFunc iommu_fn;
         ^~~~~~~~~~~~
    include/hw/pci/pci_bus.h:27:5: error: unknown type name ‘pci_set_irq_fn’
         pci_set_irq_fn set_irq;
         ^~~~~~~~~~~~~~
    include/hw/pci/pci_bus.h:28:5: error: unknown type name ‘pci_map_irq_fn’
         pci_map_irq_fn map_irq;
         ^~~~~~~~~~~~~~
    include/hw/pci/pci_bus.h:29:5: error: unknown type name ‘pci_route_irq_fn’
         pci_route_irq_fn route_intx_to_irq;
         ^~~~~~~~~~~~~~~~
    include/hw/pci/pci_bus.h:31:24: error: ‘PCI_SLOT_MAX’ undeclared here (not in a function)
         PCIDevice *devices[PCI_SLOT_MAX * PCI_FUNC_MAX];
                            ^~~~~~~~~~~~
    include/hw/pci/pci_bus.h:31:39: error: ‘PCI_FUNC_MAX’ undeclared here (not in a function)
         PCIDevice *devices[PCI_SLOT_MAX * PCI_FUNC_MAX];
                                           ^~~~~~~~~~~~
    make[1]: *** [rules.mak:69: hw/mips/gt64xxx_pci.o] Error 1
    make: *** [Makefile:482: subdir-mips-softmmu] Error 2

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/pci/pci_bus.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index b7da8f555b..dfb75752cb 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -1,6 +1,8 @@
 #ifndef QEMU_PCI_BUS_H
 #define QEMU_PCI_BUS_H
 
+#include "hw/pci/pci.h"
+
 /*
  * PCI Bus datastructures.
  *
-- 
MST

[Qemu-devel] [PULL v2 24/28] pci_bridge: fix typo in comment

[Michael S. Tsirkin]

From: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pci_bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 08b7e44e2e..ee9dff2d3a 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -399,7 +399,7 @@ void pci_bridge_exitfn(PCIDevice *pci_dev)
 
 /*
  * before qdev initialization(qdev_init()), this function sets bus_name and
- * map_irq callback which are necessry for pci_bridge_initfn() to
+ * map_irq callback which are necessary for pci_bridge_initfn() to
  * initialize bus.
  */
 void pci_bridge_map_irq(PCIBridge *br, const char* bus_name,
-- 
MST

[Qemu-devel] [PULL v2 25/28] i440fx: use ARRAY_SIZE for pam_regions

[Michael S. Tsirkin]

From: Li Qiang <liq3ea@gmail.com>

Cc: qemu-trivial@nongnu.org

Signed-off-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/piix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 999db824da..82421a86e3 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -142,7 +142,7 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
     PCIDevice *pd = PCI_DEVICE(d);
 
     memory_region_transaction_begin();
-    for (i = 0; i < 13; i++) {
+    for (i = 0; i < ARRAY_SIZE(d->pam_regions); i++) {
         pam_update(&d->pam_regions[i], i,
                    pd->config[I440FX_PAM + (DIV_ROUND_UP(i, 2))]);
     }
@@ -412,7 +412,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
 
     init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
              &f->pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
-    for (i = 0; i < 12; ++i) {
+    for (i = 0; i < ARRAY_SIZE(f->pam_regions) - 1; ++i) {
         init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
                  &f->pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE,
                  PAM_EXPAN_SIZE);
-- 
MST

[Qemu-devel] [PULL v2 26/28] piix: use TYPE_FOO constants than string constats

[Michael S. Tsirkin]

From: Li Qiang <liq3ea@gmail.com>

Make them more QOMConventional.
Cc:qemu-trivial@nongnu.org

Signed-off-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/piix.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 82421a86e3..731190d92d 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -95,6 +95,9 @@ typedef struct PIIX3State {
 #define I440FX_PCI_DEVICE(obj) \
     OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
 
+#define TYPE_PIIX3_DEVICE "PIIX3"
+#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
+
 struct PCII440FXState {
     /*< private >*/
     PCIDevice parent_obj;
@@ -424,13 +427,13 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
      * These additional routes can be discovered through ACPI. */
     if (xen_enabled()) {
         PCIDevice *pci_dev = pci_create_simple_multifunction(b,
-                             -1, true, "PIIX3-xen");
+                             -1, true, TYPE_PIIX3_XEN_DEVICE);
         piix3 = PIIX3_PCI_DEVICE(pci_dev);
         pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
                 piix3, XEN_PIIX_NUM_PIRQS);
     } else {
         PCIDevice *pci_dev = pci_create_simple_multifunction(b,
-                             -1, true, "PIIX3");
+                             -1, true, TYPE_PIIX3_DEVICE);
         piix3 = PIIX3_PCI_DEVICE(pci_dev);
         pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3,
                 PIIX_NUM_PIRQS);
@@ -748,7 +751,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo piix3_info = {
-    .name          = "PIIX3",
+    .name          = TYPE_PIIX3_DEVICE,
     .parent        = TYPE_PIIX3_PCI_DEVICE,
     .class_init    = piix3_class_init,
 };
@@ -761,7 +764,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
 };
 
 static const TypeInfo piix3_xen_info = {
-    .name          = "PIIX3-xen",
+    .name          = TYPE_PIIX3_XEN_DEVICE,
     .parent        = TYPE_PIIX3_PCI_DEVICE,
     .class_init    = piix3_xen_class_init,
 };
-- 
MST

[Qemu-devel] [PULL v2 27/28] piix_pci: fix i440fx data sheet link

[Michael S. Tsirkin]

From: Li Qiang <liq3ea@163.com>

It seems that the intel link is unavailable, change it to point to the
qemu site.

Signed-off-by: Li Qiang <liq3ea@163.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 731190d92d..213a81e669 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -40,7 +40,7 @@
 
 /*
  * I440FX chipset data sheet.
- * http://download.intel.com/design/chipsets/datashts/29054901.pdf
+ * https://wiki.qemu.org/File:29054901.pdf
  */
 
 #define I440FX_PCI_HOST_BRIDGE(obj) \
-- 
MST

[Qemu-devel] [PULL v2 28/28] vhost-scsi: prevent using uninitialized vqs

[Michael S. Tsirkin]

From: yuchenlin <yuchenlin@synology.com>

There are 3 virtqueues (ctrl, event and cmd) for virtio scsi device,
but seabios will only set the physical address for the 3rd one (cmd).
Then in vhost_virtqueue_start(), virtio_queue_get_desc_addr()
will be 0 for ctrl and event vq.

In this case, ctrl and event vq are not initialized.
vhost_verify_ring_mappings may use uninitialized vhost_virtqueue
such that vhost_verify_ring_part_mapping returns ENOMEM.

When encountered this problem, we got the following logs:

    qemu-system-x86_64: Unable to map available ring for ring 0
    qemu-system-x86_64: Verify ring failure on region 0

Signed-off-by: Forrest Liu <forrestl@synology.com>
Signed-off-by: yuchenlin <yuchenlin@synology.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/scsi/vhost-scsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index becf550085..7f21b4f9d6 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -183,7 +183,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
     }
 
     vsc->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
-    vsc->dev.vqs = g_new(struct vhost_virtqueue, vsc->dev.nvqs);
+    vsc->dev.vqs = g_new0(struct vhost_virtqueue, vsc->dev.nvqs);
     vsc->dev.vq_index = 0;
     vsc->dev.backend_features = 0;
 
-- 
MST

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Peter Maydell]

On 25 October 2018 at 01:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' into staging (2018-10-23 17:20:23 +0100)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 6a9fb4e1ba5594cde7739068617ad88e6117db93:
>
>   vhost-scsi: prevent using uninitialized vqs (2018-10-24 20:50:13 -0400)
>
> ----------------------------------------------------------------
> pci, pc, virtio: fixes, features
>
> AMD IOMMU VAPIC support + fixes all over the place.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Hi; I get some compile failures and a test assertion, I'm afraid:

On 32-bit hosts (where uint64_t and size_t are not the same):

/home/peter.maydell/qemu/include/qemu/compiler.h:80:35: error: invalid
operands to binary - (have 'uint64_t * {aka long long unsigned int *}'
and 'size_t * {aka unsigned int *}')
 #define type_check(t1,t2) ((t1*)0 - (t2*)0)
                                   ^
/home/peter.maydell/qemu/include/hw/qdev-properties.h:77:15: note: in
expansion of macro 'type_check'
             + type_check(_type, typeof_field(_state, _field)),          \
               ^
/home/peter.maydell/qemu/include/hw/qdev-properties.h:168:5: note: in
expansion of macro 'DEFINE_PROP_UNSIGNED'
     DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_size, uint64_t)
     ^
/home/peter.maydell/qemu/hw/misc/pci-testdev.c:322:5: note: in
expansion of macro 'DEFINE_PROP_SIZE'
     DEFINE_PROP_SIZE("membar", PCITestDevState, membar_size, 0),
     ^
/home/peter.maydell/qemu/rules.mak:69: recipe for target
'hw/misc/pci-testdev.o' failed

On the Windows w64 cross-compile:

In file included from /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:26:0:
/home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c: In function
'amdvi_int_remap_msi':
/home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:247:46: error: left
shift count >= width of type [-Werror=shift-count-overflow]
 #define AMDVI_DEV_NMI_PASS_MASK         (1UL << 58)
                                              ^
/home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1281:25: note: in
expansion of macro 'AMDVI_DEV_NMI_PASS_MASK'
         pass = dte[3] & AMDVI_DEV_NMI_PASS_MASK;
                         ^
/home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:245:46: error: left
shift count >= width of type [-Werror=shift-count-overflow]
 #define AMDVI_DEV_INT_PASS_MASK         (1UL << 56)
                                              ^
/home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1285:25: note: in
expansion of macro 'AMDVI_DEV_INT_PASS_MASK'
         pass = dte[3] & AMDVI_DEV_INT_PASS_MASK;
                         ^
/home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:246:46: error: left
shift count >= width of type [-Werror=shift-count-overflow]
 #define AMDVI_DEV_EINT_PASS_MASK        (1UL << 57)
                                              ^
/home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1289:25: note: in
expansion of macro 'AMDVI_DEV_EINT_PASS_MASK'
         pass = dte[3] & AMDVI_DEV_EINT_PASS_MASK;
                         ^

These should presumably all be "ULL". (The "UL" suffix is
usually a bug, as it's either unnecessary or should be ULL.)

On various Linux hosts (aarch64, s390, x86-64, sparc)
I get this assertion in the bios-tables-test:

  /x86_64/acpi/q35/mmio64:
Looking for expected file 'tests/acpi-test-data/q35/DSDT.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/DSDT'

Using expected file 'tests/acpi-test-data/q35/DSDT'

Looking for expected file 'tests/acpi-test-data/q35/FACP.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/FACP'

Using expected file 'tests/acpi-test-data/q35/FACP'

Looking for expected file 'tests/acpi-test-data/q35/APIC.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/APIC'

Using expected file 'tests/acpi-test-data/q35/APIC'

Looking for expected file 'tests/acpi-test-data/q35/HPET.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/HPET'

Using expected file 'tests/acpi-test-data/q35/HPET'

Looking for expected file 'tests/acpi-test-data/q35/SRAT.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/SRAT'
**
ERROR:/home/petmay01/linaro/qemu-for-merges/tests/bios-tables-test.c:398:load_expected_aml:
assertion failed: (exp_sdt.aml_file)
FAIL


thanks
-- PMM

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Michael S. Tsirkin]

On Thu, Oct 25, 2018 at 08:16:44PM +0100, Peter Maydell wrote:
> On 25 October 2018 at 01:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> > The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
> >
> >   Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' into staging (2018-10-23 17:20:23 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to 6a9fb4e1ba5594cde7739068617ad88e6117db93:
> >
> >   vhost-scsi: prevent using uninitialized vqs (2018-10-24 20:50:13 -0400)
> >
> > ----------------------------------------------------------------
> > pci, pc, virtio: fixes, features
> >
> > AMD IOMMU VAPIC support + fixes all over the place.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Hi; I get some compile failures and a test assertion, I'm afraid:
> 
> On 32-bit hosts (where uint64_t and size_t are not the same):
> 
> /home/peter.maydell/qemu/include/qemu/compiler.h:80:35: error: invalid
> operands to binary - (have 'uint64_t * {aka long long unsigned int *}'
> and 'size_t * {aka unsigned int *}')
>  #define type_check(t1,t2) ((t1*)0 - (t2*)0)
>                                    ^
> /home/peter.maydell/qemu/include/hw/qdev-properties.h:77:15: note: in
> expansion of macro 'type_check'
>              + type_check(_type, typeof_field(_state, _field)),          \
>                ^
> /home/peter.maydell/qemu/include/hw/qdev-properties.h:168:5: note: in
> expansion of macro 'DEFINE_PROP_UNSIGNED'
>      DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_size, uint64_t)
>      ^
> /home/peter.maydell/qemu/hw/misc/pci-testdev.c:322:5: note: in
> expansion of macro 'DEFINE_PROP_SIZE'
>      DEFINE_PROP_SIZE("membar", PCITestDevState, membar_size, 0),
>      ^
> /home/peter.maydell/qemu/rules.mak:69: recipe for target
> 'hw/misc/pci-testdev.o' failed
> 
> On the Windows w64 cross-compile:
> 
> In file included from /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:26:0:
> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c: In function
> 'amdvi_int_remap_msi':
> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:247:46: error: left
> shift count >= width of type [-Werror=shift-count-overflow]
>  #define AMDVI_DEV_NMI_PASS_MASK         (1UL << 58)
>                                               ^
> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1281:25: note: in
> expansion of macro 'AMDVI_DEV_NMI_PASS_MASK'
>          pass = dte[3] & AMDVI_DEV_NMI_PASS_MASK;
>                          ^
> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:245:46: error: left
> shift count >= width of type [-Werror=shift-count-overflow]
>  #define AMDVI_DEV_INT_PASS_MASK         (1UL << 56)
>                                               ^
> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1285:25: note: in
> expansion of macro 'AMDVI_DEV_INT_PASS_MASK'
>          pass = dte[3] & AMDVI_DEV_INT_PASS_MASK;
>                          ^
> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:246:46: error: left
> shift count >= width of type [-Werror=shift-count-overflow]
>  #define AMDVI_DEV_EINT_PASS_MASK        (1UL << 57)
>                                               ^
> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1289:25: note: in
> expansion of macro 'AMDVI_DEV_EINT_PASS_MASK'
>          pass = dte[3] & AMDVI_DEV_EINT_PASS_MASK;
>                          ^
> 
> These should presumably all be "ULL". (The "UL" suffix is
> usually a bug, as it's either unnecessary or should be ULL.)

Yea.  Fixed. Brijesh could you start cleaning up that header generally?
It has all kind of weird code like using bitfields for hardware
accesses. That isn't portable - switch to full dword fields with shift
and | to operate them and proper cpu_to_le APIs or similar please.

> On various Linux hosts (aarch64, s390, x86-64, sparc)
> I get this assertion in the bios-tables-test:
> 
>   /x86_64/acpi/q35/mmio64:
> Looking for expected file 'tests/acpi-test-data/q35/DSDT.mmio64'
> 
> Looking for expected file 'tests/acpi-test-data/q35/DSDT'
> 
> Using expected file 'tests/acpi-test-data/q35/DSDT'
> 
> Looking for expected file 'tests/acpi-test-data/q35/FACP.mmio64'
> 
> Looking for expected file 'tests/acpi-test-data/q35/FACP'
> 
> Using expected file 'tests/acpi-test-data/q35/FACP'
> 
> Looking for expected file 'tests/acpi-test-data/q35/APIC.mmio64'
> 
> Looking for expected file 'tests/acpi-test-data/q35/APIC'
> 
> Using expected file 'tests/acpi-test-data/q35/APIC'
> 
> Looking for expected file 'tests/acpi-test-data/q35/HPET.mmio64'
> 
> Looking for expected file 'tests/acpi-test-data/q35/HPET'
> 
> Using expected file 'tests/acpi-test-data/q35/HPET'
> 
> Looking for expected file 'tests/acpi-test-data/q35/SRAT.mmio64'
> 
> Looking for expected file 'tests/acpi-test-data/q35/SRAT'
> **
> ERROR:/home/petmay01/linaro/qemu-for-merges/tests/bios-tables-test.c:398:load_expected_aml:
> assertion failed: (exp_sdt.aml_file)
> FAIL
> 
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Michael S. Tsirkin]

On Thu, Oct 25, 2018 at 08:16:44PM +0100, Peter Maydell wrote:
> On 25 October 2018 at 01:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> > The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
> >
> >   Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' into staging (2018-10-23 17:20:23 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to 6a9fb4e1ba5594cde7739068617ad88e6117db93:
> >
> >   vhost-scsi: prevent using uninitialized vqs (2018-10-24 20:50:13 -0400)
> >
> > ----------------------------------------------------------------
> > pci, pc, virtio: fixes, features
> >
> > AMD IOMMU VAPIC support + fixes all over the place.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Looking for expected file 'tests/acpi-test-data/q35/SRAT.mmio64'
> 
> Looking for expected file 'tests/acpi-test-data/q35/SRAT'
> **
> ERROR:/home/petmay01/linaro/qemu-for-merges/tests/bios-tables-test.c:398:load_expected_aml:
> assertion failed: (exp_sdt.aml_file)
> FAIL
> 
> 
> thanks
> -- PMM

Stumped by this one. Doesn't SRAT.mmio64 exist for you?

-- 
MST

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Michael S. Tsirkin]

On Thu, Oct 25, 2018 at 08:16:44PM +0100, Peter Maydell wrote:
> On 25 October 2018 at 01:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> > The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
> >
> >   Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' into staging (2018-10-23 17:20:23 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to 6a9fb4e1ba5594cde7739068617ad88e6117db93:
> >
> >   vhost-scsi: prevent using uninitialized vqs (2018-10-24 20:50:13 -0400)
> >
> > ----------------------------------------------------------------
> > pci, pc, virtio: fixes, features
> >
> > AMD IOMMU VAPIC support + fixes all over the place.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Hi; I get some compile failures and a test assertion, I'm afraid:

OK  I fixed 32 bit issues. Could not reproduce the test issues,
could it be that it was a side effect of build issues maybe?

Could you retyr?
If it still ails I will drop the test for now.

Thanks!

-- 
MST

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Singh, Brijesh]

On 10/25/2018 07:59 PM, Michael S. Tsirkin wrote:
> On Thu, Oct 25, 2018 at 08:16:44PM +0100, Peter Maydell wrote:
>> On 25 October 2018 at 01:52, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
>>>
>>>    Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' into staging (2018-10-23 17:20:23 +0100)
>>>
>>> are available in the Git repository at:
>>>
>>>    git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>>>
>>> for you to fetch changes up to 6a9fb4e1ba5594cde7739068617ad88e6117db93:
>>>
>>>    vhost-scsi: prevent using uninitialized vqs (2018-10-24 20:50:13 -0400)
>>>
>>> ----------------------------------------------------------------
>>> pci, pc, virtio: fixes, features
>>>
>>> AMD IOMMU VAPIC support + fixes all over the place.
>>>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>
>> Hi; I get some compile failures and a test assertion, I'm afraid:
>>
>> On 32-bit hosts (where uint64_t and size_t are not the same):
>>
>> /home/peter.maydell/qemu/include/qemu/compiler.h:80:35: error: invalid
>> operands to binary - (have 'uint64_t * {aka long long unsigned int *}'
>> and 'size_t * {aka unsigned int *}')
>>   #define type_check(t1,t2) ((t1*)0 - (t2*)0)
>>                                     ^
>> /home/peter.maydell/qemu/include/hw/qdev-properties.h:77:15: note: in
>> expansion of macro 'type_check'
>>               + type_check(_type, typeof_field(_state, _field)),          \
>>                 ^
>> /home/peter.maydell/qemu/include/hw/qdev-properties.h:168:5: note: in
>> expansion of macro 'DEFINE_PROP_UNSIGNED'
>>       DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_size, uint64_t)
>>       ^
>> /home/peter.maydell/qemu/hw/misc/pci-testdev.c:322:5: note: in
>> expansion of macro 'DEFINE_PROP_SIZE'
>>       DEFINE_PROP_SIZE("membar", PCITestDevState, membar_size, 0),
>>       ^
>> /home/peter.maydell/qemu/rules.mak:69: recipe for target
>> 'hw/misc/pci-testdev.o' failed
>>
>> On the Windows w64 cross-compile:
>>
>> In file included from /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:26:0:
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c: In function
>> 'amdvi_int_remap_msi':
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:247:46: error: left
>> shift count >= width of type [-Werror=shift-count-overflow]
>>   #define AMDVI_DEV_NMI_PASS_MASK         (1UL << 58)
>>                                                ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1281:25: note: in
>> expansion of macro 'AMDVI_DEV_NMI_PASS_MASK'
>>           pass = dte[3] & AMDVI_DEV_NMI_PASS_MASK;
>>                           ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:245:46: error: left
>> shift count >= width of type [-Werror=shift-count-overflow]
>>   #define AMDVI_DEV_INT_PASS_MASK         (1UL << 56)
>>                                                ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1285:25: note: in
>> expansion of macro 'AMDVI_DEV_INT_PASS_MASK'
>>           pass = dte[3] & AMDVI_DEV_INT_PASS_MASK;
>>                           ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:246:46: error: left
>> shift count >= width of type [-Werror=shift-count-overflow]
>>   #define AMDVI_DEV_EINT_PASS_MASK        (1UL << 57)
>>                                                ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1289:25: note: in
>> expansion of macro 'AMDVI_DEV_EINT_PASS_MASK'
>>           pass = dte[3] & AMDVI_DEV_EINT_PASS_MASK;
>>                           ^
>>
>> These should presumably all be "ULL". (The "UL" suffix is
>> usually a bug, as it's either unnecessary or should be ULL.)
> 
> Yea.  Fixed. Brijesh could you start cleaning up that header generally?
> It has all kind of weird code like using bitfields for hardware
> accesses. That isn't portable - switch to full dword fields with shift
> and | to operate them and proper cpu_to_le APIs or similar please.
> 


Noted, I will look into cleaning up this and send patches for reviews.

thanks

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Peter Maydell]

On 26 October 2018 at 14:26, Singh, Brijesh <brijesh.singh@amd.com> wrote:
>
>
> On 10/25/2018 07:59 PM, Michael S. Tsirkin wrote:
>> On Thu, Oct 25, 2018 at 08:16:44PM +0100, Peter Maydell wrote:
>>> These should presumably all be "ULL". (The "UL" suffix is
>>> usually a bug, as it's either unnecessary or should be ULL.)
>>
>> Yea.  Fixed. Brijesh could you start cleaning up that header generally?
>> It has all kind of weird code like using bitfields for hardware
>> accesses. That isn't portable - switch to full dword fields with shift
>> and | to operate them and proper cpu_to_le APIs or similar please.
>>
>
>
> Noted, I will look into cleaning up this and send patches for reviews.

Thanks. You might like to look at the extract32()/extract64()/
deposit32()/deposit64() functions in bitops.h, which are often (but not
always) cleaner than hand-rolled shifts-and-logical-ops for assembling
and disassembling fields.

thanks
-- PMM

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Peter Maydell]

On 26 October 2018 at 03:00, Michael S. Tsirkin <mst@redhat.com> wrote:
> OK  I fixed 32 bit issues. Could not reproduce the test issues,
> could it be that it was a side effect of build issues maybe?
>
> Could you retyr?
> If it still ails I will drop the test for now.

Compiles OK, but still asserts
ERROR:/home/pm215/qemu/tests/bios-tables-test.c:398:load_expected_aml: assertion
 failed: (exp_sdt.aml_file)

on Linux/aarch64, Linux/arm32, Linux/SPARC, Linux/s390x.

pm215@gcc113:~/qemu/build/all$
QTEST_QEMU_BINARY=i386-softmmu/qemu-system-i386
./tests/bios-tables-test
/i386/acpi/piix4: OK
/i386/acpi/q35: ^[[AOK
/i386/acpi/piix4/bridge: OK
/i386/acpi/piix4/ipmi: OK
/i386/acpi/piix4/cpuhp: OK
/i386/acpi/piix4/memhp: OK
/i386/acpi/piix4/numamem: OK
/i386/acpi/piix4/dimmpxm: OK
/i386/acpi/q35/bridge: OK
/i386/acpi/q35/mmio64: **
ERROR:/home/pm215/qemu/tests/bios-tables-test.c:398:load_expected_aml:
assertion failed: (exp_sdt.aml_file)
Aborted (core dumped)

With V=1 it says
/i386/acpi/q35/mmio64:
Looking for expected file 'tests/acpi-test-data/q35/DSDT.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/DSDT'

Using expected file 'tests/acpi-test-data/q35/DSDT'

Looking for expected file 'tests/acpi-test-data/q35/FACP.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/FACP'

Using expected file 'tests/acpi-test-data/q35/FACP'

Looking for expected file 'tests/acpi-test-data/q35/APIC.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/APIC'

Using expected file 'tests/acpi-test-data/q35/APIC'

Looking for expected file 'tests/acpi-test-data/q35/HPET.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/HPET'

Using expected file 'tests/acpi-test-data/q35/HPET'

Looking for expected file 'tests/acpi-test-data/q35/SRAT.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/SRAT'
**
ERROR:/home/pm215/qemu/tests/bios-tables-test.c:398:load_expected_aml:
assertion failed: (exp_sdt.aml_file)

ET'

On an x86-64 host which works it says
/i386/acpi/q35/mmio64:
Looking for expected file 'tests/acpi-test-data/q35/DSDT.mmio64'

Using expected file 'tests/acpi-test-data/q35/DSDT.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/FACP.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/FACP'

Using expected file 'tests/acpi-test-data/q35/FACP'

Looking for expected file 'tests/acpi-test-data/q35/APIC.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/APIC'

Using expected file 'tests/acpi-test-data/q35/APIC'

Looking for expected file 'tests/acpi-test-data/q35/HPET.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/HPET'

Using expected file 'tests/acpi-test-data/q35/HPET'

Looking for expected file 'tests/acpi-test-data/q35/SRAT.mmio64'

Using expected file 'tests/acpi-test-data/q35/SRAT.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/MCFG.mmio64'

Looking for expected file 'tests/acpi-test-data/q35/MCFG'

Using expected file 'tests/acpi-test-data/q35/MCFG'
OK

So the difference is that the failing setup doesn't use
the SRAT.mmio64, and there is no SRAT file available for
fallback. (It also doesn't use DSDT.mmio64, but there there
is a fallback DSDT file in the tests/acpi-test-data/ directory.)

It looks like the test/acpi-test-data/ in the build directory
has not been populated with the MCFG.mmio64 file -- some of
the data files have symlinks from that directory back to the
source directory, but some do not. This seems to be done in
configure, but there is nothing in the makefile rules which
causes configure to be rerun because there is a new file
in your test data directory. A manual rerun of configure
adds the missing symlinks and causes the test to pass.

So I would characterise this as a makefile issue.

(PS: can we fix the extraneous extra blank lines in the test output
when run with V=1, please?)

thanks
-- PMM

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Peter Maydell]

On 27 October 2018 at 19:53, Peter Maydell <peter.maydell@linaro.org> wrote:
> It looks like the test/acpi-test-data/ in the build directory
> has not been populated with the MCFG.mmio64 file -- some of
> the data files have symlinks from that directory back to the
> source directory, but some do not. This seems to be done in
> configure, but there is nothing in the makefile rules which
> causes configure to be rerun because there is a new file
> in your test data directory. A manual rerun of configure
> adds the missing symlinks and causes the test to pass.
>
> So I would characterise this as a makefile issue.

Is there a reason why we need to symlink all the data files
in tests/acpi-test-data/ individually, rather than just
having tests/acpi-test-data itself be a symlink back to
the directory in the source tree ? That would avoid this
problem; it's how we handle tests/acceptance and tests/data.

thanks
-- PMM

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Peter Maydell]

On 25 October 2018 at 01:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' into staging (2018-10-23 17:20:23 +0100)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 6a9fb4e1ba5594cde7739068617ad88e6117db93:
>
>   vhost-scsi: prevent using uninitialized vqs (2018-10-24 20:50:13 -0400)
>
> ----------------------------------------------------------------
> pci, pc, virtio: fixes, features
>
> AMD IOMMU VAPIC support + fixes all over the place.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>

Hi Michael -- just a reminder that you need to submit a respin
of this by 1200 UTC tomorrow (with my symlink-of-data-files
patchset) if you want it to get into rc0.

thanks
-- PMM

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Michael S. Tsirkin]

On Mon, Nov 05, 2018 at 03:05:27PM +0000, Peter Maydell wrote:
> On 25 October 2018 at 01:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> > The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
> >
> >   Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' into staging (2018-10-23 17:20:23 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to 6a9fb4e1ba5594cde7739068617ad88e6117db93:
> >
> >   vhost-scsi: prevent using uninitialized vqs (2018-10-24 20:50:13 -0400)
> >
> > ----------------------------------------------------------------
> > pci, pc, virtio: fixes, features
> >
> > AMD IOMMU VAPIC support + fixes all over the place.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> 
> Hi Michael -- just a reminder that you need to submit a respin
> of this by 1200 UTC tomorrow (with my symlink-of-data-files
> patchset) if you want it to get into rc0.
> 
> thanks
> -- PMM

Working on it.

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Michael S. Tsirkin]

On Mon, Nov 05, 2018 at 03:05:27PM +0000, Peter Maydell wrote:
> On 25 October 2018 at 01:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> > The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
> >
> >   Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' into staging (2018-10-23 17:20:23 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to 6a9fb4e1ba5594cde7739068617ad88e6117db93:
> >
> >   vhost-scsi: prevent using uninitialized vqs (2018-10-24 20:50:13 -0400)
> >
> > ----------------------------------------------------------------
> > pci, pc, virtio: fixes, features
> >
> > AMD IOMMU VAPIC support + fixes all over the place.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> 
> Hi Michael -- just a reminder that you need to submit a respin
> of this by 1200 UTC tomorrow (with my symlink-of-data-files
> patchset) if you want it to get into rc0.
> 
> thanks
> -- PMM

Resent - hope it's fine now.

-- 
MST