* [PULL v2 01/47] qom: assert integer does not overflow
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 02/47] ACPI ERST: specification for ERST support Michael S. Tsirkin
` (46 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Peter Maydell, Daniel P. Berrangé, Paolo Bonzini
QOM reference counting is not designed with an infinite amount of
references in mind, trying to take a reference in a loop without
dropping a reference will overflow the integer.
It is generally a symptom of a reference leak (a missing deref, commonly
as part of error handling - such as one fixed here:
https://lore.kernel.org/r/20220228095058.27899-1-sgarzare%40redhat.com ).
All this can lead to either freeing the object too early (memory
corruption) or never freeing it (memory leak).
If we happen to dereference at just the right time (when it's wrapping
around to 0), we might eventually assert when dereferencing, but the
real problem is an extra object_ref so let's assert there to make such
issues cleaner and easier to debug.
Some micro-benchmarking shows using fetch and add this is essentially
free on x86.
Since multiple threads could be incrementing in parallel, we assert
around INT_MAX to make sure none of these approach the wrap around
point: this way we get a memory leak and not a memory corruption, the
former is generally easier to debug.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
qom/object.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/qom/object.c b/qom/object.c
index 9f7a33139d..a27532a6ba 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1167,10 +1167,14 @@ GSList *object_class_get_list_sorted(const char *implements_type,
Object *object_ref(void *objptr)
{
Object *obj = OBJECT(objptr);
+ uint32_t ref;
+
if (!obj) {
return NULL;
}
- qatomic_inc(&obj->ref);
+ ref = qatomic_fetch_inc(&obj->ref);
+ /* Assert waaay before the integer overflows */
+ g_assert(ref < INT_MAX);
return obj;
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 02/47] ACPI ERST: specification for ERST support
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 01/47] qom: assert integer does not overflow Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 03/47] MAINTAINERS: no need to add my name explicitly as a reviewer for VIOT tables Michael S. Tsirkin
` (45 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Daniel P. Berrangé,
Markus Armbruster, Ani Sinha, Igor Mammedov, Eric DeVolder
From: Eric DeVolder <eric.devolder@oracle.com>
Information on the implementation of the ACPI ERST support.
Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
Acked-by: Ani Sinha <ani@anisinha.ca>
Message-Id: <20220223143322.927136-2-ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
docs/specs/acpi_erst.rst | 200 +++++++++++++++++++++++++++++++++++++++
docs/specs/index.rst | 1 +
2 files changed, 201 insertions(+)
create mode 100644 docs/specs/acpi_erst.rst
diff --git a/docs/specs/acpi_erst.rst b/docs/specs/acpi_erst.rst
new file mode 100644
index 0000000000..a8a9d22d25
--- /dev/null
+++ b/docs/specs/acpi_erst.rst
@@ -0,0 +1,200 @@
+ACPI ERST DEVICE
+================
+
+The ACPI ERST device is utilized to support the ACPI Error Record
+Serialization Table, ERST, functionality. This feature is designed for
+storing error records in persistent storage for future reference
+and/or debugging.
+
+The ACPI specification[1], in Chapter "ACPI Platform Error Interfaces
+(APEI)", and specifically subsection "Error Serialization", outlines a
+method for storing error records into persistent storage.
+
+The format of error records is described in the UEFI specification[2],
+in Appendix N "Common Platform Error Record".
+
+While the ACPI specification allows for an NVRAM "mode" (see
+GET_ERROR_LOG_ADDRESS_RANGE_ATTRIBUTES) where non-volatile RAM is
+directly exposed for direct access by the OS/guest, this device
+implements the non-NVRAM "mode". This non-NVRAM "mode" is what is
+implemented by most BIOS (since flash memory requires programming
+operations in order to update its contents). Furthermore, as of the
+time of this writing, Linux only supports the non-NVRAM "mode".
+
+
+Background/Motivation
+---------------------
+
+Linux uses the persistent storage filesystem, pstore, to record
+information (eg. dmesg tail) upon panics and shutdowns. Pstore is
+independent of, and runs before, kdump. In certain scenarios (ie.
+hosts/guests with root filesystems on NFS/iSCSI where networking
+software and/or hardware fails, and thus kdump fails), pstore may
+contain information available for post-mortem debugging.
+
+Two common storage backends for the pstore filesystem are ACPI ERST
+and UEFI. Most BIOS implement ACPI ERST. UEFI is not utilized in all
+guests. With QEMU supporting ACPI ERST, it becomes a viable pstore
+storage backend for virtual machines (as it is now for bare metal
+machines).
+
+Enabling support for ACPI ERST facilitates a consistent method to
+capture kernel panic information in a wide range of guests: from
+resource-constrained microvms to very large guests, and in particular,
+in direct-boot environments (which would lack UEFI run-time services).
+
+Note that Microsoft Windows also utilizes the ACPI ERST for certain
+crash information, if available[3].
+
+
+Configuration|Usage
+-------------------
+
+To use ACPI ERST, a memory-backend-file object and acpi-erst device
+can be created, for example:
+
+ qemu ...
+ -object memory-backend-file,id=erstnvram,mem-path=acpi-erst.backing,size=0x10000,share=on \
+ -device acpi-erst,memdev=erstnvram
+
+For proper operation, the ACPI ERST device needs a memory-backend-file
+object with the following parameters:
+
+ - id: The id of the memory-backend-file object is used to associate
+ this memory with the acpi-erst device.
+ - size: The size of the ACPI ERST backing storage. This parameter is
+ required.
+ - mem-path: The location of the ACPI ERST backing storage file. This
+ parameter is also required.
+ - share: The share=on parameter is required so that updates to the
+ ERST backing store are written to the file.
+
+and ERST device:
+
+ - memdev: Is the object id of the memory-backend-file.
+ - record_size: Specifies the size of the records (or slots) in the
+ backend storage. Must be a power of two value greater than or
+ equal to 4096 (PAGE_SIZE).
+
+
+PCI Interface
+-------------
+
+The ERST device is a PCI device with two BARs, one for accessing the
+programming registers, and the other for accessing the record exchange
+buffer.
+
+BAR0 contains the programming interface consisting of ACTION and VALUE
+64-bit registers. All ERST actions/operations/side effects happen on
+the write to the ACTION, by design. Any data needed by the action must
+be placed into VALUE prior to writing ACTION. Reading the VALUE
+simply returns the register contents, which can be updated by a
+previous ACTION.
+
+BAR1 contains the 8KiB record exchange buffer, which is the
+implemented maximum record size.
+
+
+Backend Storage Format
+----------------------
+
+The backend storage is divided into fixed size "slots", 8KiB in
+length, with each slot storing a single record. Not all slots need to
+be occupied, and they need not be occupied in a contiguous fashion.
+The ability to clear/erase specific records allows for the formation
+of unoccupied slots.
+
+Slot 0 contains a backend storage header that identifies the contents
+as ERST and also facilitates efficient access to the records.
+Depending upon the size of the backend storage, additional slots will
+be designated to be a part of the slot 0 header. For example, at 8KiB,
+the slot 0 header can accomodate 1021 records. Thus a storage size
+of 8MiB (8KiB * 1024) requires an additional slot for use by the
+header. In this scenario, slot 0 and slot 1 form the backend storage
+header, and records can be stored starting at slot 2.
+
+Below is an example layout of the backend storage format (for storage
+size less than 8MiB). The size of the storage is a multiple of 8KiB,
+and contains N number of slots to store records. The example below
+shows two records (in CPER format) in the backend storage, while the
+remaining slots are empty/available.
+
+::
+
+ Slot Record
+ <------------------ 8KiB -------------------->
+ +--------------------------------------------+
+ 0 | storage header |
+ +--------------------------------------------+
+ 1 | empty/available |
+ +--------------------------------------------+
+ 2 | CPER |
+ +--------------------------------------------+
+ 3 | CPER |
+ +--------------------------------------------+
+ ... | |
+ +--------------------------------------------+
+ N | empty/available |
+ +--------------------------------------------+
+
+The storage header consists of some basic information and an array
+of CPER record_id's to efficiently access records in the backend
+storage.
+
+All fields in the header are stored in little endian format.
+
+::
+
+ +--------------------------------------------+
+ | magic | 0x0000
+ +--------------------------------------------+
+ | record_offset | record_size | 0x0008
+ +--------------------------------------------+
+ | record_count | reserved | version | 0x0010
+ +--------------------------------------------+
+ | record_id[0] | 0x0018
+ +--------------------------------------------+
+ | record_id[1] | 0x0020
+ +--------------------------------------------+
+ | record_id[...] |
+ +--------------------------------------------+
+ | record_id[N] | 0x1FF8
+ +--------------------------------------------+
+
+The 'magic' field contains the value 0x524F545354535245.
+
+The 'record_size' field contains the value 0x2000, 8KiB.
+
+The 'record_offset' field points to the first record_id in the array,
+0x0018.
+
+The 'version' field contains 0x0100, the first version.
+
+The 'record_count' field contains the number of valid records in the
+backend storage.
+
+The 'record_id' array fields are the 64-bit record identifiers of the
+CPER record in the corresponding slot. Stated differently, the
+location of a CPER record_id in the record_id[] array provides the
+slot index for the corresponding record in the backend storage.
+
+Note that, for example, with a backend storage less than 8MiB, slot 0
+contains the header, so the record_id[0] will never contain a valid
+CPER record_id. Instead slot 1 is the first available slot and thus
+record_id_[1] may contain a CPER.
+
+A 'record_id' of all 0s or all 1s indicates an invalid record (ie. the
+slot is available).
+
+
+References
+----------
+
+[1] "Advanced Configuration and Power Interface Specification",
+ version 4.0, June 2009.
+
+[2] "Unified Extensible Firmware Interface Specification",
+ version 2.1, October 2008.
+
+[3] "Windows Hardware Error Architecture", specfically
+ "Error Record Persistence Mechanism".
diff --git a/docs/specs/index.rst b/docs/specs/index.rst
index ecc43896bb..32863b4aa5 100644
--- a/docs/specs/index.rst
+++ b/docs/specs/index.rst
@@ -18,3 +18,4 @@ guest hardware that is specific to QEMU.
acpi_mem_hotplug
acpi_pci_hotplug
acpi_nvdimm
+ acpi_erst
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 03/47] MAINTAINERS: no need to add my name explicitly as a reviewer for VIOT tables
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 01/47] qom: assert integer does not overflow Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 02/47] ACPI ERST: specification for ERST support Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 04/47] docs/acpi/erst: add device id for ACPI ERST device in pci-ids.txt Michael S. Tsirkin
` (44 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Richard Henderson,
Philippe Mathieu-Daudé,
Ani Sinha, Alex Bennée
From: Ani Sinha <ani@anisinha.ca>
I am already listed as a reviewer for ACPI/SMBIOS subsystem. There is no need to
again add me as a reviewer for ACPI/VIOT.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Message-Id: <20220223143322.927136-3-ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
MAINTAINERS | 1 -
1 file changed, 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 68adaac373..83a9f79c1c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1819,7 +1819,6 @@ F: docs/specs/acpi_hw_reduced_hotplug.rst
ACPI/VIOT
M: Jean-Philippe Brucker <jean-philippe@linaro.org>
-R: Ani Sinha <ani@anisinha.ca>
S: Supported
F: hw/acpi/viot.c
F: hw/acpi/viot.h
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 04/47] docs/acpi/erst: add device id for ACPI ERST device in pci-ids.txt
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (2 preceding siblings ...)
2022-03-07 10:01 ` [PULL v2 03/47] MAINTAINERS: no need to add my name explicitly as a reviewer for VIOT tables Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 05/47] hw/acpi/erst: clean up unused IS_UEFI_CPER_RECORD macro Michael S. Tsirkin
` (43 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Ani Sinha, Peter Maydell, Eric DeVolder
From: Ani Sinha <ani@anisinha.ca>
Adding device ID for ERST device in pci-ids.txt. It was missed when ERST
related patches were reviewed.
CC: Eric DeVolder <eric.devolder@oracle.com>
Reviewed-by: Eric DeVolder <eric.devolder@oracle.com>
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Message-Id: <20220223143322.927136-4-ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
docs/specs/pci-ids.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
index 5e407a6f32..dd6859d039 100644
--- a/docs/specs/pci-ids.txt
+++ b/docs/specs/pci-ids.txt
@@ -65,6 +65,7 @@ PCI devices (other than virtio):
1b36:000f mdpy (mdev sample device), linux/samples/vfio-mdev/mdpy.c
1b36:0010 PCIe NVMe device (-device nvme)
1b36:0011 PCI PVPanic device (-device pvpanic-pci)
+1b36:0012 PCI ACPI ERST device (-device acpi-erst)
All these devices are documented in docs/specs.
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 05/47] hw/acpi/erst: clean up unused IS_UEFI_CPER_RECORD macro
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (3 preceding siblings ...)
2022-03-07 10:01 ` [PULL v2 04/47] docs/acpi/erst: add device id for ACPI ERST device in pci-ids.txt Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 06/47] hw/smbios: code cleanup - use macro definitions for table header handles Michael S. Tsirkin
` (42 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Ani Sinha, Peter Maydell, Eric DeVolder, Igor Mammedov
From: Ani Sinha <ani@anisinha.ca>
This change is cosmetic. IS_UEFI_CPER_RECORD macro definition that was added
as a part of the ERST implementation seems to be unused. Remove it.
CC: Eric DeVolder <eric.devolder@oracle.com>
Reviewed-by: Eric DeVolder <eric.devolder@oracle.com>
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Message-Id: <20220223143322.927136-5-ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/acpi/erst.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/hw/acpi/erst.c b/hw/acpi/erst.c
index c0a23cf467..de509c2b48 100644
--- a/hw/acpi/erst.c
+++ b/hw/acpi/erst.c
@@ -80,11 +80,6 @@
#define UEFI_CPER_RECORD_MIN_SIZE 128U
#define UEFI_CPER_RECORD_LENGTH_OFFSET 20U
#define UEFI_CPER_RECORD_ID_OFFSET 96U
-#define IS_UEFI_CPER_RECORD(ptr) \
- (((ptr)[0] == 'C') && \
- ((ptr)[1] == 'P') && \
- ((ptr)[2] == 'E') && \
- ((ptr)[3] == 'R'))
/*
* NOTE that when accessing CPER fields within a record, memcpy()
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 06/47] hw/smbios: code cleanup - use macro definitions for table header handles
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (4 preceding siblings ...)
2022-03-07 10:01 ` [PULL v2 05/47] hw/acpi/erst: clean up unused IS_UEFI_CPER_RECORD macro Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 07/47] hw/smbios: fix overlapping table handle numbers with large memory vms Michael S. Tsirkin
` (41 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel
Cc: Ani Sinha, Peter Maydell, Philippe Mathieu-Daudé, Igor Mammedov
From: Ani Sinha <ani@anisinha.ca>
This is a minor cleanup. Using macro definitions makes the code more
readable. It is at once clear which tables use which handle numbers in their
header. It also makes it easy to calculate the gaps between the numbers and
update them if needed.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Message-Id: <20220223143322.927136-6-ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/smbios/smbios.c | 38 ++++++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 6013df1698..56b412ce35 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -549,9 +549,23 @@ bool smbios_skip_table(uint8_t type, bool required_table)
return true;
}
+#define T0_BASE 0x000
+#define T1_BASE 0x100
+#define T2_BASE 0x200
+#define T3_BASE 0x300
+#define T4_BASE 0x400
+#define T11_BASE 0xe00
+
+#define T16_BASE 0x1000
+#define T17_BASE 0x1100
+#define T19_BASE 0x1300
+#define T32_BASE 0x2000
+#define T41_BASE 0x2900
+#define T127_BASE 0x7F00
+
static void smbios_build_type_0_table(void)
{
- SMBIOS_BUILD_TABLE_PRE(0, 0x000, false); /* optional, leave up to BIOS */
+ SMBIOS_BUILD_TABLE_PRE(0, T0_BASE, false); /* optional, leave up to BIOS */
SMBIOS_TABLE_SET_STR(0, vendor_str, type0.vendor);
SMBIOS_TABLE_SET_STR(0, bios_version_str, type0.version);
@@ -599,7 +613,7 @@ static void smbios_encode_uuid(struct smbios_uuid *uuid, QemuUUID *in)
static void smbios_build_type_1_table(void)
{
- SMBIOS_BUILD_TABLE_PRE(1, 0x100, true); /* required */
+ SMBIOS_BUILD_TABLE_PRE(1, T1_BASE, true); /* required */
SMBIOS_TABLE_SET_STR(1, manufacturer_str, type1.manufacturer);
SMBIOS_TABLE_SET_STR(1, product_name_str, type1.product);
@@ -619,7 +633,7 @@ static void smbios_build_type_1_table(void)
static void smbios_build_type_2_table(void)
{
- SMBIOS_BUILD_TABLE_PRE(2, 0x200, false); /* optional */
+ SMBIOS_BUILD_TABLE_PRE(2, T2_BASE, false); /* optional */
SMBIOS_TABLE_SET_STR(2, manufacturer_str, type2.manufacturer);
SMBIOS_TABLE_SET_STR(2, product_str, type2.product);
@@ -637,7 +651,7 @@ static void smbios_build_type_2_table(void)
static void smbios_build_type_3_table(void)
{
- SMBIOS_BUILD_TABLE_PRE(3, 0x300, true); /* required */
+ SMBIOS_BUILD_TABLE_PRE(3, T3_BASE, true); /* required */
SMBIOS_TABLE_SET_STR(3, manufacturer_str, type3.manufacturer);
t->type = 0x01; /* Other */
@@ -662,7 +676,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
{
char sock_str[128];
- SMBIOS_BUILD_TABLE_PRE(4, 0x400 + instance, true); /* required */
+ SMBIOS_BUILD_TABLE_PRE(4, T4_BASE + instance, true); /* required */
snprintf(sock_str, sizeof(sock_str), "%s%2x", type4.sock_pfx, instance);
SMBIOS_TABLE_SET_STR(4, socket_designation_str, sock_str);
@@ -702,7 +716,7 @@ static void smbios_build_type_11_table(void)
return;
}
- SMBIOS_BUILD_TABLE_PRE(11, 0xe00, true); /* required */
+ SMBIOS_BUILD_TABLE_PRE(11, T11_BASE, true); /* required */
snprintf(count_str, sizeof(count_str), "%zu", type11.nvalues);
t->count = type11.nvalues;
@@ -722,7 +736,7 @@ static void smbios_build_type_16_table(unsigned dimm_cnt)
{
uint64_t size_kb;
- SMBIOS_BUILD_TABLE_PRE(16, 0x1000, true); /* required */
+ SMBIOS_BUILD_TABLE_PRE(16, T16_BASE, true); /* required */
t->location = 0x01; /* Other */
t->use = 0x03; /* System memory */
@@ -749,7 +763,7 @@ static void smbios_build_type_17_table(unsigned instance, uint64_t size)
char loc_str[128];
uint64_t size_mb;
- SMBIOS_BUILD_TABLE_PRE(17, 0x1100 + instance, true); /* required */
+ SMBIOS_BUILD_TABLE_PRE(17, T17_BASE + instance, true); /* required */
t->physical_memory_array_handle = cpu_to_le16(0x1000); /* Type 16 above */
t->memory_error_information_handle = cpu_to_le16(0xFFFE); /* Not provided */
@@ -790,7 +804,7 @@ static void smbios_build_type_19_table(unsigned instance,
{
uint64_t end, start_kb, end_kb;
- SMBIOS_BUILD_TABLE_PRE(19, 0x1300 + instance, true); /* required */
+ SMBIOS_BUILD_TABLE_PRE(19, T19_BASE + instance, true); /* required */
end = start + size - 1;
assert(end > start);
@@ -814,7 +828,7 @@ static void smbios_build_type_19_table(unsigned instance,
static void smbios_build_type_32_table(void)
{
- SMBIOS_BUILD_TABLE_PRE(32, 0x2000, true); /* required */
+ SMBIOS_BUILD_TABLE_PRE(32, T32_BASE, true); /* required */
memset(t->reserved, 0, 6);
t->boot_status = 0; /* No errors detected */
@@ -828,7 +842,7 @@ static void smbios_build_type_41_table(Error **errp)
struct type41_instance *t41;
QTAILQ_FOREACH(t41, &type41, next) {
- SMBIOS_BUILD_TABLE_PRE(41, 0x2900 + instance, true);
+ SMBIOS_BUILD_TABLE_PRE(41, T41_BASE + instance, true);
SMBIOS_TABLE_SET_STR(41, reference_designation_str, t41->designation);
t->device_type = t41->kind;
@@ -871,7 +885,7 @@ static void smbios_build_type_41_table(Error **errp)
static void smbios_build_type_127_table(void)
{
- SMBIOS_BUILD_TABLE_PRE(127, 0x7F00, true); /* required */
+ SMBIOS_BUILD_TABLE_PRE(127, T127_BASE, true); /* required */
SMBIOS_BUILD_TABLE_POST;
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 07/47] hw/smbios: fix overlapping table handle numbers with large memory vms
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (5 preceding siblings ...)
2022-03-07 10:01 ` [PULL v2 06/47] hw/smbios: code cleanup - use macro definitions for table header handles Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 08/47] hw/smbios: add assertion to ensure handles of tables 19 and 32 do not collide Michael S. Tsirkin
` (40 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Ani Sinha, Peter Maydell, Igor Mammedov
From: Ani Sinha <ani@anisinha.ca>
The current smbios table implementation splits the main memory in 16 GiB
(DIMM like) chunks. With the current smbios table assignment code, we can have
only 512 such chunks before the 16 bit handle numbers in the header for tables
17 and 19 conflict. A guest with more than 8 TiB of memory will hit this
limitation and would fail with the following assertion in isa-debugcon:
ASSERT_EFI_ERROR (Status = Already started)
ASSERT /builddir/build/BUILD/edk2-ca407c7246bf/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c(125): !EFI_ERROR (Status)
This change adds an additional offset between tables 17 and 19 handle numbers
when configuring VMs larger than 8 TiB of memory. The value of the offset is
calculated to be equal to the additional space required to be reserved
in order to accomodate more DIMM entries without the table handles colliding.
In normal cases where the VM memory is smaller or equal to 8 TiB, this offset
value is 0. Hence in this case, no additional handle numbers are reserved and
table handle values remain as before.
Since smbios memory is not transmitted over the wire during migration,
this change can break migration for large memory vms if the guest is in the
middle of generating the tables during migration. However, in those
situations, qemu generates invalid table handles anyway with or without this
fix. Hence, we do not preserve the old bug by introducing compat knobs/machine
types.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2023977
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20220223143322.927136-7-ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/smbios/smbios.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 56b412ce35..44c53797a4 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -799,12 +799,13 @@ static void smbios_build_type_17_table(unsigned instance, uint64_t size)
SMBIOS_BUILD_TABLE_POST;
}
-static void smbios_build_type_19_table(unsigned instance,
+static void smbios_build_type_19_table(unsigned instance, unsigned offset,
uint64_t start, uint64_t size)
{
uint64_t end, start_kb, end_kb;
- SMBIOS_BUILD_TABLE_PRE(19, T19_BASE + instance, true); /* required */
+ SMBIOS_BUILD_TABLE_PRE(19, T19_BASE + offset + instance,
+ true); /* required */
end = start + size - 1;
assert(end > start);
@@ -996,7 +997,7 @@ void smbios_get_tables(MachineState *ms,
uint8_t **anchor, size_t *anchor_len,
Error **errp)
{
- unsigned i, dimm_cnt;
+ unsigned i, dimm_cnt, offset;
if (smbios_legacy) {
*tables = *anchor = NULL;
@@ -1026,6 +1027,16 @@ void smbios_get_tables(MachineState *ms,
dimm_cnt = QEMU_ALIGN_UP(current_machine->ram_size, MAX_DIMM_SZ) / MAX_DIMM_SZ;
+ /*
+ * The offset determines if we need to keep additional space betweeen
+ * table 17 and table 19 header handle numbers so that they do
+ * not overlap. For example, for a VM with larger than 8 TB guest
+ * memory and DIMM like chunks of 16 GiB, the default space between
+ * the two tables (T19_BASE - T17_BASE = 512) is not enough.
+ */
+ offset = (dimm_cnt > (T19_BASE - T17_BASE)) ? \
+ dimm_cnt - (T19_BASE - T17_BASE) : 0;
+
smbios_build_type_16_table(dimm_cnt);
for (i = 0; i < dimm_cnt; i++) {
@@ -1033,7 +1044,7 @@ void smbios_get_tables(MachineState *ms,
}
for (i = 0; i < mem_array_size; i++) {
- smbios_build_type_19_table(i, mem_array[i].address,
+ smbios_build_type_19_table(i, offset, mem_array[i].address,
mem_array[i].length);
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 08/47] hw/smbios: add assertion to ensure handles of tables 19 and 32 do not collide
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (6 preceding siblings ...)
2022-03-07 10:01 ` [PULL v2 07/47] hw/smbios: fix overlapping table handle numbers with large memory vms Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 09/47] vhost-user: remove VirtQ notifier restore Michael S. Tsirkin
` (39 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Ani Sinha, Peter Maydell, Igor Mammedov
From: Ani Sinha <ani@anisinha.ca>
Since change dcf359832eec02 ("hw/smbios: fix table memory corruption with large memory vms")
we reserve additional space between handle numbers of tables 17 and 19 for
large VMs. This may cause table 19 to collide with table 32 in their handle
numbers for those large VMs. This change adds an assertion to ensure numbers
do not collide. If they do, qemu crashes with useful debug information for
taking additional steps.
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20220223143322.927136-8-ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/smbios/smbios.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 44c53797a4..1f8d5c252f 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -1048,6 +1048,12 @@ void smbios_get_tables(MachineState *ms,
mem_array[i].length);
}
+ /*
+ * make sure 16 bit handle numbers in the headers of tables 19
+ * and 32 do not overlap.
+ */
+ assert((mem_array_size + offset) < (T32_BASE - T19_BASE));
+
smbios_build_type_32_table();
smbios_build_type_38_table();
smbios_build_type_41_table(errp);
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 09/47] vhost-user: remove VirtQ notifier restore
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (7 preceding siblings ...)
2022-03-07 10:01 ` [PULL v2 08/47] hw/smbios: add assertion to ensure handles of tables 19 and 32 do not collide Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 10/47] vhost-user: fix VirtQ notifier cleanup Michael S. Tsirkin
` (38 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-stable, Xueming Li
From: Xueming Li <xuemingl@nvidia.com>
Notifier set when vhost-user backend asks qemu to mmap an FD and
offset. When vhost-user backend restart or getting killed, VQ notifier
FD and mmap addresses become invalid. After backend restart, MR contains
the invalid address will be restored and fail on notifier access.
On the other hand, qemu should munmap the notifier, release underlying
hardware resources to enable backend restart and allocate hardware
notifier resources correctly.
Qemu shouldn't reference and use resources of disconnected backend.
This patch removes VQ notifier restore, uses the default vhost-user
notifier to avoid invalid address access.
After backend restart, the backend should ask qemu to install a hardware
notifier if needed.
Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers")
Cc: qemu-stable@nongnu.org
Signed-off-by: Xueming Li <xuemingl@nvidia.com>
Message-Id: <20220207071929.527149-2-xuemingl@nvidia.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/virtio/vhost-user.h | 1 -
hw/virtio/vhost-user.c | 19 +------------------
2 files changed, 1 insertion(+), 19 deletions(-)
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index a9abca3288..f6012b2078 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -14,7 +14,6 @@
typedef struct VhostUserHostNotifier {
MemoryRegion mr;
void *addr;
- bool set;
} VhostUserHostNotifier;
typedef struct VhostUserState {
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 662853513e..ebe9bd58d0 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1162,19 +1162,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev,
return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring);
}
-static void vhost_user_host_notifier_restore(struct vhost_dev *dev,
- int queue_idx)
-{
- struct vhost_user *u = dev->opaque;
- VhostUserHostNotifier *n = &u->user->notifier[queue_idx];
- VirtIODevice *vdev = dev->vdev;
-
- if (n->addr && !n->set) {
- virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true);
- n->set = true;
- }
-}
-
static void vhost_user_host_notifier_remove(struct vhost_dev *dev,
int queue_idx)
{
@@ -1182,17 +1169,14 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev,
VhostUserHostNotifier *n = &u->user->notifier[queue_idx];
VirtIODevice *vdev = dev->vdev;
- if (n->addr && n->set) {
+ if (n->addr) {
virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false);
- n->set = false;
}
}
static int vhost_user_set_vring_base(struct vhost_dev *dev,
struct vhost_vring_state *ring)
{
- vhost_user_host_notifier_restore(dev, ring->index);
-
return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
}
@@ -1558,7 +1542,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
}
n->addr = addr;
- n->set = true;
return 0;
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 10/47] vhost-user: fix VirtQ notifier cleanup
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (8 preceding siblings ...)
2022-03-07 10:01 ` [PULL v2 09/47] vhost-user: remove VirtQ notifier restore Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 11/47] virtio: fix the condition for iommu_platform not supported Michael S. Tsirkin
` (37 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-stable, Xueming Li
From: Xueming Li <xuemingl@nvidia.com>
When vhost-user device cleanup, remove notifier MR and munmaps notifier
address in the event-handling thread, VM CPU thread writing the notifier
in concurrent fails with an error of accessing invalid address. It
happens because MR is still being referenced and accessed in another
thread while the underlying notifier mmap address is being freed and
becomes invalid.
This patch calls RCU and munmap notifiers in the callback after the
memory flatview update finish.
Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers")
Cc: qemu-stable@nongnu.org
Signed-off-by: Xueming Li <xuemingl@nvidia.com>
Message-Id: <20220207071929.527149-3-xuemingl@nvidia.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/virtio/vhost-user.h | 2 ++
hw/virtio/vhost-user.c | 48 ++++++++++++++++++++--------------
2 files changed, 31 insertions(+), 19 deletions(-)
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index f6012b2078..e44a41bb70 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -12,8 +12,10 @@
#include "hw/virtio/virtio.h"
typedef struct VhostUserHostNotifier {
+ struct rcu_head rcu;
MemoryRegion mr;
void *addr;
+ void *unmap_addr;
} VhostUserHostNotifier;
typedef struct VhostUserState {
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index ebe9bd58d0..6abbc9da32 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -25,6 +25,7 @@
#include "migration/migration.h"
#include "migration/postcopy-ram.h"
#include "trace.h"
+#include "exec/ramblock.h"
#include <sys/ioctl.h>
#include <sys/socket.h>
@@ -1162,15 +1163,26 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev,
return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring);
}
-static void vhost_user_host_notifier_remove(struct vhost_dev *dev,
- int queue_idx)
+static void vhost_user_host_notifier_free(VhostUserHostNotifier *n)
{
- struct vhost_user *u = dev->opaque;
- VhostUserHostNotifier *n = &u->user->notifier[queue_idx];
- VirtIODevice *vdev = dev->vdev;
+ assert(n && n->unmap_addr);
+ munmap(n->unmap_addr, qemu_real_host_page_size);
+ n->unmap_addr = NULL;
+}
+
+static void vhost_user_host_notifier_remove(VhostUserState *user,
+ VirtIODevice *vdev, int queue_idx)
+{
+ VhostUserHostNotifier *n = &user->notifier[queue_idx];
if (n->addr) {
- virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false);
+ if (vdev) {
+ virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false);
+ }
+ assert(!n->unmap_addr);
+ n->unmap_addr = n->addr;
+ n->addr = NULL;
+ call_rcu(n, vhost_user_host_notifier_free, rcu);
}
}
@@ -1219,8 +1231,9 @@ static int vhost_user_get_vring_base(struct vhost_dev *dev,
.payload.state = *ring,
.hdr.size = sizeof(msg.payload.state),
};
+ struct vhost_user *u = dev->opaque;
- vhost_user_host_notifier_remove(dev, ring->index);
+ vhost_user_host_notifier_remove(u->user, dev->vdev, ring->index);
ret = vhost_user_write(dev, &msg, NULL, 0);
if (ret < 0) {
@@ -1506,12 +1519,7 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
n = &user->notifier[queue_idx];
- if (n->addr) {
- virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false);
- object_unparent(OBJECT(&n->mr));
- munmap(n->addr, page_size);
- n->addr = NULL;
- }
+ vhost_user_host_notifier_remove(user, vdev, queue_idx);
if (area->u64 & VHOST_USER_VRING_NOFD_MASK) {
return 0;
@@ -1530,9 +1538,12 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
name = g_strdup_printf("vhost-user/host-notifier@%p mmaps[%d]",
user, queue_idx);
- if (!n->mr.ram) /* Don't init again after suspend. */
+ if (!n->mr.ram) { /* Don't init again after suspend. */
memory_region_init_ram_device_ptr(&n->mr, OBJECT(vdev), name,
page_size, addr);
+ } else {
+ n->mr.ram_block->host = addr;
+ }
g_free(name);
if (virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true)) {
@@ -2505,17 +2516,16 @@ bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp)
void vhost_user_cleanup(VhostUserState *user)
{
int i;
+ VhostUserHostNotifier *n;
if (!user->chr) {
return;
}
memory_region_transaction_begin();
for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
- if (user->notifier[i].addr) {
- object_unparent(OBJECT(&user->notifier[i].mr));
- munmap(user->notifier[i].addr, qemu_real_host_page_size);
- user->notifier[i].addr = NULL;
- }
+ n = &user->notifier[i];
+ vhost_user_host_notifier_remove(user, NULL, i);
+ object_unparent(OBJECT(&n->mr));
}
memory_region_transaction_commit();
user->chr = NULL;
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 11/47] virtio: fix the condition for iommu_platform not supported
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (9 preceding siblings ...)
2022-03-07 10:01 ` [PULL v2 10/47] vhost-user: fix VirtQ notifier cleanup Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 12/47] hw/vhost-user-i2c: Add support for VIRTIO_I2C_F_ZERO_LENGTH_REQUEST Michael S. Tsirkin
` (36 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Daniel Henrique Barboza, Jason Wang,
Cornelia Huck, qemu-stable, Halil Pasic, Jakob Naucke
From: Halil Pasic <pasic@linux.ibm.com>
The commit 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but
unsupported") claims to fail the device hotplug when iommu_platform
is requested, but not supported by the (vhost) device. On the first
glance the condition for detecting that situation looks perfect, but
because a certain peculiarity of virtio_platform it ain't.
In fact the aforementioned commit introduces a regression. It breaks
virtio-fs support for Secure Execution, and most likely also for AMD SEV
or any other confidential guest scenario that relies encrypted guest
memory. The same also applies to any other vhost device that does not
support _F_ACCESS_PLATFORM.
The peculiarity is that iommu_platform and _F_ACCESS_PLATFORM collates
"device can not access all of the guest RAM" and "iova != gpa, thus
device needs to translate iova".
Confidential guest technologies currently rely on the device/hypervisor
offering _F_ACCESS_PLATFORM, so that, after the feature has been
negotiated, the guest grants access to the portions of memory the
device needs to see. So in for confidential guests, generally,
_F_ACCESS_PLATFORM is about the restricted access to memory, but not
about the addresses used being something else than guest physical
addresses.
This is the very reason for which commit f7ef7e6e3b ("vhost: correctly
turn on VIRTIO_F_IOMMU_PLATFORM") fences _F_ACCESS_PLATFORM from the
vhost device that does not need it, because on the vhost interface it
only means "I/O address translation is needed".
This patch takes inspiration from f7ef7e6e3b ("vhost: correctly turn on
VIRTIO_F_IOMMU_PLATFORM"), and uses the same condition for detecting the
situation when _F_ACCESS_PLATFORM is requested, but no I/O translation
by the device, and thus no device capability is needed. In this
situation claiming that the device does not support iommu_plattform=on
is counter-productive. So let us stop doing that!
Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
Reported-by: Jakob Naucke <Jakob.Naucke@ibm.com>
Fixes: 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but
unsupported")
Acked-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-stable@nongnu.org
Message-Id: <20220207112857.607829-1-pasic@linux.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
hw/virtio/virtio-bus.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index d23db98c56..0f69d1c742 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -48,6 +48,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp)
VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
bool has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+ bool vdev_has_iommu;
Error *local_err = NULL;
DPRINTF("%s: plug device.\n", qbus->name);
@@ -69,11 +70,6 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp)
return;
}
- if (has_iommu && !virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) {
- error_setg(errp, "iommu_platform=true is not supported by the device");
- return;
- }
-
if (klass->device_plugged != NULL) {
klass->device_plugged(qbus->parent, &local_err);
}
@@ -82,9 +78,15 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp)
return;
}
+ vdev_has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
if (klass->get_dma_as != NULL && has_iommu) {
virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM);
vdev->dma_as = klass->get_dma_as(qbus->parent);
+ if (!vdev_has_iommu && vdev->dma_as != &address_space_memory) {
+ error_setg(errp,
+ "iommu_platform=true is not supported by the device");
+ return;
+ }
} else {
vdev->dma_as = &address_space_memory;
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 12/47] hw/vhost-user-i2c: Add support for VIRTIO_I2C_F_ZERO_LENGTH_REQUEST
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (10 preceding siblings ...)
2022-03-07 10:01 ` [PULL v2 11/47] virtio: fix the condition for iommu_platform not supported Michael S. Tsirkin
@ 2022-03-07 10:01 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 13/47] hw/virtio: vdpa: Fix leak of host-notifier memory-region Michael S. Tsirkin
` (35 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Alex Bennée, Viresh Kumar
From: Viresh Kumar <viresh.kumar@linaro.org>
VIRTIO_I2C_F_ZERO_LENGTH_REQUEST is a mandatory feature, that must be
implemented by everyone. Add its support.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Message-Id: <fc47ab63b1cd414319c9201e8d6c7705b5ec3bd9.1644490993.git.viresh.kumar@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/virtio/vhost-user-i2c.h | 3 +++
hw/virtio/vhost-user-i2c.c | 11 +++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/include/hw/virtio/vhost-user-i2c.h b/include/hw/virtio/vhost-user-i2c.h
index deae47a76d..d8372f3b43 100644
--- a/include/hw/virtio/vhost-user-i2c.h
+++ b/include/hw/virtio/vhost-user-i2c.h
@@ -25,4 +25,7 @@ struct VHostUserI2C {
bool connected;
};
+/* Virtio Feature bits */
+#define VIRTIO_I2C_F_ZERO_LENGTH_REQUEST 0
+
#endif /* _QEMU_VHOST_USER_I2C_H */
diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c
index d172632bb0..42c7f6d9e5 100644
--- a/hw/virtio/vhost-user-i2c.c
+++ b/hw/virtio/vhost-user-i2c.c
@@ -19,6 +19,11 @@
#define VIRTIO_ID_I2C_ADAPTER 34
#endif
+static const int feature_bits[] = {
+ VIRTIO_I2C_F_ZERO_LENGTH_REQUEST,
+ VHOST_INVALID_FEATURE_BIT
+};
+
static void vu_i2c_start(VirtIODevice *vdev)
{
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
@@ -113,8 +118,10 @@ static void vu_i2c_set_status(VirtIODevice *vdev, uint8_t status)
static uint64_t vu_i2c_get_features(VirtIODevice *vdev,
uint64_t requested_features, Error **errp)
{
- /* No feature bits used yet */
- return requested_features;
+ VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
+
+ virtio_add_feature(&requested_features, VIRTIO_I2C_F_ZERO_LENGTH_REQUEST);
+ return vhost_get_features(&i2c->vhost_dev, feature_bits, requested_features);
}
static void vu_i2c_handle_output(VirtIODevice *vdev, VirtQueue *vq)
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 13/47] hw/virtio: vdpa: Fix leak of host-notifier memory-region
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (11 preceding siblings ...)
2022-03-07 10:01 ` [PULL v2 12/47] hw/vhost-user-i2c: Add support for VIRTIO_I2C_F_ZERO_LENGTH_REQUEST Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 14/47] vhost-vdpa: make notifiers _init()/_uninit() symmetric Michael S. Tsirkin
` (34 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Peter Maydell, jasowang, qemu-stable, Stefano Garzarella
From: Laurent Vivier <lvivier@redhat.com>
If call virtio_queue_set_host_notifier_mr fails, should free
host-notifier memory-region.
This problem can trigger a coredump with some vDPA drivers (mlx5,
but not with the vdpasim), if we unplug the virtio-net card from
the guest after a stop/start.
The same fix has been done for vhost-user:
1f89d3b91e3e ("hw/virtio: Fix leak of host-notifier memory-region")
Fixes: d0416d487bd5 ("vhost-vdpa: map virtqueue notification area if possible")
Cc: jasowang@redhat.com
Resolves: https://bugzilla.redhat.com/2027208
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Message-Id: <20220211170259.1388734-1-lvivier@redhat.com>
Cc: qemu-stable@nongnu.org
Acked-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/vhost-vdpa.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 04ea43704f..11f696468d 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -431,6 +431,7 @@ static int vhost_vdpa_host_notifier_init(struct vhost_dev *dev, int queue_index)
g_free(name);
if (virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, true)) {
+ object_unparent(OBJECT(&n->mr));
munmap(addr, page_size);
goto err;
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 14/47] vhost-vdpa: make notifiers _init()/_uninit() symmetric
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (12 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 13/47] hw/virtio: vdpa: Fix leak of host-notifier memory-region Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 15/47] intel_iommu: support snoop control Michael S. Tsirkin
` (33 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Laurent Vivier, Peter Maydell, jasowang, Stefano Garzarella
From: Laurent Vivier <lvivier@redhat.com>
vhost_vdpa_host_notifiers_init() initializes queue notifiers
for queues "dev->vq_index" to queue "dev->vq_index + dev->nvqs",
whereas vhost_vdpa_host_notifiers_uninit() uninitializes the
same notifiers for queue "0" to queue "dev->nvqs".
This asymmetry seems buggy, fix that by using dev->vq_index
as the base for both.
Fixes: d0416d487bd5 ("vhost-vdpa: map virtqueue notification area if possible")
Cc: jasowang@redhat.com
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Message-Id: <20220211161309.1385839-1-lvivier@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/vhost-vdpa.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 11f696468d..6c67d5f034 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -395,15 +395,6 @@ static void vhost_vdpa_host_notifier_uninit(struct vhost_dev *dev,
}
}
-static void vhost_vdpa_host_notifiers_uninit(struct vhost_dev *dev, int n)
-{
- int i;
-
- for (i = 0; i < n; i++) {
- vhost_vdpa_host_notifier_uninit(dev, i);
- }
-}
-
static int vhost_vdpa_host_notifier_init(struct vhost_dev *dev, int queue_index)
{
size_t page_size = qemu_real_host_page_size;
@@ -443,6 +434,15 @@ err:
return -1;
}
+static void vhost_vdpa_host_notifiers_uninit(struct vhost_dev *dev, int n)
+{
+ int i;
+
+ for (i = dev->vq_index; i < dev->vq_index + n; i++) {
+ vhost_vdpa_host_notifier_uninit(dev, i);
+ }
+}
+
static void vhost_vdpa_host_notifiers_init(struct vhost_dev *dev)
{
int i;
@@ -456,7 +456,7 @@ static void vhost_vdpa_host_notifiers_init(struct vhost_dev *dev)
return;
err:
- vhost_vdpa_host_notifiers_uninit(dev, i);
+ vhost_vdpa_host_notifiers_uninit(dev, i - dev->vq_index);
return;
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 15/47] intel_iommu: support snoop control
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (13 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 14/47] vhost-vdpa: make notifiers _init()/_uninit() symmetric Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 16/47] hw/i386: Improve bounds checking in OVMF table parsing Michael S. Tsirkin
` (32 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Peter Maydell, Jason Wang, Richard Henderson,
Peter Xu, Paolo Bonzini
From: Jason Wang <jasowang@redhat.com>
SC is required for some kernel features like vhost-vDPA. So this patch
implements basic SC feature. The idea is pretty simple, for software
emulated DMA it would be always coherent. In this case we can simple
advertise ECAP_SC bit. For VFIO and vhost, thing will be more much
complicated, so this patch simply fail the IOMMU notifier
registration.
In the future, we may want to have a dedicated notifiers flag or
similar mechanism to demonstrate the coherency so VFIO could advertise
that if it has VFIO_DMA_CC_IOMMU, for vhost kernel backend we don't
need that since it's a software backend.
Signed-off-by: Jason Wang <jasowang@redhat.com>
Message-Id: <20220214060346.72455-1-jasowang@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/i386/intel_iommu_internal.h | 1 +
include/hw/i386/intel_iommu.h | 1 +
hw/i386/intel_iommu.c | 14 +++++++++++++-
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index a6c788049b..1ff13b40f9 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -188,6 +188,7 @@
#define VTD_ECAP_IR (1ULL << 3)
#define VTD_ECAP_EIM (1ULL << 4)
#define VTD_ECAP_PT (1ULL << 6)
+#define VTD_ECAP_SC (1ULL << 7)
#define VTD_ECAP_MHMV (15ULL << 20)
#define VTD_ECAP_SRS (1ULL << 31)
#define VTD_ECAP_SMTS (1ULL << 43)
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 41783ee46d..3b5ac869db 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -228,6 +228,7 @@ struct IntelIOMMUState {
bool caching_mode; /* RO - is cap CM enabled? */
bool scalable_mode; /* RO - is Scalable Mode supported? */
+ bool snoop_control; /* RO - is SNP filed supported? */
dma_addr_t root; /* Current root table pointer */
bool root_scalable; /* Type of root table (scalable or not) */
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 4c6c016388..32471a44cb 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3030,6 +3030,13 @@ static int vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
IntelIOMMUState *s = vtd_as->iommu_state;
+ /* TODO: add support for VFIO and vhost users */
+ if (s->snoop_control) {
+ error_setg_errno(errp, -ENOTSUP,
+ "Snoop Control with vhost or VFIO is not supported");
+ return -ENOTSUP;
+ }
+
/* Update per-address-space notifier flags */
vtd_as->notifier_flags = new;
@@ -3113,6 +3120,7 @@ static Property vtd_properties[] = {
VTD_HOST_ADDRESS_WIDTH),
DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, FALSE),
DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode, FALSE),
+ DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState, snoop_control, false),
DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
DEFINE_PROP_END_OF_LIST(),
};
@@ -3643,7 +3651,7 @@ static void vtd_init(IntelIOMMUState *s)
vtd_spte_rsvd_large[3] = VTD_SPTE_LPAGE_L3_RSVD_MASK(s->aw_bits,
x86_iommu->dt_supported);
- if (s->scalable_mode) {
+ if (s->scalable_mode || s->snoop_control) {
vtd_spte_rsvd[1] &= ~VTD_SPTE_SNP;
vtd_spte_rsvd_large[2] &= ~VTD_SPTE_SNP;
vtd_spte_rsvd_large[3] &= ~VTD_SPTE_SNP;
@@ -3674,6 +3682,10 @@ static void vtd_init(IntelIOMMUState *s)
s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
}
+ if (s->snoop_control) {
+ s->ecap |= VTD_ECAP_SC;
+ }
+
vtd_reset_caches(s);
/* Define registers with default values and bit semantics */
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 16/47] hw/i386: Improve bounds checking in OVMF table parsing
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (14 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 15/47] intel_iommu: support snoop control Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 17/47] hw/i386: Replace magic number with field length calculation Michael S. Tsirkin
` (31 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Peter Maydell, Daniel P . Berrangé,
Richard Henderson, Dr . David Alan Gilbert,
Philippe Mathieu-Daudé,
Dov Murik, Gerd Hoffmann, Paolo Bonzini
From: Dov Murik <dovmurik@linux.ibm.com>
When pc_system_parse_ovmf_flash() parses the optional GUIDed table in
the end of the OVMF flash memory area, the table length field is checked
for sizes that are too small, but doesn't error on sizes that are too
big (bigger than the flash content itself).
Add a check for maximal size of the OVMF table, and add an error report
in case the size is invalid. In such a case, an error like this will be
displayed during launch:
qemu-system-x86_64: OVMF table has invalid size 4047
and the table parsing is skipped.
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Message-Id: <20220222071906.2632426-2-dovmurik@linux.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hw/i386/pc_sysfw_ovmf.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/i386/pc_sysfw_ovmf.c b/hw/i386/pc_sysfw_ovmf.c
index f4dd92c588..df15c9737b 100644
--- a/hw/i386/pc_sysfw_ovmf.c
+++ b/hw/i386/pc_sysfw_ovmf.c
@@ -24,6 +24,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "hw/i386/pc.h"
#include "cpu.h"
@@ -66,7 +67,13 @@ void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
ptr -= sizeof(uint16_t);
tot_len = le16_to_cpu(*(uint16_t *)ptr) - sizeof(guid) - sizeof(uint16_t);
- if (tot_len <= 0) {
+ if (tot_len < 0 || tot_len > (ptr - flash_ptr)) {
+ error_report("OVMF table has invalid size %d", tot_len);
+ return;
+ }
+
+ if (tot_len == 0) {
+ /* no entries in the OVMF table */
return;
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 17/47] hw/i386: Replace magic number with field length calculation
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (15 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 16/47] hw/i386: Improve bounds checking in OVMF table parsing Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 18/47] virtio-iommu: Default to bypass during boot Michael S. Tsirkin
` (30 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Peter Maydell, Daniel P . Berrangé,
Richard Henderson, Philippe Mathieu-Daudé,
Dov Murik, Gerd Hoffmann, Paolo Bonzini
From: Dov Murik <dovmurik@linux.ibm.com>
Replce the literal magic number 48 with length calculation (32 bytes at
the end of the firmware after the table footer + 16 bytes of the OVMF
table footer GUID).
No functional change intended.
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Message-Id: <20220222071906.2632426-3-dovmurik@linux.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
hw/i386/pc_sysfw_ovmf.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc_sysfw_ovmf.c b/hw/i386/pc_sysfw_ovmf.c
index df15c9737b..07a4c267fa 100644
--- a/hw/i386/pc_sysfw_ovmf.c
+++ b/hw/i386/pc_sysfw_ovmf.c
@@ -30,6 +30,8 @@
#define OVMF_TABLE_FOOTER_GUID "96b582de-1fb2-45f7-baea-a366c55a082d"
+static const int bytes_after_table_footer = 32;
+
static bool ovmf_flash_parsed;
static uint8_t *ovmf_table;
static int ovmf_table_len;
@@ -53,12 +55,13 @@ void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
/*
* if this is OVMF there will be a table footer
- * guid 48 bytes before the end of the flash file. If it's
- * not found, silently abort the flash parsing.
+ * guid 48 bytes before the end of the flash file
+ * (= 32 bytes after the table + 16 bytes the GUID itself).
+ * If it's not found, silently abort the flash parsing.
*/
qemu_uuid_parse(OVMF_TABLE_FOOTER_GUID, &guid);
guid = qemu_uuid_bswap(guid); /* guids are LE */
- ptr = flash_ptr + flash_size - 48;
+ ptr = flash_ptr + flash_size - (bytes_after_table_footer + sizeof(guid));
if (!qemu_uuid_is_equal((QemuUUID *)ptr, &guid)) {
return;
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 18/47] virtio-iommu: Default to bypass during boot
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (16 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 17/47] hw/i386: Replace magic number with field length calculation Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 19/47] virtio-iommu: Support bypass domain Michael S. Tsirkin
` (29 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Cornelia Huck, Eric Auger, Jean-Philippe Brucker
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
Currently the virtio-iommu device must be programmed before it allows
DMA from any PCI device. This can make the VM entirely unusable when a
virtio-iommu driver isn't present, for example in a bootloader that
loads the OS from storage.
Similarly to the other vIOMMU implementations, default to DMA bypassing
the IOMMU during boot. Add a "boot-bypass" property, defaulting to true,
that lets users change this behavior.
Replace the VIRTIO_IOMMU_F_BYPASS feature, which didn't support bypass
before feature negotiation, with VIRTIO_IOMMU_F_BYPASS_CONFIG.
We add the bypass field to the migration stream without introducing
subsections, based on the assumption that this virtio-iommu device isn't
being used in production enough to require cross-version migration at
the moment (all previous version required workarounds since they didn't
support ACPI and boot-bypass).
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Message-Id: <20220214124356.872985-3-jean-philippe@linaro.org>
Acked-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/virtio/virtio-iommu.h | 1 +
hw/virtio/virtio-iommu.c | 60 +++++++++++++++++++++++++++-----
hw/virtio/trace-events | 4 ++-
3 files changed, 56 insertions(+), 9 deletions(-)
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index e2339e5b72..84391f8448 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -58,6 +58,7 @@ struct VirtIOIOMMU {
GTree *domains;
QemuMutex mutex;
GTree *endpoints;
+ bool boot_bypass;
};
#endif
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index aa9c16a17b..4ca36db4ac 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -24,6 +24,7 @@
#include "hw/qdev-properties.h"
#include "hw/virtio/virtio.h"
#include "sysemu/kvm.h"
+#include "sysemu/reset.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "trace.h"
@@ -728,8 +729,7 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
.perm = IOMMU_NONE,
};
- bypass_allowed = virtio_vdev_has_feature(&s->parent_obj,
- VIRTIO_IOMMU_F_BYPASS);
+ bypass_allowed = s->config.bypass;
sid = virtio_iommu_get_bdf(sdev);
@@ -831,13 +831,37 @@ static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
out_config->domain_range.start = cpu_to_le32(dev_config->domain_range.start);
out_config->domain_range.end = cpu_to_le32(dev_config->domain_range.end);
out_config->probe_size = cpu_to_le32(dev_config->probe_size);
+ out_config->bypass = dev_config->bypass;
trace_virtio_iommu_get_config(dev_config->page_size_mask,
dev_config->input_range.start,
dev_config->input_range.end,
dev_config->domain_range.start,
dev_config->domain_range.end,
- dev_config->probe_size);
+ dev_config->probe_size,
+ dev_config->bypass);
+}
+
+static void virtio_iommu_set_config(VirtIODevice *vdev,
+ const uint8_t *config_data)
+{
+ VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
+ struct virtio_iommu_config *dev_config = &dev->config;
+ const struct virtio_iommu_config *in_config = (void *)config_data;
+
+ if (in_config->bypass != dev_config->bypass) {
+ if (!virtio_vdev_has_feature(vdev, VIRTIO_IOMMU_F_BYPASS_CONFIG)) {
+ virtio_error(vdev, "cannot set config.bypass");
+ return;
+ } else if (in_config->bypass != 0 && in_config->bypass != 1) {
+ virtio_error(vdev, "invalid config.bypass value '%u'",
+ in_config->bypass);
+ return;
+ }
+ dev_config->bypass = in_config->bypass;
+ }
+
+ trace_virtio_iommu_set_config(in_config->bypass);
}
static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
@@ -963,6 +987,19 @@ static int virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
return 0;
}
+static void virtio_iommu_system_reset(void *opaque)
+{
+ VirtIOIOMMU *s = opaque;
+
+ trace_virtio_iommu_system_reset();
+
+ /*
+ * config.bypass is sticky across device reset, but should be restored on
+ * system reset
+ */
+ s->config.bypass = s->boot_bypass;
+}
+
static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
{
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -988,9 +1025,9 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
virtio_add_feature(&s->features, VIRTIO_IOMMU_F_INPUT_RANGE);
virtio_add_feature(&s->features, VIRTIO_IOMMU_F_DOMAIN_RANGE);
virtio_add_feature(&s->features, VIRTIO_IOMMU_F_MAP_UNMAP);
- virtio_add_feature(&s->features, VIRTIO_IOMMU_F_BYPASS);
virtio_add_feature(&s->features, VIRTIO_IOMMU_F_MMIO);
virtio_add_feature(&s->features, VIRTIO_IOMMU_F_PROBE);
+ virtio_add_feature(&s->features, VIRTIO_IOMMU_F_BYPASS_CONFIG);
qemu_mutex_init(&s->mutex);
@@ -1001,6 +1038,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
} else {
error_setg(errp, "VIRTIO-IOMMU is not attached to any PCI bus!");
}
+
+ qemu_register_reset(virtio_iommu_system_reset, s);
}
static void virtio_iommu_device_unrealize(DeviceState *dev)
@@ -1008,6 +1047,8 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
+ qemu_unregister_reset(virtio_iommu_system_reset, s);
+
g_hash_table_destroy(s->as_by_busptr);
if (s->domains) {
g_tree_destroy(s->domains);
@@ -1141,21 +1182,22 @@ static int iommu_post_load(void *opaque, int version_id)
static const VMStateDescription vmstate_virtio_iommu_device = {
.name = "virtio-iommu-device",
- .minimum_version_id = 1,
- .version_id = 1,
+ .minimum_version_id = 2,
+ .version_id = 2,
.post_load = iommu_post_load,
.fields = (VMStateField[]) {
VMSTATE_GTREE_DIRECT_KEY_V(domains, VirtIOIOMMU, 1,
&vmstate_domain, VirtIOIOMMUDomain),
+ VMSTATE_UINT8_V(config.bypass, VirtIOIOMMU, 2),
VMSTATE_END_OF_LIST()
},
};
static const VMStateDescription vmstate_virtio_iommu = {
.name = "virtio-iommu",
- .minimum_version_id = 1,
+ .minimum_version_id = 2,
.priority = MIG_PRI_IOMMU,
- .version_id = 1,
+ .version_id = 2,
.fields = (VMStateField[]) {
VMSTATE_VIRTIO_DEVICE,
VMSTATE_END_OF_LIST()
@@ -1164,6 +1206,7 @@ static const VMStateDescription vmstate_virtio_iommu = {
static Property virtio_iommu_properties[] = {
DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus, "PCI", PCIBus *),
+ DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
DEFINE_PROP_END_OF_LIST(),
};
@@ -1180,6 +1223,7 @@ static void virtio_iommu_class_init(ObjectClass *klass, void *data)
vdc->unrealize = virtio_iommu_device_unrealize;
vdc->reset = virtio_iommu_device_reset;
vdc->get_config = virtio_iommu_get_config;
+ vdc->set_config = virtio_iommu_set_config;
vdc->get_features = virtio_iommu_get_features;
vdc->set_status = virtio_iommu_set_status;
vdc->vmsd = &vmstate_virtio_iommu_device;
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index f7ad6be5fb..a5102eac9e 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -89,9 +89,11 @@ virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d"
# virtio-iommu.c
virtio_iommu_device_reset(void) "reset!"
+virtio_iommu_system_reset(void) "system reset!"
virtio_iommu_get_features(uint64_t features) "device supports features=0x%"PRIx64
virtio_iommu_device_status(uint8_t status) "driver status = %d"
-virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_start, uint32_t domain_end, uint32_t probe_size) "page_size_mask=0x%"PRIx64" input range start=0x%"PRIx64" input range end=0x%"PRIx64" domain range start=%d domain range end=%d probe_size=0x%x"
+virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_start, uint32_t domain_end, uint32_t probe_size, uint8_t bypass) "page_size_mask=0x%"PRIx64" input range start=0x%"PRIx64" input range end=0x%"PRIx64" domain range start=%d domain range end=%d probe_size=0x%x bypass=0x%x"
+virtio_iommu_set_config(uint8_t bypass) "bypass=0x%x"
virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 19/47] virtio-iommu: Support bypass domain
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (17 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 18/47] virtio-iommu: Default to bypass during boot Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 20/47] tests/qtest/virtio-iommu-test: Check bypass config Michael S. Tsirkin
` (28 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Cornelia Huck, Eric Auger, Jean-Philippe Brucker
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
The driver can create a bypass domain by passing the
VIRTIO_IOMMU_ATTACH_F_BYPASS flag on the ATTACH request. Bypass domains
perform slightly better than domains with identity mappings since they
skip translation.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Message-Id: <20220214124356.872985-4-jean-philippe@linaro.org>
Acked-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/virtio-iommu.c | 39 ++++++++++++++++++++++++++++++++++-----
1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 4ca36db4ac..239fe97b12 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -43,6 +43,7 @@
typedef struct VirtIOIOMMUDomain {
uint32_t id;
+ bool bypass;
GTree *mappings;
QLIST_HEAD(, VirtIOIOMMUEndpoint) endpoint_list;
} VirtIOIOMMUDomain;
@@ -258,12 +259,16 @@ static void virtio_iommu_put_endpoint(gpointer data)
}
static VirtIOIOMMUDomain *virtio_iommu_get_domain(VirtIOIOMMU *s,
- uint32_t domain_id)
+ uint32_t domain_id,
+ bool bypass)
{
VirtIOIOMMUDomain *domain;
domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
if (domain) {
+ if (domain->bypass != bypass) {
+ return NULL;
+ }
return domain;
}
domain = g_malloc0(sizeof(*domain));
@@ -271,6 +276,7 @@ static VirtIOIOMMUDomain *virtio_iommu_get_domain(VirtIOIOMMU *s,
domain->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp,
NULL, (GDestroyNotify)g_free,
(GDestroyNotify)g_free);
+ domain->bypass = bypass;
g_tree_insert(s->domains, GUINT_TO_POINTER(domain_id), domain);
QLIST_INIT(&domain->endpoint_list);
trace_virtio_iommu_get_domain(domain_id);
@@ -334,11 +340,16 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
{
uint32_t domain_id = le32_to_cpu(req->domain);
uint32_t ep_id = le32_to_cpu(req->endpoint);
+ uint32_t flags = le32_to_cpu(req->flags);
VirtIOIOMMUDomain *domain;
VirtIOIOMMUEndpoint *ep;
trace_virtio_iommu_attach(domain_id, ep_id);
+ if (flags & ~VIRTIO_IOMMU_ATTACH_F_BYPASS) {
+ return VIRTIO_IOMMU_S_INVAL;
+ }
+
ep = virtio_iommu_get_endpoint(s, ep_id);
if (!ep) {
return VIRTIO_IOMMU_S_NOENT;
@@ -356,7 +367,12 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
}
}
- domain = virtio_iommu_get_domain(s, domain_id);
+ domain = virtio_iommu_get_domain(s, domain_id,
+ flags & VIRTIO_IOMMU_ATTACH_F_BYPASS);
+ if (!domain) {
+ /* Incompatible bypass flag */
+ return VIRTIO_IOMMU_S_INVAL;
+ }
QLIST_INSERT_HEAD(&domain->endpoint_list, ep, next);
ep->domain = domain;
@@ -419,6 +435,10 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
return VIRTIO_IOMMU_S_NOENT;
}
+ if (domain->bypass) {
+ return VIRTIO_IOMMU_S_INVAL;
+ }
+
interval = g_malloc0(sizeof(*interval));
interval->low = virt_start;
@@ -464,6 +484,11 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
if (!domain) {
return VIRTIO_IOMMU_S_NOENT;
}
+
+ if (domain->bypass) {
+ return VIRTIO_IOMMU_S_INVAL;
+ }
+
interval.low = virt_start;
interval.high = virt_end;
@@ -780,6 +805,9 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
entry.perm = flag;
}
goto unlock;
+ } else if (ep->domain->bypass) {
+ entry.perm = flag;
+ goto unlock;
}
found = g_tree_lookup_extended(ep->domain->mappings, (gpointer)(&interval),
@@ -1139,8 +1167,8 @@ static const VMStateDescription vmstate_endpoint = {
static const VMStateDescription vmstate_domain = {
.name = "domain",
- .version_id = 1,
- .minimum_version_id = 1,
+ .version_id = 2,
+ .minimum_version_id = 2,
.pre_load = domain_preload,
.fields = (VMStateField[]) {
VMSTATE_UINT32(id, VirtIOIOMMUDomain),
@@ -1149,6 +1177,7 @@ static const VMStateDescription vmstate_domain = {
VirtIOIOMMUInterval, VirtIOIOMMUMapping),
VMSTATE_QLIST_V(endpoint_list, VirtIOIOMMUDomain, 1,
vmstate_endpoint, VirtIOIOMMUEndpoint, next),
+ VMSTATE_BOOL_V(bypass, VirtIOIOMMUDomain, 2),
VMSTATE_END_OF_LIST()
}
};
@@ -1186,7 +1215,7 @@ static const VMStateDescription vmstate_virtio_iommu_device = {
.version_id = 2,
.post_load = iommu_post_load,
.fields = (VMStateField[]) {
- VMSTATE_GTREE_DIRECT_KEY_V(domains, VirtIOIOMMU, 1,
+ VMSTATE_GTREE_DIRECT_KEY_V(domains, VirtIOIOMMU, 2,
&vmstate_domain, VirtIOIOMMUDomain),
VMSTATE_UINT8_V(config.bypass, VirtIOIOMMU, 2),
VMSTATE_END_OF_LIST()
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 20/47] tests/qtest/virtio-iommu-test: Check bypass config
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (18 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 19/47] virtio-iommu: Support bypass domain Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 21/47] hw/i386/pc_piix: Mark the machine types from version 1.4 to 1.7 as deprecated Michael S. Tsirkin
` (27 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Peter Maydell, Thomas Huth,
Jean-Philippe Brucker, Cornelia Huck, Eric Auger, Paolo Bonzini
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
The bypass config field should be initialized to 1 by default.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Message-Id: <20220214124356.872985-5-jean-philippe@linaro.org>
Acked-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Thomas Huth <thuth@redhat.com>
---
tests/qtest/virtio-iommu-test.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/qtest/virtio-iommu-test.c b/tests/qtest/virtio-iommu-test.c
index 47e68388a0..068e7a9e6c 100644
--- a/tests/qtest/virtio-iommu-test.c
+++ b/tests/qtest/virtio-iommu-test.c
@@ -31,11 +31,13 @@ static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
uint64_t input_range_end = qvirtio_config_readq(dev, 16);
uint32_t domain_range_start = qvirtio_config_readl(dev, 24);
uint32_t domain_range_end = qvirtio_config_readl(dev, 28);
+ uint8_t bypass = qvirtio_config_readb(dev, 36);
g_assert_cmpint(input_range_start, ==, 0);
g_assert_cmphex(input_range_end, ==, UINT64_MAX);
g_assert_cmpint(domain_range_start, ==, 0);
g_assert_cmpint(domain_range_end, ==, UINT32_MAX);
+ g_assert_cmpint(bypass, ==, 1);
}
static int read_tail_status(struct virtio_iommu_req_tail *buffer)
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 21/47] hw/i386/pc_piix: Mark the machine types from version 1.4 to 1.7 as deprecated
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (19 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 20/47] tests/qtest/virtio-iommu-test: Check bypass config Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 22/47] hw/pci-bridge/pxb: Fix missing swizzle Michael S. Tsirkin
` (26 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Peter Maydell, Thomas Huth, libvir-list,
Richard Henderson, Paolo Bonzini
From: Thomas Huth <thuth@redhat.com>
The list of machine types grows larger and larger each release ... and
it is unlikely that many people still use the very old ones for live
migration. QEMU v1.7 has been released more than 8 years ago, so most
people should have updated their machines to a newer version in those
8 years at least once. Thus let's mark the very old 1.x machine types
as deprecated now.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20220117191639.278497-1-thuth@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/i386/pc_piix.c | 1 +
docs/about/deprecated.rst | 8 ++++++++
2 files changed, 9 insertions(+)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8d33cf689d..b72c03d0a6 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -757,6 +757,7 @@ static void pc_i440fx_1_7_machine_options(MachineClass *m)
m->hw_version = "1.7.0";
m->default_machine_opts = NULL;
m->option_rom_has_mr = true;
+ m->deprecation_reason = "old and unattended - use a newer version instead";
compat_props_add(m->compat_props, pc_compat_1_7, pc_compat_1_7_len);
pcmc->smbios_defaults = false;
pcmc->gigabyte_align = false;
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 85773db631..cf02ef6821 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -324,6 +324,14 @@ machine is hardly emulated at all (e.g. neither the LCD nor the USB part had
been implemented), so there is not much value added by this board. Use the
``ref405ep`` machine instead.
+``pc-i440fx-1.4`` up to ``pc-i440fx-1.7`` (since 7.0)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+These old machine types are quite neglected nowadays and thus might have
+various pitfalls with regards to live migration. Use a newer machine type
+instead.
+
+
Backend options
---------------
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 22/47] hw/pci-bridge/pxb: Fix missing swizzle
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (20 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 21/47] hw/i386/pc_piix: Mark the machine types from version 1.4 to 1.7 as deprecated Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 23/47] virtio-net: Unlimit tx queue size if peer is vdpa Michael S. Tsirkin
` (25 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Jonathan Cameron
From: Jonathan Cameron <jonathan.cameron@huawei.com>
pxb_map_irq_fn() handled the necessary removal of the swizzle
applied to the PXB interrupts by the bus to which it was attached
but neglected to apply the normal swizzle for PCI root ports
on the expander bridge.
Result of this was on ARM virt, the PME interrupts for a second
RP on a PXB instance were miss-routed to #45 rather than #46.
Tested with a selection of different configurations with 1 to 5
RP per PXB instance. Note on my x86 test setup the PME interrupts
are not triggered so I haven't been able to test this.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Message-Id: <20220118174855.19325-1-Jonathan.Cameron@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/pci-bridge/pci_expander_bridge.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 10e6e7c2ab..de932286b5 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -192,6 +192,12 @@ static int pxb_map_irq_fn(PCIDevice *pci_dev, int pin)
{
PCIDevice *pxb = pci_get_bus(pci_dev)->parent_dev;
+ /*
+ * First carry out normal swizzle to handle
+ * multple root ports on a pxb instance.
+ */
+ pin = pci_swizzle_map_irq_fn(pci_dev, pin);
+
/*
* The bios does not index the pxb slot number when
* it computes the IRQ because it resides on bus 0
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 23/47] virtio-net: Unlimit tx queue size if peer is vdpa
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (21 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 22/47] hw/pci-bridge/pxb: Fix missing swizzle Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 24/47] pcie: Add support for Single Root I/O Virtualization (SR/IOV) Michael S. Tsirkin
` (24 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Jason Wang, Eugenio Pérez
From: Eugenio Pérez <eperezma@redhat.com>
The code used to limit the maximum size of tx queue for others backends
than vhost_user since the introduction of configurable tx queue size in
9b02e1618cf2 ("virtio-net: enable configurable tx queue size").
As vhost_user, vhost_vdpa devices should deal with memory region
crosses already, so let's use the full tx size.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
Message-Id: <20220217175029.2517071-1-eperezma@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/net/virtio-net.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index cf8ab0f8af..b02a0632df 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -628,17 +628,20 @@ static int virtio_net_max_tx_queue_size(VirtIONet *n)
NetClientState *peer = n->nic_conf.peers.ncs[0];
/*
- * Backends other than vhost-user don't support max queue size.
+ * Backends other than vhost-user or vhost-vdpa don't support max queue
+ * size.
*/
if (!peer) {
return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE;
}
- if (peer->info->type != NET_CLIENT_DRIVER_VHOST_USER) {
+ switch(peer->info->type) {
+ case NET_CLIENT_DRIVER_VHOST_USER:
+ case NET_CLIENT_DRIVER_VHOST_VDPA:
+ return VIRTQUEUE_MAX_SIZE;
+ default:
return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE;
- }
-
- return VIRTQUEUE_MAX_SIZE;
+ };
}
static int peer_attach(VirtIONet *n, int index)
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 24/47] pcie: Add support for Single Root I/O Virtualization (SR/IOV)
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (22 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 23/47] virtio-net: Unlimit tx queue size if peer is vdpa Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 25/47] pcie: Add some SR/IOV API documentation in docs/pcie_sriov.txt Michael S. Tsirkin
` (23 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Knut Omang, Knut Omang
From: Knut Omang <knut.omang@oracle.com>
This patch provides the building blocks for creating an SR/IOV
PCIe Extended Capability header and register/unregister
SR/IOV Virtual Functions.
Signed-off-by: Knut Omang <knuto@ifi.uio.no>
Message-Id: <20220217174504.1051716-2-lukasz.maniak@linux.intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/pci/pci.h | 12 +-
include/hw/pci/pcie.h | 6 +
include/hw/pci/pcie_sriov.h | 71 +++++++++
include/qemu/typedefs.h | 2 +
hw/pci/pci.c | 100 +++++++++---
hw/pci/pcie.c | 5 +
hw/pci/pcie_sriov.c | 294 ++++++++++++++++++++++++++++++++++++
hw/pci/meson.build | 1 +
hw/pci/trace-events | 5 +
9 files changed, 470 insertions(+), 26 deletions(-)
create mode 100644 include/hw/pci/pcie_sriov.h
create mode 100644 hw/pci/pcie_sriov.c
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index c3f3c90473..3a32b8dd40 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -7,9 +7,6 @@
/* PCI includes legacy ISA access. */
#include "hw/isa/isa.h"
-#include "hw/pci/pcie.h"
-#include "qom/object.h"
-
extern bool pci_available;
/* PCI bus */
@@ -157,6 +154,7 @@ enum {
#define QEMU_PCI_VGA_IO_HI_SIZE 0x20
#include "hw/pci/pci_regs.h"
+#include "hw/pci/pcie.h"
/* PCI HEADER_TYPE */
#define PCI_HEADER_TYPE_MULTI_FUNCTION 0x80
@@ -499,6 +497,9 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
AddressSpace *pci_device_iommu_address_space(PCIDevice *dev);
void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque);
+pcibus_t pci_bar_address(PCIDevice *d,
+ int reg, uint8_t type, pcibus_t size);
+
static inline void
pci_set_byte(uint8_t *config, uint8_t val)
{
@@ -779,6 +780,11 @@ static inline int pci_is_express_downstream_port(const PCIDevice *d)
return type == PCI_EXP_TYPE_DOWNSTREAM || type == PCI_EXP_TYPE_ROOT_PORT;
}
+static inline int pci_is_vf(const PCIDevice *d)
+{
+ return d->exp.sriov_vf.pf != NULL;
+}
+
static inline uint32_t pci_config_size(const PCIDevice *d)
{
return pci_is_express(d) ? PCIE_CONFIG_SPACE_SIZE : PCI_CONFIG_SPACE_SIZE;
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index 6063bee0ec..168950a83b 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -24,6 +24,7 @@
#include "hw/pci/pci_regs.h"
#include "hw/pci/pcie_regs.h"
#include "hw/pci/pcie_aer.h"
+#include "hw/pci/pcie_sriov.h"
#include "hw/hotplug.h"
typedef enum {
@@ -81,6 +82,11 @@ struct PCIExpressDevice {
/* ACS */
uint16_t acs_cap;
+
+ /* SR/IOV */
+ uint16_t sriov_cap;
+ PCIESriovPF sriov_pf;
+ PCIESriovVF sriov_vf;
};
#define COMPAT_PROP_PCP "power_controller_present"
diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
new file mode 100644
index 0000000000..990cff0a1c
--- /dev/null
+++ b/include/hw/pci/pcie_sriov.h
@@ -0,0 +1,71 @@
+/*
+ * pcie_sriov.h:
+ *
+ * Implementation of SR/IOV emulation support.
+ *
+ * Copyright (c) 2015 Knut Omang <knut.omang@oracle.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_PCIE_SRIOV_H
+#define QEMU_PCIE_SRIOV_H
+
+struct PCIESriovPF {
+ uint16_t num_vfs; /* Number of virtual functions created */
+ uint8_t vf_bar_type[PCI_NUM_REGIONS]; /* Store type for each VF bar */
+ const char *vfname; /* Reference to the device type used for the VFs */
+ PCIDevice **vf; /* Pointer to an array of num_vfs VF devices */
+};
+
+struct PCIESriovVF {
+ PCIDevice *pf; /* Pointer back to owner physical function */
+ uint16_t vf_number; /* Logical VF number of this function */
+};
+
+void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
+ const char *vfname, uint16_t vf_dev_id,
+ uint16_t init_vfs, uint16_t total_vfs,
+ uint16_t vf_offset, uint16_t vf_stride);
+void pcie_sriov_pf_exit(PCIDevice *dev);
+
+/* Set up a VF bar in the SR/IOV bar area */
+void pcie_sriov_pf_init_vf_bar(PCIDevice *dev, int region_num,
+ uint8_t type, dma_addr_t size);
+
+/* Instantiate a bar for a VF */
+void pcie_sriov_vf_register_bar(PCIDevice *dev, int region_num,
+ MemoryRegion *memory);
+
+/*
+ * Default (minimal) page size support values
+ * as required by the SR/IOV standard:
+ * 0x553 << 12 = 0x553000 = 4K + 8K + 64K + 256K + 1M + 4M
+ */
+#define SRIOV_SUP_PGSIZE_MINREQ 0x553
+
+/*
+ * Optionally add supported page sizes to the mask of supported page sizes
+ * Page size values are interpreted as opt_sup_pgsize << 12.
+ */
+void pcie_sriov_pf_add_sup_pgsize(PCIDevice *dev, uint16_t opt_sup_pgsize);
+
+/* SR/IOV capability config write handler */
+void pcie_sriov_config_write(PCIDevice *dev, uint32_t address,
+ uint32_t val, int len);
+
+/* Reset SR/IOV VF Enable bit to unregister all VFs */
+void pcie_sriov_pf_disable_vfs(PCIDevice *dev);
+
+/* Get logical VF number of a VF - only valid for VFs */
+uint16_t pcie_sriov_vf_number(PCIDevice *dev);
+
+/*
+ * Get the physical function that owns this VF.
+ * Returns NULL if dev is not a virtual function
+ */
+PCIDevice *pcie_sriov_get_pf(PCIDevice *dev);
+
+#endif /* QEMU_PCIE_SRIOV_H */
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index ee60eb3de4..5b302cb214 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -86,6 +86,8 @@ typedef struct PCIDevice PCIDevice;
typedef struct PCIEAERErr PCIEAERErr;
typedef struct PCIEAERLog PCIEAERLog;
typedef struct PCIEAERMsg PCIEAERMsg;
+typedef struct PCIESriovPF PCIESriovPF;
+typedef struct PCIESriovVF PCIESriovVF;
typedef struct PCIEPort PCIEPort;
typedef struct PCIESlot PCIESlot;
typedef struct PCIExpressDevice PCIExpressDevice;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 5d30f9ca60..ba8fb92efc 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -239,6 +239,9 @@ int pci_bar(PCIDevice *d, int reg)
{
uint8_t type;
+ /* PCIe virtual functions do not have their own BARs */
+ assert(!pci_is_vf(d));
+
if (reg != PCI_ROM_SLOT)
return PCI_BASE_ADDRESS_0 + reg * 4;
@@ -304,10 +307,30 @@ void pci_device_deassert_intx(PCIDevice *dev)
}
}
-static void pci_do_device_reset(PCIDevice *dev)
+static void pci_reset_regions(PCIDevice *dev)
{
int r;
+ if (pci_is_vf(dev)) {
+ return;
+ }
+ for (r = 0; r < PCI_NUM_REGIONS; ++r) {
+ PCIIORegion *region = &dev->io_regions[r];
+ if (!region->size) {
+ continue;
+ }
+
+ if (!(region->type & PCI_BASE_ADDRESS_SPACE_IO) &&
+ region->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+ pci_set_quad(dev->config + pci_bar(dev, r), region->type);
+ } else {
+ pci_set_long(dev->config + pci_bar(dev, r), region->type);
+ }
+ }
+}
+
+static void pci_do_device_reset(PCIDevice *dev)
+{
pci_device_deassert_intx(dev);
assert(dev->irq_state == 0);
@@ -323,19 +346,7 @@ static void pci_do_device_reset(PCIDevice *dev)
pci_get_word(dev->wmask + PCI_INTERRUPT_LINE) |
pci_get_word(dev->w1cmask + PCI_INTERRUPT_LINE));
dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
- for (r = 0; r < PCI_NUM_REGIONS; ++r) {
- PCIIORegion *region = &dev->io_regions[r];
- if (!region->size) {
- continue;
- }
-
- if (!(region->type & PCI_BASE_ADDRESS_SPACE_IO) &&
- region->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
- pci_set_quad(dev->config + pci_bar(dev, r), region->type);
- } else {
- pci_set_long(dev->config + pci_bar(dev, r), region->type);
- }
- }
+ pci_reset_regions(dev);
pci_update_mappings(dev);
msi_reset(dev);
@@ -884,6 +895,16 @@ static void pci_init_multifunction(PCIBus *bus, PCIDevice *dev, Error **errp)
dev->config[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
}
+ /*
+ * With SR/IOV and ARI, a device at function 0 need not be a multifunction
+ * device, as it may just be a VF that ended up with function 0 in
+ * the legacy PCI interpretation. Avoid failing in such cases:
+ */
+ if (pci_is_vf(dev) &&
+ dev->exp.sriov_vf.pf->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
+ return;
+ }
+
/*
* multifunction bit is interpreted in two ways as follows.
* - all functions must set the bit to 1.
@@ -1083,6 +1104,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
bus->devices[devfn]->name);
return NULL;
} else if (dev->hotplugged &&
+ !pci_is_vf(pci_dev) &&
pci_get_function_0(pci_dev)) {
error_setg(errp, "PCI: slot %d function 0 already occupied by %s,"
" new func %s cannot be exposed to guest.",
@@ -1191,6 +1213,7 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
pcibus_t size = memory_region_size(memory);
uint8_t hdr_type;
+ assert(!pci_is_vf(pci_dev)); /* VFs must use pcie_sriov_vf_register_bar */
assert(region_num >= 0);
assert(region_num < PCI_NUM_REGIONS);
assert(is_power_of_2(size));
@@ -1294,11 +1317,45 @@ pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num)
return pci_dev->io_regions[region_num].addr;
}
-static pcibus_t pci_bar_address(PCIDevice *d,
- int reg, uint8_t type, pcibus_t size)
+static pcibus_t pci_config_get_bar_addr(PCIDevice *d, int reg,
+ uint8_t type, pcibus_t size)
+{
+ pcibus_t new_addr;
+ if (!pci_is_vf(d)) {
+ int bar = pci_bar(d, reg);
+ if (type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+ new_addr = pci_get_quad(d->config + bar);
+ } else {
+ new_addr = pci_get_long(d->config + bar);
+ }
+ } else {
+ PCIDevice *pf = d->exp.sriov_vf.pf;
+ uint16_t sriov_cap = pf->exp.sriov_cap;
+ int bar = sriov_cap + PCI_SRIOV_BAR + reg * 4;
+ uint16_t vf_offset =
+ pci_get_word(pf->config + sriov_cap + PCI_SRIOV_VF_OFFSET);
+ uint16_t vf_stride =
+ pci_get_word(pf->config + sriov_cap + PCI_SRIOV_VF_STRIDE);
+ uint32_t vf_num = (d->devfn - (pf->devfn + vf_offset)) / vf_stride;
+
+ if (type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+ new_addr = pci_get_quad(pf->config + bar);
+ } else {
+ new_addr = pci_get_long(pf->config + bar);
+ }
+ new_addr += vf_num * size;
+ }
+ /* The ROM slot has a specific enable bit, keep it intact */
+ if (reg != PCI_ROM_SLOT) {
+ new_addr &= ~(size - 1);
+ }
+ return new_addr;
+}
+
+pcibus_t pci_bar_address(PCIDevice *d,
+ int reg, uint8_t type, pcibus_t size)
{
pcibus_t new_addr, last_addr;
- int bar = pci_bar(d, reg);
uint16_t cmd = pci_get_word(d->config + PCI_COMMAND);
Object *machine = qdev_get_machine();
ObjectClass *oc = object_get_class(machine);
@@ -1309,7 +1366,7 @@ static pcibus_t pci_bar_address(PCIDevice *d,
if (!(cmd & PCI_COMMAND_IO)) {
return PCI_BAR_UNMAPPED;
}
- new_addr = pci_get_long(d->config + bar) & ~(size - 1);
+ new_addr = pci_config_get_bar_addr(d, reg, type, size);
last_addr = new_addr + size - 1;
/* Check if 32 bit BAR wraps around explicitly.
* TODO: make priorities correct and remove this work around.
@@ -1324,11 +1381,7 @@ static pcibus_t pci_bar_address(PCIDevice *d,
if (!(cmd & PCI_COMMAND_MEMORY)) {
return PCI_BAR_UNMAPPED;
}
- if (type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
- new_addr = pci_get_quad(d->config + bar);
- } else {
- new_addr = pci_get_long(d->config + bar);
- }
+ new_addr = pci_config_get_bar_addr(d, reg, type, size);
/* the ROM slot has a specific enable bit */
if (reg == PCI_ROM_SLOT && !(new_addr & PCI_ROM_ADDRESS_ENABLE)) {
return PCI_BAR_UNMAPPED;
@@ -1473,6 +1526,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val_in, int
msi_write_config(d, addr, val_in, l);
msix_write_config(d, addr, val_in, l);
+ pcie_sriov_config_write(d, addr, val_in, l);
}
/***********************************************************/
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index d7d73a31e4..3c44204cf3 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -446,6 +446,11 @@ void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
PCIDevice *pci_dev = PCI_DEVICE(dev);
uint32_t lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP);
+ if (pci_is_vf(pci_dev)) {
+ /* Virtual function cannot be physically disconnected */
+ return;
+ }
+
/* Don't send event when device is enabled during qemu machine creation:
* it is present on boot, no hotplug event is necessary. We do send an
* event when the device is disabled later. */
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
new file mode 100644
index 0000000000..3f256d483f
--- /dev/null
+++ b/hw/pci/pcie_sriov.c
@@ -0,0 +1,294 @@
+/*
+ * pcie_sriov.c:
+ *
+ * Implementation of SR/IOV emulation support.
+ *
+ * Copyright (c) 2015-2017 Knut Omang <knut.omang@oracle.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pcie.h"
+#include "hw/pci/pci_bus.h"
+#include "hw/qdev-properties.h"
+#include "qemu/error-report.h"
+#include "qemu/range.h"
+#include "qapi/error.h"
+#include "trace.h"
+
+static PCIDevice *register_vf(PCIDevice *pf, int devfn,
+ const char *name, uint16_t vf_num);
+static void unregister_vfs(PCIDevice *dev);
+
+void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
+ const char *vfname, uint16_t vf_dev_id,
+ uint16_t init_vfs, uint16_t total_vfs,
+ uint16_t vf_offset, uint16_t vf_stride)
+{
+ uint8_t *cfg = dev->config + offset;
+ uint8_t *wmask;
+
+ pcie_add_capability(dev, PCI_EXT_CAP_ID_SRIOV, 1,
+ offset, PCI_EXT_CAP_SRIOV_SIZEOF);
+ dev->exp.sriov_cap = offset;
+ dev->exp.sriov_pf.num_vfs = 0;
+ dev->exp.sriov_pf.vfname = g_strdup(vfname);
+ dev->exp.sriov_pf.vf = NULL;
+
+ pci_set_word(cfg + PCI_SRIOV_VF_OFFSET, vf_offset);
+ pci_set_word(cfg + PCI_SRIOV_VF_STRIDE, vf_stride);
+
+ /*
+ * Mandatory page sizes to support.
+ * Device implementations can call pcie_sriov_pf_add_sup_pgsize()
+ * to set more bits:
+ */
+ pci_set_word(cfg + PCI_SRIOV_SUP_PGSIZE, SRIOV_SUP_PGSIZE_MINREQ);
+
+ /*
+ * Default is to use 4K pages, software can modify it
+ * to any of the supported bits
+ */
+ pci_set_word(cfg + PCI_SRIOV_SYS_PGSIZE, 0x1);
+
+ /* Set up device ID and initial/total number of VFs available */
+ pci_set_word(cfg + PCI_SRIOV_VF_DID, vf_dev_id);
+ pci_set_word(cfg + PCI_SRIOV_INITIAL_VF, init_vfs);
+ pci_set_word(cfg + PCI_SRIOV_TOTAL_VF, total_vfs);
+ pci_set_word(cfg + PCI_SRIOV_NUM_VF, 0);
+
+ /* Write enable control bits */
+ wmask = dev->wmask + offset;
+ pci_set_word(wmask + PCI_SRIOV_CTRL,
+ PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE | PCI_SRIOV_CTRL_ARI);
+ pci_set_word(wmask + PCI_SRIOV_NUM_VF, 0xffff);
+ pci_set_word(wmask + PCI_SRIOV_SYS_PGSIZE, 0x553);
+
+ qdev_prop_set_bit(&dev->qdev, "multifunction", true);
+}
+
+void pcie_sriov_pf_exit(PCIDevice *dev)
+{
+ unregister_vfs(dev);
+ g_free((char *)dev->exp.sriov_pf.vfname);
+ dev->exp.sriov_pf.vfname = NULL;
+}
+
+void pcie_sriov_pf_init_vf_bar(PCIDevice *dev, int region_num,
+ uint8_t type, dma_addr_t size)
+{
+ uint32_t addr;
+ uint64_t wmask;
+ uint16_t sriov_cap = dev->exp.sriov_cap;
+
+ assert(sriov_cap > 0);
+ assert(region_num >= 0);
+ assert(region_num < PCI_NUM_REGIONS);
+ assert(region_num != PCI_ROM_SLOT);
+
+ wmask = ~(size - 1);
+ addr = sriov_cap + PCI_SRIOV_BAR + region_num * 4;
+
+ pci_set_long(dev->config + addr, type);
+ if (!(type & PCI_BASE_ADDRESS_SPACE_IO) &&
+ type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+ pci_set_quad(dev->wmask + addr, wmask);
+ pci_set_quad(dev->cmask + addr, ~0ULL);
+ } else {
+ pci_set_long(dev->wmask + addr, wmask & 0xffffffff);
+ pci_set_long(dev->cmask + addr, 0xffffffff);
+ }
+ dev->exp.sriov_pf.vf_bar_type[region_num] = type;
+}
+
+void pcie_sriov_vf_register_bar(PCIDevice *dev, int region_num,
+ MemoryRegion *memory)
+{
+ PCIIORegion *r;
+ PCIBus *bus = pci_get_bus(dev);
+ uint8_t type;
+ pcibus_t size = memory_region_size(memory);
+
+ assert(pci_is_vf(dev)); /* PFs must use pci_register_bar */
+ assert(region_num >= 0);
+ assert(region_num < PCI_NUM_REGIONS);
+ type = dev->exp.sriov_vf.pf->exp.sriov_pf.vf_bar_type[region_num];
+
+ if (!is_power_of_2(size)) {
+ error_report("%s: PCI region size must be a power"
+ " of two - type=0x%x, size=0x%"FMT_PCIBUS,
+ __func__, type, size);
+ exit(1);
+ }
+
+ r = &dev->io_regions[region_num];
+ r->memory = memory;
+ r->address_space =
+ type & PCI_BASE_ADDRESS_SPACE_IO
+ ? bus->address_space_io
+ : bus->address_space_mem;
+ r->size = size;
+ r->type = type;
+
+ r->addr = pci_bar_address(dev, region_num, r->type, r->size);
+ if (r->addr != PCI_BAR_UNMAPPED) {
+ memory_region_add_subregion_overlap(r->address_space,
+ r->addr, r->memory, 1);
+ }
+}
+
+static PCIDevice *register_vf(PCIDevice *pf, int devfn, const char *name,
+ uint16_t vf_num)
+{
+ PCIDevice *dev = pci_new(devfn, name);
+ dev->exp.sriov_vf.pf = pf;
+ dev->exp.sriov_vf.vf_number = vf_num;
+ PCIBus *bus = pci_get_bus(pf);
+ Error *local_err = NULL;
+
+ qdev_realize(&dev->qdev, &bus->qbus, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ return NULL;
+ }
+
+ /* set vid/did according to sr/iov spec - they are not used */
+ pci_config_set_vendor_id(dev->config, 0xffff);
+ pci_config_set_device_id(dev->config, 0xffff);
+
+ return dev;
+}
+
+static void register_vfs(PCIDevice *dev)
+{
+ uint16_t num_vfs;
+ uint16_t i;
+ uint16_t sriov_cap = dev->exp.sriov_cap;
+ uint16_t vf_offset =
+ pci_get_word(dev->config + sriov_cap + PCI_SRIOV_VF_OFFSET);
+ uint16_t vf_stride =
+ pci_get_word(dev->config + sriov_cap + PCI_SRIOV_VF_STRIDE);
+ int32_t devfn = dev->devfn + vf_offset;
+
+ assert(sriov_cap > 0);
+ num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
+
+ dev->exp.sriov_pf.vf = g_malloc(sizeof(PCIDevice *) * num_vfs);
+ assert(dev->exp.sriov_pf.vf);
+
+ trace_sriov_register_vfs(dev->name, PCI_SLOT(dev->devfn),
+ PCI_FUNC(dev->devfn), num_vfs);
+ for (i = 0; i < num_vfs; i++) {
+ dev->exp.sriov_pf.vf[i] = register_vf(dev, devfn,
+ dev->exp.sriov_pf.vfname, i);
+ if (!dev->exp.sriov_pf.vf[i]) {
+ num_vfs = i;
+ break;
+ }
+ devfn += vf_stride;
+ }
+ dev->exp.sriov_pf.num_vfs = num_vfs;
+}
+
+static void unregister_vfs(PCIDevice *dev)
+{
+ Error *local_err = NULL;
+ uint16_t num_vfs = dev->exp.sriov_pf.num_vfs;
+ uint16_t i;
+
+ trace_sriov_unregister_vfs(dev->name, PCI_SLOT(dev->devfn),
+ PCI_FUNC(dev->devfn), num_vfs);
+ for (i = 0; i < num_vfs; i++) {
+ PCIDevice *vf = dev->exp.sriov_pf.vf[i];
+ object_property_set_bool(OBJECT(vf), "realized", false, &local_err);
+ if (local_err) {
+ fprintf(stderr, "Failed to unplug: %s\n",
+ error_get_pretty(local_err));
+ error_free(local_err);
+ }
+ object_unparent(OBJECT(vf));
+ }
+ g_free(dev->exp.sriov_pf.vf);
+ dev->exp.sriov_pf.vf = NULL;
+ dev->exp.sriov_pf.num_vfs = 0;
+ pci_set_word(dev->config + dev->exp.sriov_cap + PCI_SRIOV_NUM_VF, 0);
+}
+
+void pcie_sriov_config_write(PCIDevice *dev, uint32_t address,
+ uint32_t val, int len)
+{
+ uint32_t off;
+ uint16_t sriov_cap = dev->exp.sriov_cap;
+
+ if (!sriov_cap || address < sriov_cap) {
+ return;
+ }
+ off = address - sriov_cap;
+ if (off >= PCI_EXT_CAP_SRIOV_SIZEOF) {
+ return;
+ }
+
+ trace_sriov_config_write(dev->name, PCI_SLOT(dev->devfn),
+ PCI_FUNC(dev->devfn), off, val, len);
+
+ if (range_covers_byte(off, len, PCI_SRIOV_CTRL)) {
+ if (dev->exp.sriov_pf.num_vfs) {
+ if (!(val & PCI_SRIOV_CTRL_VFE)) {
+ unregister_vfs(dev);
+ }
+ } else {
+ if (val & PCI_SRIOV_CTRL_VFE) {
+ register_vfs(dev);
+ }
+ }
+ }
+}
+
+
+/* Reset SR/IOV VF Enable bit to trigger an unregister of all VFs */
+void pcie_sriov_pf_disable_vfs(PCIDevice *dev)
+{
+ uint16_t sriov_cap = dev->exp.sriov_cap;
+ if (sriov_cap) {
+ uint32_t val = pci_get_byte(dev->config + sriov_cap + PCI_SRIOV_CTRL);
+ if (val & PCI_SRIOV_CTRL_VFE) {
+ val &= ~PCI_SRIOV_CTRL_VFE;
+ pcie_sriov_config_write(dev, sriov_cap + PCI_SRIOV_CTRL, val, 1);
+ }
+ }
+}
+
+/* Add optional supported page sizes to the mask of supported page sizes */
+void pcie_sriov_pf_add_sup_pgsize(PCIDevice *dev, uint16_t opt_sup_pgsize)
+{
+ uint8_t *cfg = dev->config + dev->exp.sriov_cap;
+ uint8_t *wmask = dev->wmask + dev->exp.sriov_cap;
+
+ uint16_t sup_pgsize = pci_get_word(cfg + PCI_SRIOV_SUP_PGSIZE);
+
+ sup_pgsize |= opt_sup_pgsize;
+
+ /*
+ * Make sure the new bits are set, and that system page size
+ * also can be set to any of the new values according to spec:
+ */
+ pci_set_word(cfg + PCI_SRIOV_SUP_PGSIZE, sup_pgsize);
+ pci_set_word(wmask + PCI_SRIOV_SYS_PGSIZE, sup_pgsize);
+}
+
+
+uint16_t pcie_sriov_vf_number(PCIDevice *dev)
+{
+ assert(pci_is_vf(dev));
+ return dev->exp.sriov_vf.vf_number;
+}
+
+
+PCIDevice *pcie_sriov_get_pf(PCIDevice *dev)
+{
+ return dev->exp.sriov_vf.pf;
+}
diff --git a/hw/pci/meson.build b/hw/pci/meson.build
index 5c4bbac817..bcc9c75919 100644
--- a/hw/pci/meson.build
+++ b/hw/pci/meson.build
@@ -5,6 +5,7 @@ pci_ss.add(files(
'pci.c',
'pci_bridge.c',
'pci_host.c',
+ 'pcie_sriov.c',
'shpc.c',
'slotid_cap.c'
))
diff --git a/hw/pci/trace-events b/hw/pci/trace-events
index 7570752c40..aaf46bc92d 100644
--- a/hw/pci/trace-events
+++ b/hw/pci/trace-events
@@ -10,3 +10,8 @@ pci_cfg_write(const char *dev, uint32_t bus, uint32_t slot, uint32_t func, unsig
# msix.c
msix_write_config(char *name, bool enabled, bool masked) "dev %s enabled %d masked %d"
+
+# hw/pci/pcie_sriov.c
+sriov_register_vfs(const char *name, int slot, int function, int num_vfs) "%s %02x:%x: creating %d vf devs"
+sriov_unregister_vfs(const char *name, int slot, int function, int num_vfs) "%s %02x:%x: Unregistering %d vf devs"
+sriov_config_write(const char *name, int slot, int fun, uint32_t offset, uint32_t val, uint32_t len) "%s %02x:%x: sriov offset 0x%x val 0x%x len %d"
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 25/47] pcie: Add some SR/IOV API documentation in docs/pcie_sriov.txt
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (23 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 24/47] pcie: Add support for Single Root I/O Virtualization (SR/IOV) Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 26/47] pcie: Add a helper to the SR/IOV API Michael S. Tsirkin
` (22 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Knut Omang, Knut Omang
From: Knut Omang <knut.omang@oracle.com>
Add a small intro + minimal documentation for how to
implement SR/IOV support for an emulated device.
Signed-off-by: Knut Omang <knuto@ifi.uio.no>
Message-Id: <20220217174504.1051716-3-lukasz.maniak@linux.intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
docs/pcie_sriov.txt | 115 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 115 insertions(+)
create mode 100644 docs/pcie_sriov.txt
diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
new file mode 100644
index 0000000000..f5e891e1d4
--- /dev/null
+++ b/docs/pcie_sriov.txt
@@ -0,0 +1,115 @@
+PCI SR/IOV EMULATION SUPPORT
+============================
+
+Description
+===========
+SR/IOV (Single Root I/O Virtualization) is an optional extended capability
+of a PCI Express device. It allows a single physical function (PF) to appear as multiple
+virtual functions (VFs) for the main purpose of eliminating software
+overhead in I/O from virtual machines.
+
+Qemu now implements the basic common functionality to enable an emulated device
+to support SR/IOV. Yet no fully implemented devices exists in Qemu, but a
+proof-of-concept hack of the Intel igb can be found here:
+
+git://github.com/knuto/qemu.git sriov_patches_v5
+
+Implementation
+==============
+Implementing emulation of an SR/IOV capable device typically consists of
+implementing support for two types of device classes; the "normal" physical device
+(PF) and the virtual device (VF). From Qemu's perspective, the VFs are just
+like other devices, except that some of their properties are derived from
+the PF.
+
+A virtual function is different from a physical function in that the BAR
+space for all VFs are defined by the BAR registers in the PFs SR/IOV
+capability. All VFs have the same BARs and BAR sizes.
+
+Accesses to these virtual BARs then is computed as
+
+ <VF BAR start> + <VF number> * <BAR sz> + <offset>
+
+From our emulation perspective this means that there is a separate call for
+setting up a BAR for a VF.
+
+1) To enable SR/IOV support in the PF, it must be a PCI Express device so
+ you would need to add a PCI Express capability in the normal PCI
+ capability list. You might also want to add an ARI (Alternative
+ Routing-ID Interpretation) capability to indicate that your device
+ supports functions beyond it's "own" function space (0-7),
+ which is necessary to support more than 7 functions, or
+ if functions extends beyond offset 7 because they are placed at an
+ offset > 1 or have stride > 1.
+
+ ...
+ #include "hw/pci/pcie.h"
+ #include "hw/pci/pcie_sriov.h"
+
+ pci_your_pf_dev_realize( ... )
+ {
+ ...
+ int ret = pcie_endpoint_cap_init(d, 0x70);
+ ...
+ pcie_ari_init(d, 0x100, 1);
+ ...
+
+ /* Add and initialize the SR/IOV capability */
+ pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
+ vf_devid, initial_vfs, total_vfs,
+ fun_offset, stride);
+
+ /* Set up individual VF BARs (parameters as for normal BARs) */
+ pcie_sriov_pf_init_vf_bar( ... )
+ ...
+ }
+
+ For cleanup, you simply call:
+
+ pcie_sriov_pf_exit(device);
+
+ which will delete all the virtual functions and associated resources.
+
+2) Similarly in the implementation of the virtual function, you need to
+ make it a PCI Express device and add a similar set of capabilities
+ except for the SR/IOV capability. Then you need to set up the VF BARs as
+ subregions of the PFs SR/IOV VF BARs by calling
+ pcie_sriov_vf_register_bar() instead of the normal pci_register_bar() call:
+
+ pci_your_vf_dev_realize( ... )
+ {
+ ...
+ int ret = pcie_endpoint_cap_init(d, 0x60);
+ ...
+ pcie_ari_init(d, 0x100, 1);
+ ...
+ memory_region_init(mr, ... )
+ pcie_sriov_vf_register_bar(d, bar_nr, mr);
+ ...
+ }
+
+Testing on Linux guest
+======================
+The easiest is if your device driver supports sysfs based SR/IOV
+enabling. Support for this was added in kernel v.3.8, so not all drivers
+support it yet.
+
+To enable 4 VFs for a device at 01:00.0:
+
+ modprobe yourdriver
+ echo 4 > /sys/bus/pci/devices/0000:01:00.0/sriov_numvfs
+
+You should now see 4 VFs with lspci.
+To turn SR/IOV off again - the standard requires you to turn it off before you can enable
+another VF count, and the emulation enforces this:
+
+ echo 0 > /sys/bus/pci/devices/0000:01:00.0/sriov_numvfs
+
+Older drivers typically provide a max_vfs module parameter
+to enable it at load time:
+
+ modprobe yourdriver max_vfs=4
+
+To disable the VFs again then, you simply have to unload the driver:
+
+ rmmod yourdriver
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 26/47] pcie: Add a helper to the SR/IOV API
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (24 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 25/47] pcie: Add some SR/IOV API documentation in docs/pcie_sriov.txt Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 27/47] pcie: Add 1.2 version token for the Power Management Capability Michael S. Tsirkin
` (21 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Knut Omang, Łukasz Gieryk
From: Łukasz Gieryk <lukasz.gieryk@linux.intel.com>
Convenience function for retrieving the PCIDevice object of the N-th VF.
Signed-off-by: Łukasz Gieryk <lukasz.gieryk@linux.intel.com>
Reviewed-by: Knut Omang <knuto@ifi.uio.no>
Message-Id: <20220217174504.1051716-4-lukasz.maniak@linux.intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/pci/pcie_sriov.h | 6 ++++++
hw/pci/pcie_sriov.c | 10 +++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
index 990cff0a1c..80f5c84e75 100644
--- a/include/hw/pci/pcie_sriov.h
+++ b/include/hw/pci/pcie_sriov.h
@@ -68,4 +68,10 @@ uint16_t pcie_sriov_vf_number(PCIDevice *dev);
*/
PCIDevice *pcie_sriov_get_pf(PCIDevice *dev);
+/*
+ * Get the n-th VF of this physical function - only valid for PF.
+ * Returns NULL if index is invalid
+ */
+PCIDevice *pcie_sriov_get_vf_at_index(PCIDevice *dev, int n);
+
#endif /* QEMU_PCIE_SRIOV_H */
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index 3f256d483f..87abad6ac8 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -287,8 +287,16 @@ uint16_t pcie_sriov_vf_number(PCIDevice *dev)
return dev->exp.sriov_vf.vf_number;
}
-
PCIDevice *pcie_sriov_get_pf(PCIDevice *dev)
{
return dev->exp.sriov_vf.pf;
}
+
+PCIDevice *pcie_sriov_get_vf_at_index(PCIDevice *dev, int n)
+{
+ assert(!pci_is_vf(dev));
+ if (n < dev->exp.sriov_pf.num_vfs) {
+ return dev->exp.sriov_pf.vf[n];
+ }
+ return NULL;
+}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 27/47] pcie: Add 1.2 version token for the Power Management Capability
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (25 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 26/47] pcie: Add a helper to the SR/IOV API Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 28/47] pci-bridge/xio3130_upstream: Fix error handling Michael S. Tsirkin
` (20 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Łukasz Gieryk
From: Łukasz Gieryk <lukasz.gieryk@linux.intel.com>
Signed-off-by: Łukasz Gieryk <lukasz.gieryk@linux.intel.com>
Message-Id: <20220217174504.1051716-5-lukasz.maniak@linux.intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/pci/pci_regs.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/hw/pci/pci_regs.h b/include/hw/pci/pci_regs.h
index 77ba64b931..a590140962 100644
--- a/include/hw/pci/pci_regs.h
+++ b/include/hw/pci/pci_regs.h
@@ -4,5 +4,6 @@
#include "standard-headers/linux/pci_regs.h"
#define PCI_PM_CAP_VER_1_1 0x0002 /* PCI PM spec ver. 1.1 */
+#define PCI_PM_CAP_VER_1_2 0x0003 /* PCI PM spec ver. 1.2 */
#endif
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 28/47] pci-bridge/xio3130_upstream: Fix error handling
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (26 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 27/47] pcie: Add 1.2 version token for the Power Management Capability Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 29/47] pci-bridge/xio3130_downstream: " Michael S. Tsirkin
` (19 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Jonathan Cameron
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Goto label is incorrect so msi cleanup would not occur if there is
an error in the ssvid initialization.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Message-Id: <20220218102303.7061-1-Jonathan.Cameron@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/pci-bridge/xio3130_upstream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index 5cd3af4fbc..5ff46ef050 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -75,7 +75,7 @@ static void xio3130_upstream_realize(PCIDevice *d, Error **errp)
XIO3130_SSVID_SVID, XIO3130_SSVID_SSID,
errp);
if (rc < 0) {
- goto err_bridge;
+ goto err_msi;
}
rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_UPSTREAM,
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 29/47] pci-bridge/xio3130_downstream: Fix error handling
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (27 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 28/47] pci-bridge/xio3130_upstream: Fix error handling Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 30/47] headers: Add pvpanic.h Michael S. Tsirkin
` (18 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Jonathan Cameron
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Wrong goto label, so msi cleanup would not occur if there is
an error in the ssvid initialization.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Message-Id: <20220218102303.7061-2-Jonathan.Cameron@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/pci-bridge/xio3130_downstream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index 04aae72cd6..080a6613fe 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -84,7 +84,7 @@ static void xio3130_downstream_realize(PCIDevice *d, Error **errp)
XIO3130_SSVID_SVID, XIO3130_SSVID_SSID,
errp);
if (rc < 0) {
- goto err_bridge;
+ goto err_msi;
}
rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_DOWNSTREAM,
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 30/47] headers: Add pvpanic.h
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (28 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 29/47] pci-bridge/xio3130_downstream: " Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 31/47] hw/misc/pvpanic: Use standard headers instead Michael S. Tsirkin
` (17 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Cornelia Huck, Philippe Mathieu-Daudé,
zhenwei pi, Paolo Bonzini
From: zhenwei pi <pizhenwei@bytedance.com>
Since 2020, linux kernel started to export pvpanic.h. Import the
latest version from linux into QEMU.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20220221122717.1371010-1-pizhenwei@bytedance.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/standard-headers/linux/pvpanic.h | 9 +++++++++
| 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
create mode 100644 include/standard-headers/linux/pvpanic.h
diff --git a/include/standard-headers/linux/pvpanic.h b/include/standard-headers/linux/pvpanic.h
new file mode 100644
index 0000000000..54b7485390
--- /dev/null
+++ b/include/standard-headers/linux/pvpanic.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef __PVPANIC_H__
+#define __PVPANIC_H__
+
+#define PVPANIC_PANICKED (1 << 0)
+#define PVPANIC_CRASH_LOADED (1 << 1)
+
+#endif /* __PVPANIC_H__ */
--git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index fe850763c5..839a5ec614 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -214,7 +214,8 @@ for i in "$tmpdir"/include/linux/*virtio*.h \
"$tmpdir/include/linux/const.h" \
"$tmpdir/include/linux/kernel.h" \
"$tmpdir/include/linux/vhost_types.h" \
- "$tmpdir/include/linux/sysinfo.h"; do
+ "$tmpdir/include/linux/sysinfo.h" \
+ "$tmpdir/include/misc/pvpanic.h"; do
cp_portable "$i" "$output/include/standard-headers/linux"
done
mkdir -p "$output/include/standard-headers/drm"
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 31/47] hw/misc/pvpanic: Use standard headers instead
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (29 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 30/47] headers: Add pvpanic.h Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 32/47] pci: show id info when pci BDF conflict Michael S. Tsirkin
` (16 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Philippe Mathieu-Daudé,
zhenwei pi, Alistair Francis, Bernhard Beschow
From: zhenwei pi <pizhenwei@bytedance.com>
QEMU side has already imported pvpanic.h from linux, remove bit
definitions from include/hw/misc/pvpanic.h, and use
include/standard-headers/linux/pvpanic.h instead.
Also minor changes for PVPANIC_CRASHLOADED -> PVPANIC_CRASH_LOADED.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20220221122717.1371010-2-pizhenwei@bytedance.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/misc/pvpanic.h | 8 --------
hw/misc/pvpanic-isa.c | 4 +++-
hw/misc/pvpanic-pci.c | 4 +++-
hw/misc/pvpanic.c | 5 +++--
4 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h
index ca3c5bb533..7f16cc9b16 100644
--- a/include/hw/misc/pvpanic.h
+++ b/include/hw/misc/pvpanic.h
@@ -22,14 +22,6 @@
#define PVPANIC_IOPORT_PROP "ioport"
-/* The bit of supported pv event, TODO: include uapi header and remove this */
-#define PVPANIC_F_PANICKED 0
-#define PVPANIC_F_CRASHLOADED 1
-
-/* The pv event value */
-#define PVPANIC_PANICKED (1 << PVPANIC_F_PANICKED)
-#define PVPANIC_CRASHLOADED (1 << PVPANIC_F_CRASHLOADED)
-
/*
* PVPanicState for any device type
*/
diff --git a/hw/misc/pvpanic-isa.c b/hw/misc/pvpanic-isa.c
index a39fcdd1fc..b84d4d458d 100644
--- a/hw/misc/pvpanic-isa.c
+++ b/hw/misc/pvpanic-isa.c
@@ -21,6 +21,7 @@
#include "hw/misc/pvpanic.h"
#include "qom/object.h"
#include "hw/isa/isa.h"
+#include "standard-headers/linux/pvpanic.h"
OBJECT_DECLARE_SIMPLE_TYPE(PVPanicISAState, PVPANIC_ISA_DEVICE)
@@ -64,7 +65,8 @@ static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp)
static Property pvpanic_isa_properties[] = {
DEFINE_PROP_UINT16(PVPANIC_IOPORT_PROP, PVPanicISAState, ioport, 0x505),
- DEFINE_PROP_UINT8("events", PVPanicISAState, pvpanic.events, PVPANIC_PANICKED | PVPANIC_CRASHLOADED),
+ DEFINE_PROP_UINT8("events", PVPanicISAState, pvpanic.events,
+ PVPANIC_PANICKED | PVPANIC_CRASH_LOADED),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/misc/pvpanic-pci.c b/hw/misc/pvpanic-pci.c
index 62e1be68c1..99cf7e2041 100644
--- a/hw/misc/pvpanic-pci.c
+++ b/hw/misc/pvpanic-pci.c
@@ -21,6 +21,7 @@
#include "hw/misc/pvpanic.h"
#include "qom/object.h"
#include "hw/pci/pci.h"
+#include "standard-headers/linux/pvpanic.h"
OBJECT_DECLARE_SIMPLE_TYPE(PVPanicPCIState, PVPANIC_PCI_DEVICE)
@@ -53,7 +54,8 @@ static void pvpanic_pci_realizefn(PCIDevice *dev, Error **errp)
}
static Property pvpanic_pci_properties[] = {
- DEFINE_PROP_UINT8("events", PVPanicPCIState, pvpanic.events, PVPANIC_PANICKED | PVPANIC_CRASHLOADED),
+ DEFINE_PROP_UINT8("events", PVPanicPCIState, pvpanic.events,
+ PVPANIC_PANICKED | PVPANIC_CRASH_LOADED),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index e2cb4a5d28..1540e9091a 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -21,12 +21,13 @@
#include "hw/qdev-properties.h"
#include "hw/misc/pvpanic.h"
#include "qom/object.h"
+#include "standard-headers/linux/pvpanic.h"
static void handle_event(int event)
{
static bool logged;
- if (event & ~(PVPANIC_PANICKED | PVPANIC_CRASHLOADED) && !logged) {
+ if (event & ~(PVPANIC_PANICKED | PVPANIC_CRASH_LOADED) && !logged) {
qemu_log_mask(LOG_GUEST_ERROR, "pvpanic: unknown event %#x.\n", event);
logged = true;
}
@@ -36,7 +37,7 @@ static void handle_event(int event)
return;
}
- if (event & PVPANIC_CRASHLOADED) {
+ if (event & PVPANIC_CRASH_LOADED) {
qemu_system_guest_crashloaded(NULL);
return;
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 32/47] pci: show id info when pci BDF conflict
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (30 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 31/47] hw/misc/pvpanic: Use standard headers instead Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 33/47] pci: expose TYPE_XIO3130_DOWNSTREAM name Michael S. Tsirkin
` (15 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Zhenzhong Duan
From: Zhenzhong Duan <zhenzhong.duan@intel.com>
During qemu init stage, when there is pci BDF conflicts, qemu print
a warning but not showing which device the BDF is occupied by. E.x:
"PCI: slot 2 function 0 not available for virtio-scsi-pci, in use by virtio-scsi-pci"
To facilitate user knowing the offending device and fixing it, showing
the id info in the warning.
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Message-Id: <20220223094435.64495-1-zhenzhong.duan@intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/pci/pci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index ba8fb92efc..5cb1232e27 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1099,9 +1099,9 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
return NULL;
} else if (!pci_bus_devfn_available(bus, devfn)) {
error_setg(errp, "PCI: slot %d function %d not available for %s,"
- " in use by %s",
+ " in use by %s,id=%s",
PCI_SLOT(devfn), PCI_FUNC(devfn), name,
- bus->devices[devfn]->name);
+ bus->devices[devfn]->name, bus->devices[devfn]->qdev.id);
return NULL;
} else if (dev->hotplugged &&
!pci_is_vf(pci_dev) &&
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 33/47] pci: expose TYPE_XIO3130_DOWNSTREAM name
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (31 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 32/47] pci: show id info when pci BDF conflict Michael S. Tsirkin
@ 2022-03-07 10:02 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 34/47] acpi: pcihp: pcie: set power on cap on parent slot Michael S. Tsirkin
` (14 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
Type name will be used in followup patch for cast check
in pcihp code.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20220301151200.3507298-2-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/pci-bridge/xio3130_downstream.h | 15 +++++++++++++++
hw/pci-bridge/xio3130_downstream.c | 3 ++-
2 files changed, 17 insertions(+), 1 deletion(-)
create mode 100644 include/hw/pci-bridge/xio3130_downstream.h
diff --git a/include/hw/pci-bridge/xio3130_downstream.h b/include/hw/pci-bridge/xio3130_downstream.h
new file mode 100644
index 0000000000..1d10139aea
--- /dev/null
+++ b/include/hw/pci-bridge/xio3130_downstream.h
@@ -0,0 +1,15 @@
+/*
+ * TI X3130 pci express downstream port switch
+ *
+ * Copyright (C) 2022 Igor Mammedov <imammedo@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_PCI_BRIDGE_XIO3130_DOWNSTREAM_H
+#define HW_PCI_BRIDGE_XIO3130_DOWNSTREAM_H
+
+#define TYPE_XIO3130_DOWNSTREAM "xio3130-downstream"
+
+#endif
+
diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index 080a6613fe..05e2b06c0c 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -28,6 +28,7 @@
#include "migration/vmstate.h"
#include "qapi/error.h"
#include "qemu/module.h"
+#include "hw/pci-bridge/xio3130_downstream.h"
#define PCI_DEVICE_ID_TI_XIO3130D 0x8233 /* downstream port */
#define XIO3130_REVISION 0x1
@@ -173,7 +174,7 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data)
}
static const TypeInfo xio3130_downstream_info = {
- .name = "xio3130-downstream",
+ .name = TYPE_XIO3130_DOWNSTREAM,
.parent = TYPE_PCIE_SLOT,
.class_init = xio3130_downstream_class_init,
.interfaces = (InterfaceInfo[]) {
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 34/47] acpi: pcihp: pcie: set power on cap on parent slot
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (32 preceding siblings ...)
2022-03-07 10:02 ` [PULL v2 33/47] pci: expose TYPE_XIO3130_DOWNSTREAM name Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 35/47] pc: add option to disable PS/2 mouse/keyboard Michael S. Tsirkin
` (13 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Ani Sinha, Peter Maydell, Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
on creation a PCIDevice has power turned on at the end of pci_qdev_realize()
however later on if PCIe slot isn't populated with any children
it's power is turned off. It's fine if native hotplug is used
as plug callback will power slot on among other things.
However when ACPI hotplug is enabled it replaces native PCIe plug
callbacks with ACPI specific ones (acpi_pcihp_device_*plug_cb) and
as result slot stays powered off. It works fine as ACPI hotplug
on guest side takes care of enumerating/initializing hotplugged
device. But when later guest is migrated, call chain introduced by]
commit d5daff7d312 (pcie: implement slot power control for pcie root ports)
pcie_cap_slot_post_load()
-> pcie_cap_update_power()
-> pcie_set_power_device()
-> pci_set_power()
-> pci_update_mappings()
will disable earlier initialized BARs for the hotplugged device
in powered off slot due to commit 23786d13441 (pci: implement power state)
which disables BARs if power is off.
Fix it by setting PCI_EXP_SLTCTL_PCC to PCI_EXP_SLTCTL_PWR_ON
on slot (root port/downstream port) at the time a device
hotplugged into it. As result PCI_EXP_SLTCTL_PWR_ON is migrated
to target and above call chain keeps device plugged into it
powered on.
Fixes: d5daff7d312 ("pcie: implement slot power control for pcie root ports")
Fixes: 23786d13441 ("pci: implement power state")
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2053584
Suggested-by: "Michael S. Tsirkin" <mst@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20220301151200.3507298-3-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/pci/pcie.h | 1 +
hw/acpi/pcihp.c | 12 +++++++++++-
hw/pci/pcie.c | 11 +++++++++++
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index 168950a83b..798a262a0a 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -118,6 +118,7 @@ void pcie_cap_slot_write_config(PCIDevice *dev,
uint32_t addr, uint32_t val, int len);
int pcie_cap_slot_post_load(void *opaque, int version_id);
void pcie_cap_slot_push_attention_button(PCIDevice *dev);
+void pcie_cap_slot_enable_power(PCIDevice *dev);
void pcie_cap_root_init(PCIDevice *dev);
void pcie_cap_root_reset(PCIDevice *dev);
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 6befd23e16..6351bd3424 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -32,6 +32,7 @@
#include "hw/pci/pci_bridge.h"
#include "hw/pci/pci_host.h"
#include "hw/pci/pcie_port.h"
+#include "hw/pci-bridge/xio3130_downstream.h"
#include "hw/i386/acpi-build.h"
#include "hw/acpi/acpi.h"
#include "hw/pci/pci_bus.h"
@@ -336,6 +337,8 @@ void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
{
PCIDevice *pdev = PCI_DEVICE(dev);
int slot = PCI_SLOT(pdev->devfn);
+ PCIDevice *bridge;
+ PCIBus *bus;
int bsel;
/* Don't send event when device is enabled during qemu machine creation:
@@ -365,7 +368,14 @@ void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
return;
}
- bsel = acpi_pcihp_get_bsel(pci_get_bus(pdev));
+ bus = pci_get_bus(pdev);
+ bridge = pci_bridge_get_device(bus);
+ if (object_dynamic_cast(OBJECT(bridge), TYPE_PCIE_ROOT_PORT) ||
+ object_dynamic_cast(OBJECT(bridge), TYPE_XIO3130_DOWNSTREAM)) {
+ pcie_cap_slot_enable_power(bridge);
+ }
+
+ bsel = acpi_pcihp_get_bsel(bus);
g_assert(bsel >= 0);
s->acpi_pcihp_pci_status[bsel].up |= (1U << slot);
acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 3c44204cf3..67a5d67372 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -366,6 +366,17 @@ static void hotplug_event_clear(PCIDevice *dev)
}
}
+void pcie_cap_slot_enable_power(PCIDevice *dev)
+{
+ uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
+ uint32_t sltcap = pci_get_long(exp_cap + PCI_EXP_SLTCAP);
+
+ if (sltcap & PCI_EXP_SLTCAP_PCP) {
+ pci_set_word_by_mask(exp_cap + PCI_EXP_SLTCTL,
+ PCI_EXP_SLTCTL_PCC, PCI_EXP_SLTCTL_PWR_ON);
+ }
+}
+
static void pcie_set_power_device(PCIBus *bus, PCIDevice *dev, void *opaque)
{
bool *power = opaque;
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 35/47] pc: add option to disable PS/2 mouse/keyboard
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (33 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 34/47] acpi: pcihp: pcie: set power on cap on parent slot Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 36/47] vhost-vsock: detach the virqueue element in case of error Michael S. Tsirkin
` (12 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Peter Maydell, Richard Henderson,
Joelle van Dyne, Paolo Bonzini
From: Joelle van Dyne <j@getutm.app>
On some older software like Windows 7 installer, having both a PS/2
mouse and USB mouse results in only one device working property (which
might be a different device each boot). While the workaround to not use
a USB mouse with such software is valid, it creates an inconsistent
experience if the user wishes to always use a USB mouse.
This introduces a new machine property to inhibit the creation of the
i8042 PS/2 controller.
Signed-off-by: Joelle van Dyne <j@getutm.app>
Message-Id: <20220227210655.45592-1-j@getutm.app>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/i386/pc.h | 2 ++
hw/i386/pc.c | 28 ++++++++++++++++++++++++++--
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9c9f4ac748..1a27de9c8b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -48,6 +48,7 @@ typedef struct PCMachineState {
bool sata_enabled;
bool pit_enabled;
bool hpet_enabled;
+ bool i8042_enabled;
bool default_bus_bypass_iommu;
uint64_t max_fw_size;
@@ -64,6 +65,7 @@ typedef struct PCMachineState {
#define PC_MACHINE_SMBUS "smbus"
#define PC_MACHINE_SATA "sata"
#define PC_MACHINE_PIT "pit"
+#define PC_MACHINE_I8042 "i8042"
#define PC_MACHINE_MAX_FW_SIZE "max-fw-size"
#define PC_MACHINE_SMBIOS_EP "smbios-entry-point-type"
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c8696ac01e..32bf12421e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1014,7 +1014,8 @@ static const MemoryRegionOps ioportF0_io_ops = {
},
};
-static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport)
+static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
+ bool create_i8042, bool no_vmport)
{
int i;
DriveInfo *fd[MAX_FD];
@@ -1036,6 +1037,10 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport)
}
}
+ if (!create_i8042) {
+ return;
+ }
+
i8042 = isa_create_simple(isa_bus, "i8042");
if (!no_vmport) {
isa_create_simple(isa_bus, TYPE_VMPORT);
@@ -1131,7 +1136,8 @@ void pc_basic_device_init(struct PCMachineState *pcms,
i8257_dma_init(isa_bus, 0);
/* Super I/O */
- pc_superio_init(isa_bus, create_fdctrl, pcms->vmport != ON_OFF_AUTO_ON);
+ pc_superio_init(isa_bus, create_fdctrl, pcms->i8042_enabled,
+ pcms->vmport != ON_OFF_AUTO_ON);
}
void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus)
@@ -1512,6 +1518,20 @@ static void pc_machine_set_hpet(Object *obj, bool value, Error **errp)
pcms->hpet_enabled = value;
}
+static bool pc_machine_get_i8042(Object *obj, Error **errp)
+{
+ PCMachineState *pcms = PC_MACHINE(obj);
+
+ return pcms->i8042_enabled;
+}
+
+static void pc_machine_set_i8042(Object *obj, bool value, Error **errp)
+{
+ PCMachineState *pcms = PC_MACHINE(obj);
+
+ pcms->i8042_enabled = value;
+}
+
static bool pc_machine_get_default_bus_bypass_iommu(Object *obj, Error **errp)
{
PCMachineState *pcms = PC_MACHINE(obj);
@@ -1641,6 +1661,7 @@ static void pc_machine_initfn(Object *obj)
pcms->smbus_enabled = true;
pcms->sata_enabled = true;
pcms->pit_enabled = true;
+ pcms->i8042_enabled = true;
pcms->max_fw_size = 8 * MiB;
#ifdef CONFIG_HPET
pcms->hpet_enabled = true;
@@ -1777,6 +1798,9 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
object_class_property_set_description(oc, "hpet",
"Enable/disable high precision event timer emulation");
+ object_class_property_add_bool(oc, PC_MACHINE_I8042,
+ pc_machine_get_i8042, pc_machine_set_i8042);
+
object_class_property_add_bool(oc, "default-bus-bypass-iommu",
pc_machine_get_default_bus_bypass_iommu,
pc_machine_set_default_bus_bypass_iommu);
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 36/47] vhost-vsock: detach the virqueue element in case of error
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (34 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 35/47] pc: add option to disable PS/2 mouse/keyboard Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 37/47] x86: cleanup unused compat_apic_id_mode Michael S. Tsirkin
` (11 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel
Cc: VictorV, Peter Maydell, qemu-stable, Stefan Hajnoczi, Stefano Garzarella
From: Stefano Garzarella <sgarzare@redhat.com>
In vhost_vsock_common_send_transport_reset(), if an element popped from
the virtqueue is invalid, we should call virtqueue_detach_element() to
detach it from the virtqueue before freeing its memory.
Fixes: fc0b9b0e1c ("vhost-vsock: add virtio sockets device")
Fixes: CVE-2022-26354
Cc: qemu-stable@nongnu.org
Reported-by: VictorV <vv474172261@gmail.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20220228095058.27899-1-sgarzare@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/vhost-vsock-common.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c
index 3f3771274e..ed706681ac 100644
--- a/hw/virtio/vhost-vsock-common.c
+++ b/hw/virtio/vhost-vsock-common.c
@@ -153,19 +153,23 @@ static void vhost_vsock_common_send_transport_reset(VHostVSockCommon *vvc)
if (elem->out_num) {
error_report("invalid vhost-vsock event virtqueue element with "
"out buffers");
- goto out;
+ goto err;
}
if (iov_from_buf(elem->in_sg, elem->in_num, 0,
&event, sizeof(event)) != sizeof(event)) {
error_report("vhost-vsock event virtqueue element is too short");
- goto out;
+ goto err;
}
virtqueue_push(vq, elem, sizeof(event));
virtio_notify(VIRTIO_DEVICE(vvc), vq);
-out:
+ g_free(elem);
+ return;
+
+err:
+ virtqueue_detach_element(vq, elem, 0);
g_free(elem);
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 37/47] x86: cleanup unused compat_apic_id_mode
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (35 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 36/47] vhost-vsock: detach the virqueue element in case of error Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 38/47] hw/smbios: Add table 4 parameter, "processor-id" Michael S. Tsirkin
` (10 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Peter Maydell, Thomas Huth, Richard Henderson,
Philippe Mathieu-Daudé,
Igor Mammedov, Paolo Bonzini
From: Igor Mammedov <imammedo@redhat.com>
commit
f862ddbb1a4 (hw/i386: Remove the deprecated pc-1.x machine types)
removed the last user of broken APIC ID compat knob,
but compat_apic_id_mode itself was forgotten.
Clean it up and simplify x86_cpu_apic_id_from_index()
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20220228131634.3389805-1-imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/i386/x86.h | 2 --
hw/i386/x86.c | 16 +---------------
2 files changed, 1 insertion(+), 17 deletions(-)
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index a145a30370..916cc325ee 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -35,8 +35,6 @@ struct X86MachineClass {
/* TSC rate migration: */
bool save_tsc_khz;
- /* Enables contiguous-apic-ID mode */
- bool compat_apic_id_mode;
/* use DMA capable linuxboot option rom */
bool fwcfg_dma_enabled;
};
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index b84840a1bb..4cf107baea 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -83,24 +83,11 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
unsigned int cpu_index)
{
- X86MachineClass *x86mc = X86_MACHINE_GET_CLASS(x86ms);
X86CPUTopoInfo topo_info;
- uint32_t correct_id;
- static bool warned;
init_topo_info(&topo_info, x86ms);
- correct_id = x86_apicid_from_cpu_idx(&topo_info, cpu_index);
- if (x86mc->compat_apic_id_mode) {
- if (cpu_index != correct_id && !warned && !qtest_enabled()) {
- error_report("APIC IDs set in compatibility mode, "
- "CPU topology won't match the configuration");
- warned = true;
- }
- return cpu_index;
- } else {
- return correct_id;
- }
+ return x86_apicid_from_cpu_idx(&topo_info, cpu_index);
}
@@ -1330,7 +1317,6 @@ static void x86_machine_class_init(ObjectClass *oc, void *data)
mc->cpu_index_to_instance_props = x86_cpu_index_to_props;
mc->get_default_cpu_node_id = x86_get_default_cpu_node_id;
mc->possible_cpu_arch_ids = x86_possible_cpu_arch_ids;
- x86mc->compat_apic_id_mode = false;
x86mc->save_tsc_khz = true;
x86mc->fwcfg_dma_enabled = true;
nc->nmi_monitor_handler = x86_nmi;
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 38/47] hw/smbios: Add table 4 parameter, "processor-id"
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (36 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 37/47] x86: cleanup unused compat_apic_id_mode Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 39/47] pci: drop COMPAT_PROP_PCP for 2.0 machine types Michael S. Tsirkin
` (9 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Titus Rwantare, Patrick Venture, Ani Sinha,
Igor Mammedov, Peter Foley
From: Patrick Venture <venture@google.com>
This parameter is to be used in the processor_id entry in the type 4
table.
This parameter is set as optional and if left will use the values from
the CPU model.
This enables hiding the host information from the guest and allowing AMD
VMs to run pretending to be Intel for some userspace software concerns.
Reviewed-by: Peter Foley <pefoley@google.com>
Reviewed-by: Titus Rwantare <titusr@google.com>
Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20220125163118.1011809-1-venture@google.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/smbios/smbios.c | 19 ++++++++++++++++---
qemu-options.hx | 3 ++-
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 1f8d5c252f..60349ee402 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -104,9 +104,11 @@ static struct {
const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
uint64_t max_speed;
uint64_t current_speed;
+ uint64_t processor_id;
} type4 = {
.max_speed = DEFAULT_CPU_SPEED,
- .current_speed = DEFAULT_CPU_SPEED
+ .current_speed = DEFAULT_CPU_SPEED,
+ .processor_id = 0,
};
static struct {
@@ -327,6 +329,10 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
.name = "part",
.type = QEMU_OPT_STRING,
.help = "part number",
+ }, {
+ .name = "processor-id",
+ .type = QEMU_OPT_NUMBER,
+ .help = "processor id",
},
{ /* end of list */ }
};
@@ -683,8 +689,13 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
t->processor_type = 0x03; /* CPU */
t->processor_family = 0x01; /* Other */
SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
- t->processor_id[0] = cpu_to_le32(smbios_cpuid_version);
- t->processor_id[1] = cpu_to_le32(smbios_cpuid_features);
+ if (type4.processor_id == 0) {
+ t->processor_id[0] = cpu_to_le32(smbios_cpuid_version);
+ t->processor_id[1] = cpu_to_le32(smbios_cpuid_features);
+ } else {
+ t->processor_id[0] = cpu_to_le32((uint32_t)type4.processor_id);
+ t->processor_id[1] = cpu_to_le32(type4.processor_id >> 32);
+ }
SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version);
t->voltage = 0;
t->external_clock = cpu_to_le16(0); /* Unknown */
@@ -1323,6 +1334,8 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
save_opt(&type4.serial, opts, "serial");
save_opt(&type4.asset, opts, "asset");
save_opt(&type4.part, opts, "part");
+ /* If the value is 0, it will take the value from the CPU model. */
+ type4.processor_id = qemu_opt_get_number(opts, "processor-id", 0);
type4.max_speed = qemu_opt_get_number(opts, "max-speed",
DEFAULT_CPU_SPEED);
type4.current_speed = qemu_opt_get_number(opts, "current-speed",
diff --git a/qemu-options.hx b/qemu-options.hx
index 094a6c1d7c..5ce0ada75e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2537,6 +2537,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
" specify SMBIOS type 3 fields\n"
"-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n"
" [,asset=str][,part=str][,max-speed=%d][,current-speed=%d]\n"
+ " [,processor-id=%d]\n"
" specify SMBIOS type 4 fields\n"
"-smbios type=11[,value=str][,path=filename]\n"
" specify SMBIOS type 11 fields\n"
@@ -2562,7 +2563,7 @@ SRST
``-smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str][,sku=str]``
Specify SMBIOS type 3 fields
-``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str]``
+``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-id=%d]``
Specify SMBIOS type 4 fields
``-smbios type=11[,value=str][,path=filename]``
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 39/47] pci: drop COMPAT_PROP_PCP for 2.0 machine types
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (37 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 38/47] hw/smbios: Add table 4 parameter, "processor-id" Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 40/47] event_notifier: add event_notifier_get_wfd() Michael S. Tsirkin
` (8 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Peter Maydell, Richard Henderson, Igor Mammedov,
Paolo Bonzini
From: Igor Mammedov <imammedo@redhat.com>
COMPAT_PROP_PCP is 'on' by default and it's used for turning
off PCP capability on PCIe slots for 2.0 machine types using
compat machinery.
Drop not needed compat glue as Q35 supports migration starting
from 2.4 machine types.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20220222102504.3080104-1-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/i386/pc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 32bf12421e..fd55fc725c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -318,8 +318,6 @@ GlobalProperty pc_compat_2_0[] = {
{ "pci-serial-4x", "prog_if", "0" },
{ "virtio-net-pci", "guest_announce", "off" },
{ "ICH9-LPC", "memory-hotplug-support", "off" },
- { "xio3130-downstream", COMPAT_PROP_PCP, "off" },
- { "ioh3420", COMPAT_PROP_PCP, "off" },
};
const size_t pc_compat_2_0_len = G_N_ELEMENTS(pc_compat_2_0);
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 40/47] event_notifier: add event_notifier_get_wfd()
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (38 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 39/47] pci: drop COMPAT_PROP_PCP for 2.0 machine types Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 41/47] vhost: use wfd on functions setting vring call fd Michael S. Tsirkin
` (7 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Stefan Hajnoczi, Sergio Lopez, Paolo Bonzini
From: Sergio Lopez <slp@redhat.com>
event_notifier_get_fd(const EventNotifier *e) always returns
EventNotifier's read file descriptor (rfd). This is not a problem when
the EventNotifier is backed by a an eventfd, as a single file
descriptor is used both for reading and triggering events (rfd ==
wfd).
But, when EventNotifier is backed by a pipe pair, we have two file
descriptors, one that can only be used for reads (rfd), and the other
only for writes (wfd).
There's, at least, one known situation in which we need to obtain wfd
instead of rfd, which is when setting up the file that's going to be
sent to the peer in vhost's SET_VRING_CALL.
Add a new event_notifier_get_wfd(const EventNotifier *e) that can be
used to obtain wfd where needed.
Signed-off-by: Sergio Lopez <slp@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20220304100854.14829-2-slp@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/qemu/event_notifier.h | 1 +
util/event_notifier-posix.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/include/qemu/event_notifier.h b/include/qemu/event_notifier.h
index b79add035d..8a4ff308e1 100644
--- a/include/qemu/event_notifier.h
+++ b/include/qemu/event_notifier.h
@@ -38,6 +38,7 @@ int event_notifier_test_and_clear(EventNotifier *);
#ifdef CONFIG_POSIX
void event_notifier_init_fd(EventNotifier *, int fd);
int event_notifier_get_fd(const EventNotifier *);
+int event_notifier_get_wfd(const EventNotifier *);
#else
HANDLE event_notifier_get_handle(EventNotifier *);
#endif
diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
index 8307013c5d..16294e98d4 100644
--- a/util/event_notifier-posix.c
+++ b/util/event_notifier-posix.c
@@ -99,6 +99,11 @@ int event_notifier_get_fd(const EventNotifier *e)
return e->rfd;
}
+int event_notifier_get_wfd(const EventNotifier *e)
+{
+ return e->wfd;
+}
+
int event_notifier_set(EventNotifier *e)
{
static const uint64_t value = 1;
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 41/47] vhost: use wfd on functions setting vring call fd
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (39 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 40/47] event_notifier: add event_notifier_get_wfd() Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 42/47] configure, meson: allow enabling vhost-user on all POSIX systems Michael S. Tsirkin
` (6 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Stefan Hajnoczi, Sergio Lopez
From: Sergio Lopez <slp@redhat.com>
When ioeventfd is emulated using qemu_pipe(), only EventNotifier's wfd
can be used for writing.
Use the recently introduced event_notifier_get_wfd() function to
obtain the fd that our peer must use to signal the vring.
Signed-off-by: Sergio Lopez <slp@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20220304100854.14829-3-slp@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/vhost.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 7b03efccec..b643f42ea4 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1287,7 +1287,7 @@ static int vhost_virtqueue_init(struct vhost_dev *dev,
return r;
}
- file.fd = event_notifier_get_fd(&vq->masked_notifier);
+ file.fd = event_notifier_get_wfd(&vq->masked_notifier);
r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
if (r) {
VHOST_OPS_DEBUG(r, "vhost_set_vring_call failed");
@@ -1542,9 +1542,9 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
if (mask) {
assert(vdev->use_guest_notifier_mask);
- file.fd = event_notifier_get_fd(&hdev->vqs[index].masked_notifier);
+ file.fd = event_notifier_get_wfd(&hdev->vqs[index].masked_notifier);
} else {
- file.fd = event_notifier_get_fd(virtio_queue_get_guest_notifier(vvq));
+ file.fd = event_notifier_get_wfd(virtio_queue_get_guest_notifier(vvq));
}
file.index = hdev->vhost_ops->vhost_get_vq_index(hdev, n);
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 42/47] configure, meson: allow enabling vhost-user on all POSIX systems
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (40 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 41/47] vhost: use wfd on functions setting vring call fd Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 43/47] docs: vhost-user: add subsection for non-Linux platforms Michael S. Tsirkin
` (5 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Daniel P. Berrangé,
Sergio Lopez, Richard Henderson, Philippe Mathieu-Daudé,
Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini
From: Sergio Lopez <slp@redhat.com>
With the possibility of using a pipe pair via qemu_pipe() as a
replacement on operating systems that doesn't support eventfd,
vhost-user can also work on all POSIX systems.
This change allows enabling vhost-user on all non-Windows platforms
and makes libvhost_user (which still depends on eventfd) a linux-only
feature.
Signed-off-by: Sergio Lopez <slp@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20220304100854.14829-4-slp@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
configure | 4 ++--
meson.build | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index c56ed53ee3..daccf4be7c 100755
--- a/configure
+++ b/configure
@@ -1659,8 +1659,8 @@ fi
# vhost interdependencies and host support
# vhost backends
-if test "$vhost_user" = "yes" && test "$linux" != "yes"; then
- error_exit "vhost-user is only available on Linux"
+if test "$vhost_user" = "yes" && test "$mingw32" = "yes"; then
+ error_exit "vhost-user is not available on Windows"
fi
test "$vhost_vdpa" = "" && vhost_vdpa=$linux
if test "$vhost_vdpa" = "yes" && test "$linux" != "yes"; then
diff --git a/meson.build b/meson.build
index a5b63e62cd..28612fca36 100644
--- a/meson.build
+++ b/meson.build
@@ -2714,7 +2714,7 @@ if have_system or have_user
endif
vhost_user = not_found
-if 'CONFIG_VHOST_USER' in config_host
+if targetos == 'linux' and 'CONFIG_VHOST_USER' in config_host
libvhost_user = subproject('libvhost-user')
vhost_user = libvhost_user.get_variable('vhost_user_dep')
endif
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 43/47] docs: vhost-user: add subsection for non-Linux platforms
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (41 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 42/47] configure, meson: allow enabling vhost-user on all POSIX systems Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 44/47] tests/acpi: i386: allow FACP acpi table changes Michael S. Tsirkin
` (4 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Stefan Hajnoczi, Sergio Lopez
From: Sergio Lopez <slp@redhat.com>
Add a section explaining how vhost-user is supported on platforms
other than Linux.
Signed-off-by: Sergio Lopez <slp@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20220304100854.14829-5-slp@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
docs/interop/vhost-user.rst | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index edc3ad84a3..4dbc84fd00 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -38,6 +38,26 @@ conventions <backend_conventions>`.
*Master* and *slave* can be either a client (i.e. connecting) or
server (listening) in the socket communication.
+Support for platforms other than Linux
+--------------------------------------
+
+While vhost-user was initially developed targeting Linux, nowadays it
+is supported on any platform that provides the following features:
+
+- A way for requesting shared memory represented by a file descriptor
+ so it can be passed over a UNIX domain socket and then mapped by the
+ other process.
+
+- AF_UNIX sockets with SCM_RIGHTS, so QEMU and the other process can
+ exchange messages through it, including ancillary data when needed.
+
+- Either eventfd or pipe/pipe2. On platforms where eventfd is not
+ available, QEMU will automatically fall back to pipe2 or, as a last
+ resort, pipe. Each file descriptor will be used for receiving or
+ sending events by reading or writing (respectively) an 8-byte value
+ to the corresponding it. The 8-value itself has no meaning and
+ should not be interpreted.
+
Message Specification
=====================
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 44/47] tests/acpi: i386: allow FACP acpi table changes
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (42 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 43/47] docs: vhost-user: add subsection for non-Linux platforms Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 45/47] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table Michael S. Tsirkin
` (3 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Ani Sinha, Peter Maydell, Liav Albani, Igor Mammedov
From: Liav Albani <liavalb@gmail.com>
The FACP table is going to be changed for x86/q35 machines. To be sure
the following changes are not breaking any QEMU test this change follows
step 2 from the bios-tables-test.c guide on changes that affect ACPI
tables.
Signed-off-by: Liav Albani <liavalb@gmail.com>
Acked-by: Ani Sinha <ani@anisinha.ca>
Message-Id: <20220304154032.2071585-2-ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..7570e39369 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,5 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/FACP",
+"tests/data/acpi/q35/FACP.nosmm",
+"tests/data/acpi/q35/FACP.slic",
+"tests/data/acpi/q35/FACP.xapic",
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 45/47] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (43 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 44/47] tests/acpi: i386: allow FACP acpi table changes Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 46/47] tests/acpi: i386: update FACP table differences Michael S. Tsirkin
` (2 subsequent siblings)
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Peter Maydell, Richard Henderson, Liav Albani,
Igor Mammedov, Ani Sinha, Paolo Bonzini
From: Liav Albani <liavalb@gmail.com>
This can allow the guest OS to determine more easily if i8042 controller
is present in the system or not, so it doesn't need to do probing of the
controller, but just initialize it immediately, before enumerating the
ACPI AML namespace.
The 8042 bit in IAPC_BOOT_ARCH was introduced from ACPI spec v2 (FADT
revision 2 and above). Therefore, in this change, we only enable this bit for
x86/q35 machine types since x86/i440fx machines use FADT ACPI table with
revision 1.
Signed-off-by: Liav Albani <liavalb@gmail.com>
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Message-Id: <20220304154032.2071585-3-ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/acpi/acpi-defs.h | 1 +
include/hw/input/i8042.h | 15 +++++++++++++++
hw/acpi/aml-build.c | 8 +++++++-
hw/i386/acpi-build.c | 8 ++++++++
4 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index c97e8633ad..2b42e4192b 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -77,6 +77,7 @@ typedef struct AcpiFadtData {
uint16_t plvl2_lat; /* P_LVL2_LAT */
uint16_t plvl3_lat; /* P_LVL3_LAT */
uint16_t arm_boot_arch; /* ARM_BOOT_ARCH */
+ uint16_t iapc_boot_arch; /* IAPC_BOOT_ARCH */
uint8_t minor_ver; /* FADT Minor Version */
/*
diff --git a/include/hw/input/i8042.h b/include/hw/input/i8042.h
index 1d90432dae..e070f546e4 100644
--- a/include/hw/input/i8042.h
+++ b/include/hw/input/i8042.h
@@ -23,4 +23,19 @@ void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq,
void i8042_isa_mouse_fake_event(ISAKBDState *isa);
void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out);
+static inline bool i8042_present(void)
+{
+ bool amb = false;
+ return object_resolve_path_type("", TYPE_I8042, &amb) || amb;
+}
+
+/*
+ * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
+ * Flags, bit offset 1 - 8042.
+ */
+static inline uint16_t iapc_boot_arch_8042(void)
+{
+ return i8042_present() ? 0x1 << 1 : 0x0 ;
+}
+
#endif /* HW_INPUT_I8042_H */
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 8966e16320..1773cf55f1 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2152,7 +2152,13 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
- build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
+ /* IAPC_BOOT_ARCH */
+ if (f->rev == 1) {
+ build_append_int_noprefix(tbl, 0, 2);
+ } else {
+ /* since ACPI v2.0 */
+ build_append_int_noprefix(tbl, f->iapc_boot_arch, 2);
+ }
build_append_int_noprefix(tbl, 0, 1); /* Reserved */
build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebd47aa26f..4ad4d7286c 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -38,6 +38,7 @@
#include "hw/nvram/fw_cfg.h"
#include "hw/acpi/bios-linker-loader.h"
#include "hw/isa/isa.h"
+#include "hw/input/i8042.h"
#include "hw/block/fdc.h"
#include "hw/acpi/memory_hotplug.h"
#include "sysemu/tpm.h"
@@ -192,6 +193,13 @@ static void init_common_fadt_data(MachineState *ms, Object *o,
.address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
},
};
+
+ /*
+ * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
+ * Flags, bit offset 1 - 8042.
+ */
+ fadt.iapc_boot_arch = iapc_boot_arch_8042();
+
*data = fadt;
}
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 46/47] tests/acpi: i386: update FACP table differences
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (44 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 45/47] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 47/47] hw/acpi/microvm: turn on 8042 bit in FADT boot architecture flags if present Michael S. Tsirkin
2022-03-07 17:06 ` [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Peter Maydell
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Ani Sinha, Peter Maydell, Liav Albani, Igor Mammedov
From: Liav Albani <liavalb@gmail.com>
After changing the IAPC boot flags register to indicate support of i8042
in the machine chipset to help the guest OS to determine its existence
"faster", we need to have the updated FACP ACPI binary images in tree.
The ASL changes introduced are shown by the following diff:
@@ -42,35 +42,35 @@
[059h 0089 1] PM1 Control Block Length : 02
[05Ah 0090 1] PM2 Control Block Length : 00
[05Bh 0091 1] PM Timer Block Length : 04
[05Ch 0092 1] GPE0 Block Length : 10
[05Dh 0093 1] GPE1 Block Length : 00
[05Eh 0094 1] GPE1 Base Offset : 00
[05Fh 0095 1] _CST Support : 00
[060h 0096 2] C2 Latency : 0FFF
[062h 0098 2] C3 Latency : 0FFF
[064h 0100 2] CPU Cache Size : 0000
[066h 0102 2] Cache Flush Stride : 0000
[068h 0104 1] Duty Cycle Offset : 00
[069h 0105 1] Duty Cycle Width : 00
[06Ah 0106 1] RTC Day Alarm Index : 00
[06Bh 0107 1] RTC Month Alarm Index : 00
[06Ch 0108 1] RTC Century Index : 32
-[06Dh 0109 2] Boot Flags (decoded below) : 0000
+[06Dh 0109 2] Boot Flags (decoded below) : 0002
Legacy Devices Supported (V2) : 0
- 8042 Present on ports 60/64 (V2) : 0
+ 8042 Present on ports 60/64 (V2) : 1
VGA Not Present (V4) : 0
MSI Not Supported (V4) : 0
PCIe ASPM Not Supported (V4) : 0
CMOS RTC Not Present (V5) : 0
[06Fh 0111 1] Reserved : 00
[070h 0112 4] Flags (decoded below) : 000084A5
WBINVD instruction is operational (V1) : 1
WBINVD flushes all caches (V1) : 0
All CPUs support C1 (V1) : 1
C2 works on MP system (V1) : 0
Control Method Power Button (V1) : 0
Control Method Sleep Button (V1) : 1
RTC wake not in fixed reg space (V1) : 0
RTC can wake system from S4 (V1) : 1
32-bit PM Timer (V1) : 0
Docking Supported (V1) : 0
Signed-off-by: Liav Albani <liavalb@gmail.com>
Acked-by: Ani Sinha <ani@anisinha.ca>
Message-Id: <20220304154032.2071585-4-ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 4 ----
tests/data/acpi/q35/FACP | Bin 244 -> 244 bytes
tests/data/acpi/q35/FACP.nosmm | Bin 244 -> 244 bytes
tests/data/acpi/q35/FACP.slic | Bin 244 -> 244 bytes
tests/data/acpi/q35/FACP.xapic | Bin 244 -> 244 bytes
5 files changed, 4 deletions(-)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 7570e39369..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,5 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/FACP",
-"tests/data/acpi/q35/FACP.nosmm",
-"tests/data/acpi/q35/FACP.slic",
-"tests/data/acpi/q35/FACP.xapic",
diff --git a/tests/data/acpi/q35/FACP b/tests/data/acpi/q35/FACP
index f6a864cc863c7763f6c09d3814ad184a658fa0a0..a8f6a8961109d01059aceef9f1869cde09a2f10c 100644
GIT binary patch
delta 23
ecmeyu_=S<n&CxmF3j+fK^Y)2c$&5@B^V$GgGY3Ne
delta 23
ecmeyu_=S<n&CxmF3j+fK^UjG}$&3sW^V$GgJqJSo
diff --git a/tests/data/acpi/q35/FACP.nosmm b/tests/data/acpi/q35/FACP.nosmm
index 6a9aa5f370eb9af6a03dc739d8a159be58fdee01..c4e6d18ee5fc64159160d4589aa96b4d648c913a 100644
GIT binary patch
delta 23
ecmeyu_=S<n&CxmF3j+fKbKXR*WJacmd2Ik#q6Yc^
delta 23
ecmeyu_=S<n&CxmF3j+fKbHPNeWJZRGd2Ik#tOoi3
diff --git a/tests/data/acpi/q35/FACP.slic b/tests/data/acpi/q35/FACP.slic
index 15986e095cf2db7ee92f7ce113c1d46d54018c62..48bbb1cf5ad0ceda1d2f6d56edf5c1e207bd1a04 100644
GIT binary patch
delta 23
ecmeyu_=S<n&CxmF3j+fK^M#3A$&5@B^V$Gh6bD=Y
delta 23
ecmeyu_=S<n&CxmF3j+fK^QDPg$&3sW^V$Gh9tT_i
diff --git a/tests/data/acpi/q35/FACP.xapic b/tests/data/acpi/q35/FACP.xapic
index 2d3659c9c6753d07c3d48742343cb8e8cc034de7..31fa5dd19c213034eef4eeefa6a04e61dadd8a2a 100644
GIT binary patch
delta 23
ecmeyu_=S<n&CxmF3j+fK^X7?M$&5@B^V$Gg4+lR0
delta 23
ecmeyu_=S<n&CxmF3j+fK^VW%6$&3sW^V$Gg83#WA
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PULL v2 47/47] hw/acpi/microvm: turn on 8042 bit in FADT boot architecture flags if present
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (45 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 46/47] tests/acpi: i386: update FACP table differences Michael S. Tsirkin
@ 2022-03-07 10:03 ` Michael S. Tsirkin
2022-03-07 17:06 ` [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Peter Maydell
47 siblings, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 10:03 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Peter Maydell, Richard Henderson, Ani Sinha,
Paolo Bonzini
From: Ani Sinha <ani@anisinha.ca>
The second bit of IAPC_BOOT_ARCH in FADT table indicates the presence of
keyboard controller implemented as 8042 or equivalent micro controller. This
change enables this flag for microvms if such a device exists (for example,
when added explicitly from the QEMU commandline). Change
654701e292d98b308b0 ("hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table")
enabled this flag for i386 q35 based machines. The reason for doing the same
for micro-vms is to make sure we provide the correct tables to the guest OS
uniformly in all cases when an i8042 device is present. When this bit is not
enabled, guest OSes has to find other indirect methods to detect the device
which we would like to avoid.
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Message-Id: <20220304154032.2071585-5-ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/i386/acpi-microvm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
index 68ca7e7fc2..fb09185cbd 100644
--- a/hw/i386/acpi-microvm.c
+++ b/hw/i386/acpi-microvm.c
@@ -37,6 +37,7 @@
#include "hw/pci/pcie_host.h"
#include "hw/usb/xhci.h"
#include "hw/virtio/virtio-mmio.h"
+#include "hw/input/i8042.h"
#include "acpi-common.h"
#include "acpi-microvm.h"
@@ -187,6 +188,11 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
.address = GED_MMIO_BASE_REGS + ACPI_GED_REG_RESET,
},
.reset_val = ACPI_GED_RESET_VALUE,
+ /*
+ * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
+ * Flags, bit offset 1 - 8042.
+ */
+ .iapc_boot_arch = iapc_boot_arch_8042(),
};
table_offsets = g_array_new(false, true /* clear */,
--
MST
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
` (46 preceding siblings ...)
2022-03-07 10:03 ` [PULL v2 47/47] hw/acpi/microvm: turn on 8042 bit in FADT boot architecture flags if present Michael S. Tsirkin
@ 2022-03-07 17:06 ` Peter Maydell
2022-03-07 17:13 ` Peter Maydell
47 siblings, 1 reply; 57+ messages in thread
From: Peter Maydell @ 2022-03-07 17:06 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On Mon, 7 Mar 2022 at 10:01, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> The following changes since commit 6629bf78aac7e53f83fd0bcbdbe322e2302dfd1f:
>
> Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20220302' into staging (2022-03-03 14:46:48 +0000)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 41d137fc631bd9315ff84727d780757d25054c58:
>
> hw/acpi/microvm: turn on 8042 bit in FADT boot architecture flags if present (2022-03-06 16:06:16 -0500)
>
> ----------------------------------------------------------------
> virtio,pc,pci: features, cleanups, fixes
>
> vhost-user enabled on non-linux systems
> beginning of nvme sriov support
> bigger tx queue for vdpa
> virtio iommu bypass
> An FADT flag to detect legacy keyboards.
>
> Fixes, cleanups all over the place
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Fails to build on the build-system-centos job:
libqemu-ppc64-softmmu.fa.p/hw_virtio_virtio.c.o: In function
`qmp_decode_features':
/builds/qemu-project/qemu/build/../hw/virtio/virtio.c:4155: undefined
reference to `gpu_map'
/builds/qemu-project/qemu/build/../hw/virtio/virtio.c:4155: undefined
reference to `gpu_map'
collect2: error: ld returned 1 exit status
https://gitlab.com/qemu-project/qemu/-/jobs/2172339948
thanks
-- PMM
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes
2022-03-07 17:06 ` [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Peter Maydell
@ 2022-03-07 17:13 ` Peter Maydell
2022-03-07 22:52 ` Michael S. Tsirkin
0 siblings, 1 reply; 57+ messages in thread
From: Peter Maydell @ 2022-03-07 17:13 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On Mon, 7 Mar 2022 at 17:06, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 7 Mar 2022 at 10:01, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > The following changes since commit 6629bf78aac7e53f83fd0bcbdbe322e2302dfd1f:
> >
> > Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20220302' into staging (2022-03-03 14:46:48 +0000)
> >
> > are available in the Git repository at:
> >
> > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to 41d137fc631bd9315ff84727d780757d25054c58:
> >
> > hw/acpi/microvm: turn on 8042 bit in FADT boot architecture flags if present (2022-03-06 16:06:16 -0500)
> >
> > ----------------------------------------------------------------
> > virtio,pc,pci: features, cleanups, fixes
> >
> > vhost-user enabled on non-linux systems
> > beginning of nvme sriov support
> > bigger tx queue for vdpa
> > virtio iommu bypass
> > An FADT flag to detect legacy keyboards.
> >
> > Fixes, cleanups all over the place
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> Fails to build on the build-system-centos job:
>
> libqemu-ppc64-softmmu.fa.p/hw_virtio_virtio.c.o: In function
> `qmp_decode_features':
> /builds/qemu-project/qemu/build/../hw/virtio/virtio.c:4155: undefined
> reference to `gpu_map'
> /builds/qemu-project/qemu/build/../hw/virtio/virtio.c:4155: undefined
> reference to `gpu_map'
> collect2: error: ld returned 1 exit status
>
> https://gitlab.com/qemu-project/qemu/-/jobs/2172339948
Also fails on cross-win64-system:
https://gitlab.com/qemu-project/qemu/-/jobs/2172339938
../hw/virtio/virtio.c: In function 'qmp_x_query_virtio_vhost_queue_status':
../hw/virtio/virtio.c:4358:30: error: cast from pointer to integer of
different size [-Werror=pointer-to-int-cast]
4358 | status->desc = (uint64_t)(unsigned long)hdev->vqs[queue].desc;
| ^
../hw/virtio/virtio.c:4359:31: error: cast from pointer to integer of
different size [-Werror=pointer-to-int-cast]
4359 | status->avail = (uint64_t)(unsigned long)hdev->vqs[queue].avail;
| ^
../hw/virtio/virtio.c:4360:30: error: cast from pointer to integer of
different size [-Werror=pointer-to-int-cast]
4360 | status->used = (uint64_t)(unsigned long)hdev->vqs[queue].used;
| ^
cc1: all warnings being treated as errors
-- PMM
>
> thanks
> -- PMM
--
12345678901234567890123456789012345678901234567890123456789012345678901234567890
1 2 3 4 5 6 7 8
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes
2022-03-07 17:13 ` Peter Maydell
@ 2022-03-07 22:52 ` Michael S. Tsirkin
2022-03-08 9:05 ` Peter Maydell
0 siblings, 1 reply; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-07 22:52 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel
On Mon, Mar 07, 2022 at 05:13:16PM +0000, Peter Maydell wrote:
> On Mon, 7 Mar 2022 at 17:06, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 7 Mar 2022 at 10:01, Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > The following changes since commit 6629bf78aac7e53f83fd0bcbdbe322e2302dfd1f:
> > >
> > > Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20220302' into staging (2022-03-03 14:46:48 +0000)
> > >
> > > are available in the Git repository at:
> > >
> > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> > >
> > > for you to fetch changes up to 41d137fc631bd9315ff84727d780757d25054c58:
> > >
> > > hw/acpi/microvm: turn on 8042 bit in FADT boot architecture flags if present (2022-03-06 16:06:16 -0500)
> > >
> > > ----------------------------------------------------------------
> > > virtio,pc,pci: features, cleanups, fixes
> > >
> > > vhost-user enabled on non-linux systems
> > > beginning of nvme sriov support
> > > bigger tx queue for vdpa
> > > virtio iommu bypass
> > > An FADT flag to detect legacy keyboards.
> > >
> > > Fixes, cleanups all over the place
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > Fails to build on the build-system-centos job:
> >
> > libqemu-ppc64-softmmu.fa.p/hw_virtio_virtio.c.o: In function
> > `qmp_decode_features':
> > /builds/qemu-project/qemu/build/../hw/virtio/virtio.c:4155: undefined
> > reference to `gpu_map'
> > /builds/qemu-project/qemu/build/../hw/virtio/virtio.c:4155: undefined
> > reference to `gpu_map'
> > collect2: error: ld returned 1 exit status
> >
> > https://gitlab.com/qemu-project/qemu/-/jobs/2172339948
>
> Also fails on cross-win64-system:
>
> https://gitlab.com/qemu-project/qemu/-/jobs/2172339938
>
> ../hw/virtio/virtio.c: In function 'qmp_x_query_virtio_vhost_queue_status':
> ../hw/virtio/virtio.c:4358:30: error: cast from pointer to integer of
> different size [-Werror=pointer-to-int-cast]
> 4358 | status->desc = (uint64_t)(unsigned long)hdev->vqs[queue].desc;
> | ^
> ../hw/virtio/virtio.c:4359:31: error: cast from pointer to integer of
> different size [-Werror=pointer-to-int-cast]
> 4359 | status->avail = (uint64_t)(unsigned long)hdev->vqs[queue].avail;
> | ^
> ../hw/virtio/virtio.c:4360:30: error: cast from pointer to integer of
> different size [-Werror=pointer-to-int-cast]
> 4360 | status->used = (uint64_t)(unsigned long)hdev->vqs[queue].used;
> | ^
> cc1: all warnings being treated as errors
>
> -- PMM
I dropped these for now but I really question the value of this warning,
as you can see the reason we have the buggy cast to unsigned long
is because someone wanted to shut up the warning on a 32 bit system.
Now, I could maybe get behind this if it simply warned about a cast that
loses information (cast to a smaller integer) or integer/pointer cast
that does not go through uintptr_t without regard to size.
>
>
> >
> > thanks
> > -- PMM
>
>
>
> --
> 12345678901234567890123456789012345678901234567890123456789012345678901234567890
> 1 2 3 4 5 6 7 8
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes
2022-03-07 22:52 ` Michael S. Tsirkin
@ 2022-03-08 9:05 ` Peter Maydell
2022-03-08 11:01 ` Michael S. Tsirkin
0 siblings, 1 reply; 57+ messages in thread
From: Peter Maydell @ 2022-03-08 9:05 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On Mon, 7 Mar 2022 at 22:52, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Mar 07, 2022 at 05:13:16PM +0000, Peter Maydell wrote:
> > Also fails on cross-win64-system:
> >
> > https://gitlab.com/qemu-project/qemu/-/jobs/2172339938
> >
> > ../hw/virtio/virtio.c: In function 'qmp_x_query_virtio_vhost_queue_status':
> > ../hw/virtio/virtio.c:4358:30: error: cast from pointer to integer of
> > different size [-Werror=pointer-to-int-cast]
> > 4358 | status->desc = (uint64_t)(unsigned long)hdev->vqs[queue].desc;
> > | ^
> > ../hw/virtio/virtio.c:4359:31: error: cast from pointer to integer of
> > different size [-Werror=pointer-to-int-cast]
> > 4359 | status->avail = (uint64_t)(unsigned long)hdev->vqs[queue].avail;
> > | ^
> > ../hw/virtio/virtio.c:4360:30: error: cast from pointer to integer of
> > different size [-Werror=pointer-to-int-cast]
> > 4360 | status->used = (uint64_t)(unsigned long)hdev->vqs[queue].used;
> > | ^
> > cc1: all warnings being treated as errors
> I dropped these for now but I really question the value of this warning,
> as you can see the reason we have the buggy cast to unsigned long
> is because someone wanted to shut up the warning on a 32 bit system.
>
> Now, I could maybe get behind this if it simply warned about a cast that
> loses information (cast to a smaller integer) or integer/pointer cast
> that does not go through uintptr_t without regard to size.
This *is* warning about losing information. On 64-bit Windows
pointers are 64 bits but 'long' is 32 bits, so the path
pointer -> long -> uint64_t drops the top half of the pointer.
thanks
-- PMM
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes
2022-03-08 9:05 ` Peter Maydell
@ 2022-03-08 11:01 ` Michael S. Tsirkin
2022-03-08 11:18 ` Peter Maydell
0 siblings, 1 reply; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-08 11:01 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel
On Tue, Mar 08, 2022 at 09:05:27AM +0000, Peter Maydell wrote:
> On Mon, 7 Mar 2022 at 22:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Mon, Mar 07, 2022 at 05:13:16PM +0000, Peter Maydell wrote:
> > > Also fails on cross-win64-system:
> > >
> > > https://gitlab.com/qemu-project/qemu/-/jobs/2172339938
> > >
> > > ../hw/virtio/virtio.c: In function 'qmp_x_query_virtio_vhost_queue_status':
> > > ../hw/virtio/virtio.c:4358:30: error: cast from pointer to integer of
> > > different size [-Werror=pointer-to-int-cast]
> > > 4358 | status->desc = (uint64_t)(unsigned long)hdev->vqs[queue].desc;
> > > | ^
> > > ../hw/virtio/virtio.c:4359:31: error: cast from pointer to integer of
> > > different size [-Werror=pointer-to-int-cast]
> > > 4359 | status->avail = (uint64_t)(unsigned long)hdev->vqs[queue].avail;
> > > | ^
> > > ../hw/virtio/virtio.c:4360:30: error: cast from pointer to integer of
> > > different size [-Werror=pointer-to-int-cast]
> > > 4360 | status->used = (uint64_t)(unsigned long)hdev->vqs[queue].used;
> > > | ^
> > > cc1: all warnings being treated as errors
>
> > I dropped these for now but I really question the value of this warning,
> > as you can see the reason we have the buggy cast to unsigned long
> > is because someone wanted to shut up the warning on a 32 bit system.
> >
> > Now, I could maybe get behind this if it simply warned about a cast that
> > loses information (cast to a smaller integer) or integer/pointer cast
> > that does not go through uintptr_t without regard to size.
>
> This *is* warning about losing information. On 64-bit Windows
> pointers are 64 bits but 'long' is 32 bits, so the path
> pointer -> long -> uint64_t drops the top half of the pointer.
>
> thanks
> -- PMM
Yes obviously. My point is that this:
(uint64_t)hdev->vqs[queue].avail
is always harmless but it warns on a 32 bit system.
And someone trying to fix that *is* what resulted in
(uint64_t)(unsigned long)hdev->vqs[queue].avail
IOW I don't really see how
(uint64_t)(uintptr_t)hdev->vqs[queue].avail
is better than
(uint64_t)hdev->vqs[queue].avail
except as a way to say "yes I do intend to cast pointer to integer
here, I did not forget to dereference the pointer". But if that
latter is what gcc is trying to warn about, then it should
just warn about any cast to integer except to uintptr_t,
without respect to size.
--
MST
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes
2022-03-08 11:01 ` Michael S. Tsirkin
@ 2022-03-08 11:18 ` Peter Maydell
2022-03-08 11:23 ` Michael S. Tsirkin
2022-03-08 13:19 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 57+ messages in thread
From: Peter Maydell @ 2022-03-08 11:18 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On Tue, 8 Mar 2022 at 11:01, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Mar 08, 2022 at 09:05:27AM +0000, Peter Maydell wrote:
> > On Mon, 7 Mar 2022 at 22:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Mon, Mar 07, 2022 at 05:13:16PM +0000, Peter Maydell wrote:
> > > > Also fails on cross-win64-system:
> > > >
> > > > https://gitlab.com/qemu-project/qemu/-/jobs/2172339938
> > > >
> > > > ../hw/virtio/virtio.c: In function 'qmp_x_query_virtio_vhost_queue_status':
> > > > ../hw/virtio/virtio.c:4358:30: error: cast from pointer to integer of
> > > > different size [-Werror=pointer-to-int-cast]
> > > > 4358 | status->desc = (uint64_t)(unsigned long)hdev->vqs[queue].desc;
> > > > | ^
> > > > ../hw/virtio/virtio.c:4359:31: error: cast from pointer to integer of
> > > > different size [-Werror=pointer-to-int-cast]
> > > > 4359 | status->avail = (uint64_t)(unsigned long)hdev->vqs[queue].avail;
> > > > | ^
> > > > ../hw/virtio/virtio.c:4360:30: error: cast from pointer to integer of
> > > > different size [-Werror=pointer-to-int-cast]
> > > > 4360 | status->used = (uint64_t)(unsigned long)hdev->vqs[queue].used;
> > > > | ^
> > > > cc1: all warnings being treated as errors
> >
> > > I dropped these for now but I really question the value of this warning,
> > > as you can see the reason we have the buggy cast to unsigned long
> > > is because someone wanted to shut up the warning on a 32 bit system.
> > >
> > > Now, I could maybe get behind this if it simply warned about a cast that
> > > loses information (cast to a smaller integer) or integer/pointer cast
> > > that does not go through uintptr_t without regard to size.
> >
> > This *is* warning about losing information. On 64-bit Windows
> > pointers are 64 bits but 'long' is 32 bits, so the path
> > pointer -> long -> uint64_t drops the top half of the pointer.
> Yes obviously. My point is that this:
> (uint64_t)hdev->vqs[queue].avail
> is always harmless but it warns on a 32 bit system.
True, I suppose. But compiler warnings are often like that: we
take the hit of having to tweak some things we know to be OK in
order to catch the real bugs in other cases.
> And someone trying to fix that *is* what resulted in
> (uint64_t)(unsigned long)hdev->vqs[queue].avail
Using 'unsigned long' in a cast (or anything else) is often
the wrong thing in QEMU...
> IOW I don't really see how
> (uint64_t)(uintptr_t)hdev->vqs[queue].avail
> is better than
> (uint64_t)hdev->vqs[queue].avail
>
> except as a way to say "yes I do intend to cast pointer to integer
> here, I did not forget to dereference the pointer". But if that
> latter is what gcc is trying to warn about, then it should
> just warn about any cast to integer except to uintptr_t,
> without respect to size.
What is the uint64_t cast bringing to the table? Wouldn't
just status->desc = (uintptr_t)hdev->vqs[queue].desc;
work ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes
2022-03-08 11:18 ` Peter Maydell
@ 2022-03-08 11:23 ` Michael S. Tsirkin
2022-03-08 13:19 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 57+ messages in thread
From: Michael S. Tsirkin @ 2022-03-08 11:23 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel
On Tue, Mar 08, 2022 at 11:18:38AM +0000, Peter Maydell wrote:
> On Tue, 8 Mar 2022 at 11:01, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Mar 08, 2022 at 09:05:27AM +0000, Peter Maydell wrote:
> > > On Mon, 7 Mar 2022 at 22:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Mon, Mar 07, 2022 at 05:13:16PM +0000, Peter Maydell wrote:
> > > > > Also fails on cross-win64-system:
> > > > >
> > > > > https://gitlab.com/qemu-project/qemu/-/jobs/2172339938
> > > > >
> > > > > ../hw/virtio/virtio.c: In function 'qmp_x_query_virtio_vhost_queue_status':
> > > > > ../hw/virtio/virtio.c:4358:30: error: cast from pointer to integer of
> > > > > different size [-Werror=pointer-to-int-cast]
> > > > > 4358 | status->desc = (uint64_t)(unsigned long)hdev->vqs[queue].desc;
> > > > > | ^
> > > > > ../hw/virtio/virtio.c:4359:31: error: cast from pointer to integer of
> > > > > different size [-Werror=pointer-to-int-cast]
> > > > > 4359 | status->avail = (uint64_t)(unsigned long)hdev->vqs[queue].avail;
> > > > > | ^
> > > > > ../hw/virtio/virtio.c:4360:30: error: cast from pointer to integer of
> > > > > different size [-Werror=pointer-to-int-cast]
> > > > > 4360 | status->used = (uint64_t)(unsigned long)hdev->vqs[queue].used;
> > > > > | ^
> > > > > cc1: all warnings being treated as errors
> > >
> > > > I dropped these for now but I really question the value of this warning,
> > > > as you can see the reason we have the buggy cast to unsigned long
> > > > is because someone wanted to shut up the warning on a 32 bit system.
> > > >
> > > > Now, I could maybe get behind this if it simply warned about a cast that
> > > > loses information (cast to a smaller integer) or integer/pointer cast
> > > > that does not go through uintptr_t without regard to size.
> > >
> > > This *is* warning about losing information. On 64-bit Windows
> > > pointers are 64 bits but 'long' is 32 bits, so the path
> > > pointer -> long -> uint64_t drops the top half of the pointer.
>
> > Yes obviously. My point is that this:
> > (uint64_t)hdev->vqs[queue].avail
> > is always harmless but it warns on a 32 bit system.
>
> True, I suppose. But compiler warnings are often like that: we
> take the hit of having to tweak some things we know to be OK in
> order to catch the real bugs in other cases.
>
> > And someone trying to fix that *is* what resulted in
> > (uint64_t)(unsigned long)hdev->vqs[queue].avail
>
> Using 'unsigned long' in a cast (or anything else) is often
> the wrong thing in QEMU...
>
> > IOW I don't really see how
> > (uint64_t)(uintptr_t)hdev->vqs[queue].avail
> > is better than
> > (uint64_t)hdev->vqs[queue].avail
> >
> > except as a way to say "yes I do intend to cast pointer to integer
> > here, I did not forget to dereference the pointer". But if that
> > latter is what gcc is trying to warn about, then it should
> > just warn about any cast to integer except to uintptr_t,
> > without respect to size.
>
> What is the uint64_t cast bringing to the table? Wouldn't
> just status->desc = (uintptr_t)hdev->vqs[queue].desc;
> work ?
>
> thanks
> -- PMM
True too. I would be happy if this gave a warning:
struct {
uint32_t foo;
} bar = { .foo = (uintptr_t)hdev->vqs[queue].avail }
but of course it doesn't, uintptr_t is just an integer
type as far as gcc is concerned :(
Maybe adding things like that to sparse might be practical.
Generally sparse seems to mainly gain features from the
kernel, QEMU's needs are different. Could be useful to collect
ideas e.g. for a GSoC project.
--
MST
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes
2022-03-08 11:18 ` Peter Maydell
2022-03-08 11:23 ` Michael S. Tsirkin
@ 2022-03-08 13:19 ` Philippe Mathieu-Daudé
2022-03-15 18:41 ` Peter Maydell
1 sibling, 1 reply; 57+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-08 13:19 UTC (permalink / raw)
To: Peter Maydell, Michael S. Tsirkin; +Cc: qemu-devel
On 8/3/22 12:18, Peter Maydell wrote:
> On Tue, 8 Mar 2022 at 11:01, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> On Tue, Mar 08, 2022 at 09:05:27AM +0000, Peter Maydell wrote:
>>> On Mon, 7 Mar 2022 at 22:52, Michael S. Tsirkin <mst@redhat.com> wrote:
>>>> Now, I could maybe get behind this if it simply warned about a cast that
>>>> loses information (cast to a smaller integer) or integer/pointer cast
>>>> that does not go through uintptr_t without regard to size.
>>>
>>> This *is* warning about losing information. On 64-bit Windows
>>> pointers are 64 bits but 'long' is 32 bits, so the path
>>> pointer -> long -> uint64_t drops the top half of the pointer.
>
>> Yes obviously. My point is that this:
>> (uint64_t)hdev->vqs[queue].avail
>> is always harmless but it warns on a 32 bit system.
>
> True, I suppose. But compiler warnings are often like that: we
> take the hit of having to tweak some things we know to be OK in
> order to catch the real bugs in other cases.
>
>> And someone trying to fix that *is* what resulted in
>> (uint64_t)(unsigned long)hdev->vqs[queue].avail
>
> Using 'unsigned long' in a cast (or anything else) is often
> the wrong thing in QEMU...
$ git grep -F '(unsigned long)' | wc -l
273
Ouch :/
These require cleanup:
target/i386/sev.c:170: input.data = (__u64)(unsigned long)data;
target/i386/sev.c:188: arg.data = (unsigned long)data;
target/i386/sev.c:243: range.addr = (__u64)(unsigned long)host;
target/i386/sev.c:273: range.addr = (__u64)(unsigned long)host;
target/i386/sev.c:730: update.uaddr = (__u64)(unsigned long)addr;
And we might add a Gitlab issue to look at the hw/ ones:
$ git grep -F '(unsigned long)' hw | wc -l
76
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes
2022-03-08 13:19 ` Philippe Mathieu-Daudé
@ 2022-03-15 18:41 ` Peter Maydell
0 siblings, 0 replies; 57+ messages in thread
From: Peter Maydell @ 2022-03-15 18:41 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Michael S. Tsirkin
On Tue, 15 Mar 2022 at 18:35, Philippe Mathieu-Daudé
<philippe.mathieu.daude@gmail.com> wrote:
> On 8/3/22 12:18, Peter Maydell wrote:
> > Using 'unsigned long' in a cast (or anything else) is often
> > the wrong thing in QEMU...
>
> $ git grep -F '(unsigned long)' | wc -l
> 273
>
> Ouch :/
Only "often", not "always" :-) We have some APIs that work on
'long', usually because they're generic APIs borrowed from the
Linux kernel like the clear_bit/set_bit functions. And sometimes
you're interfacing to a host OS API whose types are 'long'.
So it's only one of those things that I tend to have in the
back of my head during code review, rather than something I think
we could enforce automatically.
The stuff in sev.c you list does look a bit suspicious, but
it's not actually buggy because that's all KVM code so we
know 'unsigned long' and pointers are the same size.
'uintptr_t' would be better, though.
thanks
-- PMM
^ permalink raw reply [flat|nested] 57+ messages in thread