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

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

[Michael S. Tsirkin]

The following changes since commit b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:

  Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' into staging (2018-11-05 10:32:49 +0000)

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 6196df5c8e6688c1c3f06f73442820066335337c:

  vhost-scsi: prevent using uninitialized vqs (2018-11-05 12:59:35 -0500)

----------------------------------------------------------------
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

Michael S. Tsirkin (1):
      bios-tables-test: prepare expected files for mmio64

Peter Maydell (4):
      tests: Move tests/acpi-test-data/ to tests/data/acpi/
      tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/
      configure: Rename FILES variable to LINKS
      configure: Use LINKS loop for all build tree symlinks

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 +-
 configure                                          |  57 ++-
 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                           |  18 +-
 tests/hexloader-test.c                             |   2 +-
 MAINTAINERS                                        |   4 +-
 hw/i386/trace-events                               |  14 +
 tests/{acpi-test-data => data/acpi}/pc/APIC        | Bin
 tests/{acpi-test-data => data/acpi}/pc/APIC.cphp   | Bin
 .../{acpi-test-data => data/acpi}/pc/APIC.dimmpxm  | Bin
 tests/{acpi-test-data => data/acpi}/pc/DSDT        | Bin
 tests/{acpi-test-data => data/acpi}/pc/DSDT.bridge | Bin
 tests/{acpi-test-data => data/acpi}/pc/DSDT.cphp   | Bin
 .../{acpi-test-data => data/acpi}/pc/DSDT.dimmpxm  | Bin
 .../{acpi-test-data => data/acpi}/pc/DSDT.ipmikcs  | Bin
 tests/{acpi-test-data => data/acpi}/pc/DSDT.memhp  | Bin
 .../{acpi-test-data => data/acpi}/pc/DSDT.numamem  | Bin
 tests/{acpi-test-data => data/acpi}/pc/FACP        | Bin
 tests/{acpi-test-data => data/acpi}/pc/FACS        | Bin
 tests/{acpi-test-data => data/acpi}/pc/HPET        | Bin
 .../{acpi-test-data => data/acpi}/pc/NFIT.dimmpxm  | Bin
 tests/{acpi-test-data => data/acpi}/pc/SLIT.cphp   | Bin
 tests/{acpi-test-data => data/acpi}/pc/SLIT.memhp  | Bin
 tests/{acpi-test-data => data/acpi}/pc/SRAT.cphp   | Bin
 .../{acpi-test-data => data/acpi}/pc/SRAT.dimmpxm  | Bin
 tests/{acpi-test-data => data/acpi}/pc/SRAT.memhp  | Bin
 .../{acpi-test-data => data/acpi}/pc/SRAT.numamem  | Bin
 .../{acpi-test-data => data/acpi}/pc/SSDT.dimmpxm  | Bin
 tests/{acpi-test-data => data/acpi}/q35/APIC       | Bin
 tests/{acpi-test-data => data/acpi}/q35/APIC.cphp  | Bin
 .../{acpi-test-data => data/acpi}/q35/APIC.dimmpxm | Bin
 tests/{acpi-test-data => data/acpi}/q35/DSDT       | Bin
 .../{acpi-test-data => data/acpi}/q35/DSDT.bridge  | Bin
 tests/{acpi-test-data => data/acpi}/q35/DSDT.cphp  | Bin
 .../{acpi-test-data => data/acpi}/q35/DSDT.dimmpxm | Bin
 .../{acpi-test-data => data/acpi}/q35/DSDT.ipmibt  | Bin
 tests/{acpi-test-data => data/acpi}/q35/DSDT.memhp | Bin
 tests/data/acpi/q35/DSDT.mmio64                    | Bin 0 -> 8947 bytes
 .../{acpi-test-data => data/acpi}/q35/DSDT.numamem | Bin
 tests/{acpi-test-data => data/acpi}/q35/FACP       | Bin
 tests/{acpi-test-data => data/acpi}/q35/FACS       | Bin
 tests/{acpi-test-data => data/acpi}/q35/HPET       | Bin
 tests/{acpi-test-data => data/acpi}/q35/MCFG       | Bin
 .../{acpi-test-data => data/acpi}/q35/NFIT.dimmpxm | Bin
 tests/{acpi-test-data => data/acpi}/q35/SLIT.cphp  | Bin
 tests/{acpi-test-data => data/acpi}/q35/SLIT.memhp | Bin
 tests/{acpi-test-data => data/acpi}/q35/SRAT.cphp  | Bin
 .../{acpi-test-data => data/acpi}/q35/SRAT.dimmpxm | Bin
 tests/{acpi-test-data => data/acpi}/q35/SRAT.memhp | Bin
 tests/data/acpi/q35/SRAT.mmio64                    | Bin 0 -> 224 bytes
 .../{acpi-test-data => data/acpi}/q35/SRAT.numamem | Bin
 .../{acpi-test-data => data/acpi}/q35/SSDT.dimmpxm | Bin
 .../acpi}/rebuild-expected-aml.sh                  |   2 -
 .../hex-loader}/test.hex                           |   0
 74 files changed, 838 insertions(+), 290 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
 rename tests/{acpi-test-data => data/acpi}/pc/APIC (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/APIC.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/APIC.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.bridge (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.ipmikcs (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.memhp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.numamem (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/FACP (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/FACS (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/HPET (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/NFIT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SLIT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SLIT.memhp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SRAT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SRAT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SRAT.memhp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SRAT.numamem (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SSDT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/APIC (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/APIC.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/APIC.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.bridge (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.ipmibt (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.memhp (100%)
 create mode 100644 tests/data/acpi/q35/DSDT.mmio64
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.numamem (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/FACP (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/FACS (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/HPET (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/MCFG (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/NFIT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SLIT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SLIT.memhp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SRAT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SRAT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SRAT.memhp (100%)
 create mode 100644 tests/data/acpi/q35/SRAT.mmio64
 rename tests/{acpi-test-data => data/acpi}/q35/SRAT.numamem (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SSDT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/rebuild-expected-aml.sh (86%)
 rename tests/{hex-loader-check-data => data/hex-loader}/test.hex (100%)

[Qemu-devel] [PULL 01/33] tests: Move tests/acpi-test-data/ to tests/data/acpi/

[Michael S. Tsirkin]

From: Peter Maydell <peter.maydell@linaro.org>

Currently tests/acpi-test-data contains data files used by the
bios-tables-test, and configure individually symlinks those
data files into the build directory using a wildcard.

Using a wildcard like this is a bad idea, because if a new
data file is added, nothing causes configure to be rerun,
and so no symlink is added for the new file. This can cause
tests to spuriously fail when they can't find their data.
Instead, it's better to symlink an entire directory of
data files. We already have such a directory: tests/data.

Move the data files from tests/acpi-test-data/ to
tests/data/acpi/, and remove the unnecessary symlinking.

We can remove entirely the note in rebuild-expected-aml.sh
about copying any new data files, because now they will
be in the source directory, not the build directory, and
no copying is required.

(We can't just change the existing tests/acpi-test-data/
to being a symlinked directory, because if we did that and
a developer switched git branches from one after that change
to one before it then configure would end up trashing all
the test files by making them symlinks to themselves.
Changing their path avoids this annoyance.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure                                           |   4 ----
 tests/bios-tables-test.c                            |   2 +-
 tests/{acpi-test-data => data/acpi}/pc/APIC         | Bin
 tests/{acpi-test-data => data/acpi}/pc/APIC.cphp    | Bin
 tests/{acpi-test-data => data/acpi}/pc/APIC.dimmpxm | Bin
 tests/{acpi-test-data => data/acpi}/pc/DSDT         | Bin
 tests/{acpi-test-data => data/acpi}/pc/DSDT.bridge  | Bin
 tests/{acpi-test-data => data/acpi}/pc/DSDT.cphp    | Bin
 tests/{acpi-test-data => data/acpi}/pc/DSDT.dimmpxm | Bin
 tests/{acpi-test-data => data/acpi}/pc/DSDT.ipmikcs | Bin
 tests/{acpi-test-data => data/acpi}/pc/DSDT.memhp   | Bin
 tests/{acpi-test-data => data/acpi}/pc/DSDT.numamem | Bin
 tests/{acpi-test-data => data/acpi}/pc/FACP         | Bin
 tests/{acpi-test-data => data/acpi}/pc/FACS         | Bin
 tests/{acpi-test-data => data/acpi}/pc/HPET         | Bin
 tests/{acpi-test-data => data/acpi}/pc/NFIT.dimmpxm | Bin
 tests/{acpi-test-data => data/acpi}/pc/SLIT.cphp    | Bin
 tests/{acpi-test-data => data/acpi}/pc/SLIT.memhp   | Bin
 tests/{acpi-test-data => data/acpi}/pc/SRAT.cphp    | Bin
 tests/{acpi-test-data => data/acpi}/pc/SRAT.dimmpxm | Bin
 tests/{acpi-test-data => data/acpi}/pc/SRAT.memhp   | Bin
 tests/{acpi-test-data => data/acpi}/pc/SRAT.numamem | Bin
 tests/{acpi-test-data => data/acpi}/pc/SSDT.dimmpxm | Bin
 tests/{acpi-test-data => data/acpi}/q35/APIC        | Bin
 tests/{acpi-test-data => data/acpi}/q35/APIC.cphp   | Bin
 .../{acpi-test-data => data/acpi}/q35/APIC.dimmpxm  | Bin
 tests/{acpi-test-data => data/acpi}/q35/DSDT        | Bin
 tests/{acpi-test-data => data/acpi}/q35/DSDT.bridge | Bin
 tests/{acpi-test-data => data/acpi}/q35/DSDT.cphp   | Bin
 .../{acpi-test-data => data/acpi}/q35/DSDT.dimmpxm  | Bin
 tests/{acpi-test-data => data/acpi}/q35/DSDT.ipmibt | Bin
 tests/{acpi-test-data => data/acpi}/q35/DSDT.memhp  | Bin
 .../{acpi-test-data => data/acpi}/q35/DSDT.numamem  | Bin
 tests/{acpi-test-data => data/acpi}/q35/FACP        | Bin
 tests/{acpi-test-data => data/acpi}/q35/FACS        | Bin
 tests/{acpi-test-data => data/acpi}/q35/HPET        | Bin
 tests/{acpi-test-data => data/acpi}/q35/MCFG        | Bin
 .../{acpi-test-data => data/acpi}/q35/NFIT.dimmpxm  | Bin
 tests/{acpi-test-data => data/acpi}/q35/SLIT.cphp   | Bin
 tests/{acpi-test-data => data/acpi}/q35/SLIT.memhp  | Bin
 tests/{acpi-test-data => data/acpi}/q35/SRAT.cphp   | Bin
 .../{acpi-test-data => data/acpi}/q35/SRAT.dimmpxm  | Bin
 tests/{acpi-test-data => data/acpi}/q35/SRAT.memhp  | Bin
 .../{acpi-test-data => data/acpi}/q35/SRAT.numamem  | Bin
 .../{acpi-test-data => data/acpi}/q35/SSDT.dimmpxm  | Bin
 .../acpi}/rebuild-expected-aml.sh                   |   2 --
 46 files changed, 1 insertion(+), 7 deletions(-)
 rename tests/{acpi-test-data => data/acpi}/pc/APIC (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/APIC.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/APIC.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.bridge (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.ipmikcs (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.memhp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/DSDT.numamem (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/FACP (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/FACS (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/HPET (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/NFIT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SLIT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SLIT.memhp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SRAT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SRAT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SRAT.memhp (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SRAT.numamem (100%)
 rename tests/{acpi-test-data => data/acpi}/pc/SSDT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/APIC (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/APIC.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/APIC.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.bridge (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.ipmibt (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.memhp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/DSDT.numamem (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/FACP (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/FACS (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/HPET (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/MCFG (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/NFIT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SLIT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SLIT.memhp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SRAT.cphp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SRAT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SRAT.memhp (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SRAT.numamem (100%)
 rename tests/{acpi-test-data => data/acpi}/q35/SSDT.dimmpxm (100%)
 rename tests/{acpi-test-data => data/acpi}/rebuild-expected-aml.sh (86%)

diff --git a/configure b/configure
index 46ae1e8c76..895b7483b8 100755
--- a/configure
+++ b/configure
@@ -7421,10 +7421,6 @@ for bios_file in \
 do
     FILES="$FILES pc-bios/$(basename $bios_file)"
 done
-for test_file in $(find $source_path/tests/acpi-test-data -type f)
-do
-    FILES="$FILES tests/acpi-test-data$(echo $test_file | sed -e 's/.*acpi-test-data//')"
-done
 for test_file in $(find $source_path/tests/hex-loader-check-data -type f)
 do
     FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed -e 's/.*hex-loader-check-data//')"
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 02e77ec811..b14c8eaa17 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -42,7 +42,7 @@ typedef struct {
 } test_data;
 
 static char disk[] = "tests/acpi-test-disk-XXXXXX";
-static const char *data_dir = "tests/acpi-test-data";
+static const char *data_dir = "tests/data/acpi";
 #ifdef CONFIG_IASL
 static const char *iasl = stringify(CONFIG_IASL);
 #else
diff --git a/tests/acpi-test-data/pc/APIC b/tests/data/acpi/pc/APIC
similarity index 100%
rename from tests/acpi-test-data/pc/APIC
rename to tests/data/acpi/pc/APIC
diff --git a/tests/acpi-test-data/pc/APIC.cphp b/tests/data/acpi/pc/APIC.cphp
similarity index 100%
rename from tests/acpi-test-data/pc/APIC.cphp
rename to tests/data/acpi/pc/APIC.cphp
diff --git a/tests/acpi-test-data/pc/APIC.dimmpxm b/tests/data/acpi/pc/APIC.dimmpxm
similarity index 100%
rename from tests/acpi-test-data/pc/APIC.dimmpxm
rename to tests/data/acpi/pc/APIC.dimmpxm
diff --git a/tests/acpi-test-data/pc/DSDT b/tests/data/acpi/pc/DSDT
similarity index 100%
rename from tests/acpi-test-data/pc/DSDT
rename to tests/data/acpi/pc/DSDT
diff --git a/tests/acpi-test-data/pc/DSDT.bridge b/tests/data/acpi/pc/DSDT.bridge
similarity index 100%
rename from tests/acpi-test-data/pc/DSDT.bridge
rename to tests/data/acpi/pc/DSDT.bridge
diff --git a/tests/acpi-test-data/pc/DSDT.cphp b/tests/data/acpi/pc/DSDT.cphp
similarity index 100%
rename from tests/acpi-test-data/pc/DSDT.cphp
rename to tests/data/acpi/pc/DSDT.cphp
diff --git a/tests/acpi-test-data/pc/DSDT.dimmpxm b/tests/data/acpi/pc/DSDT.dimmpxm
similarity index 100%
rename from tests/acpi-test-data/pc/DSDT.dimmpxm
rename to tests/data/acpi/pc/DSDT.dimmpxm
diff --git a/tests/acpi-test-data/pc/DSDT.ipmikcs b/tests/data/acpi/pc/DSDT.ipmikcs
similarity index 100%
rename from tests/acpi-test-data/pc/DSDT.ipmikcs
rename to tests/data/acpi/pc/DSDT.ipmikcs
diff --git a/tests/acpi-test-data/pc/DSDT.memhp b/tests/data/acpi/pc/DSDT.memhp
similarity index 100%
rename from tests/acpi-test-data/pc/DSDT.memhp
rename to tests/data/acpi/pc/DSDT.memhp
diff --git a/tests/acpi-test-data/pc/DSDT.numamem b/tests/data/acpi/pc/DSDT.numamem
similarity index 100%
rename from tests/acpi-test-data/pc/DSDT.numamem
rename to tests/data/acpi/pc/DSDT.numamem
diff --git a/tests/acpi-test-data/pc/FACP b/tests/data/acpi/pc/FACP
similarity index 100%
rename from tests/acpi-test-data/pc/FACP
rename to tests/data/acpi/pc/FACP
diff --git a/tests/acpi-test-data/pc/FACS b/tests/data/acpi/pc/FACS
similarity index 100%
rename from tests/acpi-test-data/pc/FACS
rename to tests/data/acpi/pc/FACS
diff --git a/tests/acpi-test-data/pc/HPET b/tests/data/acpi/pc/HPET
similarity index 100%
rename from tests/acpi-test-data/pc/HPET
rename to tests/data/acpi/pc/HPET
diff --git a/tests/acpi-test-data/pc/NFIT.dimmpxm b/tests/data/acpi/pc/NFIT.dimmpxm
similarity index 100%
rename from tests/acpi-test-data/pc/NFIT.dimmpxm
rename to tests/data/acpi/pc/NFIT.dimmpxm
diff --git a/tests/acpi-test-data/pc/SLIT.cphp b/tests/data/acpi/pc/SLIT.cphp
similarity index 100%
rename from tests/acpi-test-data/pc/SLIT.cphp
rename to tests/data/acpi/pc/SLIT.cphp
diff --git a/tests/acpi-test-data/pc/SLIT.memhp b/tests/data/acpi/pc/SLIT.memhp
similarity index 100%
rename from tests/acpi-test-data/pc/SLIT.memhp
rename to tests/data/acpi/pc/SLIT.memhp
diff --git a/tests/acpi-test-data/pc/SRAT.cphp b/tests/data/acpi/pc/SRAT.cphp
similarity index 100%
rename from tests/acpi-test-data/pc/SRAT.cphp
rename to tests/data/acpi/pc/SRAT.cphp
diff --git a/tests/acpi-test-data/pc/SRAT.dimmpxm b/tests/data/acpi/pc/SRAT.dimmpxm
similarity index 100%
rename from tests/acpi-test-data/pc/SRAT.dimmpxm
rename to tests/data/acpi/pc/SRAT.dimmpxm
diff --git a/tests/acpi-test-data/pc/SRAT.memhp b/tests/data/acpi/pc/SRAT.memhp
similarity index 100%
rename from tests/acpi-test-data/pc/SRAT.memhp
rename to tests/data/acpi/pc/SRAT.memhp
diff --git a/tests/acpi-test-data/pc/SRAT.numamem b/tests/data/acpi/pc/SRAT.numamem
similarity index 100%
rename from tests/acpi-test-data/pc/SRAT.numamem
rename to tests/data/acpi/pc/SRAT.numamem
diff --git a/tests/acpi-test-data/pc/SSDT.dimmpxm b/tests/data/acpi/pc/SSDT.dimmpxm
similarity index 100%
rename from tests/acpi-test-data/pc/SSDT.dimmpxm
rename to tests/data/acpi/pc/SSDT.dimmpxm
diff --git a/tests/acpi-test-data/q35/APIC b/tests/data/acpi/q35/APIC
similarity index 100%
rename from tests/acpi-test-data/q35/APIC
rename to tests/data/acpi/q35/APIC
diff --git a/tests/acpi-test-data/q35/APIC.cphp b/tests/data/acpi/q35/APIC.cphp
similarity index 100%
rename from tests/acpi-test-data/q35/APIC.cphp
rename to tests/data/acpi/q35/APIC.cphp
diff --git a/tests/acpi-test-data/q35/APIC.dimmpxm b/tests/data/acpi/q35/APIC.dimmpxm
similarity index 100%
rename from tests/acpi-test-data/q35/APIC.dimmpxm
rename to tests/data/acpi/q35/APIC.dimmpxm
diff --git a/tests/acpi-test-data/q35/DSDT b/tests/data/acpi/q35/DSDT
similarity index 100%
rename from tests/acpi-test-data/q35/DSDT
rename to tests/data/acpi/q35/DSDT
diff --git a/tests/acpi-test-data/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge
similarity index 100%
rename from tests/acpi-test-data/q35/DSDT.bridge
rename to tests/data/acpi/q35/DSDT.bridge
diff --git a/tests/acpi-test-data/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp
similarity index 100%
rename from tests/acpi-test-data/q35/DSDT.cphp
rename to tests/data/acpi/q35/DSDT.cphp
diff --git a/tests/acpi-test-data/q35/DSDT.dimmpxm b/tests/data/acpi/q35/DSDT.dimmpxm
similarity index 100%
rename from tests/acpi-test-data/q35/DSDT.dimmpxm
rename to tests/data/acpi/q35/DSDT.dimmpxm
diff --git a/tests/acpi-test-data/q35/DSDT.ipmibt b/tests/data/acpi/q35/DSDT.ipmibt
similarity index 100%
rename from tests/acpi-test-data/q35/DSDT.ipmibt
rename to tests/data/acpi/q35/DSDT.ipmibt
diff --git a/tests/acpi-test-data/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp
similarity index 100%
rename from tests/acpi-test-data/q35/DSDT.memhp
rename to tests/data/acpi/q35/DSDT.memhp
diff --git a/tests/acpi-test-data/q35/DSDT.numamem b/tests/data/acpi/q35/DSDT.numamem
similarity index 100%
rename from tests/acpi-test-data/q35/DSDT.numamem
rename to tests/data/acpi/q35/DSDT.numamem
diff --git a/tests/acpi-test-data/q35/FACP b/tests/data/acpi/q35/FACP
similarity index 100%
rename from tests/acpi-test-data/q35/FACP
rename to tests/data/acpi/q35/FACP
diff --git a/tests/acpi-test-data/q35/FACS b/tests/data/acpi/q35/FACS
similarity index 100%
rename from tests/acpi-test-data/q35/FACS
rename to tests/data/acpi/q35/FACS
diff --git a/tests/acpi-test-data/q35/HPET b/tests/data/acpi/q35/HPET
similarity index 100%
rename from tests/acpi-test-data/q35/HPET
rename to tests/data/acpi/q35/HPET
diff --git a/tests/acpi-test-data/q35/MCFG b/tests/data/acpi/q35/MCFG
similarity index 100%
rename from tests/acpi-test-data/q35/MCFG
rename to tests/data/acpi/q35/MCFG
diff --git a/tests/acpi-test-data/q35/NFIT.dimmpxm b/tests/data/acpi/q35/NFIT.dimmpxm
similarity index 100%
rename from tests/acpi-test-data/q35/NFIT.dimmpxm
rename to tests/data/acpi/q35/NFIT.dimmpxm
diff --git a/tests/acpi-test-data/q35/SLIT.cphp b/tests/data/acpi/q35/SLIT.cphp
similarity index 100%
rename from tests/acpi-test-data/q35/SLIT.cphp
rename to tests/data/acpi/q35/SLIT.cphp
diff --git a/tests/acpi-test-data/q35/SLIT.memhp b/tests/data/acpi/q35/SLIT.memhp
similarity index 100%
rename from tests/acpi-test-data/q35/SLIT.memhp
rename to tests/data/acpi/q35/SLIT.memhp
diff --git a/tests/acpi-test-data/q35/SRAT.cphp b/tests/data/acpi/q35/SRAT.cphp
similarity index 100%
rename from tests/acpi-test-data/q35/SRAT.cphp
rename to tests/data/acpi/q35/SRAT.cphp
diff --git a/tests/acpi-test-data/q35/SRAT.dimmpxm b/tests/data/acpi/q35/SRAT.dimmpxm
similarity index 100%
rename from tests/acpi-test-data/q35/SRAT.dimmpxm
rename to tests/data/acpi/q35/SRAT.dimmpxm
diff --git a/tests/acpi-test-data/q35/SRAT.memhp b/tests/data/acpi/q35/SRAT.memhp
similarity index 100%
rename from tests/acpi-test-data/q35/SRAT.memhp
rename to tests/data/acpi/q35/SRAT.memhp
diff --git a/tests/acpi-test-data/q35/SRAT.numamem b/tests/data/acpi/q35/SRAT.numamem
similarity index 100%
rename from tests/acpi-test-data/q35/SRAT.numamem
rename to tests/data/acpi/q35/SRAT.numamem
diff --git a/tests/acpi-test-data/q35/SSDT.dimmpxm b/tests/data/acpi/q35/SSDT.dimmpxm
similarity index 100%
rename from tests/acpi-test-data/q35/SSDT.dimmpxm
rename to tests/data/acpi/q35/SSDT.dimmpxm
diff --git a/tests/acpi-test-data/rebuild-expected-aml.sh b/tests/data/acpi/rebuild-expected-aml.sh
similarity index 86%
rename from tests/acpi-test-data/rebuild-expected-aml.sh
rename to tests/data/acpi/rebuild-expected-aml.sh
index 11bf743914..bf9ba242ad 100755
--- a/tests/acpi-test-data/rebuild-expected-aml.sh
+++ b/tests/data/acpi/rebuild-expected-aml.sh
@@ -32,5 +32,3 @@ fi
 TEST_ACPI_REBUILD_AML=y QTEST_QEMU_BINARY=$qemu tests/bios-tables-test
 
 echo "The files were rebuilt and can be added to git."
-echo "However, if new files were created, please copy them manually" \
-     "to tests/acpi-test-data/pc/ or tests/acpi-test-data/q35/ ."
-- 
MST

[Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Michael S. Tsirkin]

From: Peter Maydell <peter.maydell@linaro.org>

Currently tests/hex-loader-check-data contains data files used
by the hexloader-test, and configure individually symlinks those
data files into the build directory using a wildcard.

Using a wildcard like this is a bad idea, because if a new
data file is added, nothing causes configure to be rerun,
and so no symlink is added for the new file. This can cause
tests to spuriously fail when they can't find their data.
Instead, it's better to symlink an entire directory of
data files. We already have such a directory: tests/data.

Move the data files from tests/hex-loader-check-data/ to
tests/data/hex-loader/, and remove the unnecessary symlinking.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure                                                 | 4 ----
 tests/hexloader-test.c                                    | 2 +-
 MAINTAINERS                                               | 2 +-
 tests/{hex-loader-check-data => data/hex-loader}/test.hex | 0
 4 files changed, 2 insertions(+), 6 deletions(-)
 rename tests/{hex-loader-check-data => data/hex-loader}/test.hex (100%)

diff --git a/configure b/configure
index 895b7483b8..bfdca8b814 100755
--- a/configure
+++ b/configure
@@ -7421,10 +7421,6 @@ for bios_file in \
 do
     FILES="$FILES pc-bios/$(basename $bios_file)"
 done
-for test_file in $(find $source_path/tests/hex-loader-check-data -type f)
-do
-    FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed -e 's/.*hex-loader-check-data//')"
-done
 mkdir -p $DIRS
 for f in $FILES ; do
     if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; then
diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
index b653d44ba1..834ed52c22 100644
--- a/tests/hexloader-test.c
+++ b/tests/hexloader-test.c
@@ -23,7 +23,7 @@ static void hex_loader_test(void)
     const unsigned int base_addr = 0x00010000;
 
     QTestState *s = qtest_initf(
-        "-M vexpress-a9 -nographic -device loader,file=tests/hex-loader-check-data/test.hex");
+        "-M vexpress-a9 -nographic -device loader,file=tests/data/hex-loader/test.hex");
 
     for (i = 0; i < 256; ++i) {
         uint8_t val = qtest_readb(s, base_addr + i);
diff --git a/MAINTAINERS b/MAINTAINERS
index 98a1856afc..cfabc14b59 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1370,7 +1370,7 @@ Intel Hexadecimal Object File Loader
 M: Su Hang <suhang16@mails.ucas.ac.cn>
 S: Maintained
 F: tests/hexloader-test.c
-F: tests/hex-loader-check-data/test.hex
+F: tests/data/hex-loader/test.hex
 
 CHRP NVRAM
 M: Thomas Huth <thuth@redhat.com>
diff --git a/tests/hex-loader-check-data/test.hex b/tests/data/hex-loader/test.hex
similarity index 100%
rename from tests/hex-loader-check-data/test.hex
rename to tests/data/hex-loader/test.hex
-- 
MST

[Qemu-devel] [PULL 03/33] configure: Rename FILES variable to LINKS

[Michael S. Tsirkin]

From: Peter Maydell <peter.maydell@linaro.org>

The FILES variable is used to accumulate a list of things to symlink
from the source tree into the build tree.  These don't have to be
individual files; symlinking an entire directory of data files is
also fine.  Rename it to something less confusing before we add a few
directories to it.

Improve the comment to clarify what DIRS and LINKS do and why
it's not a good idea to add things to LINKS with wildcarding.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/configure b/configure
index bfdca8b814..52408ed307 100755
--- a/configure
+++ b/configure
@@ -7392,22 +7392,31 @@ if test "$ccache_cpp2" = "yes"; then
   echo "export CCACHE_CPP2=y" >> $config_host_mak
 fi
 
-# build tree in object directory in case the source is not in the current directory
+# If we're using a separate build tree, set it up now.
+# DIRS are directories which we simply mkdir in the build tree;
+# LINKS are things to symlink back into the source tree
+# (these can be both files and directories).
+# Caution: do not add files or directories here using wildcards. This
+# will result in problems later if a new file matching the wildcard is
+# added to the source tree -- nothing will cause configure to be rerun
+# so the build tree will be missing the link back to the new file, and
+# tests might fail. Prefer to keep the relevant files in their own
+# directory and symlink the directory instead.
 DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm"
 DIRS="$DIRS tests/fp"
 DIRS="$DIRS docs docs/interop fsdev scsi"
 DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
 DIRS="$DIRS roms/seabios roms/vgabios"
-FILES="Makefile tests/tcg/Makefile qdict-test-data.txt"
-FILES="$FILES tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit"
-FILES="$FILES tests/tcg/lm32/Makefile tests/tcg/xtensa/Makefile po/Makefile"
-FILES="$FILES tests/fp/Makefile"
-FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps"
-FILES="$FILES pc-bios/spapr-rtas/Makefile"
-FILES="$FILES pc-bios/s390-ccw/Makefile"
-FILES="$FILES roms/seabios/Makefile roms/vgabios/Makefile"
-FILES="$FILES pc-bios/qemu-icon.bmp"
-FILES="$FILES .gdbinit scripts" # scripts needed by relative path in .gdbinit
+LINKS="Makefile tests/tcg/Makefile qdict-test-data.txt"
+LINKS="$LINKS tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit"
+LINKS="$LINKS tests/tcg/lm32/Makefile tests/tcg/xtensa/Makefile po/Makefile"
+LINKS="$LINKS tests/fp/Makefile"
+LINKS="$LINKS pc-bios/optionrom/Makefile pc-bios/keymaps"
+LINKS="$LINKS pc-bios/spapr-rtas/Makefile"
+LINKS="$LINKS pc-bios/s390-ccw/Makefile"
+LINKS="$LINKS roms/seabios/Makefile roms/vgabios/Makefile"
+LINKS="$LINKS pc-bios/qemu-icon.bmp"
+LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
 for bios_file in \
     $source_path/pc-bios/*.bin \
     $source_path/pc-bios/*.lid \
@@ -7419,10 +7428,10 @@ for bios_file in \
     $source_path/pc-bios/u-boot.* \
     $source_path/pc-bios/palcode-*
 do
-    FILES="$FILES pc-bios/$(basename $bios_file)"
+    LINKS="$LINKS pc-bios/$(basename $bios_file)"
 done
 mkdir -p $DIRS
-for f in $FILES ; do
+for f in $LINKS ; do
     if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; then
         symlink "$source_path/$f" "$f"
     fi
-- 
MST

[Qemu-devel] [PULL 04/33] configure: Use LINKS loop for all build tree symlinks

[Michael S. Tsirkin]

From: Peter Maydell <peter.maydell@linaro.org>

A few places in configure were doing ad-hoc calls to
the symlink function to set up symlinks from the build tree
back to the source tree. We have a loop that does this
already for all files and directories listed in the LINKS
environment variable; use that instead.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/configure b/configure
index 52408ed307..74e313a810 100755
--- a/configure
+++ b/configure
@@ -7417,6 +7417,8 @@ LINKS="$LINKS pc-bios/s390-ccw/Makefile"
 LINKS="$LINKS roms/seabios/Makefile roms/vgabios/Makefile"
 LINKS="$LINKS pc-bios/qemu-icon.bmp"
 LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
+LINKS="$LINKS tests/acceptance tests/data"
+LINKS="$LINKS tests/qemu-iotests/check"
 for bios_file in \
     $source_path/pc-bios/*.bin \
     $source_path/pc-bios/*.lid \
@@ -7453,25 +7455,13 @@ for rom in seabios vgabios ; do
     echo "RANLIB=$ranlib" >> $config_mak
 done
 
-# set up tests data directory
-for tests_subdir in acceptance data; do
-    if [ ! -e tests/$tests_subdir ]; then
-        symlink "$source_path/tests/$tests_subdir" tests/$tests_subdir
-    fi
-done
-
 # set up qemu-iotests in this build directory
 iotests_common_env="tests/qemu-iotests/common.env"
-iotests_check="tests/qemu-iotests/check"
 
 echo "# Automatically generated by configure - do not modify" > "$iotests_common_env"
 echo >> "$iotests_common_env"
 echo "export PYTHON='$python'" >> "$iotests_common_env"
 
-if [ ! -e "$iotests_check" ]; then
-    symlink "$source_path/$iotests_check" "$iotests_check"
-fi
-
 # Save the configure command line for later reuse.
 cat <<EOD >config.status
 #!/bin/sh
-- 
MST

[Qemu-devel] [PULL 05/33] 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 06/33] 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 07/33] 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 08/33] 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 09/33] 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 10/33] 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 11/33] 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 12/33] 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 13/33] 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 14/33] 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 15/33] 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 16/33] 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 17/33] 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 18/33] 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 19/33] 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  | 12 ++++++------
 hw/i386/acpi-build.c |  3 ++-
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 8061e9c49c..c52886f3ed 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 | \
@@ -242,11 +242,11 @@
 #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)
+#define AMDVI_DEV_INT_PASS_MASK         (1ULL << 56)
+#define AMDVI_DEV_EINT_PASS_MASK        (1ULL << 57)
+#define AMDVI_DEV_NMI_PASS_MASK         (1ULL << 58)
+#define AMDVI_DEV_LINT0_PASS_MASK       (1ULL << 62)
+#define AMDVI_DEV_LINT1_PASS_MASK       (1ULL << 63)
 
 /* Interrupt remapping table fields (Guest VAPIC not enabled) */
 union irte {
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 20/33] 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 cfabc14b59..c121a99fc0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1116,6 +1116,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 21/33] 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..1282d151cb 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;
+
+    uint64_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 22/33] 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 47293a3915..ce271da9b0 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 966a7cf92d..ceb00f7706 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 23/33] 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 ce271da9b0..ec5441583a 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 ceb00f7706..0c38a8dfd3 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 24/33] bios-tables-test: prepare expected files for mmio64

[Michael S. Tsirkin]

test will be added by follow-up patch.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/data/acpi/q35/DSDT.mmio64 | Bin 0 -> 8947 bytes
 tests/data/acpi/q35/SRAT.mmio64 | Bin 0 -> 224 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/data/acpi/q35/DSDT.mmio64
 create mode 100644 tests/data/acpi/q35/SRAT.mmio64

diff --git a/tests/data/acpi/q35/DSDT.mmio64 b/tests/data/acpi/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/data/acpi/q35/SRAT.mmio64 b/tests/data/acpi/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 25/33] 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.

Previous patch fixed the issue that 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.

This patch adds a test to make sure it won't happen again.

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 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index b14c8eaa17..d661d9be62 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);
-- 
MST

[Qemu-devel] [PULL 26/33] 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 27/33] 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 28/33] 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 29/33] 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 30/33] 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 ec5441583a..b5ded1a806 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 31/33] 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 b5ded1a806..761a8158d7 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 32/33] 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 761a8158d7..d9c70f7ce6 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 33/33] 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 05/33] virtio-blk: fix comment for virtio_blk_rw_complete

[Dongli Zhang]

On 11/06/2018 02:15 AM, Michael S. Tsirkin wrote:
> 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

Should it be "If nalloc is != -1" in the comment? Seems the initial state is -1.

> -             * 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);
>          }
>  
> 

Dongli Zhang

Re: [Qemu-devel] [PULL 05/33] virtio-blk: fix comment for virtio_blk_rw_complete

[Michael S. Tsirkin]

On Tue, Nov 06, 2018 at 11:17:03AM +0800, Dongli Zhang wrote:
> 
> 
> On 11/06/2018 02:15 AM, Michael S. Tsirkin wrote:
> > 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
> 
> Should it be "If nalloc is != -1" in the comment? Seems the initial state is -1.

Makes sense. Patch?

> > -             * 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);
> >          }
> >  
> > 
> 
> Dongli Zhang

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

[Thomas Huth]

On 2018-11-05 19:14, Michael S. Tsirkin wrote:
> The following changes since commit b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:
> 
>   Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' into staging (2018-11-05 10:32:49 +0000)
> 
> 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 6196df5c8e6688c1c3f06f73442820066335337c:
> 
>   vhost-scsi: prevent using uninitialized vqs (2018-11-05 12:59:35 -0500)
> 
> ----------------------------------------------------------------
> 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
[...]
>  tests/{acpi-test-data => data/acpi}/pc/APIC        | Bin
>  tests/{acpi-test-data => data/acpi}/pc/APIC.cphp   | Bin
>  .../{acpi-test-data => data/acpi}/pc/APIC.dimmpxm  | Bin
>  tests/{acpi-test-data => data/acpi}/pc/DSDT        | Bin

So patch 1 moves tests/acpi-test-data/ to tests/data/acpi/ and patch 20
adds an entry for tests/acpi-test-data/ ? Does not make much sense to me
... I think patch 20 needs to be adapted now.

 Thomas

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

[Michael S. Tsirkin]

On Tue, Nov 06, 2018 at 09:18:49AM +0100, Thomas Huth wrote:
> On 2018-11-05 19:14, Michael S. Tsirkin wrote:
> > The following changes since commit b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:
> > 
> >   Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' into staging (2018-11-05 10:32:49 +0000)
> > 
> > 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 6196df5c8e6688c1c3f06f73442820066335337c:
> > 
> >   vhost-scsi: prevent using uninitialized vqs (2018-11-05 12:59:35 -0500)
> > 
> > ----------------------------------------------------------------
> > 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
> [...]
> >  tests/{acpi-test-data => data/acpi}/pc/APIC        | Bin
> >  tests/{acpi-test-data => data/acpi}/pc/APIC.cphp   | Bin
> >  .../{acpi-test-data => data/acpi}/pc/APIC.dimmpxm  | Bin
> >  tests/{acpi-test-data => data/acpi}/pc/DSDT        | Bin
> 
> So patch 1 moves tests/acpi-test-data/ to tests/data/acpi/ and patch 20
> adds an entry for tests/acpi-test-data/ ? Does not make much sense to me
> ... I think patch 20 needs to be adapted now.
> 
>  Thomas

Oh right, MAINTAINERS needs to be fixed. Can be done with a patch on top
though.

-- 
MST

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

[Peter Maydell]

On 6 November 2018 at 11:07, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Nov 06, 2018 at 09:18:49AM +0100, Thomas Huth wrote:
>> On 2018-11-05 19:14, Michael S. Tsirkin wrote:
>> > The following changes since commit b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:
>> >
>> >   Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' into staging (2018-11-05 10:32:49 +0000)
>> >
>> > 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 6196df5c8e6688c1c3f06f73442820066335337c:
>> >
>> >   vhost-scsi: prevent using uninitialized vqs (2018-11-05 12:59:35 -0500)
>> >
>> > ----------------------------------------------------------------
>> > 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
>> [...]
>> >  tests/{acpi-test-data => data/acpi}/pc/APIC        | Bin
>> >  tests/{acpi-test-data => data/acpi}/pc/APIC.cphp   | Bin
>> >  .../{acpi-test-data => data/acpi}/pc/APIC.dimmpxm  | Bin
>> >  tests/{acpi-test-data => data/acpi}/pc/DSDT        | Bin
>>
>> So patch 1 moves tests/acpi-test-data/ to tests/data/acpi/ and patch 20
>> adds an entry for tests/acpi-test-data/ ? Does not make much sense to me
>> ... I think patch 20 needs to be adapted now.
>>
>>  Thomas
>
> Oh right, MAINTAINERS needs to be fixed. Can be done with a patch on top
> though.

Yeah, given the timing for rc0 I'll just apply this version of
the pullreq, and we can fix up MAINTAINERS afterwards.

thanks
-- PMM

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

[Peter Maydell]

On 6 November 2018 at 11:20, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 November 2018 at 11:07, Michael S. Tsirkin <mst@redhat.com> wrote:
>> On Tue, Nov 06, 2018 at 09:18:49AM +0100, Thomas Huth wrote:
>>> On 2018-11-05 19:14, Michael S. Tsirkin wrote:
>>> > The following changes since commit b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:
>>> >
>>> >   Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' into staging (2018-11-05 10:32:49 +0000)
>>> >
>>> > 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 6196df5c8e6688c1c3f06f73442820066335337c:
>>> >
>>> >   vhost-scsi: prevent using uninitialized vqs (2018-11-05 12:59:35 -0500)
>>> >
>>> > ----------------------------------------------------------------
>>> > 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
>>> [...]
>>> >  tests/{acpi-test-data => data/acpi}/pc/APIC        | Bin
>>> >  tests/{acpi-test-data => data/acpi}/pc/APIC.cphp   | Bin
>>> >  .../{acpi-test-data => data/acpi}/pc/APIC.dimmpxm  | Bin
>>> >  tests/{acpi-test-data => data/acpi}/pc/DSDT        | Bin
>>>
>>> So patch 1 moves tests/acpi-test-data/ to tests/data/acpi/ and patch 20
>>> adds an entry for tests/acpi-test-data/ ? Does not make much sense to me
>>> ... I think patch 20 needs to be adapted now.
>>>
>>>  Thomas
>>
>> Oh right, MAINTAINERS needs to be fixed. Can be done with a patch on top
>> though.
>
> Yeah, given the timing for rc0 I'll just apply this version of
> the pullreq, and we can fix up MAINTAINERS afterwards.

...applied.

thanks
-- PMM

Re: [Qemu-devel] [PULL 01/33] tests: Move tests/acpi-test-data/ to tests/data/acpi/

[Philippe Mathieu-Daudé]

On 5/11/18 19:14, Michael S. Tsirkin wrote:
> From: Peter Maydell <peter.maydell@linaro.org>
> 
> Currently tests/acpi-test-data contains data files used by the
> bios-tables-test, and configure individually symlinks those
> data files into the build directory using a wildcard.
> 
> Using a wildcard like this is a bad idea, because if a new
> data file is added, nothing causes configure to be rerun,
> and so no symlink is added for the new file. This can cause
> tests to spuriously fail when they can't find their data.
> Instead, it's better to symlink an entire directory of
> data files. We already have such a directory: tests/data.
> 
> Move the data files from tests/acpi-test-data/ to
> tests/data/acpi/, and remove the unnecessary symlinking.
> 
> We can remove entirely the note in rebuild-expected-aml.sh
> about copying any new data files, because now they will
> be in the source directory, not the build directory, and
> no copying is required.
> 
> (We can't just change the existing tests/acpi-test-data/
> to being a symlinked directory, because if we did that and
> a developer switched git branches from one after that change
> to one before it then configure would end up trashing all
> the test files by making them symlinks to themselves.
> Changing their path avoids this annoyance.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

I reviewed/tested this patch.

> ---
>   configure                                           |   4 ----
>   tests/bios-tables-test.c                            |   2 +-
>   tests/{acpi-test-data => data/acpi}/pc/APIC         | Bin
>   tests/{acpi-test-data => data/acpi}/pc/APIC.cphp    | Bin
>   tests/{acpi-test-data => data/acpi}/pc/APIC.dimmpxm | Bin
>   tests/{acpi-test-data => data/acpi}/pc/DSDT         | Bin
>   tests/{acpi-test-data => data/acpi}/pc/DSDT.bridge  | Bin
>   tests/{acpi-test-data => data/acpi}/pc/DSDT.cphp    | Bin
>   tests/{acpi-test-data => data/acpi}/pc/DSDT.dimmpxm | Bin
>   tests/{acpi-test-data => data/acpi}/pc/DSDT.ipmikcs | Bin
>   tests/{acpi-test-data => data/acpi}/pc/DSDT.memhp   | Bin
>   tests/{acpi-test-data => data/acpi}/pc/DSDT.numamem | Bin
>   tests/{acpi-test-data => data/acpi}/pc/FACP         | Bin
>   tests/{acpi-test-data => data/acpi}/pc/FACS         | Bin
>   tests/{acpi-test-data => data/acpi}/pc/HPET         | Bin
>   tests/{acpi-test-data => data/acpi}/pc/NFIT.dimmpxm | Bin
>   tests/{acpi-test-data => data/acpi}/pc/SLIT.cphp    | Bin
>   tests/{acpi-test-data => data/acpi}/pc/SLIT.memhp   | Bin
>   tests/{acpi-test-data => data/acpi}/pc/SRAT.cphp    | Bin
>   tests/{acpi-test-data => data/acpi}/pc/SRAT.dimmpxm | Bin
>   tests/{acpi-test-data => data/acpi}/pc/SRAT.memhp   | Bin
>   tests/{acpi-test-data => data/acpi}/pc/SRAT.numamem | Bin
>   tests/{acpi-test-data => data/acpi}/pc/SSDT.dimmpxm | Bin
>   tests/{acpi-test-data => data/acpi}/q35/APIC        | Bin
>   tests/{acpi-test-data => data/acpi}/q35/APIC.cphp   | Bin
>   .../{acpi-test-data => data/acpi}/q35/APIC.dimmpxm  | Bin
>   tests/{acpi-test-data => data/acpi}/q35/DSDT        | Bin
>   tests/{acpi-test-data => data/acpi}/q35/DSDT.bridge | Bin
>   tests/{acpi-test-data => data/acpi}/q35/DSDT.cphp   | Bin
>   .../{acpi-test-data => data/acpi}/q35/DSDT.dimmpxm  | Bin
>   tests/{acpi-test-data => data/acpi}/q35/DSDT.ipmibt | Bin
>   tests/{acpi-test-data => data/acpi}/q35/DSDT.memhp  | Bin
>   .../{acpi-test-data => data/acpi}/q35/DSDT.numamem  | Bin
>   tests/{acpi-test-data => data/acpi}/q35/FACP        | Bin
>   tests/{acpi-test-data => data/acpi}/q35/FACS        | Bin
>   tests/{acpi-test-data => data/acpi}/q35/HPET        | Bin
>   tests/{acpi-test-data => data/acpi}/q35/MCFG        | Bin
>   .../{acpi-test-data => data/acpi}/q35/NFIT.dimmpxm  | Bin
>   tests/{acpi-test-data => data/acpi}/q35/SLIT.cphp   | Bin
>   tests/{acpi-test-data => data/acpi}/q35/SLIT.memhp  | Bin
>   tests/{acpi-test-data => data/acpi}/q35/SRAT.cphp   | Bin
>   .../{acpi-test-data => data/acpi}/q35/SRAT.dimmpxm  | Bin
>   tests/{acpi-test-data => data/acpi}/q35/SRAT.memhp  | Bin
>   .../{acpi-test-data => data/acpi}/q35/SRAT.numamem  | Bin
>   .../{acpi-test-data => data/acpi}/q35/SSDT.dimmpxm  | Bin
>   .../acpi}/rebuild-expected-aml.sh                   |   2 --
>   46 files changed, 1 insertion(+), 7 deletions(-)
>   rename tests/{acpi-test-data => data/acpi}/pc/APIC (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/APIC.cphp (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/APIC.dimmpxm (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/DSDT (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/DSDT.bridge (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/DSDT.cphp (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/DSDT.dimmpxm (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/DSDT.ipmikcs (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/DSDT.memhp (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/DSDT.numamem (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/FACP (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/FACS (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/HPET (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/NFIT.dimmpxm (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/SLIT.cphp (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/SLIT.memhp (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/SRAT.cphp (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/SRAT.dimmpxm (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/SRAT.memhp (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/SRAT.numamem (100%)
>   rename tests/{acpi-test-data => data/acpi}/pc/SSDT.dimmpxm (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/APIC (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/APIC.cphp (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/APIC.dimmpxm (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/DSDT (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/DSDT.bridge (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/DSDT.cphp (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/DSDT.dimmpxm (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/DSDT.ipmibt (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/DSDT.memhp (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/DSDT.numamem (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/FACP (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/FACS (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/HPET (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/MCFG (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/NFIT.dimmpxm (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/SLIT.cphp (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/SLIT.memhp (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/SRAT.cphp (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/SRAT.dimmpxm (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/SRAT.memhp (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/SRAT.numamem (100%)
>   rename tests/{acpi-test-data => data/acpi}/q35/SSDT.dimmpxm (100%)
>   rename tests/{acpi-test-data => data/acpi}/rebuild-expected-aml.sh (86%)
> 
> diff --git a/configure b/configure
> index 46ae1e8c76..895b7483b8 100755
> --- a/configure
> +++ b/configure
> @@ -7421,10 +7421,6 @@ for bios_file in \
>   do
>       FILES="$FILES pc-bios/$(basename $bios_file)"
>   done
> -for test_file in $(find $source_path/tests/acpi-test-data -type f)
> -do
> -    FILES="$FILES tests/acpi-test-data$(echo $test_file | sed -e 's/.*acpi-test-data//')"
> -done
>   for test_file in $(find $source_path/tests/hex-loader-check-data -type f)
>   do
>       FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed -e 's/.*hex-loader-check-data//')"
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 02e77ec811..b14c8eaa17 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -42,7 +42,7 @@ typedef struct {
>   } test_data;
>   
>   static char disk[] = "tests/acpi-test-disk-XXXXXX";
> -static const char *data_dir = "tests/acpi-test-data";
> +static const char *data_dir = "tests/data/acpi";
>   #ifdef CONFIG_IASL
>   static const char *iasl = stringify(CONFIG_IASL);
>   #else
> diff --git a/tests/acpi-test-data/pc/APIC b/tests/data/acpi/pc/APIC
> similarity index 100%
> rename from tests/acpi-test-data/pc/APIC
> rename to tests/data/acpi/pc/APIC
> diff --git a/tests/acpi-test-data/pc/APIC.cphp b/tests/data/acpi/pc/APIC.cphp
> similarity index 100%
> rename from tests/acpi-test-data/pc/APIC.cphp
> rename to tests/data/acpi/pc/APIC.cphp
> diff --git a/tests/acpi-test-data/pc/APIC.dimmpxm b/tests/data/acpi/pc/APIC.dimmpxm
> similarity index 100%
> rename from tests/acpi-test-data/pc/APIC.dimmpxm
> rename to tests/data/acpi/pc/APIC.dimmpxm
> diff --git a/tests/acpi-test-data/pc/DSDT b/tests/data/acpi/pc/DSDT
> similarity index 100%
> rename from tests/acpi-test-data/pc/DSDT
> rename to tests/data/acpi/pc/DSDT
> diff --git a/tests/acpi-test-data/pc/DSDT.bridge b/tests/data/acpi/pc/DSDT.bridge
> similarity index 100%
> rename from tests/acpi-test-data/pc/DSDT.bridge
> rename to tests/data/acpi/pc/DSDT.bridge
> diff --git a/tests/acpi-test-data/pc/DSDT.cphp b/tests/data/acpi/pc/DSDT.cphp
> similarity index 100%
> rename from tests/acpi-test-data/pc/DSDT.cphp
> rename to tests/data/acpi/pc/DSDT.cphp
> diff --git a/tests/acpi-test-data/pc/DSDT.dimmpxm b/tests/data/acpi/pc/DSDT.dimmpxm
> similarity index 100%
> rename from tests/acpi-test-data/pc/DSDT.dimmpxm
> rename to tests/data/acpi/pc/DSDT.dimmpxm
> diff --git a/tests/acpi-test-data/pc/DSDT.ipmikcs b/tests/data/acpi/pc/DSDT.ipmikcs
> similarity index 100%
> rename from tests/acpi-test-data/pc/DSDT.ipmikcs
> rename to tests/data/acpi/pc/DSDT.ipmikcs
> diff --git a/tests/acpi-test-data/pc/DSDT.memhp b/tests/data/acpi/pc/DSDT.memhp
> similarity index 100%
> rename from tests/acpi-test-data/pc/DSDT.memhp
> rename to tests/data/acpi/pc/DSDT.memhp
> diff --git a/tests/acpi-test-data/pc/DSDT.numamem b/tests/data/acpi/pc/DSDT.numamem
> similarity index 100%
> rename from tests/acpi-test-data/pc/DSDT.numamem
> rename to tests/data/acpi/pc/DSDT.numamem
> diff --git a/tests/acpi-test-data/pc/FACP b/tests/data/acpi/pc/FACP
> similarity index 100%
> rename from tests/acpi-test-data/pc/FACP
> rename to tests/data/acpi/pc/FACP
> diff --git a/tests/acpi-test-data/pc/FACS b/tests/data/acpi/pc/FACS
> similarity index 100%
> rename from tests/acpi-test-data/pc/FACS
> rename to tests/data/acpi/pc/FACS
> diff --git a/tests/acpi-test-data/pc/HPET b/tests/data/acpi/pc/HPET
> similarity index 100%
> rename from tests/acpi-test-data/pc/HPET
> rename to tests/data/acpi/pc/HPET
> diff --git a/tests/acpi-test-data/pc/NFIT.dimmpxm b/tests/data/acpi/pc/NFIT.dimmpxm
> similarity index 100%
> rename from tests/acpi-test-data/pc/NFIT.dimmpxm
> rename to tests/data/acpi/pc/NFIT.dimmpxm
> diff --git a/tests/acpi-test-data/pc/SLIT.cphp b/tests/data/acpi/pc/SLIT.cphp
> similarity index 100%
> rename from tests/acpi-test-data/pc/SLIT.cphp
> rename to tests/data/acpi/pc/SLIT.cphp
> diff --git a/tests/acpi-test-data/pc/SLIT.memhp b/tests/data/acpi/pc/SLIT.memhp
> similarity index 100%
> rename from tests/acpi-test-data/pc/SLIT.memhp
> rename to tests/data/acpi/pc/SLIT.memhp
> diff --git a/tests/acpi-test-data/pc/SRAT.cphp b/tests/data/acpi/pc/SRAT.cphp
> similarity index 100%
> rename from tests/acpi-test-data/pc/SRAT.cphp
> rename to tests/data/acpi/pc/SRAT.cphp
> diff --git a/tests/acpi-test-data/pc/SRAT.dimmpxm b/tests/data/acpi/pc/SRAT.dimmpxm
> similarity index 100%
> rename from tests/acpi-test-data/pc/SRAT.dimmpxm
> rename to tests/data/acpi/pc/SRAT.dimmpxm
> diff --git a/tests/acpi-test-data/pc/SRAT.memhp b/tests/data/acpi/pc/SRAT.memhp
> similarity index 100%
> rename from tests/acpi-test-data/pc/SRAT.memhp
> rename to tests/data/acpi/pc/SRAT.memhp
> diff --git a/tests/acpi-test-data/pc/SRAT.numamem b/tests/data/acpi/pc/SRAT.numamem
> similarity index 100%
> rename from tests/acpi-test-data/pc/SRAT.numamem
> rename to tests/data/acpi/pc/SRAT.numamem
> diff --git a/tests/acpi-test-data/pc/SSDT.dimmpxm b/tests/data/acpi/pc/SSDT.dimmpxm
> similarity index 100%
> rename from tests/acpi-test-data/pc/SSDT.dimmpxm
> rename to tests/data/acpi/pc/SSDT.dimmpxm
> diff --git a/tests/acpi-test-data/q35/APIC b/tests/data/acpi/q35/APIC
> similarity index 100%
> rename from tests/acpi-test-data/q35/APIC
> rename to tests/data/acpi/q35/APIC
> diff --git a/tests/acpi-test-data/q35/APIC.cphp b/tests/data/acpi/q35/APIC.cphp
> similarity index 100%
> rename from tests/acpi-test-data/q35/APIC.cphp
> rename to tests/data/acpi/q35/APIC.cphp
> diff --git a/tests/acpi-test-data/q35/APIC.dimmpxm b/tests/data/acpi/q35/APIC.dimmpxm
> similarity index 100%
> rename from tests/acpi-test-data/q35/APIC.dimmpxm
> rename to tests/data/acpi/q35/APIC.dimmpxm
> diff --git a/tests/acpi-test-data/q35/DSDT b/tests/data/acpi/q35/DSDT
> similarity index 100%
> rename from tests/acpi-test-data/q35/DSDT
> rename to tests/data/acpi/q35/DSDT
> diff --git a/tests/acpi-test-data/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge
> similarity index 100%
> rename from tests/acpi-test-data/q35/DSDT.bridge
> rename to tests/data/acpi/q35/DSDT.bridge
> diff --git a/tests/acpi-test-data/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp
> similarity index 100%
> rename from tests/acpi-test-data/q35/DSDT.cphp
> rename to tests/data/acpi/q35/DSDT.cphp
> diff --git a/tests/acpi-test-data/q35/DSDT.dimmpxm b/tests/data/acpi/q35/DSDT.dimmpxm
> similarity index 100%
> rename from tests/acpi-test-data/q35/DSDT.dimmpxm
> rename to tests/data/acpi/q35/DSDT.dimmpxm
> diff --git a/tests/acpi-test-data/q35/DSDT.ipmibt b/tests/data/acpi/q35/DSDT.ipmibt
> similarity index 100%
> rename from tests/acpi-test-data/q35/DSDT.ipmibt
> rename to tests/data/acpi/q35/DSDT.ipmibt
> diff --git a/tests/acpi-test-data/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp
> similarity index 100%
> rename from tests/acpi-test-data/q35/DSDT.memhp
> rename to tests/data/acpi/q35/DSDT.memhp
> diff --git a/tests/acpi-test-data/q35/DSDT.numamem b/tests/data/acpi/q35/DSDT.numamem
> similarity index 100%
> rename from tests/acpi-test-data/q35/DSDT.numamem
> rename to tests/data/acpi/q35/DSDT.numamem
> diff --git a/tests/acpi-test-data/q35/FACP b/tests/data/acpi/q35/FACP
> similarity index 100%
> rename from tests/acpi-test-data/q35/FACP
> rename to tests/data/acpi/q35/FACP
> diff --git a/tests/acpi-test-data/q35/FACS b/tests/data/acpi/q35/FACS
> similarity index 100%
> rename from tests/acpi-test-data/q35/FACS
> rename to tests/data/acpi/q35/FACS
> diff --git a/tests/acpi-test-data/q35/HPET b/tests/data/acpi/q35/HPET
> similarity index 100%
> rename from tests/acpi-test-data/q35/HPET
> rename to tests/data/acpi/q35/HPET
> diff --git a/tests/acpi-test-data/q35/MCFG b/tests/data/acpi/q35/MCFG
> similarity index 100%
> rename from tests/acpi-test-data/q35/MCFG
> rename to tests/data/acpi/q35/MCFG
> diff --git a/tests/acpi-test-data/q35/NFIT.dimmpxm b/tests/data/acpi/q35/NFIT.dimmpxm
> similarity index 100%
> rename from tests/acpi-test-data/q35/NFIT.dimmpxm
> rename to tests/data/acpi/q35/NFIT.dimmpxm
> diff --git a/tests/acpi-test-data/q35/SLIT.cphp b/tests/data/acpi/q35/SLIT.cphp
> similarity index 100%
> rename from tests/acpi-test-data/q35/SLIT.cphp
> rename to tests/data/acpi/q35/SLIT.cphp
> diff --git a/tests/acpi-test-data/q35/SLIT.memhp b/tests/data/acpi/q35/SLIT.memhp
> similarity index 100%
> rename from tests/acpi-test-data/q35/SLIT.memhp
> rename to tests/data/acpi/q35/SLIT.memhp
> diff --git a/tests/acpi-test-data/q35/SRAT.cphp b/tests/data/acpi/q35/SRAT.cphp
> similarity index 100%
> rename from tests/acpi-test-data/q35/SRAT.cphp
> rename to tests/data/acpi/q35/SRAT.cphp
> diff --git a/tests/acpi-test-data/q35/SRAT.dimmpxm b/tests/data/acpi/q35/SRAT.dimmpxm
> similarity index 100%
> rename from tests/acpi-test-data/q35/SRAT.dimmpxm
> rename to tests/data/acpi/q35/SRAT.dimmpxm
> diff --git a/tests/acpi-test-data/q35/SRAT.memhp b/tests/data/acpi/q35/SRAT.memhp
> similarity index 100%
> rename from tests/acpi-test-data/q35/SRAT.memhp
> rename to tests/data/acpi/q35/SRAT.memhp
> diff --git a/tests/acpi-test-data/q35/SRAT.numamem b/tests/data/acpi/q35/SRAT.numamem
> similarity index 100%
> rename from tests/acpi-test-data/q35/SRAT.numamem
> rename to tests/data/acpi/q35/SRAT.numamem
> diff --git a/tests/acpi-test-data/q35/SSDT.dimmpxm b/tests/data/acpi/q35/SSDT.dimmpxm
> similarity index 100%
> rename from tests/acpi-test-data/q35/SSDT.dimmpxm
> rename to tests/data/acpi/q35/SSDT.dimmpxm
> diff --git a/tests/acpi-test-data/rebuild-expected-aml.sh b/tests/data/acpi/rebuild-expected-aml.sh
> similarity index 86%
> rename from tests/acpi-test-data/rebuild-expected-aml.sh
> rename to tests/data/acpi/rebuild-expected-aml.sh
> index 11bf743914..bf9ba242ad 100755
> --- a/tests/acpi-test-data/rebuild-expected-aml.sh
> +++ b/tests/data/acpi/rebuild-expected-aml.sh
> @@ -32,5 +32,3 @@ fi
>   TEST_ACPI_REBUILD_AML=y QTEST_QEMU_BINARY=$qemu tests/bios-tables-test
>   
>   echo "The files were rebuilt and can be added to git."
> -echo "However, if new files were created, please copy them manually" \
> -     "to tests/acpi-test-data/pc/ or tests/acpi-test-data/q35/ ."
> 

Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Philippe Mathieu-Daudé]

On 5/11/18 19:14, Michael S. Tsirkin wrote:
> From: Peter Maydell <peter.maydell@linaro.org>
> 
> Currently tests/hex-loader-check-data contains data files used
> by the hexloader-test, and configure individually symlinks those
> data files into the build directory using a wildcard.
> 
> Using a wildcard like this is a bad idea, because if a new
> data file is added, nothing causes configure to be rerun,
> and so no symlink is added for the new file. This can cause
> tests to spuriously fail when they can't find their data.
> Instead, it's better to symlink an entire directory of
> data files. We already have such a directory: tests/data.
> 
> Move the data files from tests/hex-loader-check-data/ to
> tests/data/hex-loader/, and remove the unnecessary symlinking.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

I reviewed/tested this patch too.

> ---
>   configure                                                 | 4 ----
>   tests/hexloader-test.c                                    | 2 +-
>   MAINTAINERS                                               | 2 +-
>   tests/{hex-loader-check-data => data/hex-loader}/test.hex | 0
>   4 files changed, 2 insertions(+), 6 deletions(-)
>   rename tests/{hex-loader-check-data => data/hex-loader}/test.hex (100%)
> 
> diff --git a/configure b/configure
> index 895b7483b8..bfdca8b814 100755
> --- a/configure
> +++ b/configure
> @@ -7421,10 +7421,6 @@ for bios_file in \
>   do
>       FILES="$FILES pc-bios/$(basename $bios_file)"
>   done
> -for test_file in $(find $source_path/tests/hex-loader-check-data -type f)
> -do
> -    FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed -e 's/.*hex-loader-check-data//')"
> -done
>   mkdir -p $DIRS
>   for f in $FILES ; do
>       if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; then
> diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
> index b653d44ba1..834ed52c22 100644
> --- a/tests/hexloader-test.c
> +++ b/tests/hexloader-test.c
> @@ -23,7 +23,7 @@ static void hex_loader_test(void)
>       const unsigned int base_addr = 0x00010000;
>   
>       QTestState *s = qtest_initf(
> -        "-M vexpress-a9 -nographic -device loader,file=tests/hex-loader-check-data/test.hex");
> +        "-M vexpress-a9 -nographic -device loader,file=tests/data/hex-loader/test.hex");
>   
>       for (i = 0; i < 256; ++i) {
>           uint8_t val = qtest_readb(s, base_addr + i);
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 98a1856afc..cfabc14b59 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1370,7 +1370,7 @@ Intel Hexadecimal Object File Loader
>   M: Su Hang <suhang16@mails.ucas.ac.cn>
>   S: Maintained
>   F: tests/hexloader-test.c
> -F: tests/hex-loader-check-data/test.hex
> +F: tests/data/hex-loader/test.hex
>   
>   CHRP NVRAM
>   M: Thomas Huth <thuth@redhat.com>
> diff --git a/tests/hex-loader-check-data/test.hex b/tests/data/hex-loader/test.hex
> similarity index 100%
> rename from tests/hex-loader-check-data/test.hex
> rename to tests/data/hex-loader/test.hex
> 

Re: [Qemu-devel] [PULL 03/33] configure: Rename FILES variable to LINKS

[Philippe Mathieu-Daudé]

On 5/11/18 19:14, Michael S. Tsirkin wrote:
> From: Peter Maydell <peter.maydell@linaro.org>
> 
> The FILES variable is used to accumulate a list of things to symlink
> from the source tree into the build tree.  These don't have to be
> individual files; symlinking an entire directory of data files is
> also fine.  Rename it to something less confusing before we add a few
> directories to it.
> 
> Improve the comment to clarify what DIRS and LINKS do and why
> it's not a good idea to add things to LINKS with wildcarding.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Again, I reviewed/tested this patch.

> ---
>   configure | 35 ++++++++++++++++++++++-------------
>   1 file changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/configure b/configure
> index bfdca8b814..52408ed307 100755
> --- a/configure
> +++ b/configure
> @@ -7392,22 +7392,31 @@ if test "$ccache_cpp2" = "yes"; then
>     echo "export CCACHE_CPP2=y" >> $config_host_mak
>   fi
>   
> -# build tree in object directory in case the source is not in the current directory
> +# If we're using a separate build tree, set it up now.
> +# DIRS are directories which we simply mkdir in the build tree;
> +# LINKS are things to symlink back into the source tree
> +# (these can be both files and directories).
> +# Caution: do not add files or directories here using wildcards. This
> +# will result in problems later if a new file matching the wildcard is
> +# added to the source tree -- nothing will cause configure to be rerun
> +# so the build tree will be missing the link back to the new file, and
> +# tests might fail. Prefer to keep the relevant files in their own
> +# directory and symlink the directory instead.
>   DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm"
>   DIRS="$DIRS tests/fp"
>   DIRS="$DIRS docs docs/interop fsdev scsi"
>   DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
>   DIRS="$DIRS roms/seabios roms/vgabios"
> -FILES="Makefile tests/tcg/Makefile qdict-test-data.txt"
> -FILES="$FILES tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit"
> -FILES="$FILES tests/tcg/lm32/Makefile tests/tcg/xtensa/Makefile po/Makefile"
> -FILES="$FILES tests/fp/Makefile"
> -FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps"
> -FILES="$FILES pc-bios/spapr-rtas/Makefile"
> -FILES="$FILES pc-bios/s390-ccw/Makefile"
> -FILES="$FILES roms/seabios/Makefile roms/vgabios/Makefile"
> -FILES="$FILES pc-bios/qemu-icon.bmp"
> -FILES="$FILES .gdbinit scripts" # scripts needed by relative path in .gdbinit
> +LINKS="Makefile tests/tcg/Makefile qdict-test-data.txt"
> +LINKS="$LINKS tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit"
> +LINKS="$LINKS tests/tcg/lm32/Makefile tests/tcg/xtensa/Makefile po/Makefile"
> +LINKS="$LINKS tests/fp/Makefile"
> +LINKS="$LINKS pc-bios/optionrom/Makefile pc-bios/keymaps"
> +LINKS="$LINKS pc-bios/spapr-rtas/Makefile"
> +LINKS="$LINKS pc-bios/s390-ccw/Makefile"
> +LINKS="$LINKS roms/seabios/Makefile roms/vgabios/Makefile"
> +LINKS="$LINKS pc-bios/qemu-icon.bmp"
> +LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
>   for bios_file in \
>       $source_path/pc-bios/*.bin \
>       $source_path/pc-bios/*.lid \
> @@ -7419,10 +7428,10 @@ for bios_file in \
>       $source_path/pc-bios/u-boot.* \
>       $source_path/pc-bios/palcode-*
>   do
> -    FILES="$FILES pc-bios/$(basename $bios_file)"
> +    LINKS="$LINKS pc-bios/$(basename $bios_file)"
>   done
>   mkdir -p $DIRS
> -for f in $FILES ; do
> +for f in $LINKS ; do
>       if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; then
>           symlink "$source_path/$f" "$f"
>       fi
> 

Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Michael S. Tsirkin]

On Tue, Nov 06, 2018 at 02:27:18PM +0100, Philippe Mathieu-Daudé wrote:
> On 5/11/18 19:14, Michael S. Tsirkin wrote:
> > From: Peter Maydell <peter.maydell@linaro.org>
> > 
> > Currently tests/hex-loader-check-data contains data files used
> > by the hexloader-test, and configure individually symlinks those
> > data files into the build directory using a wildcard.
> > 
> > Using a wildcard like this is a bad idea, because if a new
> > data file is added, nothing causes configure to be rerun,
> > and so no symlink is added for the new file. This can cause
> > tests to spuriously fail when they can't find their data.
> > Instead, it's better to symlink an entire directory of
> > data files. We already have such a directory: tests/data.
> > 
> > Move the data files from tests/hex-loader-check-data/ to
> > tests/data/hex-loader/, and remove the unnecessary symlinking.
> > 
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> 
> I reviewed/tested this patch too.


Thanks a lot Philippe!
It is unfortunately too late to update this patch info in git
commit history, however your help is still greatly appreciated!


> > ---
> >   configure                                                 | 4 ----
> >   tests/hexloader-test.c                                    | 2 +-
> >   MAINTAINERS                                               | 2 +-
> >   tests/{hex-loader-check-data => data/hex-loader}/test.hex | 0
> >   4 files changed, 2 insertions(+), 6 deletions(-)
> >   rename tests/{hex-loader-check-data => data/hex-loader}/test.hex (100%)
> > 
> > diff --git a/configure b/configure
> > index 895b7483b8..bfdca8b814 100755
> > --- a/configure
> > +++ b/configure
> > @@ -7421,10 +7421,6 @@ for bios_file in \
> >   do
> >       FILES="$FILES pc-bios/$(basename $bios_file)"
> >   done
> > -for test_file in $(find $source_path/tests/hex-loader-check-data -type f)
> > -do
> > -    FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed -e 's/.*hex-loader-check-data//')"
> > -done
> >   mkdir -p $DIRS
> >   for f in $FILES ; do
> >       if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; then
> > diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
> > index b653d44ba1..834ed52c22 100644
> > --- a/tests/hexloader-test.c
> > +++ b/tests/hexloader-test.c
> > @@ -23,7 +23,7 @@ static void hex_loader_test(void)
> >       const unsigned int base_addr = 0x00010000;
> >       QTestState *s = qtest_initf(
> > -        "-M vexpress-a9 -nographic -device loader,file=tests/hex-loader-check-data/test.hex");
> > +        "-M vexpress-a9 -nographic -device loader,file=tests/data/hex-loader/test.hex");
> >       for (i = 0; i < 256; ++i) {
> >           uint8_t val = qtest_readb(s, base_addr + i);
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 98a1856afc..cfabc14b59 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1370,7 +1370,7 @@ Intel Hexadecimal Object File Loader
> >   M: Su Hang <suhang16@mails.ucas.ac.cn>
> >   S: Maintained
> >   F: tests/hexloader-test.c
> > -F: tests/hex-loader-check-data/test.hex
> > +F: tests/data/hex-loader/test.hex
> >   CHRP NVRAM
> >   M: Thomas Huth <thuth@redhat.com>
> > diff --git a/tests/hex-loader-check-data/test.hex b/tests/data/hex-loader/test.hex
> > similarity index 100%
> > rename from tests/hex-loader-check-data/test.hex
> > rename to tests/data/hex-loader/test.hex
> > 

Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Philippe Mathieu-Daudé]

On 6/11/18 15:13, Michael S. Tsirkin wrote:
> On Tue, Nov 06, 2018 at 02:27:18PM +0100, Philippe Mathieu-Daudé wrote:
>> On 5/11/18 19:14, Michael S. Tsirkin wrote:
>>> From: Peter Maydell <peter.maydell@linaro.org>
>>>
>>> Currently tests/hex-loader-check-data contains data files used
>>> by the hexloader-test, and configure individually symlinks those
>>> data files into the build directory using a wildcard.
>>>
>>> Using a wildcard like this is a bad idea, because if a new
>>> data file is added, nothing causes configure to be rerun,
>>> and so no symlink is added for the new file. This can cause
>>> tests to spuriously fail when they can't find their data.
>>> Instead, it's better to symlink an entire directory of
>>> data files. We already have such a directory: tests/data.
>>>
>>> Move the data files from tests/hex-loader-check-data/ to
>>> tests/data/hex-loader/, and remove the unnecessary symlinking.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>
>> I reviewed/tested this patch too.
> 
> 
> Thanks a lot Philippe!
> It is unfortunately too late to update this patch info in git
> commit history, however your help is still greatly appreciated!

No worry, I'm not mad at all, but there might be an issue in your git PR 
workflow, this series also missed your maintainer S-o-b.

Peter: Can you add a such check in your scripts? (during next merge 
window, no hurry).

Rather than your scripts, this should be in scripts a maintainer can run 
locally, such ./scripts/checkpatch.pl --maintainer or 
./scripts/checkseries.xx.

> 
> 
>>> ---
>>>    configure                                                 | 4 ----
>>>    tests/hexloader-test.c                                    | 2 +-
>>>    MAINTAINERS                                               | 2 +-
>>>    tests/{hex-loader-check-data => data/hex-loader}/test.hex | 0
>>>    4 files changed, 2 insertions(+), 6 deletions(-)
>>>    rename tests/{hex-loader-check-data => data/hex-loader}/test.hex (100%)
>>>
>>> diff --git a/configure b/configure
>>> index 895b7483b8..bfdca8b814 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -7421,10 +7421,6 @@ for bios_file in \
>>>    do
>>>        FILES="$FILES pc-bios/$(basename $bios_file)"
>>>    done
>>> -for test_file in $(find $source_path/tests/hex-loader-check-data -type f)
>>> -do
>>> -    FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed -e 's/.*hex-loader-check-data//')"
>>> -done
>>>    mkdir -p $DIRS
>>>    for f in $FILES ; do
>>>        if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; then
>>> diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
>>> index b653d44ba1..834ed52c22 100644
>>> --- a/tests/hexloader-test.c
>>> +++ b/tests/hexloader-test.c
>>> @@ -23,7 +23,7 @@ static void hex_loader_test(void)
>>>        const unsigned int base_addr = 0x00010000;
>>>        QTestState *s = qtest_initf(
>>> -        "-M vexpress-a9 -nographic -device loader,file=tests/hex-loader-check-data/test.hex");
>>> +        "-M vexpress-a9 -nographic -device loader,file=tests/data/hex-loader/test.hex");
>>>        for (i = 0; i < 256; ++i) {
>>>            uint8_t val = qtest_readb(s, base_addr + i);
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 98a1856afc..cfabc14b59 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -1370,7 +1370,7 @@ Intel Hexadecimal Object File Loader
>>>    M: Su Hang <suhang16@mails.ucas.ac.cn>
>>>    S: Maintained
>>>    F: tests/hexloader-test.c
>>> -F: tests/hex-loader-check-data/test.hex
>>> +F: tests/data/hex-loader/test.hex
>>>    CHRP NVRAM
>>>    M: Thomas Huth <thuth@redhat.com>
>>> diff --git a/tests/hex-loader-check-data/test.hex b/tests/data/hex-loader/test.hex
>>> similarity index 100%
>>> rename from tests/hex-loader-check-data/test.hex
>>> rename to tests/data/hex-loader/test.hex
>>>

Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Peter Maydell]

On 6 November 2018 at 15:15, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> No worry, I'm not mad at all, but there might be an issue in your git PR
> workflow, this series also missed your maintainer S-o-b.
>
> Peter: Can you add a such check in your scripts? (during next merge window,
> no hurry).
>
> Rather than your scripts, this should be in scripts a maintainer can run
> locally, such ./scripts/checkpatch.pl --maintainer or
> ./scripts/checkseries.xx.

I have such a check in my own make-pullreq script which I use
for arm pull requests:
https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/make-pullreq#n98

I can't do much about other submaintainers' workflows
(and the block tree's "sub-submaintainers" setup means
"check all commits have a signoff from the same person as
the merge commit did" won't work either.)

thanks
-- PMM

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

[Laszlo Ersek]

On 11/06/18 13:39, Peter Maydell wrote:
> On 6 November 2018 at 11:20, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 6 November 2018 at 11:07, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> On Tue, Nov 06, 2018 at 09:18:49AM +0100, Thomas Huth wrote:
>>>> On 2018-11-05 19:14, Michael S. Tsirkin wrote:
>>>>> The following changes since commit b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:
>>>>>
>>>>>   Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' into staging (2018-11-05 10:32:49 +0000)
>>>>>
>>>>> 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 6196df5c8e6688c1c3f06f73442820066335337c:
>>>>>
>>>>>   vhost-scsi: prevent using uninitialized vqs (2018-11-05 12:59:35 -0500)
>>>>>
>>>>> ----------------------------------------------------------------
>>>>> 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
>>>> [...]
>>>>>  tests/{acpi-test-data => data/acpi}/pc/APIC        | Bin
>>>>>  tests/{acpi-test-data => data/acpi}/pc/APIC.cphp   | Bin
>>>>>  .../{acpi-test-data => data/acpi}/pc/APIC.dimmpxm  | Bin
>>>>>  tests/{acpi-test-data => data/acpi}/pc/DSDT        | Bin
>>>>
>>>> So patch 1 moves tests/acpi-test-data/ to tests/data/acpi/ and patch 20
>>>> adds an entry for tests/acpi-test-data/ ? Does not make much sense to me
>>>> ... I think patch 20 needs to be adapted now.
>>>>
>>>>  Thomas
>>>
>>> Oh right, MAINTAINERS needs to be fixed. Can be done with a patch on top
>>> though.
>>
>> Yeah, given the timing for rc0 I'll just apply this version of
>> the pullreq, and we can fix up MAINTAINERS afterwards.
> 
> ...applied.

Thanks!
Laszlo

Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Michael S. Tsirkin]

On Tue, Nov 06, 2018 at 03:31:08PM +0000, Peter Maydell wrote:
> On 6 November 2018 at 15:15, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> > No worry, I'm not mad at all, but there might be an issue in your git PR
> > workflow, this series also missed your maintainer S-o-b.
> >
> > Peter: Can you add a such check in your scripts? (during next merge window,
> > no hurry).
> >
> > Rather than your scripts, this should be in scripts a maintainer can run
> > locally, such ./scripts/checkpatch.pl --maintainer or
> > ./scripts/checkseries.xx.
> 
> I have such a check in my own make-pullreq script which I use
> for arm pull requests:
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/make-pullreq#n98
> 
> I can't do much about other submaintainers' workflows
> (and the block tree's "sub-submaintainers" setup means
> "check all commits have a signoff from the same person as
> the merge commit did" won't work either.)
> 
> thanks
> -- PMM

I don't have anything validating that but I agree it's a good idea.
Will add, thanks!

Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Michael S. Tsirkin]

On Tue, Nov 06, 2018 at 04:15:03PM +0100, Philippe Mathieu-Daudé wrote:
> On 6/11/18 15:13, Michael S. Tsirkin wrote:
> > On Tue, Nov 06, 2018 at 02:27:18PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 5/11/18 19:14, Michael S. Tsirkin wrote:
> > > > From: Peter Maydell <peter.maydell@linaro.org>
> > > > 
> > > > Currently tests/hex-loader-check-data contains data files used
> > > > by the hexloader-test, and configure individually symlinks those
> > > > data files into the build directory using a wildcard.
> > > > 
> > > > Using a wildcard like this is a bad idea, because if a new
> > > > data file is added, nothing causes configure to be rerun,
> > > > and so no symlink is added for the new file. This can cause
> > > > tests to spuriously fail when they can't find their data.
> > > > Instead, it's better to symlink an entire directory of
> > > > data files. We already have such a directory: tests/data.
> > > > 
> > > > Move the data files from tests/hex-loader-check-data/ to
> > > > tests/data/hex-loader/, and remove the unnecessary symlinking.
> > > > 
> > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > > 
> > > I reviewed/tested this patch too.
> > 
> > 
> > Thanks a lot Philippe!
> > It is unfortunately too late to update this patch info in git
> > commit history, however your help is still greatly appreciated!
> 
> No worry, I'm not mad at all, but there might be an issue in your git PR
> workflow, this series also missed your maintainer S-o-b.

It's just that I could not figure out the failures that were blocking
the pull, so when I saw that Peter finally posted the fix I rushed to
merge and test it and didn't look for any acks.  My mistake, sorry about
that. That's also why I forgot to sign it.

> Peter: Can you add a such check in your scripts? (during next merge window,
> no hurry).
> 
> Rather than your scripts, this should be in scripts a maintainer can run
> locally, such ./scripts/checkpatch.pl --maintainer or
> ./scripts/checkseries.xx.
> 
> > 
> > 
> > > > ---
> > > >    configure                                                 | 4 ----
> > > >    tests/hexloader-test.c                                    | 2 +-
> > > >    MAINTAINERS                                               | 2 +-
> > > >    tests/{hex-loader-check-data => data/hex-loader}/test.hex | 0
> > > >    4 files changed, 2 insertions(+), 6 deletions(-)
> > > >    rename tests/{hex-loader-check-data => data/hex-loader}/test.hex (100%)
> > > > 
> > > > diff --git a/configure b/configure
> > > > index 895b7483b8..bfdca8b814 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -7421,10 +7421,6 @@ for bios_file in \
> > > >    do
> > > >        FILES="$FILES pc-bios/$(basename $bios_file)"
> > > >    done
> > > > -for test_file in $(find $source_path/tests/hex-loader-check-data -type f)
> > > > -do
> > > > -    FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed -e 's/.*hex-loader-check-data//')"
> > > > -done
> > > >    mkdir -p $DIRS
> > > >    for f in $FILES ; do
> > > >        if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; then
> > > > diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
> > > > index b653d44ba1..834ed52c22 100644
> > > > --- a/tests/hexloader-test.c
> > > > +++ b/tests/hexloader-test.c
> > > > @@ -23,7 +23,7 @@ static void hex_loader_test(void)
> > > >        const unsigned int base_addr = 0x00010000;
> > > >        QTestState *s = qtest_initf(
> > > > -        "-M vexpress-a9 -nographic -device loader,file=tests/hex-loader-check-data/test.hex");
> > > > +        "-M vexpress-a9 -nographic -device loader,file=tests/data/hex-loader/test.hex");
> > > >        for (i = 0; i < 256; ++i) {
> > > >            uint8_t val = qtest_readb(s, base_addr + i);
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 98a1856afc..cfabc14b59 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -1370,7 +1370,7 @@ Intel Hexadecimal Object File Loader
> > > >    M: Su Hang <suhang16@mails.ucas.ac.cn>
> > > >    S: Maintained
> > > >    F: tests/hexloader-test.c
> > > > -F: tests/hex-loader-check-data/test.hex
> > > > +F: tests/data/hex-loader/test.hex
> > > >    CHRP NVRAM
> > > >    M: Thomas Huth <thuth@redhat.com>
> > > > diff --git a/tests/hex-loader-check-data/test.hex b/tests/data/hex-loader/test.hex
> > > > similarity index 100%
> > > > rename from tests/hex-loader-check-data/test.hex
> > > > rename to tests/data/hex-loader/test.hex
> > > > 

Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Laurent Vivier]

On 06/11/2018 16:15, Philippe Mathieu-Daudé wrote:
> On 6/11/18 15:13, Michael S. Tsirkin wrote:
>> On Tue, Nov 06, 2018 at 02:27:18PM +0100, Philippe Mathieu-Daudé wrote:
>>> On 5/11/18 19:14, Michael S. Tsirkin wrote:
>>>> From: Peter Maydell <peter.maydell@linaro.org>
>>>>
>>>> Currently tests/hex-loader-check-data contains data files used
>>>> by the hexloader-test, and configure individually symlinks those
>>>> data files into the build directory using a wildcard.
>>>>
>>>> Using a wildcard like this is a bad idea, because if a new
>>>> data file is added, nothing causes configure to be rerun,
>>>> and so no symlink is added for the new file. This can cause
>>>> tests to spuriously fail when they can't find their data.
>>>> Instead, it's better to symlink an entire directory of
>>>> data files. We already have such a directory: tests/data.
>>>>
>>>> Move the data files from tests/hex-loader-check-data/ to
>>>> tests/data/hex-loader/, and remove the unnecessary symlinking.
>>>>
>>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>
>>> I reviewed/tested this patch too.
>>
>>
>> Thanks a lot Philippe!
>> It is unfortunately too late to update this patch info in git
>> commit history, however your help is still greatly appreciated!
> 
> No worry, I'm not mad at all, but there might be an issue in your git PR
> workflow, this series also missed your maintainer S-o-b.
> 
> Peter: Can you add a such check in your scripts? (during next merge
> window, no hurry).
> 
> Rather than your scripts, this should be in scripts a maintainer can run
> locally, such ./scripts/checkpatch.pl --maintainer or
> ./scripts/checkseries.xx.

I think such tool already exists: with git-publish you can configure a
"pre-publish-send-email" hook, and check your S-o-B is present.

Thanks,
Laurent

Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2314 bytes --]

On Tue, Nov 06, 2018 at 05:16:14PM +0100, Laurent Vivier wrote:
> On 06/11/2018 16:15, Philippe Mathieu-Daudé wrote:
> > On 6/11/18 15:13, Michael S. Tsirkin wrote:
> >> On Tue, Nov 06, 2018 at 02:27:18PM +0100, Philippe Mathieu-Daudé wrote:
> >>> On 5/11/18 19:14, Michael S. Tsirkin wrote:
> >>>> From: Peter Maydell <peter.maydell@linaro.org>
> >>>>
> >>>> Currently tests/hex-loader-check-data contains data files used
> >>>> by the hexloader-test, and configure individually symlinks those
> >>>> data files into the build directory using a wildcard.
> >>>>
> >>>> Using a wildcard like this is a bad idea, because if a new
> >>>> data file is added, nothing causes configure to be rerun,
> >>>> and so no symlink is added for the new file. This can cause
> >>>> tests to spuriously fail when they can't find their data.
> >>>> Instead, it's better to symlink an entire directory of
> >>>> data files. We already have such a directory: tests/data.
> >>>>
> >>>> Move the data files from tests/hex-loader-check-data/ to
> >>>> tests/data/hex-loader/, and remove the unnecessary symlinking.
> >>>>
> >>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >>>
> >>> I reviewed/tested this patch too.
> >>
> >>
> >> Thanks a lot Philippe!
> >> It is unfortunately too late to update this patch info in git
> >> commit history, however your help is still greatly appreciated!
> > 
> > No worry, I'm not mad at all, but there might be an issue in your git PR
> > workflow, this series also missed your maintainer S-o-b.
> > 
> > Peter: Can you add a such check in your scripts? (during next merge
> > window, no hurry).
> > 
> > Rather than your scripts, this should be in scripts a maintainer can run
> > locally, such ./scripts/checkpatch.pl --maintainer or
> > ./scripts/checkseries.xx.
> 
> I think such tool already exists: with git-publish you can configure a
> "pre-publish-send-email" hook, and check your S-o-B is present.

For making my own commits .git/hooks/pre-commit is useful:
http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html

For sending patch series, git-publish is useful:
https://github.com/stefanha/git-publish/blob/master/hooks/pre-publish-send-email.example
https://github.com/stefanha/git-publish/

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Laurent Vivier]

On 08/11/2018 11:24, Stefan Hajnoczi wrote:
> On Tue, Nov 06, 2018 at 05:16:14PM +0100, Laurent Vivier wrote:
>> On 06/11/2018 16:15, Philippe Mathieu-Daudé wrote:
>>> On 6/11/18 15:13, Michael S. Tsirkin wrote:
>>>> On Tue, Nov 06, 2018 at 02:27:18PM +0100, Philippe Mathieu-Daudé wrote:
>>>>> On 5/11/18 19:14, Michael S. Tsirkin wrote:
>>>>>> From: Peter Maydell <peter.maydell@linaro.org>
>>>>>>
>>>>>> Currently tests/hex-loader-check-data contains data files used
>>>>>> by the hexloader-test, and configure individually symlinks those
>>>>>> data files into the build directory using a wildcard.
>>>>>>
>>>>>> Using a wildcard like this is a bad idea, because if a new
>>>>>> data file is added, nothing causes configure to be rerun,
>>>>>> and so no symlink is added for the new file. This can cause
>>>>>> tests to spuriously fail when they can't find their data.
>>>>>> Instead, it's better to symlink an entire directory of
>>>>>> data files. We already have such a directory: tests/data.
>>>>>>
>>>>>> Move the data files from tests/hex-loader-check-data/ to
>>>>>> tests/data/hex-loader/, and remove the unnecessary symlinking.
>>>>>>
>>>>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>>>
>>>>> I reviewed/tested this patch too.
>>>>
>>>>
>>>> Thanks a lot Philippe!
>>>> It is unfortunately too late to update this patch info in git
>>>> commit history, however your help is still greatly appreciated!
>>>
>>> No worry, I'm not mad at all, but there might be an issue in your git PR
>>> workflow, this series also missed your maintainer S-o-b.
>>>
>>> Peter: Can you add a such check in your scripts? (during next merge
>>> window, no hurry).
>>>
>>> Rather than your scripts, this should be in scripts a maintainer can run
>>> locally, such ./scripts/checkpatch.pl --maintainer or
>>> ./scripts/checkseries.xx.
>>
>> I think such tool already exists: with git-publish you can configure a
>> "pre-publish-send-email" hook, and check your S-o-B is present.
> 
> For making my own commits .git/hooks/pre-commit is useful:
> http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html
> 
> For sending patch series, git-publish is useful:
> https://github.com/stefanha/git-publish/blob/master/hooks/pre-publish-send-email.example
> https://github.com/stefanha/git-publish/

A pre-publish-send-email like this should do the trick:

#!/bin/bash

NAME=$(git config --get user.name)
EMAIL=$(git config --get user.email)

for PATCH in $1/*.patch; do
    if [ $(basename $PATCH) = "0000-cover-letter.patch" ]; then
        continue
    fi
    if ! grep -q "^Signed-off-by: $NAME <$EMAIL>" $PATCH; then
        echo "Error: Missing sender S-o-B in $PATCH"
        exit 1
    fi
done
exit 0

Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Philippe Mathieu-Daudé]

On Thu, Nov 8, 2018 at 3:30 PM Laurent Vivier <lvivier@redhat.com> wrote:
> On 08/11/2018 11:24, Stefan Hajnoczi wrote:
> > On Tue, Nov 06, 2018 at 05:16:14PM +0100, Laurent Vivier wrote:
> >> On 06/11/2018 16:15, Philippe Mathieu-Daudé wrote:
> >>> On 6/11/18 15:13, Michael S. Tsirkin wrote:
> >>>> On Tue, Nov 06, 2018 at 02:27:18PM +0100, Philippe Mathieu-Daudé wrote:
> >>>>> On 5/11/18 19:14, Michael S. Tsirkin wrote:
> >>>>>> From: Peter Maydell <peter.maydell@linaro.org>
> >>>>>>
> >>>>>> Currently tests/hex-loader-check-data contains data files used
> >>>>>> by the hexloader-test, and configure individually symlinks those
> >>>>>> data files into the build directory using a wildcard.
> >>>>>>
> >>>>>> Using a wildcard like this is a bad idea, because if a new
> >>>>>> data file is added, nothing causes configure to be rerun,
> >>>>>> and so no symlink is added for the new file. This can cause
> >>>>>> tests to spuriously fail when they can't find their data.
> >>>>>> Instead, it's better to symlink an entire directory of
> >>>>>> data files. We already have such a directory: tests/data.
> >>>>>>
> >>>>>> Move the data files from tests/hex-loader-check-data/ to
> >>>>>> tests/data/hex-loader/, and remove the unnecessary symlinking.
> >>>>>>
> >>>>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >>>>>
> >>>>> I reviewed/tested this patch too.
> >>>>
> >>>>
> >>>> Thanks a lot Philippe!
> >>>> It is unfortunately too late to update this patch info in git
> >>>> commit history, however your help is still greatly appreciated!
> >>>
> >>> No worry, I'm not mad at all, but there might be an issue in your git PR
> >>> workflow, this series also missed your maintainer S-o-b.
> >>>
> >>> Peter: Can you add a such check in your scripts? (during next merge
> >>> window, no hurry).
> >>>
> >>> Rather than your scripts, this should be in scripts a maintainer can run
> >>> locally, such ./scripts/checkpatch.pl --maintainer or
> >>> ./scripts/checkseries.xx.
> >>
> >> I think such tool already exists: with git-publish you can configure a
> >> "pre-publish-send-email" hook, and check your S-o-B is present.
> >
> > For making my own commits .git/hooks/pre-commit is useful:
> > http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html
> >
> > For sending patch series, git-publish is useful:
> > https://github.com/stefanha/git-publish/blob/master/hooks/pre-publish-send-email.example
> > https://github.com/stefanha/git-publish/
>
> A pre-publish-send-email like this should do the trick:
>
> #!/bin/bash
>
> NAME=$(git config --get user.name)
> EMAIL=$(git config --get user.email)
>
> for PATCH in $1/*.patch; do
>     if [ $(basename $PATCH) = "0000-cover-letter.patch" ]; then
>         continue
>     fi

Maybe easier (to handle series versions > 1):

    case ${PATCH} in
    *-cover-letter.patch)
        continue
        ;;
    *)
    ...

>     if ! grep -q "^Signed-off-by: $NAME <$EMAIL>" $PATCH; then
>         echo "Error: Missing sender S-o-B in $PATCH"
>         exit 1
>     fi
> done
> exit 0
>
>