From 438c78dab75e3b9d1bc8da1d5401da77c84e27b7 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Nov 2018 11:52:36 +0000 Subject: [PATCH 01/33] tests: Move tests/acpi-test-data/ to tests/data/acpi/ 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 Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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/ ." From 4b2ff65a1f33b6130bfd92a8aaf6289909af9d35 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Nov 2018 11:52:37 +0000 Subject: [PATCH 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/ 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 Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- MAINTAINERS | 2 +- configure | 4 ---- tests/{hex-loader-check-data => data/hex-loader}/test.hex | 0 tests/hexloader-test.c | 2 +- 4 files changed, 2 insertions(+), 6 deletions(-) rename tests/{hex-loader-check-data => data/hex-loader}/test.hex (100%) 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 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 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/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 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); From e29e5c6ee0b2770912dfea6a05926352ff30171d Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Nov 2018 11:52:38 +0000 Subject: [PATCH 03/33] configure: Rename FILES variable to LINKS 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 Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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 From 39950353959932320763c9ef27fb4ff0b774fe0c Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 2 Nov 2018 11:52:39 +0000 Subject: [PATCH 04/33] configure: Use LINKS loop for all build tree symlinks 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 Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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 <config.status #!/bin/sh From 9bb192a4fca3e50194b6ac2e0eaaee20d996bba1 Mon Sep 17 00:00:00 2001 From: Yaowei Bai Date: Sat, 28 Jul 2018 13:18:44 +0800 Subject: [PATCH 05/33] virtio-blk: fix comment for virtio_blk_rw_complete Here should be submit_requests, there is no submit_merged_requests function. Signed-off-by: Yaowei Bai Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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); } From 06aba4ca52fd2c8718b8ba486f22f0aa7c99ed55 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Sat, 29 Sep 2018 11:36:14 +0800 Subject: [PATCH 06/33] intel_iommu: introduce vtd_reset_caches() 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 Reviewed-by: Eric Auger Reviewed-by: Jason Wang Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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); From 2cc9ddccebcaa48b3debfc279a83761fcbb7616c Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Sat, 29 Sep 2018 11:36:15 +0800 Subject: [PATCH 07/33] intel_iommu: better handling of dmar state switch 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 Reported-by: Cong Li Signed-off-by: Peter Xu -- 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 Reviewed-by: Eric Auger Reviewed-by: Jason Wang Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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) From 95ecd3df7815b4bc4f9a0f47e1c64d81434715aa Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Tue, 9 Oct 2018 15:45:42 +0800 Subject: [PATCH 08/33] intel_iommu: move ce fetching out when sync shadow 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 Reviewed-by: Eric Auger Reviewed-by: Maxime Coquelin Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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); } /* From c28b535d083d0a263d38d9ceeada83cdae8c64f0 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Tue, 9 Oct 2018 15:45:43 +0800 Subject: [PATCH 09/33] intel_iommu: handle invalid ce for shadow sync 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 Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1627272 Signed-off-by: Peter Xu Reviewed-by: Eric Auger Reviewed-by: Maxime Coquelin Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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; } From 110b9463d5c820120c8311db79f55a64c9d81ebe Mon Sep 17 00:00:00 2001 From: Yongji Xie Date: Wed, 6 Jun 2018 21:24:48 +0800 Subject: [PATCH 10/33] vhost-user-blk: start vhost when guest kicks 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 Signed-off-by: Chai Wen Signed-off-by: Ni Xun Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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) From 50662ce16da37479533e7ae9f515d4e9d49b9d14 Mon Sep 17 00:00:00 2001 From: "Singh, Brijesh" Date: Mon, 1 Oct 2018 19:44:27 +0000 Subject: [PATCH 11/33] x86_iommu: move the kernel-irqchip check in common code Interrupt remapping needs kernel-irqchip={off|split} on both Intel and AMD platforms. Move the check in common place. Signed-off-by: Brijesh Singh Reviewed-by: Peter Xu Cc: Peter Xu Cc: "Michael S. Tsirkin" Cc: Paolo Bonzini Cc: Richard Henderson Cc: Eduardo Habkost Cc: Marcel Apfelbaum Cc: Tom Lendacky Cc: Suravee Suthikulpanit Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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); } From 35c2450191634bd0ac0ae3c6e9da3f590124e8e2 Mon Sep 17 00:00:00 2001 From: "Singh, Brijesh" Date: Mon, 1 Oct 2018 19:44:29 +0000 Subject: [PATCH 12/33] x86_iommu: move vtd_generate_msi_message in common file 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 Suggested-by: Peter Xu Reviewed-by: Eduardo Habkost Cc: Peter Xu Cc: "Michael S. Tsirkin" Cc: Paolo Bonzini Cc: Richard Henderson Cc: Eduardo Habkost Cc: Marcel Apfelbaum Cc: Tom Lendacky Cc: Suravee Suthikulpanit Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/intel_iommu.c | 32 +++-------------- hw/i386/x86-iommu.c | 24 +++++++++++++ include/hw/i386/intel_iommu.h | 59 ------------------------------- include/hw/i386/x86-iommu.h | 66 +++++++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 87 deletions(-) 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; 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 From 470506b5821c62d6b00d0dd82eea999b61c3719e Mon Sep 17 00:00:00 2001 From: "Singh, Brijesh" Date: Mon, 1 Oct 2018 19:44:32 +0000 Subject: [PATCH 13/33] x86_iommu/amd: remove V=1 check from amdvi_validate_dte() 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 Reviewed-by: Peter Xu Cc: Peter Xu Cc: "Michael S. Tsirkin" Cc: Paolo Bonzini Cc: Richard Henderson Cc: Eduardo Habkost Cc: Marcel Apfelbaum Cc: Tom Lendacky Cc: Suravee Suthikulpanit Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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; } From 53244386b0674dc91ae2987c77a95e82476bb6f7 Mon Sep 17 00:00:00 2001 From: "Singh, Brijesh" Date: Mon, 1 Oct 2018 19:44:34 +0000 Subject: [PATCH 14/33] x86_iommu/amd: make the address space naming consistent with intel-iommu To be consistent with intel-iommu: - rename the address space to use '_' instead of '-' - update the memory region relationships Signed-off-by: Brijesh Singh Reviewed-by: Peter Xu Cc: Peter Xu Cc: "Michael S. Tsirkin" Cc: Paolo Bonzini Cc: Richard Henderson Cc: Eduardo Habkost Cc: Marcel Apfelbaum Cc: Tom Lendacky Cc: Suravee Suthikulpanit Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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; } From 577c470f4326eae22e4e412946b167075f4440d2 Mon Sep 17 00:00:00 2001 From: "Singh, Brijesh" Date: Mon, 1 Oct 2018 19:44:37 +0000 Subject: [PATCH 15/33] x86_iommu/amd: Prepare for interrupt remap support 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 Cc: Peter Xu Cc: "Michael S. Tsirkin" Cc: Paolo Bonzini Cc: Richard Henderson Cc: Eduardo Habkost Cc: Marcel Apfelbaum Cc: Tom Lendacky Cc: Suravee Suthikulpanit Reviewed-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/amd_iommu.c | 106 +++++++++++++++++++++++++++++++++++++++++++ hw/i386/amd_iommu.h | 14 +++++- hw/i386/trace-events | 5 ++ 3 files changed, 123 insertions(+), 2 deletions(-) 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/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/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" From b44159fe00784f67f766ba1bf6eec14f3c709806 Mon Sep 17 00:00:00 2001 From: "Singh, Brijesh" Date: Mon, 1 Oct 2018 19:44:39 +0000 Subject: [PATCH 16/33] x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled 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 Cc: Peter Xu Cc: "Michael S. Tsirkin" Cc: Paolo Bonzini Cc: Richard Henderson Cc: Eduardo Habkost Cc: Marcel Apfelbaum Cc: Tom Lendacky Cc: Suravee Suthikulpanit Reviewed-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/amd_iommu.c | 199 ++++++++++++++++++++++++++++++++++++++++++- hw/i386/amd_iommu.h | 44 ++++++++++ hw/i386/trace-events | 7 ++ 3 files changed, 249 insertions(+), 1 deletion(-) 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/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/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" From c028818d573ab3d86ed916a70359fedb2d24a8ad Mon Sep 17 00:00:00 2001 From: "Singh, Brijesh" Date: Mon, 1 Oct 2018 19:44:41 +0000 Subject: [PATCH 17/33] i386: acpi: add IVHD device entry for IOAPIC When interrupt remapping is enabled, add a special IVHD device (type IOAPIC). Signed-off-by: Brijesh Singh Acked-by: Peter Xu Cc: Peter Xu Cc: "Michael S. Tsirkin" Cc: Paolo Bonzini Cc: Richard Henderson Cc: Eduardo Habkost Cc: Marcel Apfelbaum Cc: Tom Lendacky Cc: Suravee Suthikulpanit Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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); } From 135f866e609c2ddbd0f352e8e0fee8dfb5d56c3e Mon Sep 17 00:00:00 2001 From: "Singh, Brijesh" Date: Mon, 1 Oct 2018 19:44:43 +0000 Subject: [PATCH 18/33] x86_iommu/amd: Add interrupt remap support when VAPIC is enabled 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 Reviewed-by: Peter Xu Cc: Peter Xu Cc: "Michael S. Tsirkin" Cc: Paolo Bonzini Cc: Richard Henderson Cc: Eduardo Habkost Cc: Marcel Apfelbaum Cc: Tom Lendacky Cc: Suravee Suthikulpanit Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/amd_iommu.c | 69 +++++++++++++++++++++++++++++++++++++++++++- hw/i386/amd_iommu.h | 36 +++++++++++++++++++++++ hw/i386/trace-events | 2 ++ 3 files changed, 106 insertions(+), 1 deletion(-) 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/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/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" From 12499b23315bac605516986f4de4fc55ddfaca78 Mon Sep 17 00:00:00 2001 From: "Singh, Brijesh" Date: Mon, 1 Oct 2018 19:44:45 +0000 Subject: [PATCH 19/33] x86_iommu/amd: Enable Guest virtual APIC support 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 Reviewed-by: Peter Xu Cc: Peter Xu Cc: "Michael S. Tsirkin" Cc: Paolo Bonzini Cc: Richard Henderson Cc: Eduardo Habkost Cc: Marcel Apfelbaum Cc: Tom Lendacky Cc: Suravee Suthikulpanit Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 3 ++- hw/i386/amd_iommu.h | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) 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 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 { From 7115dcf4f19e1d590aec7afe315095af9491dad6 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 27 Sep 2018 23:24:35 +0200 Subject: [PATCH 20/33] MAINTAINERS: list "tests/acpi-test-data" files in ACPI/SMBIOS section 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" Cc: Alex Williamson Cc: Gerd Hoffmann Cc: Igor Mammedov Cc: Marcel Apfelbaum Signed-off-by: Laszlo Ersek Reviewed-by: Marcel Apfelbaum Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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 From 417463341e3e35c9be80f54b0d6ae8cfdb4b0d84 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 28 Sep 2018 08:44:19 +0200 Subject: [PATCH 21/33] pci-testdev: add optional memory bar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Laszlo Ersek Tested-by: Laszlo Ersek Signed-off-by: Gerd Hoffmann Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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= 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 = { From ccef5b1fcf1ca33976583c88489a55d6acd13b78 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 27 Sep 2018 23:24:37 +0200 Subject: [PATCH 22/33] hw/pci-host/x86: extract get_pci_hole64_start_value() helpers 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" Cc: Alex Williamson Cc: Gerd Hoffmann Cc: Igor Mammedov Cc: Marcel Apfelbaum Signed-off-by: Laszlo Ersek Reviewed-by: Marcel Apfelbaum Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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); } /* From ed6bb4b581050fe27aba80d6937138adf883d41e Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 27 Sep 2018 23:24:38 +0200 Subject: [PATCH 23/33] hw/pci-host/x86: extend the 64-bit PCI hole relative to the fw-assigned base 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" Cc: Alex Williamson Cc: Gerd Hoffmann Cc: Igor Mammedov Cc: Marcel Apfelbaum Fixes: 9fa99d2519cbf71f871e46871df12cb446dc1c3e Signed-off-by: Laszlo Ersek Reviewed-by: Marcel Apfelbaum Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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; From f5f4002ddc0c0159735fc1a2dc44c97493fdff35 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 25 Oct 2018 21:55:32 -0400 Subject: [PATCH 24/33] bios-tables-test: prepare expected files for mmio64 test will be added by follow-up patch. Signed-off-by: Michael S. Tsirkin --- 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`MZVWfQbOALxMpqxlJWYk&d;`r6l^h@YaKZ)QiHSyDi(2XemIbG~oR z?ChN7uKcdw`*w{n>)+KKuhA`4ueUuPeHLSk+Vt%-GdEcO(6819Rz8)n`gil9jgPAB zKI2!f)vdqogosUE7X2foN>3n|cX7u4F;Vnj>cWyXTzK9FEkXI)mSOBY=3Je5j)p_ihUv>SyOn{MDhyy1gK0NAyD1*>LjSOZGnnFsJk`H@j@r!I z2`Xu3;j4F++P39Y@zHAI!?}9)`LwD?)%r)zc6uI)%5#>- zIiiuS@H2HQVi9vvYs{)!2kd~Qa__PJ(F|8q`E-WR9HqWjxhk!3Z%Ly>U+!Z4{me;0htJimchm2tZF}ud z6slz~C+kX4){0kMoQ5Xkv@{RW57NwDb3~d=N^>cZhNskUXiiVflX{Rs9vgY=cGKHr zZef6LE83&T`O|9_odU=Euee;=onCu!>+QqEdt13-o{V5BXht&C;1TLNvxd&BU@B-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{KSv6{|8Z}qVnh0f08wOg@ z47A>Do){?FC>$Fo0+f|O8i6Vb1C=;oNfz#R1W-9TCkBcTLl;;WsKmw*G1cUffl8=gl7S*rI$@v^E0{1)4N3+o zp@K;Uicsl$|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;UicslAsK$hWYD^fY z#v}vPm}H(JJRC7xPiqL27 z`2j4HjS55YV|7eFNbl$9tJ1&wi|5nyE0vxV(9;O3-Px_cad)fq;n9b0GdvsFtx@5EYq*G?&4i_32&%V!^<{H9iZQ9ik#!TZhceureGEDXdAu*L(G3uzytlePf$={C6AuVd(G{x<#mErEU)%**(@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?<*>#)%dIu?G1^OgQl<3)J|A+y}g6Pf*H5R_iO{fB+Jxb48- z?%-AFbK2CS}Z~By_Lkaf3UW9nz5SP7HU~eJf~rTg$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`(rGuUS8@d&mQxczv3w<@f3%Ye+GYGN09~Brcdn+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 From 0259e96687fa9b01d87113693e076dfa83a649f0 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 27 Sep 2018 23:24:36 +0200 Subject: [PATCH 25/33] tests/bios-tables-test: add 64-bit PCI MMIO aperture round-up test on Q35 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" Cc: Alex Williamson Cc: Gerd Hoffmann Cc: Igor Mammedov Cc: Marcel Apfelbaum Signed-off-by: Laszlo Ersek Reviewed-by: Marcel Apfelbaum Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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); From c6329a2d0976661a466e02e29f50ce7b708cb8eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 10 Oct 2018 23:53:53 +0200 Subject: [PATCH 26/33] hw/pci-bridge/xio3130: Remove unused functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduced in 48ebf2f90f8 and faf1e708d5b, these functions were never used. Remove them. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci-bridge/xio3130_downstream.c | 28 +--------------------------- hw/pci-bridge/xio3130_downstream.h | 11 ----------- hw/pci-bridge/xio3130_upstream.c | 24 +----------------------- hw/pci-bridge/xio3130_upstream.h | 10 ---------- 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.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_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.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, 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 */ From cd1f0ca29d3be55398d6def7ad9db9ad7bfb4e9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 10 Oct 2018 23:53:54 +0200 Subject: [PATCH 27/33] hw/pci-bridge/ioh3420: Remove unuseful header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci-bridge/ioh3420.c | 2 +- hw/pci-bridge/ioh3420.h | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) delete mode 100644 hw/pci-bridge/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 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 */ From 2728a57a061af92b476f926b7fb66cb3ac60ab50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 10 Oct 2018 23:53:55 +0200 Subject: [PATCH 28/33] hw/pci: Add missing include MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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é Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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. * From d05eec73e209b0ddca98fdb048de3a18b28223f4 Mon Sep 17 00:00:00 2001 From: Mao Zhongyi Date: Thu, 11 Oct 2018 16:25:08 +0800 Subject: [PATCH 29/33] pci_bridge: fix typo in comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mao Zhongyi Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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, From 0118c01cabdf6e37a7fb79aa267d18e787c2bd7b Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Thu, 11 Oct 2018 05:14:03 -0700 Subject: [PATCH 30/33] i440fx: use ARRAY_SIZE for pam_regions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: qemu-trivial@nongnu.org Signed-off-by: Li Qiang Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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); From ee31e901ef3ad3bfa340fab7ec18f0a7eee6bab1 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Thu, 11 Oct 2018 05:38:54 -0700 Subject: [PATCH 31/33] piix: use TYPE_FOO constants than string constats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make them more QOMConventional. Cc:qemu-trivial@nongnu.org Signed-off-by: Li Qiang Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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, }; From 9b178f0e80cb007e1a96ac01f10d39e8955ddeb0 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Tue, 23 Oct 2018 18:44:25 -0400 Subject: [PATCH 32/33] piix_pci: fix i440fx data sheet link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It seems that the intel link is unavailable, change it to point to the qemu site. Signed-off-by: Li Qiang Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Marcel Apfelbaum Acked-by: Michael S. Tsirkin Reviewed-by: Michael S. Tsirkin --- 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) \ From e6cc11d64fc998c11a4dfcde8fda3fc33a74d844 Mon Sep 17 00:00:00 2001 From: yuchenlin Date: Fri, 12 Oct 2018 17:07:52 +0800 Subject: [PATCH 33/33] vhost-scsi: prevent using uninitialized vqs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: yuchenlin Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- 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;