Re: [PATCH v4 1/5] qemu-ga: 'Null' check for mandatory parameters
okay, I'll summarize all of the patches. thanks for reviewing.
On Fri, Oct 25, 2024 at 8:48 PM Daniel P. Berrangé
wrote:
> On Tue, Oct 22, 2024 at 10:29:44PM +0800, Dehan Meng wrote:
> > sscanf return values are checked and add 'Null' check for
> > mandatory parameters.
> >
> > Signed-off-by: Dehan Meng
> > ---
> > qga/commands-linux.c | 12 +++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/qga/commands-linux.c b/qga/commands-linux.c
> > index 51d5e3d927..f0e9cdd27c 100644
> > --- a/qga/commands-linux.c
> > +++ b/qga/commands-linux.c
> > @@ -2103,7 +2103,9 @@ static char *hexToIPAddress(const void *hexValue,
> int is_ipv6)
> > int i;
> >
> > for (i = 0; i < 16; i++) {
> > -sscanf(&hexStr[i * 2], "%02hhx", &in6.s6_addr[i]);
> > +if (sscanf(&hex_str[i * 2], "%02hhx", &in6.s6_addr[i]) !=
> 1) {
> > +return NULL;
> > +}
> > }
> > inet_ntop(AF_INET6, &in6, addr, INET6_ADDRSTRLEN);
> >
> > @@ -2164,6 +2166,10 @@ GuestNetworkRouteList
> *qmp_guest_network_get_route(Error **errp)
> > networkroute = route;
> > networkroute->iface = g_strdup(Iface);
> > networkroute->destination = hexToIPAddress(Destination,
> 1);
> > +if (networkroute->destination == NULL) {
> > +g_free(route);
> > +continue;
> > +}
>
> This still hasn't fixed the leak problems identified in the previous
> review of the last version
>
> > networkroute->metric = Metric;
> > networkroute->source = hexToIPAddress(Source, 1);
> > networkroute->desprefixlen = g_strdup_printf(
> > @@ -2195,6 +2201,10 @@ GuestNetworkRouteList
> *qmp_guest_network_get_route(Error **errp)
> > networkroute = route;
> > networkroute->iface = g_strdup(Iface);
> > networkroute->destination =
> hexToIPAddress(&Destination, 0);
> > +if (networkroute->destination == NULL) {
> > +g_free(route);
> > +continue;
> > +}
> > networkroute->gateway = hexToIPAddress(&Gateway, 0);
> > networkroute->mask = hexToIPAddress(&Mask, 0);
> > networkroute->metric = Metric;
> > --
> > 2.40.1
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-
> https://www.instagram.com/dberrange :|
>
>
Re: [PATCH v4 5/5] qemu-ga: Replace g_new0() with g_autoptr()
Thank you for reviewing, I'll summarize all of the patches.
On Fri, Oct 25, 2024 at 8:50 PM Daniel P. Berrangé
wrote:
> On Tue, Oct 22, 2024 at 10:29:48PM +0800, Dehan Meng wrote:
> > Replace g_new0() with g_autoptr() to simplify the code
> >
> > Signed-off-by: Dehan Meng
> > ---
> > qga/commands-linux.c | 16 ++--
> > 1 file changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/qga/commands-linux.c b/qga/commands-linux.c
> > index 9fb31956b4..ee4f345938 100644
> > --- a/qga/commands-linux.c
> > +++ b/qga/commands-linux.c
> > @@ -2158,15 +2158,13 @@ GuestNetworkRouteList
> *qmp_guest_network_get_route(Error **errp)
> > continue;
> > }
> >
> > -GuestNetworkRoute *route = g_new0(GuestNetworkRoute, 1);
> > +g_autoptr(GuestNetworkRoute) route =
> g_new0(GuestNetworkRoute, 1);
> >
> > route->destination = hex_to_ip_address(destination, 1);
> > -if (route->destination == NULL) {
> > -g_free(route);
> > +route->iface = g_strdup(iface);
> > +if (route->destination == NULL || route->iface ==
> NULL) {
>
> Checking "iface" for NULL is not required, since g_strdup will never
> fail to allocate memory.
>
> Also, these changes to use g_autoptr need to be part of the first patch,
> as each step in the patch series needs to be correct.
>
> > continue;
> > }
> > -route->iface = g_strdup(iface);
> > -route->destination = hex_to_ip_address(destination, 1);
> > route->source = hex_to_ip_address(source, 1);
> > route->nexthop = hex_to_ip_address(next_hop, 1);
> > route->desprefixlen = g_strdup_printf("%d",
> des_prefixlen);
> > @@ -2188,15 +2186,13 @@ GuestNetworkRouteList
> *qmp_guest_network_get_route(Error **errp)
> > continue;
> > }
> >
> > -GuestNetworkRoute *route = g_new0(GuestNetworkRoute, 1);
> > +g_autoptr(GuestNetworkRoute) route =
> g_new0(GuestNetworkRoute, 1);
> >
> > route->destination = hex_to_ip_address(destination, 1);
> > -if (route->destination == NULL) {
> > -g_free(route);
> > +route->iface = g_strdup(iface);
> > +if (route->destination == NULL || route->iface ==
> NULL) {
> > continue;
> > }
> > -route->iface = g_strdup(iface);
> > -route->destination = hex_to_ip_address(&destination, 0);
> > route->gateway = hex_to_ip_address(&gateway, 0);
> > route->mask = hex_to_ip_address(&mask, 0);
> > route->metric = metric;
> > --
> > 2.40.1
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-
> https://www.instagram.com/dberrange :|
>
>
[PATCH v6 3/3] tests/qtest/bios-tables-test: Update virt SPCR golden reference for RISC-V
Update the virt SPCR golden reference file for RISC-V to accommodate the
SPCR Table revision 4 [1], utilizing the iasl binary compiled from the
latest ACPICA repository. The SPCR table has been modified to
adhere to the revision 4 format [2].
[1]:
https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
[2]: https://github.com/acpica/acpica/pull/931
Diffs from iasl:
/*
* Intel ACPI Component Architecture
* AML/ASL+ Disassembler version 20200925 (64-bit version)
* Copyright (c) 2000 - 2020 Intel Corporation
*
- * Disassembly of tests/data/acpi/riscv64/virt/SPCR, Wed Aug 28 18:28:19 2024
+ * Disassembly of /tmp/aml-MN0NS2, Wed Aug 28 18:28:19 2024
*
* ACPI Data Table [SPCR]
*
* Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue
*/
[000h 4]Signature : "SPCR"[Serial Port Console
Redirection table]
-[004h 0004 4] Table Length : 0050
-[008h 0008 1] Revision : 02
-[009h 0009 1] Checksum : B9
+[004h 0004 4] Table Length : 005A
+[008h 0008 1] Revision : 04
+[009h 0009 1] Checksum : 13
[00Ah 0010 6] Oem ID : "BOCHS "
[010h 0016 8] Oem Table ID : "BXPC"
[018h 0024 4] Oem Revision : 0001
[01Ch 0028 4] Asl Compiler ID : "BXPC"
[020h 0032 4]Asl Compiler Revision : 0001
-[024h 0036 1] Interface Type : 00
+[024h 0036 1] Interface Type : 12
[025h 0037 3] Reserved : 00
[028h 0040 12] Serial Port Register : [Generic Address Structure]
[028h 0040 1] Space ID : 00 [SystemMemory]
[029h 0041 1]Bit Width : 20
[02Ah 0042 1] Bit Offset : 00
[02Bh 0043 1] Encoded Access Width : 01 [Byte Access:8]
[02Ch 0044 8] Address : 1000
[034h 0052 1] Interrupt Type : 10
[035h 0053 1] PCAT-compatible IRQ : 00
[036h 0054 4]Interrupt : 000A
[03Ah 0058 1]Baud Rate : 07
[03Bh 0059 1] Parity : 00
[03Ch 0060 1]Stop Bits : 01
[03Dh 0061 1] Flow Control : 00
[03Eh 0062 1]Terminal Type : 00
[04Ch 0076 1] Reserved : 00
[040h 0064 2]PCI Device ID :
[042h 0066 2]PCI Vendor ID :
[044h 0068 1] PCI Bus : 00
[045h 0069 1] PCI Device : 00
[046h 0070 1] PCI Function : 00
[047h 0071 4]PCI Flags :
[04Bh 0075 1] PCI Segment : 00
-[04Ch 0076 4] Reserved :
+[04Ch 0076 004h] Uart Clock Freq :
+[050h 0080 004h] Precise Baud rate :
+[054h 0084 002h] NameSpaceStringLength : 0002
+[056h 0086 002h] NameSpaceStringOffset : 0058
+[058h 0088 002h] NamespaceString : "."
-Raw Table Data: Length 80 (0x50)
+Raw Table Data: Length 90 (0x5A)
-: 53 50 43 52 50 00 00 00 02 B9 42 4F 43 48 53 20 // SPCRP.BOCHS
+: 53 50 43 52 5A 00 00 00 04 13 42 4F 43 48 53 20 // SPCRZ.BOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43 // BXPCBXPC
-0020: 01 00 00 00 00 00 00 00 00 20 00 01 00 00 00 10 // . ..
+0020: 01 00 00 00 12 00 00 00 00 20 00 01 00 00 00 10 // . ..
0030: 00 00 00 00 10 00 0A 00 00 00 07 00 01 00 00 03 //
0040: FF FF FF FF 00 00 00 00 00 00 00 00 00 00 00 00 //
+0050: 00 00 00 00 02 00 58 00 2E 00// ..X...
Signed-off-by: Sia Jee Heng
Reviewed-by: Sunil V L
---
tests/data/acpi/riscv64/virt/SPCR | Bin 80 -> 90 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 1 -
2 files changed, 1 deletion(-)
diff --git a/tests/data/acpi/riscv64/virt/SPCR
b/tests/data/acpi/riscv64/virt/SPCR
index
4da9daf65f71a13ac2b488d4e9728f194b569a43..09617f8793a6f7b1f08172f735b58aa748671540
100644
GIT binary patch
delta 32
mcmWHD;tCFM4vJ!6U|+&Vu)bSV*mhNumqU^
delta 21
ccmazF;0g|K4hmpkU|`xgkxPn^VWO%w05v59j{pDw
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h
b/tests/qtest/bios-tables-test-allowed-diff.h
index aae973048a..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/riscv64/virt/SPCR",
--
2.43.0
[PATCH v6 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table revision 4 format
Update the SPCR table to accommodate the SPCR Table revision 4 [1].
The SPCR table has been modified to adhere to the revision 4 format [2].
[1]:
https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
[2]: https://github.com/acpica/acpica/pull/931
Signed-off-by: Sia Jee Heng
Reviewed-by: Sunil V L
Reviewed-by: Michael S. Tsirkin
Acked-by: Alistair Francis
---
hw/acpi/aml-build.c | 20
hw/arm/virt-acpi-build.c| 8 ++--
hw/loongarch/acpi-build.c | 6 +-
hw/riscv/virt-acpi-build.c | 12 +---
include/hw/acpi/acpi-defs.h | 7 +--
include/hw/acpi/aml-build.h | 2 +-
6 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 34e0ddbde8..69c4bdfa22 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1995,7 +1995,7 @@ static void build_processor_hierarchy_node(GArray *tbl,
uint32_t flags,
void build_spcr(GArray *table_data, BIOSLinker *linker,
const AcpiSpcrData *f, const uint8_t rev,
-const char *oem_id, const char *oem_table_id)
+const char *oem_id, const char *oem_table_id, const char *name)
{
AcpiTable table = { .sig = "SPCR", .rev = rev, .oem_id = oem_id,
.oem_table_id = oem_table_id };
@@ -2041,9 +2041,21 @@ void build_spcr(GArray *table_data, BIOSLinker *linker,
build_append_int_noprefix(table_data, f->pci_flags, 4);
/* PCI Segment */
build_append_int_noprefix(table_data, f->pci_segment, 1);
-/* Reserved */
-build_append_int_noprefix(table_data, 0, 4);
-
+if (rev < 4) {
+/* Reserved */
+build_append_int_noprefix(table_data, 0, 4);
+} else {
+/* UartClkFreq */
+build_append_int_noprefix(table_data, f->uart_clk_freq, 4);
+/* PreciseBaudrate */
+build_append_int_noprefix(table_data, f->precise_baudrate, 4);
+/* NameSpaceStringLength */
+build_append_int_noprefix(table_data, f->namespace_string_length, 2);
+/* NameSpaceStringOffset */
+build_append_int_noprefix(table_data, f->namespace_string_offset, 2);
+/* NamespaceString[] */
+g_array_append_vals(table_data, name, f->namespace_string_length);
+}
acpi_table_end(linker, &table);
}
/*
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index f76fb117ad..0b6f5f8d8d 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -464,8 +464,12 @@ spcr_setup(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
.pci_flags = 0,
.pci_segment = 0,
};
-
-build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id);
+/*
+ * Passing NULL as the SPCR Table for Revision 2 doesn't support
+ * NameSpaceString.
+ */
+build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id,
+ NULL);
}
/*
diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 50709bda0f..4e04f7b6c1 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -276,8 +276,12 @@ spcr_setup(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
};
lvms = LOONGARCH_VIRT_MACHINE(machine);
+/*
+ * Passing NULL as the SPCR Table for Revision 2 doesn't support
+ * NameSpaceString.
+ */
build_spcr(table_data, linker, &serial, 2, lvms->oem_id,
- lvms->oem_table_id);
+ lvms->oem_table_id, NULL);
}
typedef
diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 36d6a3a412..68ef15acac 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -200,14 +200,15 @@ acpi_dsdt_add_uart(Aml *scope, const MemMapEntry
*uart_memmap,
/*
* Serial Port Console Redirection Table (SPCR)
- * Rev: 1.07
+ * Rev: 1.10
*/
static void
spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
{
+const char name[] = ".";
AcpiSpcrData serial = {
-.interface_type = 0, /* 16550 compatible */
+.interface_type = 0x12, /* 16550 compatible */
.base_addr.id = AML_AS_SYSTEM_MEMORY,
.base_addr.width = 32,
.base_addr.offset = 0,
@@ -229,9 +230,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker,
RISCVVirtState *s)
.pci_function = 0,
.pci_flags = 0,
.pci_segment = 0,
+.uart_clk_freq = 0,
+.precise_baudrate = 0,
+.namespace_string_length = sizeof(name),
+.namespace_string_offset = 88,
};
-build_spcr(table_data, linker, &serial, 2, s->oem_id, s->oem_table_id);
+build_spcr(table_data, linker, &serial, 4, s->oem_id, s->oem_table_id,
+ name);
}
/* RHCT Node[N] starts at offset 56 */
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index 0e6e82b339..2e6e341998 100644
--- a/include/hw/acpi/acpi-defs.
[PATCH v6 0/3] Upgrade ACPI SPCR table to support SPCR table revision 4 format
Update the SPCR table to accommodate the SPCR Table revision 4 [1]. The SPCR table has been modified to adhere to the revision 4 format [2]. Meanwhile, the virt SPCR golden reference file for RISC-V have been updated to accommodate the SPCR Table revision 4. [1]: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table [2]: https://github.com/acpica/acpica/pull/931 Changes in v6: - Added Reviewed-by: Michael S. Tsirkin - Rebase and update the build_spcr() function for the LoongArch virt machine. Changes in v5: - Reverted the SPCR table revision history for the ARM architecture. - Corrected the output of the SPCR Table diff. Changes in v4: - Remove the SPCR table revision 4 update for the ARM architecture. Changes in v3: - Rebased on the latest QEMU. - Added Acked-by: Alistair Francis Changes in v2: - Utilizes a three-patch approach to modify the ACPI pre-built binary files required by the Bios-Table-Test. - Rebases and incorporates changes to support both ARM and RISC-V ACPI pre-built binary files. Sia Jee Heng (3): qtest: allow SPCR acpi table changes hw/acpi: Upgrade ACPI SPCR table to support SPCR table revision 4 format tests/qtest/bios-tables-test: Update virt SPCR golden reference for RISC-V hw/acpi/aml-build.c | 20 hw/arm/virt-acpi-build.c | 8 ++-- hw/loongarch/acpi-build.c | 6 +- hw/riscv/virt-acpi-build.c| 12 +--- include/hw/acpi/acpi-defs.h | 7 +-- include/hw/acpi/aml-build.h | 2 +- tests/data/acpi/riscv64/virt/SPCR | Bin 80 -> 90 bytes 7 files changed, 42 insertions(+), 13 deletions(-) base-commit: cea8ac78545a83e1f01c94d89d6f5a3f6b5c05d2 -- 2.43.0
[PATCH v6 1/3] qtest: allow SPCR acpi table changes
Signed-off-by: Sia Jee Heng Reviewed-by: Sunil V L Acked-by: Alistair Francis --- tests/qtest/bios-tables-test-allowed-diff.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..aae973048a 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,2 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/riscv64/virt/SPCR", -- 2.43.0
Re: [PATCH v3 2/7] target/i386: Add RAS feature bits on EPYC CPU models
(+John) Hi Babu, This patch is fine for me. However, users recently reported an issue with SUCCOR support on AMD hosts: https://gitlab.com/qemu-project/qemu/-/issues/2571. Could you please double check and clarify that issue on AMD host? Thanks, Zhao On Thu, Oct 24, 2024 at 05:18:20PM -0500, Babu Moger wrote: > Date: Thu, 24 Oct 2024 17:18:20 -0500 > From: Babu Moger > Subject: [PATCH v3 2/7] target/i386: Add RAS feature bits on EPYC CPU models > X-Mailer: git-send-email 2.34.1 > > Add the support for following RAS features bits on AMD guests. > > SUCCOR: Software uncorrectable error containment and recovery capability. > The processor supports software containment of uncorrectable errors > through context synchronizing data poisoning and deferred error > interrupts. > > McaOverflowRecov: MCA overflow recovery support. > > Reviewed-by: Zhao Liu > Signed-off-by: Babu Moger > --- > v3: No changes > > v2: Added reviewed by from Zhao. > --- > target/i386/cpu.c | 30 ++ > 1 file changed, 30 insertions(+)
Re: [PATCH v3 00/23] rust: fix CI + allow older versions of rustc and bindgen
I think this is the wrong direction (ie, backwards). Sacrificing current code to be compatible with old stuff feels wrong. Especially for really old, like rustc in debian bookworm. bookworm has rustc-web (and a few related packages) which is regular rustc version 1.78, just renamed. It is regular bookworm, not backports. It has some packages disabled (compared to regular rust) and is a hack, but it exists and can be used for now (dunno if it is sufficient for qemu though). Also debian has backports mechanism, which also can be used for qemu - I can try back-porting regular rust (and llvm) to bookworm. I think this is a better way (at least a way forward) than trying to move backwards. But generally, what is the reason to support debian stable? I understand the CI thing, - we need a way to test stuff. For this, I'd say a better alternative would be to target debian testing (currently trixie), not debian stable. Thanks, /mjt
Re: [PATCH v3 00/23] rust: fix CI + allow older versions of rustc and bindgen
On Sun, Oct 27, 2024 at 8:02 AM Michael Tokarev wrote: >i > I think this is the wrong direction (ie, backwards). > > Sacrificing current code to be compatible with old stuff feels wrong. > Especially for really old, like rustc in debian bookworm. > > bookworm has rustc-web (and a few related packages) which is regular > rustc version 1.78, just renamed. It is regular bookworm, not backports. > It has some packages disabled (compared to regular rust) and is a hack, > but it exists and can be used for now (dunno if it is sufficient for > qemu though). Thanks for pointing it out! It is indeed better, however it does not support mipsel. > Also debian has backports mechanism, which also can be used for qemu - > I can try back-porting regular rust (and llvm) to bookworm. > I think this is a better way (at least a way forward) than trying to > move backwards. > > But generally, what is the reason to support debian stable? I understand > the CI thing, - we need a way to test stuff. For this, I'd say a better > alternative would be to target debian testing (currently trixie), not > debian stable. Basically: it is not too hard and it can be reverted without much hassle once we stop supporting Debian 12 in general. Also, the next-lowest version (in Ubuntu 22.04, which has 1.75.0) would still have some relatively invasive changes, for example patch 11. We'll always need some workarounds until all supported distros have rustc 1.77.0. Paolo
Re: [PATCH v3 00/23] rust: fix CI + allow older versions of rustc and bindgen
27.10.2024 11:00, Paolo Bonzini wrpte: [rustc-web] Thanks for pointing it out! It is indeed better, however it does not support mipsel. mipsel? do you mean mips64el? /mjt
Re: [PATCH v3 00/23] rust: fix CI + allow older versions of rustc and bindgen
27.10.2024 12:38, Michael Tokarev wrote: 27.10.2024 11:00, Paolo Bonzini wrpte: [rustc-web] Thanks for pointing it out! It is indeed better, however it does not support mipsel. mipsel? do you mean mips64el? Ah. I see what you mean. https://buildd.debian.org/status/package.php?p=rustc-web&suite=bookworm FWIW, mipsel has been removed for the next debian, it isn't supported anymore in sid or testing (trixie). /mjt
Re: [PATCH 00/11] Rust device model patches and misc cleanups
On Sat, Oct 26, 2024 at 12:06 PM Manos Pitsidianakis
wrote:
> Please reply with review comments underneath individual patches, this
> is hard to follow and I might miss some points.
Will do.
> > >Revert "rust: add PL011 device model"
> > >rust: add PL011 device model
> >
> > ... should definitely be moved on top to clean up the authorship in "git
> > blame" and other similar tools. Sorry about that.
>
> I will send them on a separate series and merge them from my tree when
> reviewed.
Those are trivial to review, just "git diff HEAD^^". No need to
separate them into a new submission.
> > >rust: add support for migration in device models
> > >rust/pl011: move CLK_NAME static to function scope
> > >rust/pl011: add TYPE_PL011_LUMINARY device
> > >rust/pl011: remove commented out C code
> > >rust/pl011: Use correct masks for IBRD and FBRD
> >
> > (minus the usage of #[derive()] should be included in that series, so
> > that qtests pass. It's not a huge amount of work and I can extract it,
> > of course with proper attribution/authorship.
>
> These are independent from CI; i.e. you can merge your CI patches after those.
That's what I did: I put them at the beginning of the series of
pending patches, so they _are_ indeed merged after the CI patches. The
only issue here is that patch 4 ("rust: add support for migration in
device models") was dependent on the Device proc macro, but that was
easy enough to extract.
> >
> > The rest are future work:
> >
> > >rust/qemu-api-macros: introduce Device proc macro
> >
> > As I said above, we first need to agree on the design.
>
> Post your review under the patches please,
Yep.
> > >rust/pl011: move pub callback decl to local scope
> >
> > This depends a lot on how we go implementing bindings to chardev. For
> > example if the callbacks turn out to be a trait, it would have to be
> > undone. Possibly the C callback wrappers would move to
> > rust/qemu-api/chardev. For now I'd leave it aside.
>
> This patch moves the callbacks scope from public to inside the
> function, no functional change related. It doesn't change or have
> anything to do with chardev interfaces
I understand. My point is that the callbacks you move
(pl011_can_receive, pl011_receive, pl011_event) in the end will belong
into the C<->Rust chardev bindings. A patch to remove "pub" would have
basically the same benefit without the churn in indentation.
> > >rust/qemu-api: add log module
> > >rust/pl011: log guest/unimp errors
> >
> > This also needs design discussion. Do we want the API to be the same as
> > C, i.e. keep the qemu_* prefix? Do we want wrapper macros that include
> > the format!() call?
>
> I'm guessing you did not see the patch messages, which cover these
> points. Post your review under the patches please,
I will but for now I'll say that the commit messages do not address
the questions I'm asking above.
More in general, we need to establish conventions on what the Rust
APIs look like (about qemu_* prefixes, for example). Personally I
prefer to have APIs that, while easy enough to connect to the C ones,
are idiomatic. But you can post these two patches separately and we
can discuss it there.
> > You also have "pub enum LogMask" to work around the fact that log masks
> > are preprocessor macros. Is that okay, or do we want to modify C code
> > to make the bindings nicer? I for example would prefer the latter and
> > then declaring LogMask as a bitfield in bindgen.
>
> A bindgen type is definitely preferred but for a Rust idiomatic
> interface a wrapper type is a UX improvement (`CPU_LOG_PCALL`?
> `LOG_GUEST_ERROR`? We can use friendlier symbols in the LogMask
> variants for that)
What I'm saying is that these are discussions where we need to reach
an agreement on, and document the choices.
Paolo
Re: [PATCH] rust: do not always select X_PL011_RUST
On Fri, 25 Oct 2024 12:42, Paolo Bonzini wrote: >Right now the Rust pl011 device is included in all QEMU system >emulator binaries if --enable-rust is passed. This is not needed >since the board logic in hw/arm/Kconfig will pick it. > >Signed-off-by: Paolo Bonzini >--- > rust/hw/char/Kconfig | 1 - > 1 file changed, 1 deletion(-) > >diff --git a/rust/hw/char/Kconfig b/rust/hw/char/Kconfig >index a1732a9e97f..5fe800c4806 100644 >--- a/rust/hw/char/Kconfig >+++ b/rust/hw/char/Kconfig >@@ -1,3 +1,2 @@ > config X_PL011_RUST > bool >-default y if HAVE_RUST >-- >2.47.0 > (Do I need someone else reviewing this before picking this up in my tree?) Acked-by: Manos Pitsidianakis
Re: [PATCH 0/2] arm: Add collie and sx functional tests
On 10/27/24 15:26, Cédric Le Goater wrote:
On 10/27/24 23:11, Guenter Roeck wrote:
On 10/27/24 14:13, Cédric Le Goater wrote:
On 10/26/24 17:32, Guenter Roeck wrote:
On 10/26/24 03:02, Cédric Le Goater wrote:
[ ... ]
I don't mind a single file. What bothers me is that the partitioning is made
mandatory for ast2600 even if not used.
Our only use case, in 2019, was to boot QEMU ast2600 machines from an
eMMC device using an OpenBMC FW image like the ones we find on IBM
Power10 Rainier systems. I agree we only focused on this scenario.
Most of the support should be there for other use cases, and it's now
a question of finding the right tunables for the user.
I did a quick experiment using 2 patches,
one on hw/sd/sd.c to fix c8cb19876d3e ("hw/sd/sdcard: Support boot
area in emmc image")
@@ -826,7 +826,9 @@ static void sd_reset(DeviceState *dev)
sect = 0;
}
size = sect << HWBLOCK_SHIFT;
- size -= sd_bootpart_offset(sd);
+ if (sd_is_emmc(sd)) {
+ size -= sd->boot_part_size * 2;
+ }
sect = sd_addr_to_wpnum(size) + 1;
and another on hw/arm/aspeed.c to remove the setting of the eMMC
device properties when it is not bootable :
@@ -338,7 +338,7 @@ static void sdhci_attach_drive(SDHCIStat
return;
}
card = qdev_new(emmc ? TYPE_EMMC : TYPE_SD_CARD);
- if (emmc) {
+ if (emmc && boot_emmc) {
qdev_prop_set_uint64(card, "boot-partition-size", 1 * MiB);
qdev_prop_set_uint8(card, "boot-config",
boot_emmc ? 0x1 << 3 : 0x0);
(I am not saying this is correct)
Works for me, though, and it is much better than mandating the existence
of boot partitions.
Yes. However, if the emmc device was user creatable, we could use :
-blockdev node-name=emmc0,driver=file,filename=mmc-ast2600-evb-noboot.raw \
-device emmc,bus=sdhci-bus.2,drive=emmc0
and with boot partitions:
-M boot-emmc=true \
-blockdev node-name=emmc0,driver=file,filename=mmc-ast2600-evb.raw \
-device
emmc,bus=sdhci-bus.2,drive=emmc0,boot-partition-size=1048576,boot-config=8
The above would be my preferred approach if acceptable. The "sd-bus"
bus identifier should be changed in other machines tough.
No real preference here, though my understanding is that emmc devices
are by definition built-in, and that is what emmc_class_init() says as well.
Also, there does not seem to be an sdhci-bus, only sd-bus, and that does
not support any index values. That may be just my lack of knowledge, though.
No, you are right. On a real ast2600-evb, the eMMC device is indeed
soldered on the board. But, for testing purposes, it is sometime
interesting to add some flexibility in the machine definition and
in the modeling too. This avoids "hard-coding" default devices in
the machines and lets the user define its own variant models using
the QEMU command line.
I would agree, but I had a number of my patches rejected because while
they would be useful for testing they would not accurately reflect the
hardware. So nowadays I gave up even trying to upstream such changes.
Guenter
Re: [PATCH v3 1/7] target/i386: Fix minor typo in NO_NESTED_DATA_BP feature bit
On Thu, Oct 24, 2024 at 05:18:19PM -0500, Babu Moger wrote: > Date: Thu, 24 Oct 2024 17:18:19 -0500 > From: Babu Moger > Subject: [PATCH v3 1/7] target/i386: Fix minor typo in NO_NESTED_DATA_BP > feature bit > X-Mailer: git-send-email 2.34.1 > > Rename CPUID_8000_0021_EAX_No_NESTED_DATA_BP to >CPUID_8000_0021_EAX_NO_NESTED_DATA_BP. > > No functional change intended. > > Signed-off-by: Babu Moger > --- > v3: New patch. > --- > target/i386/cpu.c | 2 +- > target/i386/cpu.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Zhao Liu
Re: [RFC v3 3/3] vhost: Allocate memory for packed vring
Hi,
It's been a while since I gave my last update. I have one more update
that I would like to give.
> On Tue, Sep 24, 2024 at 7:31 AM Sahil wrote:
> > And I booted L2 by running:
> >
> > # ./qemu/build/qemu-system-x86_64 \
> > -nographic \
> > -m 4G \
> > -enable-kvm \
> > -M q35 \
> > -drive file=//root/L2.qcow2,media=disk,if=virtio \
> > -netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhost-vdpa0 \
> > -device
> > virtio-net-pci,netdev=vhost-vdpa0,disable-legacy=on,disable-modern=off,ev
> > ent_idx=off,bus=pcie.0,addr=0x7 \ -smp 4 \
> > -cpu host \
> > 2>&1 | tee vm.log
>
> With packed=on in the device option, I see that the packed feature bit is
> set in L2 :)
>
> However, I see that vhost shadow virtqueues are still not being used. I am
> currently trying to find the reason behind this. I have narrowed down the
> issue to hw/virtio/vhost-vdpa.c [1]. The "vhost_vdpa_svqs_start" function
> is being called but in the loop, vhost_svq_start is never called. I think it
> might be because there's an issue with "vhost_vdpa_svq_setup".
>
> I'll send an update once I find something.
>
> Thanks,
> Sahil
>
> [1] https://github.com/qemu/qemu/blob/master/hw/virtio/vhost-vdpa.c#L1243
I spent some time tinkering with the L0-L1-L2 test environment setup,
and understanding QEMU's hw/virtio/vhost-vdpa.c [1] as well as Linux's
drivers/vhost/vdpa.c [2] and /drivers/vhost/vhost.c [3]. I don't think there
is an issue with the environment itself.
When I boot L2 with the following combinations of "x-svq" and
"packed", this is what I observe:
1. x-svq=on and packed=off
The virtio device in L2 has the packed feature bit turned off. Vhost
shadow virtqueues are used as expected.
2. x-svq=off and packed=on
The virtio device in L2 has the packed feature bit turned on. Vhost
shadow virtqueues are not used.
I don't see any issues in either of the above environment
configurations.
3. x-svq=on and packed=on
This is the configuration that I need for testing. The virtio device in
L2 has the packed feature bit turned on. However, vhost shadow
virtqueues are not being used. This is due to the
VHOST_SET_VRING_BASE ioctl call returning a EOPNOTSUPP in
hw/virtio/vhost-vdpa.c:vhost_vdpa_set_dev_vring_base() [4].
I spent some time going through the ioctl's implementation in Linux.
I used ftrace to trace the functions that were being called in the kernel.
With x-svq=on (regardless of whether split virtqueues are used or packed
virtqueues), I got the following trace:
[...]
qemu-system-x86-1737[001] ...1. 3613.371358:
vhost_vdpa_unlocked_ioctl <-__x64_sys_ioctl
qemu-system-x86-1737[001] ...1. 3613.371358: vhost_vring_ioctl
<-vhost_vdpa_unlocked_ioctl
qemu-system-x86-1737[001] ...1. 3613.371362:
vp_vdpa_set_vq_state <-vhost_vdpa_unlocked_ioctl
[...]
There are 3 virtqueues that the vdpa device offers in L1. There were no
issues when using split virtqueues and the trace shown above appears
3 times. With packed virtqueues, the first call to VHOST_SET_VRING_BASE
fails because drivers/vdpa/virtio_pci/vp_vdpa.c:vp_vdpa_set_vq_state_packed
[5] returns EOPNOTSUPP.
The payload that VHOST_SET_VRING_BASE accepts depends on whether
split virtqueues or packed virtqueues are used [6]. In hw/virtio/vhost-
vdpa.c:vhost_vdpa_svq_setup() [7], the following payload is used which is
not suitable for packed virtqueues:
struct vhost_vring_state s = {
.index = vq_index,
};
Based on the implementation in the linux kernel, the payload needs to
be as shown below for the ioctl to succeed for packed virtqueues:
struct vhost_vring_state s = {
.index = vq_index,
.num = 0x80008000,
};
After making these changes, it looks like QEMU is able to set up the
virtqueues
and shadow virtqueues are enabled as well.
Unfortunately, before the L2 VM can finish booting the kernel crashes.
The reason is that even though packed virtqueues are to be used, the
kernel tries to run
drivers/virtio/virtio_ring.c:virtqueue_get_buf_ctx_split() [8]
(instead of virtqueue_get_buf_ctx_packed) and throws an "invalid vring
head" error. I am still investigating this issue.
I'll send an update once I resolve this issue. I'll also send a patch that
crafts the payload correctly based on the format of the virtqueue in
vhost_vdpa_svq_setup().
Thanks,
Sahil
[1] https://gitlab.com/qemu-project/qemu/-/blob/master/hw/virtio/vhost-vdpa.c
[2] https://github.com/torvalds/linux/blob/master/drivers/vhost/vdpa.c
[3] https://github.com/torvalds/linux/blob/master/drivers/vhost/vhost.c
[4]
https://gitlab.com/qemu-project/qemu/-/blob/master/hw/virtio/vhost-vdpa.c#L1002
[5]
https://github.com/torvalds/linux/blob/master/drivers/vdpa/virtio_pci/vp_vdpa.c#L278
[6]
https://qemu-project.gitlab.io/qemu/interop/vhost-user.html#front-end-message-types
[7]
https://gitlab.com/qemu-project/qemu/-/blob/master/hw/virtio/vhost-vdpa.c#L1223
[8]
https://github.com/torvalds/linux/blob/master/drivers/virtio/virtio_ring.c#L823
Re: [PATCH v17 02/14] hw/ppc/spapr_pci: Do not create DT for disabled PCI device
On 10/22/24 2:06 PM, Akihiko Odaki wrote:
Disabled means it is a disabled SR-IOV VF and hidden from the guest.
Do not create DT when starting the system and also keep the disabled PCI
device not linked to DRC, which generates DT in case of hotplug.
Signed-off-by: Akihiko Odaki
---
hw/ppc/spapr_pci.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 5c0024bef9c4..679a22fe4e79 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1291,8 +1291,7 @@ static void spapr_dt_pci_device_cb(PCIBus *bus, PCIDevice
*pdev,
PciWalkFdt *p = opaque;
int err;
-if (p->err) {
-/* Something's already broken, don't keep going */
+if (p->err || !pdev->enabled) {
return;
}
@@ -1592,10 +1591,10 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
uint32_t slotnr = PCI_SLOT(pdev->devfn);
/*
- * If DR is disabled we don't need to do anything in the case of
- * hotplug or coldplug callbacks.
+ * If DR or the PCI device is disabled we don't need to do anything
+ * in the case of hotplug or coldplug callbacks.
*/
-if (!phb->dr_enabled) {
+if (!phb->dr_enabled || !pdev->enabled) {
return;
}
Thank you. This is the right place to be called from the hotplug handler
instead of the spapr_pci_dt_populate() unlike I mentioned before..
@@ -1680,6 +1679,11 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
}
g_assert(drc);
+
+if (!drc->dev) {
+return;
I agree with the change here, but were you able to get to this path? I
don't see
this if condition being entered with,
qemu-system-ppc64 -nographic -serial none -device
spapr-pci-host-bridge,index=4,id=pci.1 -device igb,id=igb0 <<<
'device_del igb0'
Regards,
Shivaprasad
+}
+
g_assert(drc->dev == plugged_dev);
if (!spapr_drc_unplug_requested(drc)) {
Re: [PATCH] configure, meson: deprecate 32-bit MIPS
On 27/10/24 10:07, Paolo Bonzini wrote: The mipsel architecture is not available in Debian Bookworm, and it will likely be a hard failure as soon as we drop support for the old Rust toolchain in Debian Bullseye. Prepare by deprecating 32-bit little endian MIPS in QEMU 9.2. Signed-off-by: Paolo Bonzini --- docs/about/build-platforms.rst | 2 +- docs/about/deprecated.rst | 12 meson.build| 8 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/docs/about/build-platforms.rst b/docs/about/build-platforms.rst index ff56091078e..6102f00aec0 100644 --- a/docs/about/build-platforms.rst +++ b/docs/about/build-platforms.rst @@ -41,7 +41,7 @@ Those hosts are officially supported, with various accelerators: - Accelerators * - Arm - kvm (64 bit only), tcg, xen - * - MIPS (little endian only) + * - MIPS (64 bit little endian only) - kvm, tcg * - PPC - kvm, tcg diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 74c75666c31..32b4cd16ec3 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -170,15 +170,19 @@ property types. Host Architectures -- -BE MIPS (since 7.2) -''' +32-bit MIPS (big endian since 7.2; little endian since 9.2) +''' Big endian MIPS since 7.2; 32-bit little endian MIPS since 9.2 ''
Re: [PATCH] configure: detect 64-bit MIPS
On 27/10/24 10:05, Paolo Bonzini wrote:
While right now 64-bit MIPS and 32-bit MIPS share the code in QEMU,
Rust uses different rules for the target. Set $cpu correctly to
either mips or mips64 (--cpu=mips64* is already accepted in the case
statement that canonicalizes cpu/host_arch/linux_arch), and adjust
the checks to account for the different between $cpu (which handles
mips/mips64 separately) and $host_arch (which does not).
Fixes: 1a6ef6ff624 ("configure, meson: detect Rust toolchain", 2024-10-11)
Signed-off-by: Paolo Bonzini
---
configure | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
Acked-by: Philippe Mathieu-Daudé
Re: [PATCH v17 02/14] hw/ppc/spapr_pci: Do not create DT for disabled PCI device
On 2024/10/28 12:08, Shivaprasad G Bhat wrote:
On 10/22/24 2:06 PM, Akihiko Odaki wrote:
Disabled means it is a disabled SR-IOV VF and hidden from the guest.
Do not create DT when starting the system and also keep the disabled PCI
device not linked to DRC, which generates DT in case of hotplug.
Signed-off-by: Akihiko Odaki
---
hw/ppc/spapr_pci.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 5c0024bef9c4..679a22fe4e79 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1291,8 +1291,7 @@ static void spapr_dt_pci_device_cb(PCIBus *bus,
PCIDevice *pdev,
PciWalkFdt *p = opaque;
int err;
- if (p->err) {
- /* Something's already broken, don't keep going */
+ if (p->err || !pdev->enabled) {
return;
}
@@ -1592,10 +1591,10 @@ static void spapr_pci_plug(HotplugHandler
*plug_handler,
uint32_t slotnr = PCI_SLOT(pdev->devfn);
/*
- * If DR is disabled we don't need to do anything in the case of
- * hotplug or coldplug callbacks.
+ * If DR or the PCI device is disabled we don't need to do anything
+ * in the case of hotplug or coldplug callbacks.
*/
- if (!phb->dr_enabled) {
+ if (!phb->dr_enabled || !pdev->enabled) {
return;
}
Thank you. This is the right place to be called from the hotplug handler
instead of the spapr_pci_dt_populate() unlike I mentioned before..
@@ -1680,6 +1679,11 @@ static void
spapr_pci_unplug_request(HotplugHandler *plug_handler,
}
g_assert(drc);
+
+ if (!drc->dev) {
+ return;
I agree with the change here, but were you able to get to this path? I
don't see
this if condition being entered with,
qemu-system-ppc64 -nographic -serial none -device spapr-pci-host-
bridge,index=4,id=pci.1 -device igb,id=igb0 <<< 'device_del igb0'
No. VFs bypass the hotplug path when unplugging. For context, see
unparent_vfs() in "[PATCH v17 09/14] pcie_sriov: Reuse SR-IOV VF device
instances.
Regards,
Akihiko Odaki
Regards,
Shivaprasad
+ }
+
g_assert(drc->dev == plugged_dev);
if (!spapr_drc_unplug_requested(drc)) {
Re: [PATCH v5] intel_iommu: Introduce property "stale-tm" to control Transient Mapping (TM) field
Reviewed-by: Clément Mathieu--Drif
On 28/10/2024 03:25, Zhenzhong Duan wrote:
> Caution: External email. Do not open attachments or click links, unless this
> email comes from a known sender and you know the content is safe.
>
>
> VT-d spec removed Transient Mapping (TM) field from second-level page-tables
> and treat the field as Reserved(0) since revision 3.2.
>
> Changing the field as reserved(0) will break backward compatibility, so
> introduce a property "stale-tm" to allow user to control the setting.
>
> Use pc_compat_9_1 to handle the compatibility for machines before 9.2 which
> allow guest to set the field. Starting from 9.2, this field is reserved(0)
> by default to match spec. Of course, user can force it on command line.
>
> This doesn't impact function of vIOMMU as there was no logic to emulate
> Transient Mapping.
>
> Suggested-by: Yi Liu
> Suggested-by: Jason Wang
> Signed-off-by: Zhenzhong Duan
> Acked-by: Jason Wang
> Reviewed-by: Yi Liu
> ---
> v5: fix typo, s/hw_compat_9_1/pc_compat_9_1 (Liuyi)
> v4: s/x-stale-tm/stale-tm (Jason)
> v3: still need to check x86_iommu->dt_supported
> v2: introcude "x-stale-tm" to handle migration compatibility (Jason)
>
> hw/i386/intel_iommu_internal.h | 12 ++--
> include/hw/i386/intel_iommu.h | 3 +++
> hw/i386/intel_iommu.c | 7 ---
> hw/i386/pc.c | 1 +
> 4 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 13d5d129ae..2f9bc0147d 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -412,8 +412,8 @@ typedef union VTDInvDesc VTDInvDesc;
> /* Rsvd field masks for spte */
> #define VTD_SPTE_SNP 0x800ULL
>
> -#define VTD_SPTE_PAGE_L1_RSVD_MASK(aw, dt_supported) \
> -dt_supported ? \
> +#define VTD_SPTE_PAGE_L1_RSVD_MASK(aw, stale_tm) \
> +stale_tm ? \
> (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
> (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
> #define VTD_SPTE_PAGE_L2_RSVD_MASK(aw) \
> @@ -423,12 +423,12 @@ typedef union VTDInvDesc VTDInvDesc;
> #define VTD_SPTE_PAGE_L4_RSVD_MASK(aw) \
> (0x880ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>
> -#define VTD_SPTE_LPAGE_L2_RSVD_MASK(aw, dt_supported) \
> -dt_supported ? \
> +#define VTD_SPTE_LPAGE_L2_RSVD_MASK(aw, stale_tm) \
> +stale_tm ? \
> (0x1ff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
> (0x1ff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
> -#define VTD_SPTE_LPAGE_L3_RSVD_MASK(aw, dt_supported) \
> -dt_supported ? \
> +#define VTD_SPTE_LPAGE_L3_RSVD_MASK(aw, stale_tm) \
> +stale_tm ? \
> (0x3800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM))
> : \
> (0x3800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 1eb05c29fc..d372cd396b 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -306,6 +306,9 @@ struct IntelIOMMUState {
> bool dma_translation; /* Whether DMA translation supported */
> bool pasid; /* Whether to support PASID */
>
> +/* Transient Mapping, Reserved(0) since VTD spec revision 3.2 */
> +bool stale_tm;
> +
> /*
>* Protects IOMMU states in general. Currently it protects the
>* per-IOMMU IOTLB cache, and context entry cache in VTDAddressSpace.
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 08fe218935..8612d0917b 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3372,6 +3372,7 @@ static Property vtd_properties[] = {
> DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
> DEFINE_PROP_BOOL("dma-translation", IntelIOMMUState, dma_translation,
> true),
> +DEFINE_PROP_BOOL("stale-tm", IntelIOMMUState, stale_tm, false),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> @@ -4138,15 +4139,15 @@ static void vtd_init(IntelIOMMUState *s)
>*/
> vtd_spte_rsvd[0] = ~0ULL;
> vtd_spte_rsvd[1] = VTD_SPTE_PAGE_L1_RSVD_MASK(s->aw_bits,
> - x86_iommu->dt_supported);
> +x86_iommu->dt_supported &&
> s->stale_tm);
> vtd_spte_rsvd[2] = VTD_SPTE_PAGE_L2_RSVD_MASK(s->aw_bits);
> vtd_spte_rsvd[3] = VTD_SPTE_PAGE_L3_RSVD_MASK(s->aw_bits);
> vtd_spte_rsvd[4] = VTD_SPTE_PAGE_L4_RSVD_MASK(s->aw_bits);
>
> vtd_spte_rsvd_large[2] = VTD_SPTE_LPAGE_L2_RSVD_MASK(s->aw_bits,
> -x86_iommu->dt_supported);
> +x86_iommu->dt_supported &&
> s->stale_tm);
> vtd_spte_rsvd_large[3] = VTD_SPTE_LPAGE_L3_RSVD_MASK(s->aw_bits,
Re: [PATCH v3 00/23] rust: fix CI + allow older versions of rustc and bindgen
27.10.2024 12:42, Michael Tokarev пишет: 27.10.2024 12:38, Michael Tokarev wrote: 27.10.2024 11:00, Paolo Bonzini wrpte: [rustc-web] Thanks for pointing it out! It is indeed better, however it does not support mipsel. mipsel? do you mean mips64el? Please note upstream rust does not provide rustup binaries for mipsel. So there's really no way to bootstrap mipsel rust currently. And mipsel is gone in debian. It looks like we should not expect rust to work on mipsel. /mjt
Re: [PATCH 0/2] arm: Add collie and sx functional tests
On 10/27/24 23:11, Guenter Roeck wrote:
On 10/27/24 14:13, Cédric Le Goater wrote:
On 10/26/24 17:32, Guenter Roeck wrote:
On 10/26/24 03:02, Cédric Le Goater wrote:
[ ... ]
I don't mind a single file. What bothers me is that the partitioning is made
mandatory for ast2600 even if not used.
Our only use case, in 2019, was to boot QEMU ast2600 machines from an
eMMC device using an OpenBMC FW image like the ones we find on IBM
Power10 Rainier systems. I agree we only focused on this scenario.
Most of the support should be there for other use cases, and it's now
a question of finding the right tunables for the user.
I did a quick experiment using 2 patches,
one on hw/sd/sd.c to fix c8cb19876d3e ("hw/sd/sdcard: Support boot
area in emmc image")
@@ -826,7 +826,9 @@ static void sd_reset(DeviceState *dev)
sect = 0;
}
size = sect << HWBLOCK_SHIFT;
- size -= sd_bootpart_offset(sd);
+ if (sd_is_emmc(sd)) {
+ size -= sd->boot_part_size * 2;
+ }
sect = sd_addr_to_wpnum(size) + 1;
and another on hw/arm/aspeed.c to remove the setting of the eMMC
device properties when it is not bootable :
@@ -338,7 +338,7 @@ static void sdhci_attach_drive(SDHCIStat
return;
}
card = qdev_new(emmc ? TYPE_EMMC : TYPE_SD_CARD);
- if (emmc) {
+ if (emmc && boot_emmc) {
qdev_prop_set_uint64(card, "boot-partition-size", 1 * MiB);
qdev_prop_set_uint8(card, "boot-config",
boot_emmc ? 0x1 << 3 : 0x0);
(I am not saying this is correct)
Works for me, though, and it is much better than mandating the existence
of boot partitions.
Yes. However, if the emmc device was user creatable, we could use :
-blockdev node-name=emmc0,driver=file,filename=mmc-ast2600-evb-noboot.raw \
-device emmc,bus=sdhci-bus.2,drive=emmc0
and with boot partitions:
-M boot-emmc=true \
-blockdev node-name=emmc0,driver=file,filename=mmc-ast2600-evb.raw \
-device
emmc,bus=sdhci-bus.2,drive=emmc0,boot-partition-size=1048576,boot-config=8
The above would be my preferred approach if acceptable. The "sd-bus"
bus identifier should be changed in other machines tough.
No real preference here, though my understanding is that emmc devices
are by definition built-in, and that is what emmc_class_init() says as well.
Also, there does not seem to be an sdhci-bus, only sd-bus, and that does
not support any index values. That may be just my lack of knowledge, though.
No, you are right. On a real ast2600-evb, the eMMC device is indeed
soldered on the board. But, for testing purposes, it is sometime
interesting to add some flexibility in the machine definition and
in the modeling too. This avoids "hard-coding" default devices in
the machines and lets the user define its own variant models using
the QEMU command line.
C.
Guenter
If you end up submitting those patches, please feel free to add
Tested-by: Guenter Roeck
I should send these fixes for QEMU 9.2.
Thanks,
C.
Re: [PATCH 03/11] rust/qemu-api-macros: introduce Device proc macro
Thank you for the review comments Paolo. I will address any bits I did
wrong and not much the rest, it's obvious you have a disagreement over
how things are done and that's fine. This series does not attempt to
solve everything at once and arguing again and again over "this Trait
should have been OtherTrait and this thing should have been thing!()"
is not productive. Your review style of relentless disagreement after
disagreement is tiresome and impossible to deal with; it's like a
denial of service for other human beings. I suggest you take a step
back and take a deep breath before reviewing Rust patches again. I
assure you I will make sure to address all your comments either in
code, TODO comments, or patch messages.
In the meantime, take it easy.
On Sun, Oct 27, 2024 at 10:58 PM Paolo Bonzini wrote:
>
> Hello,
>
> here is my second attempt to review this, this time placing the remarks
> as close as possible to the code that is affected. However, the meat is
> the same as in my previous replies to the 03/11 thread.
>
> I hope this shows that I have practical concerns about the patch and
> it's not just FUD that it's not acceptable.
>
> On 10/24/24 16:03, Manos Pitsidianakis wrote:
> > Add a new derive procedural macro to declare device models. Add
> > corresponding DeviceImpl trait after already existing ObjectImpl trait.
> > At the same time, add instance_init, instance_post_init,
> > instance_finalize methods to the ObjectImpl trait and call them from the
> > ObjectImplUnsafe trait, which is generated by the procedural macro.
> >
> > This allows all the boilerplate device model registration to be handled
> > by macros, and all pertinent details to be declared through proc macro
> > attributes or trait associated constants and methods.
> >
> > The device class can now be generated automatically and the name can be
> > optionally overridden:
> >
> > >8
> > #[repr(C)]
> > #[derive(Debug, qemu_api_macros::Object, qemu_api_macros::Device)]
> > #[device(class_name_override = PL011Class)]
> > /// PL011 Device Model in QEMU
> > pub struct PL011State {
>
> The first design issue is already visible here in this example. I could
> place the same comment when the code appears in rust/hw/char/pl011, but
> it's easier to do it here.
>
> You have two derive macros, Object and Device. Object is derived by all
> objects (even if right now we have only devices), Device is derived by
> devices only.
>
> The class name is a property of any object, not just devices. It should
> not be part of the #[device()] attribute. #[derive(Device)] and
> #[device()] instead should take care of properties and categories (and
> possibly vmstate, but I'm not sure about that and there's already enough
> to say about this patch).
>
>
> You also have no documentation, which means that users will have no idea
> of what are the other sub-attributes of #[device()], including the
> difference between class_name and class_name_override, or how categories
> are defined.
>
> Even if we don't have support for rustdoc yet in tree, we should have
> all the needed documentation as soon as the API moves from "ad hoc usage
> of C symbols" to idiomatic.
>
> > Object methods (instance_init, etc) methods are now trait methods:
> >
> > >8
> > /// Trait a type must implement to be registered with QEMU.
> > pub trait ObjectImpl {
> > type Class: ClassImpl;
> > const TYPE_NAME: &'static CStr;
> > const PARENT_TYPE_NAME: Option<&'static CStr>;
> > const ABSTRACT: bool;
>
>
> Class, TYPE_NAME, PARENT_TYPE_NAME, ABSTRACT should be defined via
> #[object()].
>
> But actually, there is already room for defining a separate trait:
>
> /// # Safety
> ///
> /// - the first field of the struct must be of `Object` type,
> /// or derived from it
> ///
> /// - `TYPE` must match the type name used in the `TypeInfo` (no matter
> /// if it is defined in C or Rust).
> ///
> /// - the struct must be `#[repr(C)]`
> pub unsafe trait ObjectType {
> type Class: ClassImpl;
> const TYPE_NAME: &'static CStr;
> }
>
> ... because you can implement it even for classes that are defined in C
> code. Then #[derive(Object)] can find the TYPE_NAME directly from the
> first field of the struct, i.e.
>
> parent_obj: SysBusDevice;
>
> becomes
>
> const PARENT_TYPE_NAME: Option<&'static CStr> =
> Some(::TYPE_NAME);
>
> while #[object()] would be just
>
> #[object(class_type = PL011Class, type_name = "pl011")]
>
> Accessing the type of the first field is easy using the get_fields()
> function that Junjie added at
> https://lore.kernel.org/qemu-devel/20241025160209.194307-16-pbonz...@redhat.com/
>
> This shows another reason why I prefer to get CI to work first. Having
> to do simple, but still non-trivial work, often provides code that can
> be reused in more complex setups.
>
> > unsafe fn instance_init(&m
Re: [PATCH] rust: do not always select X_PL011_RUST
On Sun, Oct 27, 2024 at 3:15 PM Paolo Bonzini wrote: > > On 10/27/24 10:49, Manos Pitsidianakis wrote: > > On Fri, 25 Oct 2024 12:42, Paolo Bonzini wrote: > >> Right now the Rust pl011 device is included in all QEMU system > >> emulator binaries if --enable-rust is passed. This is not needed > >> since the board logic in hw/arm/Kconfig will pick it. > >> > >> Signed-off-by: Paolo Bonzini > >> --- > >> rust/hw/char/Kconfig | 1 - > >> 1 file changed, 1 deletion(-) > >> > >> diff --git a/rust/hw/char/Kconfig b/rust/hw/char/Kconfig > >> index a1732a9e97f..5fe800c4806 100644 > >> --- a/rust/hw/char/Kconfig > >> +++ b/rust/hw/char/Kconfig > >> @@ -1,3 +1,2 @@ > >> config X_PL011_RUST > >> bool > >> -default y if HAVE_RUST > >> -- > >> 2.47.0 > > > > (Do I need someone else reviewing this before picking this up in my > > tree?) > > > > Acked-by: Manos Pitsidianakis > > Oh, I wasn't aware that you were setting up a Rust tree, since you had > asked me to include the first round of patches. Generally "acked-by" > means that you are *not* going to include it in your pull requests but > are delegating this to someone else; which would work for me because I > have this patch included in my next pull request, which I plan on > sending out tomorrow. > > Paolo > I wasn't in the MAINTAINERS list before that, thus had no tree for pull requests, that's why. But yes you can pick this one sure.
[PATCH v5] intel_iommu: Introduce property "stale-tm" to control Transient Mapping (TM) field
VT-d spec removed Transient Mapping (TM) field from second-level page-tables
and treat the field as Reserved(0) since revision 3.2.
Changing the field as reserved(0) will break backward compatibility, so
introduce a property "stale-tm" to allow user to control the setting.
Use pc_compat_9_1 to handle the compatibility for machines before 9.2 which
allow guest to set the field. Starting from 9.2, this field is reserved(0)
by default to match spec. Of course, user can force it on command line.
This doesn't impact function of vIOMMU as there was no logic to emulate
Transient Mapping.
Suggested-by: Yi Liu
Suggested-by: Jason Wang
Signed-off-by: Zhenzhong Duan
Acked-by: Jason Wang
Reviewed-by: Yi Liu
---
v5: fix typo, s/hw_compat_9_1/pc_compat_9_1 (Liuyi)
v4: s/x-stale-tm/stale-tm (Jason)
v3: still need to check x86_iommu->dt_supported
v2: introcude "x-stale-tm" to handle migration compatibility (Jason)
hw/i386/intel_iommu_internal.h | 12 ++--
include/hw/i386/intel_iommu.h | 3 +++
hw/i386/intel_iommu.c | 7 ---
hw/i386/pc.c | 1 +
4 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 13d5d129ae..2f9bc0147d 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -412,8 +412,8 @@ typedef union VTDInvDesc VTDInvDesc;
/* Rsvd field masks for spte */
#define VTD_SPTE_SNP 0x800ULL
-#define VTD_SPTE_PAGE_L1_RSVD_MASK(aw, dt_supported) \
-dt_supported ? \
+#define VTD_SPTE_PAGE_L1_RSVD_MASK(aw, stale_tm) \
+stale_tm ? \
(0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
(0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
#define VTD_SPTE_PAGE_L2_RSVD_MASK(aw) \
@@ -423,12 +423,12 @@ typedef union VTDInvDesc VTDInvDesc;
#define VTD_SPTE_PAGE_L4_RSVD_MASK(aw) \
(0x880ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
-#define VTD_SPTE_LPAGE_L2_RSVD_MASK(aw, dt_supported) \
-dt_supported ? \
+#define VTD_SPTE_LPAGE_L2_RSVD_MASK(aw, stale_tm) \
+stale_tm ? \
(0x1ff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
(0x1ff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
-#define VTD_SPTE_LPAGE_L3_RSVD_MASK(aw, dt_supported) \
-dt_supported ? \
+#define VTD_SPTE_LPAGE_L3_RSVD_MASK(aw, stale_tm) \
+stale_tm ? \
(0x3800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
(0x3800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 1eb05c29fc..d372cd396b 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -306,6 +306,9 @@ struct IntelIOMMUState {
bool dma_translation; /* Whether DMA translation supported */
bool pasid; /* Whether to support PASID */
+/* Transient Mapping, Reserved(0) since VTD spec revision 3.2 */
+bool stale_tm;
+
/*
* Protects IOMMU states in general. Currently it protects the
* per-IOMMU IOTLB cache, and context entry cache in VTDAddressSpace.
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 08fe218935..8612d0917b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3372,6 +3372,7 @@ static Property vtd_properties[] = {
DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
DEFINE_PROP_BOOL("dma-translation", IntelIOMMUState, dma_translation,
true),
+DEFINE_PROP_BOOL("stale-tm", IntelIOMMUState, stale_tm, false),
DEFINE_PROP_END_OF_LIST(),
};
@@ -4138,15 +4139,15 @@ static void vtd_init(IntelIOMMUState *s)
*/
vtd_spte_rsvd[0] = ~0ULL;
vtd_spte_rsvd[1] = VTD_SPTE_PAGE_L1_RSVD_MASK(s->aw_bits,
- x86_iommu->dt_supported);
+x86_iommu->dt_supported &&
s->stale_tm);
vtd_spte_rsvd[2] = VTD_SPTE_PAGE_L2_RSVD_MASK(s->aw_bits);
vtd_spte_rsvd[3] = VTD_SPTE_PAGE_L3_RSVD_MASK(s->aw_bits);
vtd_spte_rsvd[4] = VTD_SPTE_PAGE_L4_RSVD_MASK(s->aw_bits);
vtd_spte_rsvd_large[2] = VTD_SPTE_LPAGE_L2_RSVD_MASK(s->aw_bits,
-x86_iommu->dt_supported);
+x86_iommu->dt_supported &&
s->stale_tm);
vtd_spte_rsvd_large[3] = VTD_SPTE_LPAGE_L3_RSVD_MASK(s->aw_bits,
-x86_iommu->dt_supported);
+x86_iommu->dt_supported &&
s->stale_tm);
if (s->scalable_mode || s->snoop_control) {
vtd_spte_rsvd[1] &= ~VTD_SPTE_SNP;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2047633e4c..830614d930 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -82,6 +82,7 @@
GlobalProperty pc_compat_9_1[] = {
{ "ICH9-LPC", "x-smi-swsmi-tim
Re: [PATCH v6 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table revision 4 format
Reviewed-by: Bibo Mao
On 2024/10/28 上午9:57, Sia Jee Heng wrote:
Update the SPCR table to accommodate the SPCR Table revision 4 [1].
The SPCR table has been modified to adhere to the revision 4 format [2].
[1]:
https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
[2]: https://github.com/acpica/acpica/pull/931
Signed-off-by: Sia Jee Heng
Reviewed-by: Sunil V L
Reviewed-by: Michael S. Tsirkin
Acked-by: Alistair Francis
---
hw/acpi/aml-build.c | 20
hw/arm/virt-acpi-build.c| 8 ++--
hw/loongarch/acpi-build.c | 6 +-
hw/riscv/virt-acpi-build.c | 12 +---
include/hw/acpi/acpi-defs.h | 7 +--
include/hw/acpi/aml-build.h | 2 +-
6 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 34e0ddbde8..69c4bdfa22 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1995,7 +1995,7 @@ static void build_processor_hierarchy_node(GArray *tbl,
uint32_t flags,
void build_spcr(GArray *table_data, BIOSLinker *linker,
const AcpiSpcrData *f, const uint8_t rev,
-const char *oem_id, const char *oem_table_id)
+const char *oem_id, const char *oem_table_id, const char *name)
{
AcpiTable table = { .sig = "SPCR", .rev = rev, .oem_id = oem_id,
.oem_table_id = oem_table_id };
@@ -2041,9 +2041,21 @@ void build_spcr(GArray *table_data, BIOSLinker *linker,
build_append_int_noprefix(table_data, f->pci_flags, 4);
/* PCI Segment */
build_append_int_noprefix(table_data, f->pci_segment, 1);
-/* Reserved */
-build_append_int_noprefix(table_data, 0, 4);
-
+if (rev < 4) {
+/* Reserved */
+build_append_int_noprefix(table_data, 0, 4);
+} else {
+/* UartClkFreq */
+build_append_int_noprefix(table_data, f->uart_clk_freq, 4);
+/* PreciseBaudrate */
+build_append_int_noprefix(table_data, f->precise_baudrate, 4);
+/* NameSpaceStringLength */
+build_append_int_noprefix(table_data, f->namespace_string_length, 2);
+/* NameSpaceStringOffset */
+build_append_int_noprefix(table_data, f->namespace_string_offset, 2);
+/* NamespaceString[] */
+g_array_append_vals(table_data, name, f->namespace_string_length);
+}
acpi_table_end(linker, &table);
}
/*
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index f76fb117ad..0b6f5f8d8d 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -464,8 +464,12 @@ spcr_setup(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
.pci_flags = 0,
.pci_segment = 0,
};
-
-build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id);
+/*
+ * Passing NULL as the SPCR Table for Revision 2 doesn't support
+ * NameSpaceString.
+ */
+build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id,
+ NULL);
}
/*
diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 50709bda0f..4e04f7b6c1 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -276,8 +276,12 @@ spcr_setup(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
};
lvms = LOONGARCH_VIRT_MACHINE(machine);
+/*
+ * Passing NULL as the SPCR Table for Revision 2 doesn't support
+ * NameSpaceString.
+ */
build_spcr(table_data, linker, &serial, 2, lvms->oem_id,
- lvms->oem_table_id);
+ lvms->oem_table_id, NULL);
}
typedef
diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 36d6a3a412..68ef15acac 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -200,14 +200,15 @@ acpi_dsdt_add_uart(Aml *scope, const MemMapEntry
*uart_memmap,
/*
* Serial Port Console Redirection Table (SPCR)
- * Rev: 1.07
+ * Rev: 1.10
*/
static void
spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
{
+const char name[] = ".";
AcpiSpcrData serial = {
-.interface_type = 0, /* 16550 compatible */
+.interface_type = 0x12, /* 16550 compatible */
.base_addr.id = AML_AS_SYSTEM_MEMORY,
.base_addr.width = 32,
.base_addr.offset = 0,
@@ -229,9 +230,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker,
RISCVVirtState *s)
.pci_function = 0,
.pci_flags = 0,
.pci_segment = 0,
+.uart_clk_freq = 0,
+.precise_baudrate = 0,
+.namespace_string_length = sizeof(name),
+.namespace_string_offset = 88,
};
-build_spcr(table_data, linker, &serial, 2, s->oem_id, s->oem_table_id);
+build_spcr(table_data, linker, &serial, 4, s->oem_id, s->oem_table_id,
+ name);
}
/* RHCT Node[N] starts at offset 56 */
diff --git
[PATCH v3 1/3] linux-headers: Add unistd_64.h
since 6.11, unistd.h includes header file unistd_64.h directly on some platforms, here add unistd_64.h on these platforms. Affected platforms are ARM64, LoongArch64 and Riscv. Otherwise there will be compiling error such as: linux-headers/asm/unistd.h:3:10: fatal error: asm/unistd_64.h: No such file or directory #include Signed-off-by: Bibo Mao --- scripts/update-linux-headers.sh | 6 ++ 1 file changed, 6 insertions(+) diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh index c34ac6454e..203f48d089 100755 --- a/scripts/update-linux-headers.sh +++ b/scripts/update-linux-headers.sh @@ -163,6 +163,7 @@ EOF fi if [ $arch = arm64 ]; then cp "$hdrdir/include/asm/sve_context.h" "$output/linux-headers/asm-arm64/" +cp "$hdrdir/include/asm/unistd_64.h" "$output/linux-headers/asm-arm64/" fi if [ $arch = x86 ]; then cp "$hdrdir/include/asm/unistd_32.h" "$output/linux-headers/asm-x86/" @@ -185,6 +186,11 @@ EOF fi if [ $arch = riscv ]; then cp "$hdrdir/include/asm/ptrace.h" "$output/linux-headers/asm-riscv/" +cp "$hdrdir/include/asm/unistd_32.h" "$output/linux-headers/asm-riscv/" +cp "$hdrdir/include/asm/unistd_64.h" "$output/linux-headers/asm-riscv/" +fi +if [ $arch = loongarch ]; then +cp "$hdrdir/include/asm/unistd_64.h" "$output/linux-headers/asm-loongarch/" fi done arch= -- 2.39.3
[PATCH v3 3/3] linux-headers: Update to Linux v6.12-rc5
update linux-headers to v6.12-rc5. Pass to compile on aarch64, arm,
loongarch64, x86_64, i386, riscv64,riscv32 softmmu and linux-user.
Signed-off-by: Bibo Mao
---
include/standard-headers/drm/drm_fourcc.h | 43 +++
include/standard-headers/linux/const.h| 17 +
include/standard-headers/linux/ethtool.h | 226
include/standard-headers/linux/fuse.h | 22 +-
.../linux/input-event-codes.h | 2 +
include/standard-headers/linux/pci_regs.h | 41 ++-
.../standard-headers/linux/virtio_balloon.h | 16 +-
include/standard-headers/linux/virtio_gpu.h | 1 +
linux-headers/asm-arm64/mman.h| 9 +
linux-headers/asm-arm64/unistd.h | 25 +-
linux-headers/asm-arm64/unistd_64.h | 324 +
linux-headers/asm-generic/unistd.h| 6 +-
linux-headers/asm-loongarch/kvm.h | 24 ++
linux-headers/asm-loongarch/kvm_para.h| 21 ++
linux-headers/asm-loongarch/unistd.h | 4 +-
linux-headers/asm-loongarch/unistd_64.h | 320 +
linux-headers/asm-riscv/kvm.h | 7 +
linux-headers/asm-riscv/unistd.h | 41 +--
linux-headers/asm-riscv/unistd_32.h | 315 +
linux-headers/asm-riscv/unistd_64.h | 325 ++
linux-headers/asm-x86/kvm.h | 2 +
linux-headers/asm-x86/unistd_64.h | 1 +
linux-headers/asm-x86/unistd_x32.h| 1 +
linux-headers/linux/bits.h| 3 +
linux-headers/linux/const.h | 17 +
linux-headers/linux/iommufd.h | 143 +++-
linux-headers/linux/kvm.h | 23 +-
linux-headers/linux/mman.h| 1 +
linux-headers/linux/psp-sev.h | 28 ++
29 files changed, 1915 insertions(+), 93 deletions(-)
create mode 100644 linux-headers/asm-arm64/unistd_64.h
create mode 100644 linux-headers/asm-loongarch/kvm_para.h
create mode 100644 linux-headers/asm-loongarch/unistd_64.h
create mode 100644 linux-headers/asm-riscv/unistd_32.h
create mode 100644 linux-headers/asm-riscv/unistd_64.h
diff --git a/include/standard-headers/drm/drm_fourcc.h
b/include/standard-headers/drm/drm_fourcc.h
index b72917073d..d4a2231306 100644
--- a/include/standard-headers/drm/drm_fourcc.h
+++ b/include/standard-headers/drm/drm_fourcc.h
@@ -701,6 +701,31 @@ extern "C" {
*/
#define I915_FORMAT_MOD_4_TILED_MTL_RC_CCS_CC fourcc_mod_code(INTEL, 15)
+/*
+ * Intel Color Control Surfaces (CCS) for graphics ver. 20 unified compression
+ * on integrated graphics
+ *
+ * The main surface is Tile 4 and at plane index 0. For semi-planar formats
+ * like NV12, the Y and UV planes are Tile 4 and are located at plane indices
+ * 0 and 1, respectively. The CCS for all planes are stored outside of the
+ * GEM object in a reserved memory area dedicated for the storage of the
+ * CCS data for all compressible GEM objects.
+ */
+#define I915_FORMAT_MOD_4_TILED_LNL_CCS fourcc_mod_code(INTEL, 16)
+
+/*
+ * Intel Color Control Surfaces (CCS) for graphics ver. 20 unified compression
+ * on discrete graphics
+ *
+ * The main surface is Tile 4 and at plane index 0. For semi-planar formats
+ * like NV12, the Y and UV planes are Tile 4 and are located at plane indices
+ * 0 and 1, respectively. The CCS for all planes are stored outside of the
+ * GEM object in a reserved memory area dedicated for the storage of the
+ * CCS data for all compressible GEM objects. The GEM object must be stored in
+ * contiguous memory with a size aligned to 64KB
+ */
+#define I915_FORMAT_MOD_4_TILED_BMG_CCS fourcc_mod_code(INTEL, 17)
+
/*
* Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
*
@@ -1475,6 +1500,7 @@ drm_fourcc_canonicalize_nvidia_format_mod(uint64_t
modifier)
#define AMD_FMT_MOD_TILE_VER_GFX10 2
#define AMD_FMT_MOD_TILE_VER_GFX10_RBPLUS 3
#define AMD_FMT_MOD_TILE_VER_GFX11 4
+#define AMD_FMT_MOD_TILE_VER_GFX12 5
/*
* 64K_S is the same for GFX9/GFX10/GFX10_RBPLUS and hence has GFX9 as
canonical
@@ -1485,6 +1511,8 @@ drm_fourcc_canonicalize_nvidia_format_mod(uint64_t
modifier)
/*
* 64K_D for non-32 bpp is the same for GFX9/GFX10/GFX10_RBPLUS and hence has
* GFX9 as canonical version.
+ *
+ * 64K_D_2D on GFX12 is identical to 64K_D on GFX11.
*/
#define AMD_FMT_MOD_TILE_GFX9_64K_D 10
#define AMD_FMT_MOD_TILE_GFX9_64K_S_X 25
@@ -1492,6 +1520,21 @@ drm_fourcc_canonicalize_nvidia_format_mod(uint64_t
modifier)
#define AMD_FMT_MOD_TILE_GFX9_64K_R_X 27
#define AMD_FMT_MOD_TILE_GFX11_256K_R_X 31
+/* Gfx12 swizzle modes:
+ *0 - LINEAR
+ *1 - 256B_2D - 2D block dimensions
+ *2 - 4KB_2D
+ *3 - 64KB_2D
+ *4 - 256KB_2D
+ *5 - 4KB_3D - 3D block dimensions
+ *6 - 64KB_3D
+ *7 - 256KB_3D
+ */
+#define AMD_FMT_MOD_TILE_GFX12_256B_2D 1
+#define AMD_FMT_MOD_TILE_GFX12_4K_2D 2
+#define AMD_FMT_MOD_TILE_GFX12_64K_2D 3
+#d
[PATCH v3 2/3] linux-headers: loongarch: Add kvm_para.h
KVM LBT supports on LoongArch depends on the linux-header file kvm_para.h, add header file kvm_para.h here. Signed-off-by: Bibo Mao --- scripts/update-linux-headers.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh index 203f48d089..99a8d9fa4c 100755 --- a/scripts/update-linux-headers.sh +++ b/scripts/update-linux-headers.sh @@ -190,6 +190,7 @@ EOF cp "$hdrdir/include/asm/unistd_64.h" "$output/linux-headers/asm-riscv/" fi if [ $arch = loongarch ]; then +cp "$hdrdir/include/asm/kvm_para.h" "$output/linux-headers/asm-loongarch/" cp "$hdrdir/include/asm/unistd_64.h" "$output/linux-headers/asm-loongarch/" fi done -- 2.39.3
RE: [PATCH v4] intel_iommu: Introduce property "stale-tm" to control Transient Mapping (TM) field
>-Original Message-
>From: Liu, Yi L
>Sent: Friday, October 25, 2024 5:51 PM
>Subject: Re: [PATCH v4] intel_iommu: Introduce property "stale-tm" to control
>Transient Mapping (TM) field
>
>On 2024/10/23 15:57, Zhenzhong Duan wrote:
>> VT-d spec removed Transient Mapping (TM) field from second-level page-tables
>> and treat the field as Reserved(0) since revision 3.2.
>>
>> Changing the field as reserved(0) will break backward compatibility, so
>> introduce a property "stale-tm" to allow user to control the setting.
>>
>> Use hw_compat_9_1 to handle the compatibility for machines before 9.2 which
>
>is hw_compat_9_1 a typo? Looks to be pc_compat_9_1. :)
Good catch! Yes, will fix.
Thanks
Zhenzhong
>
>Otherwise I think it is good.
>
>Reviewed-by: Yi Liu
>
>> allow guest to set the field. Starting from 9.2, this field is reserved(0)
>> by default to match spec. Of course, user can force it on command line.
>>
>> This doesn't impact function of vIOMMU as there was no logic to emulate
>> Transient Mapping.
>>
>> Suggested-by: Yi Liu
>> Suggested-by: Jason Wang
>> Signed-off-by: Zhenzhong Duan
>> ---
>> v4: s/x-stale-tm/stale-tm (Jason)
>> v3: still need to check x86_iommu->dt_supported
>> v2: introcude "x-stale-tm" to handle migration compatibility (Jason)
>>
>> hw/i386/intel_iommu_internal.h | 12 ++--
>> include/hw/i386/intel_iommu.h | 3 +++
>> hw/i386/intel_iommu.c | 7 ---
>> hw/i386/pc.c | 1 +
>> 4 files changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
>> index 13d5d129ae..2f9bc0147d 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -412,8 +412,8 @@ typedef union VTDInvDesc VTDInvDesc;
>> /* Rsvd field masks for spte */
>> #define VTD_SPTE_SNP 0x800ULL
>>
>> -#define VTD_SPTE_PAGE_L1_RSVD_MASK(aw, dt_supported) \
>> -dt_supported ? \
>> +#define VTD_SPTE_PAGE_L1_RSVD_MASK(aw, stale_tm) \
>> +stale_tm ? \
>> (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) :
>\
>> (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>> #define VTD_SPTE_PAGE_L2_RSVD_MASK(aw) \
>> @@ -423,12 +423,12 @@ typedef union VTDInvDesc VTDInvDesc;
>> #define VTD_SPTE_PAGE_L4_RSVD_MASK(aw) \
>> (0x880ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>>
>> -#define VTD_SPTE_LPAGE_L2_RSVD_MASK(aw, dt_supported) \
>> -dt_supported ? \
>> +#define VTD_SPTE_LPAGE_L2_RSVD_MASK(aw, stale_tm) \
>> +stale_tm ? \
>> (0x1ff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM |
>VTD_SL_TM)) : \
>> (0x1ff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>> -#define VTD_SPTE_LPAGE_L3_RSVD_MASK(aw, dt_supported) \
>> -dt_supported ? \
>> +#define VTD_SPTE_LPAGE_L3_RSVD_MASK(aw, stale_tm) \
>> +stale_tm ? \
>> (0x3800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM |
>VTD_SL_TM)) : \
>> (0x3800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>>
>> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
>> index 1eb05c29fc..d372cd396b 100644
>> --- a/include/hw/i386/intel_iommu.h
>> +++ b/include/hw/i386/intel_iommu.h
>> @@ -306,6 +306,9 @@ struct IntelIOMMUState {
>> bool dma_translation; /* Whether DMA translation supported */
>> bool pasid; /* Whether to support PASID */
>>
>> +/* Transient Mapping, Reserved(0) since VTD spec revision 3.2 */
>> +bool stale_tm;
>> +
>> /*
>>* Protects IOMMU states in general. Currently it protects the
>>* per-IOMMU IOTLB cache, and context entry cache in VTDAddressSpace.
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 08fe218935..8612d0917b 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -3372,6 +3372,7 @@ static Property vtd_properties[] = {
>> DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
>> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
>> DEFINE_PROP_BOOL("dma-translation", IntelIOMMUState, dma_translation,
>true),
>> +DEFINE_PROP_BOOL("stale-tm", IntelIOMMUState, stale_tm, false),
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>> @@ -4138,15 +4139,15 @@ static void vtd_init(IntelIOMMUState *s)
>>*/
>> vtd_spte_rsvd[0] = ~0ULL;
>> vtd_spte_rsvd[1] = VTD_SPTE_PAGE_L1_RSVD_MASK(s->aw_bits,
>> - x86_iommu->dt_supported);
>> +x86_iommu->dt_supported &&
>> s->stale_tm);
>> vtd_spte_rsvd[2] = VTD_SPTE_PAGE_L2_RSVD_MASK(s->aw_bits);
>> vtd_spte_rsvd[3] = VTD_SPTE_PAGE_L3_RSVD_MASK(s->aw_bits);
>> vtd_spte_rsvd[4] = VTD_SPTE_PAGE_L4_RSVD_MASK(s->aw_bits);
>>
>> vtd_spte_rsvd_large[2] = VTD_SPTE_LPAGE_L2_RSVD_MASK(s->aw_bits,
>> -
>> x86_iommu->d
[PATCH v3 0/3] linux-headers: Update to Linux v6.12-rc5
Add unistd_64.h on arm64,loongarch and riscv platform, and update linux headers to Linux v6.12-rc5. Pass to compile on aarch64, arm, loongarch64, x86_64, i386, riscv64, riscv32 softmmu and linux-user. --- v2 ... v3: 1. Add unistd_64.h on arm64 and riscv platform also 2. Update header files to Linux v6.12-rc5 v1 ... v2: 1. update header files in directory linux-headers to v6.12-rc3 --- Bibo Mao (3): linux-headers: Add unistd_64.h linux-headers: loongarch: Add kvm_para.h linux-headers: Update to Linux v6.12-rc5 include/standard-headers/drm/drm_fourcc.h | 43 +++ include/standard-headers/linux/const.h| 17 + include/standard-headers/linux/ethtool.h | 226 include/standard-headers/linux/fuse.h | 22 +- .../linux/input-event-codes.h | 2 + include/standard-headers/linux/pci_regs.h | 41 ++- .../standard-headers/linux/virtio_balloon.h | 16 +- include/standard-headers/linux/virtio_gpu.h | 1 + linux-headers/asm-arm64/mman.h| 9 + linux-headers/asm-arm64/unistd.h | 25 +- linux-headers/asm-arm64/unistd_64.h | 324 + linux-headers/asm-generic/unistd.h| 6 +- linux-headers/asm-loongarch/kvm.h | 24 ++ linux-headers/asm-loongarch/kvm_para.h| 21 ++ linux-headers/asm-loongarch/unistd.h | 4 +- linux-headers/asm-loongarch/unistd_64.h | 320 + linux-headers/asm-riscv/kvm.h | 7 + linux-headers/asm-riscv/unistd.h | 41 +-- linux-headers/asm-riscv/unistd_32.h | 315 + linux-headers/asm-riscv/unistd_64.h | 325 ++ linux-headers/asm-x86/kvm.h | 2 + linux-headers/asm-x86/unistd_64.h | 1 + linux-headers/asm-x86/unistd_x32.h| 1 + linux-headers/linux/bits.h| 3 + linux-headers/linux/const.h | 17 + linux-headers/linux/iommufd.h | 143 +++- linux-headers/linux/kvm.h | 23 +- linux-headers/linux/mman.h| 1 + linux-headers/linux/psp-sev.h | 28 ++ scripts/update-linux-headers.sh | 7 + 30 files changed, 1922 insertions(+), 93 deletions(-) create mode 100644 linux-headers/asm-arm64/unistd_64.h create mode 100644 linux-headers/asm-loongarch/kvm_para.h create mode 100644 linux-headers/asm-loongarch/unistd_64.h create mode 100644 linux-headers/asm-riscv/unistd_32.h create mode 100644 linux-headers/asm-riscv/unistd_64.h base-commit: cea8ac78545a83e1f01c94d89d6f5a3f6b5c05d2 -- 2.39.3
[PATCH 1/6] target/i386: Add AVX512 state when AVX10 is supported
AVX10 state enumeration in CPUID leaf D and enabling in XCR0 register
are identical to AVX512 state regardless of the supported vector lengths.
Given that some E-cores will support AVX10 but not support AVX512, add
AVX512 state components to guest when AVX10 is enabled.
Tested-by: Xuelian Guo
Signed-off-by: Tao Su
---
target/i386/cpu.c | 14 ++
target/i386/cpu.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1ff1af032e..d845ff5e4e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7177,6 +7177,13 @@ static void x86_cpu_reset_hold(Object *obj, ResetType
type)
}
if (env->features[esa->feature] & esa->bits) {
xcr0 |= 1ull << i;
+continue;
+}
+if (i == XSTATE_OPMASK_BIT || i == XSTATE_ZMM_Hi256_BIT ||
+i == XSTATE_Hi16_ZMM_BIT) {
+if (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) {
+xcr0 |= 1ull << i;
+}
}
}
@@ -7315,6 +7322,13 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
const ExtSaveArea *esa = &x86_ext_save_areas[i];
if (env->features[esa->feature] & esa->bits) {
mask |= (1ULL << i);
+continue;
+}
+if (i == XSTATE_OPMASK_BIT || i == XSTATE_ZMM_Hi256_BIT ||
+i == XSTATE_Hi16_ZMM_BIT) {
+if (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) {
+mask |= (1ULL << i);
+}
}
}
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 74886d1580..280bec701c 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -972,6 +972,8 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu,
FeatureWord w);
#define CPUID_7_1_EDX_AMX_COMPLEX (1U << 8)
/* PREFETCHIT0/1 Instructions */
#define CPUID_7_1_EDX_PREFETCHITI (1U << 14)
+/* Support for Advanced Vector Extensions 10 */
+#define CPUID_7_1_EDX_AVX10 (1U << 19)
/* Flexible return and event delivery (FRED) */
#define CPUID_7_1_EAX_FRED (1U << 17)
/* Load into IA32_KERNEL_GS_BASE (LKGS) */
--
2.34.1
[PATCH 3/6] target/i386: Add CPUID.24 leaf for AVX10
When AVX10 enable bit is set, the 0x24 leaf will be present as "AVX10
Converged Vector ISA leaf" containing fields for the version number and
the supported vector bit lengths.
Tested-by: Xuelian Guo
Signed-off-by: Tao Su
---
target/i386/cpu.c | 40
target/i386/cpu.h | 8
target/i386/kvm/kvm.c | 3 ++-
3 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 5b434a107a..91fae0dcb7 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -898,6 +898,7 @@ void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
#define TCG_SGX_12_0_EAX_FEATURES 0
#define TCG_SGX_12_0_EBX_FEATURES 0
#define TCG_SGX_12_1_EAX_FEATURES 0
+#define TCG_24_0_EBX_FEATURES 0
#if defined CONFIG_USER_ONLY
#define CPUID_8000_0008_EBX_KERNEL_FEATURES (CPUID_8000_0008_EBX_IBPB | \
@@ -1163,6 +1164,20 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
.tcg_features = TCG_7_2_EDX_FEATURES,
},
+[FEAT_24_0_EBX] = {
+.type = CPUID_FEATURE_WORD,
+.feat_names = {
+[16] = "avx10-128",
+[17] = "avx10-256",
+[18] = "avx10-512",
+},
+.cpuid = {
+.eax = 0x24,
+.needs_ecx = true, .ecx = 0,
+.reg = R_EBX,
+},
+.tcg_features = TCG_24_0_EBX_FEATURES,
+},
[FEAT_8000_0007_EDX] = {
.type = CPUID_FEATURE_WORD,
.feat_names = {
@@ -6835,6 +6850,26 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
uint32_t count,
}
break;
}
+case 0x24: {
+*eax = 0;
+*ebx = 0;
+*ecx = 0;
+*edx = 0;
+if (!(env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10)) {
+break;
+}
+
+if (count == 0) {
+uint8_t v = kvm_arch_get_supported_cpuid(cs->kvm_state, 0x24,
+ 0, R_EBX);
+if (env->avx10_version && env->avx10_version < v) {
+v = env->avx10_version;
+}
+
+*ebx = env->features[FEAT_24_0_EBX] | v;
+}
+break;
+}
case 0x4000:
/*
* CPUID code in kvm_arch_init_vcpu() ignores stuff
@@ -7483,6 +7518,11 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
x86_cpu_adjust_level(cpu, &env->cpuid_min_level, 0x1F);
}
+/* Advanced Vector Extensions 10 (AVX10) requires CPUID[0x24] */
+if (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) {
+x86_cpu_adjust_level(cpu, &env->cpuid_min_level, 0x24);
+}
+
/* SVM requires CPUID[0x800A] */
if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) {
x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x800A);
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index d845384dcd..5566a13f4f 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -662,6 +662,7 @@ typedef enum FeatureWord {
FEAT_XSAVE_XSS_HI, /* CPUID[EAX=0xd,ECX=1].EDX */
FEAT_7_1_EDX, /* CPUID[EAX=7,ECX=1].EDX */
FEAT_7_2_EDX, /* CPUID[EAX=7,ECX=2].EDX */
+FEAT_24_0_EBX, /* CPUID[EAX=0x24,ECX=0].EBX */
FEATURE_WORDS,
} FeatureWord;
@@ -990,6 +991,13 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu,
FeatureWord w);
/* Packets which contain IP payload have LIP values */
#define CPUID_14_0_ECX_LIP (1U << 31)
+/* AVX10 128-bit vector support is present */
+#define CPUID_24_0_EBX_AVX10_128(1U << 16)
+/* AVX10 256-bit vector support is present */
+#define CPUID_24_0_EBX_AVX10_256(1U << 17)
+/* AVX10 512-bit vector support is present */
+#define CPUID_24_0_EBX_AVX10_512(1U << 18)
+
/* RAS Features */
#define CPUID_8000_0007_EBX_OVERFLOW_RECOV(1U << 0)
#define CPUID_8000_0007_EBX_SUCCOR (1U << 1)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index fd9f198892..8e17942c3b 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1923,7 +1923,8 @@ static uint32_t kvm_x86_build_cpuid(CPUX86State *env,
case 0x7:
case 0x14:
case 0x1d:
-case 0x1e: {
+case 0x1e:
+case 0x24: {
uint32_t times;
c->function = i;
--
2.34.1
[PATCH 4/6] target/i386: Add feature dependencies for AVX10
Since the highest supported vector length for a processor implies that
all lesser vector lengths are also supported, add the dependencies of
the supported vector lengths. If all vector lengths aren't supported,
clear AVX10 enable bit as well.
Note that the order of AVX10 related dependencies should be kept as:
CPUID_24_0_EBX_AVX10_128 -> CPUID_24_0_EBX_AVX10_256,
CPUID_24_0_EBX_AVX10_256 -> CPUID_24_0_EBX_AVX10_512,
CPUID_24_0_EBX_AVX10_VL_MASK -> CPUID_7_1_EDX_AVX10,
CPUID_7_1_EDX_AVX10 -> CPUID_24_0_EBX,
so that prevent user from setting weird CPUID combinations, e.g. 256-bits
and 512-bits are supported but 128-bits is not, no vector lengths are
supported but AVX10 enable bit is still set.
Since AVX10_128 will be reserved as 1, adding these dependencies has the
bonus that when user sets -cpu host,-avx10-128, CPUID_7_1_EDX_AVX10 and
CPUID_24_0_EBX will be disabled automatically.
Tested-by: Xuelian Guo
Signed-off-by: Tao Su
---
target/i386/cpu.c | 16
target/i386/cpu.h | 4
2 files changed, 20 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 91fae0dcb7..9666dbf29c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1760,6 +1760,22 @@ static FeatureDep feature_dependencies[] = {
.from = { FEAT_7_0_EBX, CPUID_7_0_EBX_SGX },
.to = { FEAT_SGX_12_1_EAX, ~0ull },
},
+{
+.from = { FEAT_24_0_EBX,CPUID_24_0_EBX_AVX10_128 },
+.to = { FEAT_24_0_EBX, CPUID_24_0_EBX_AVX10_256 },
+},
+{
+.from = { FEAT_24_0_EBX,CPUID_24_0_EBX_AVX10_256 },
+.to = { FEAT_24_0_EBX, CPUID_24_0_EBX_AVX10_512 },
+},
+{
+.from = { FEAT_24_0_EBX,CPUID_24_0_EBX_AVX10_VL_MASK },
+.to = { FEAT_7_1_EDX, CPUID_7_1_EDX_AVX10 },
+},
+{
+.from = { FEAT_7_1_EDX, CPUID_7_1_EDX_AVX10 },
+.to = { FEAT_24_0_EBX, ~0ull },
+},
};
typedef struct X86RegisterInfo32 {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 5566a13f4f..e4c947b478 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -997,6 +997,10 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu,
FeatureWord w);
#define CPUID_24_0_EBX_AVX10_256(1U << 17)
/* AVX10 512-bit vector support is present */
#define CPUID_24_0_EBX_AVX10_512(1U << 18)
+/* AVX10 vector length support mask */
+#define CPUID_24_0_EBX_AVX10_VL_MASK(CPUID_24_0_EBX_AVX10_128 | \
+ CPUID_24_0_EBX_AVX10_256 | \
+ CPUID_24_0_EBX_AVX10_512)
/* RAS Features */
#define CPUID_8000_0007_EBX_OVERFLOW_RECOV(1U << 0)
--
2.34.1
[PATCH 0/6] Add AVX10.1 CPUID support and GraniteRapids-v2 model
Add AVX10.1 CPUID support, i.e. add AVX10 support bit via CPUID.(EAX=07H, ECX=01H):EDX[bit 19] and new CPUID leaf 0x24H so that guest OS and applications can query the AVX10 CPUIDs directly. The AVX10.1 spec can be found in [*], it is worth mentioning that VL128 (CPUID.(EAX=24H, ECX=00H):EBX[bit 16]) was dropped in rev3.0, but it will be added back and reserved as 1. Since GraniteRapids (stepping 1) is the first platform to support AVX10, introduce GraniteRapids-v2 CPU model to add AVX10 in this patch set, and add some missing features as well. [*] https://cdrdv2.intel.com/v1/dl/getContent/784267 --- Tao Su (6): target/i386: Add AVX512 state when AVX10 is supported target/i386: add avx10-version property target/i386: Add CPUID.24 leaf for AVX10 target/i386: Add feature dependencies for AVX10 target/i386: Add support for AVX10 in CPUID enumeration target/i386: Introduce GraniteRapids-v2 model target/i386/cpu.c | 90 ++- target/i386/cpu.h | 16 target/i386/kvm/kvm.c | 3 +- 3 files changed, 107 insertions(+), 2 deletions(-) base-commit: cea8ac78545a83e1f01c94d89d6f5a3f6b5c05d2 -- 2.34.1
[PATCH 2/6] target/i386: add avx10-version property
Introduce avx10-version property so that avx10 version can be controlled
by user and cpu model. Per spec, avx10 version can never be 0, the default
value of avx10-version is set to 0 to determine whether it is specified by
user.
Tested-by: Xuelian Guo
Signed-off-by: Tao Su
---
target/i386/cpu.c | 1 +
target/i386/cpu.h | 2 ++
2 files changed, 3 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d845ff5e4e..5b434a107a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8343,6 +8343,7 @@ static Property x86_cpu_properties[] = {
DEFINE_PROP_UINT32("min-level", X86CPU, env.cpuid_min_level, 0),
DEFINE_PROP_UINT32("min-xlevel", X86CPU, env.cpuid_min_xlevel, 0),
DEFINE_PROP_UINT32("min-xlevel2", X86CPU, env.cpuid_min_xlevel2, 0),
+DEFINE_PROP_UINT8("avx10-version", X86CPU, env.avx10_version, 0),
DEFINE_PROP_UINT64("ucode-rev", X86CPU, ucode_rev, 0),
DEFINE_PROP_BOOL("full-cpuid-auto-level", X86CPU, full_cpuid_auto_level,
true),
DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 280bec701c..d845384dcd 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1920,6 +1920,8 @@ typedef struct CPUArchState {
uint32_t cpuid_vendor3;
uint32_t cpuid_version;
FeatureWordArray features;
+/* AVX10 version */
+uint8_t avx10_version;
/* Features that were explicitly enabled/disabled */
FeatureWordArray user_features;
uint32_t cpuid_model[12];
--
2.34.1
[PATCH 5/6] target/i386: Add support for AVX10 in CPUID enumeration
Intel AVX10 represents the first major new vector ISA since the
introduction of Intel AVX512, which will establish a common, converged
vector instruction set across all Intel architectures.
AVX10 enable bit is enumerated via CPUID.(EAX=7,ECX=1):EDX[bit 19]. Add
the CPUID definition for AVX10 enable bit, AVX10 will be enabled
automatically when using '-cpu host' and KVM advertises AVX10 to
userspace.
Tested-by: Xuelian Guo
Signed-off-by: Tao Su
---
target/i386/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9666dbf29c..adde98fd26 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1133,7 +1133,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
"avx-vnni-int8", "avx-ne-convert", NULL, NULL,
"amx-complex", NULL, "avx-vnni-int16", NULL,
NULL, NULL, "prefetchiti", NULL,
-NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, "avx10",
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
--
2.34.1
[PATCH 6/6] target/i386: Introduce GraniteRapids-v2 model
Update GraniteRapids CPU model to add AVX10 and the missing features(ss,
tsc-adjust, cldemote, movdiri, movdir64b).
Tested-by: Xuelian Guo
Signed-off-by: Tao Su
---
target/i386/cpu.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index adde98fd26..8d72c08b66 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4375,6 +4375,23 @@ static const X86CPUDefinition builtin_x86_defs[] = {
.model_id = "Intel Xeon Processor (GraniteRapids)",
.versions = (X86CPUVersionDefinition[]) {
{ .version = 1 },
+{
+.version = 2,
+.props = (PropValue[]) {
+{ "ss", "on" },
+{ "tsc-adjust", "on" },
+{ "cldemote", "on" },
+{ "movdiri", "on" },
+{ "movdir64b", "on" },
+{ "avx10", "on" },
+{ "avx10-128", "on" },
+{ "avx10-256", "on" },
+{ "avx10-512", "on" },
+{ "avx10-version", "1" },
+{ "stepping", "1" },
+{ /* end of list */ }
+}
+},
{ /* end of list */ },
},
},
--
2.34.1
Re: [PATCH 07/36] next-cube: introduce next_pc_init() object init function
Am Wed, 23 Oct 2024 09:58:23 +0100
schrieb Mark Cave-Ayland :
> Move initialisation of the memory regions and GPIOs from next_pc_realize() to
> the new next_pc_init() function.
>
> Signed-off-by: Mark Cave-Ayland
> ---
> hw/m68k/next-cube.c | 17 +++--
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
> index 0c3f8780a1..3b4c361156 100644
> --- a/hw/m68k/next-cube.c
> +++ b/hw/m68k/next-cube.c
> @@ -897,20 +897,24 @@ static void next_pc_reset(DeviceState *dev)
>
> static void next_pc_realize(DeviceState *dev, Error **errp)
> {
> -NeXTPC *s = NEXT_PC(dev);
> -SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +/* SCSI */
> +next_scsi_init(dev);
> +}
> +
> +static void next_pc_init(Object *obj)
> +{
> +NeXTPC *s = NEXT_PC(obj);
> +SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>
> -qdev_init_gpio_in(dev, next_irq, NEXT_NUM_IRQS);
> +qdev_init_gpio_in(DEVICE(obj), next_irq, NEXT_NUM_IRQS);
>
> memory_region_init_io(&s->mmiomem, OBJECT(s), &next_mmio_ops, s,
>"next.mmio", 0x9000);
> memory_region_init_io(&s->scrmem, OBJECT(s), &next_scr_ops, s,
>"next.scr", 0x2);
> +
> sysbus_init_mmio(sbd, &s->mmiomem);
> sysbus_init_mmio(sbd, &s->scrmem);
> -
> -/* SCSI */
> -next_scsi_init(dev);
> }
>
> /*
> @@ -972,6 +976,7 @@ static void next_pc_class_init(ObjectClass *klass, void
> *data)
> static const TypeInfo next_pc_info = {
> .name = TYPE_NEXT_PC,
> .parent = TYPE_SYS_BUS_DEVICE,
> +.instance_init = next_pc_init,
> .instance_size = sizeof(NeXTPC),
> .class_init = next_pc_class_init,
> };
Reviewed-by: Thomas Huth
[PATCH v2] rust/wrapper.h: define memory_order enum
Add stub definition of memory_order enum in wrapper.h.
Creating Rust bindings from C code is done by passing the wrapper.h
header to `bindgen`. This fails when library dependencies that use
compiler headers are enabled, and the libclang that bindgen detects does
not match the expected clang version. So far this has only been observed
with the memory_order enum symbols from stdatomic.h. If we add the enum
definition to wrapper.h ourselves, the error does not happen.
Before this commit, if the mismatch happened the following error could
come up:
/usr/include/liburing/barrier.h:72:10: error: use of undeclared identifier
'memory_order_release'
/usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier
'memory_order_acquire'
/usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier
'memory_order_acquire'
/usr/include/liburing/barrier.h:68:9: error: use of undeclared identifier
'memory_order_relaxed'
/usr/include/liburing/barrier.h:65:17: error: use of undeclared identifier
'memory_order_relaxed'
/usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier
'memory_order_acquire'
/usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier
'memory_order_acquire'
/usr/include/liburing/barrier.h:72:10: error: use of undeclared identifier
'memory_order_release'
panicked at
[..]/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-cli-0.70.1/main.rs:45:36:
Unable to generate bindings
To fix this (on my system) I would have to export CLANG_PATH and
LIBCLANG_PATH:
export CLANG_PATH=/bin/clang-17
export LIBCLANG_PATH=/usr/lib/llvm-17/lib
With these changes applied, bindgen is successful with both the
environment variables set and unset.
Since we're not using those symbols in the bindings (they are only used
by dependencies) this does not affect the generated bindings in any way.
Signed-off-by: Manos Pitsidianakis
---
Changes in v2:
- Restored warning print in `configure` script (thanks Paolo and Junjie)
- Link to v1:
https://lore.kernel.org/r/20241015-rust-wrapper-stdatomic-v1-1-f22c0bd31...@linaro.org
---
rust/wrapper.h | 17 +
1 file changed, 17 insertions(+)
diff --git a/rust/wrapper.h b/rust/wrapper.h
index
77e40213efb686d23f6b768b78602e4337623280..285d0eb6ad01e227a82f13e17c79390b4c34d37e
100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -30,6 +30,23 @@
* in order to generate C FFI compatible Rust bindings.
*/
+#ifndef __CLANG_STDATOMIC_H
+#define __CLANG_STDATOMIC_H
+/*
+ * Fix potential missing stdatomic.h error in case bindgen does not insert the
+ * correct libclang header paths on its own. We do not use stdatomic.h symbols
+ * in QEMU code, so it's fine to declare dummy types instead.
+ */
+typedef enum memory_order {
+ memory_order_relaxed,
+ memory_order_consume,
+ memory_order_acquire,
+ memory_order_release,
+ memory_order_acq_rel,
+ memory_order_seq_cst,
+} memory_order;
+#endif /* __CLANG_STDATOMIC_H */
+
#include "qemu/osdep.h"
#include "qemu/module.h"
#include "qemu-io.h"
---
base-commit: cea8ac78545a83e1f01c94d89d6f5a3f6b5c05d2
change-id: 20241015-rust-wrapper-stdatomic-18d58292c243
--
γαῖα πυρί μιχθήτω
Re: [PATCH 06/36] next-cube: move next_scsi_init() to next_pc_realize()
Am Wed, 23 Oct 2024 09:58:22 +0100 schrieb Mark Cave-Ayland : > This reflects that the SCSI interface exists within the NeXT Peripheral > Controller (PC). > > Signed-off-by: Mark Cave-Ayland > --- > hw/m68k/next-cube.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Thomas Huth
Re: [PATCH qemu.git 1/1] docs/devel/reset: add missing words
Hello Axel, On Thu, 17 Oct 2024 20:58, ~axelheider wrote: >From: Axel Heider > >Signed-off-by: Axel Heider >--- > docs/devel/reset.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst >index 74c7c0171a..3e64a7259f 100644 >--- a/docs/devel/reset.rst >+++ b/docs/devel/reset.rst >@@ -286,7 +286,7 @@ every reset child of the given resettable object. All >children must be > resettable too. Additional parameters (a reset type and an opaque pointer) > must > be passed to the callback too. > >-In ``DeviceClass`` and ``BusClass`` the ``ResettableState`` is located >+In ``DeviceClass`` and ``BusClass`` the ``ResettableState`` is located in the > ``DeviceState`` and ``BusState`` structure. ``child_foreach()`` is implemented > to follow the bus hierarchy; for a bus, it calls the function on every child > device; for a device, it calls the function on every bus child. When we reset >-- >2.45.2 > FYI: the `[PATCH ...]` part of the patch subject does not need the `qemu.git` string inside it, I suspect it was added by some tooling you were using however. If you decide to spin this patch in a new version I suggest also making this separate change: (adding a plural `s` suffix at `structure`) - ``DeviceState`` and ``BusState`` structure. ``child_foreach()`` is implemented + ``DeviceState`` and ``BusState`` structures. ``child_foreach()`` is implemented Reviewed-by: Manos Pitsidianakis
[PATCH] configure: detect 64-bit MIPS
While right now 64-bit MIPS and 32-bit MIPS share the code in QEMU,
Rust uses different rules for the target. Set $cpu correctly to
either mips or mips64 (--cpu=mips64* is already accepted in the case
statement that canonicalizes cpu/host_arch/linux_arch), and adjust
the checks to account for the different between $cpu (which handles
mips/mips64 separately) and $host_arch (which does not).
Fixes: 1a6ef6ff624 ("configure, meson: detect Rust toolchain", 2024-10-11)
Signed-off-by: Paolo Bonzini
---
configure | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index 97bd4495f86..c0ba649d309 100755
--- a/configure
+++ b/configure
@@ -395,7 +395,11 @@ elif check_define _ARCH_PPC ; then
cpu="ppc"
fi
elif check_define __mips__ ; then
- cpu="mips"
+ if check_define __mips64 ; then
+cpu="mips64"
+ else
+cpu="mips"
+ fi
elif check_define __s390__ ; then
if check_define __s390x__ ; then
cpu="s390x"
@@ -1230,7 +1234,7 @@ EOF
fi
fi
-case "$host_arch" in
+case "$cpu" in
arm)
# e.g. arm-unknown-linux-gnueabi, arm-unknown-linux-gnueabihf
write_c_skeleton
@@ -1278,7 +1282,7 @@ EOF
test "$rust_arch" = arm && test "$rust_os" != linux && rust_arch=armv7
;;
- mips|mips64)
+ mips)
# preserve ISA version (mipsisa64r6 etc.) and include endianness
rust_arch=${raw_cpu%el}
test "$bigendian" = no && rust_arch=${rust_arch}el
--
2.47.0
[PATCH] configure, meson: deprecate 32-bit MIPS
The mipsel architecture is not available in Debian Bookworm, and it will
likely be a hard failure as soon as we drop support for the old Rust
toolchain in Debian Bullseye. Prepare by deprecating 32-bit little
endian MIPS in QEMU 9.2.
Signed-off-by: Paolo Bonzini
---
docs/about/build-platforms.rst | 2 +-
docs/about/deprecated.rst | 12
meson.build| 8
3 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/docs/about/build-platforms.rst b/docs/about/build-platforms.rst
index ff56091078e..6102f00aec0 100644
--- a/docs/about/build-platforms.rst
+++ b/docs/about/build-platforms.rst
@@ -41,7 +41,7 @@ Those hosts are officially supported, with various
accelerators:
- Accelerators
* - Arm
- kvm (64 bit only), tcg, xen
- * - MIPS (little endian only)
+ * - MIPS (64 bit little endian only)
- kvm, tcg
* - PPC
- kvm, tcg
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 74c75666c31..32b4cd16ec3 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -170,15 +170,19 @@ property types.
Host Architectures
--
-BE MIPS (since 7.2)
-'''
+32-bit MIPS (big endian since 7.2; little endian since 9.2)
+'''
As Debian 10 ("Buster") moved into LTS the big endian 32 bit version of
MIPS moved out of support making it hard to maintain our
cross-compilation CI tests of the architecture. As we no longer have
CI coverage support may bitrot away before the deprecation process
-completes. The little endian variants of MIPS (both 32 and 64 bit) are
-still a supported host architecture.
+completes.
+
+Likewise, the little endian variant of 32 bit MIPS is not supported by
+Debian 13 ("Trixie") and newer.
+
+64 bit little endian MIPS is still a supported host architecture.
System emulation on 32-bit x86 hosts (since 8.0)
diff --git a/meson.build b/meson.build
index f5df5cdc49f..e69287acfc5 100644
--- a/meson.build
+++ b/meson.build
@@ -4807,6 +4807,14 @@ if host_arch == 'unknown'
message('configure has succeeded and you can continue to build, but')
message('QEMU will use a slow interpreter to emulate the target CPU.')
endif
+elif host_arch == 'mips'
+ message()
+ warning('DEPRECATED HOST CPU')
+ message()
+ message('Support for CPU host architecture ' + cpu + ' is going to be')
+ message('dropped as soon as the QEMU project stops supporting Debian 12')
+ message('("Bookworm"). Going forward, the QEMU project will not guarantee')
+ message('that QEMU will compile or work on this host CPU.')
endif
if not supported_oses.contains(host_os)
--
2.47.0
Re: [PATCH] configure, meson: deprecate 32-bit MIPS
27.10.2024 16:07, Paolo Bonzini wrote: The mipsel architecture is not available in Debian Bookworm, and it will likely be a hard failure as soon as we drop support for the old Rust toolchain in Debian Bullseye. Prepare by deprecating 32-bit little endian MIPS in QEMU 9.2. Correction. mipsel *is* available in debian bookworm, it is the last release of debian which has mipsel. It's been removed from trixie, the *next* debian release. Basically, s/Bookworm/Trixie/ s/Bullseye/Bookworm/. (it's the rust-web which is not available FOR BOOKWORM, because of no upstream support from rust people) Thanks, /mjt
Re: [PATCH] rust: do not always select X_PL011_RUST
On 10/27/24 10:49, Manos Pitsidianakis wrote: On Fri, 25 Oct 2024 12:42, Paolo Bonzini wrote: Right now the Rust pl011 device is included in all QEMU system emulator binaries if --enable-rust is passed. This is not needed since the board logic in hw/arm/Kconfig will pick it. Signed-off-by: Paolo Bonzini --- rust/hw/char/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/rust/hw/char/Kconfig b/rust/hw/char/Kconfig index a1732a9e97f..5fe800c4806 100644 --- a/rust/hw/char/Kconfig +++ b/rust/hw/char/Kconfig @@ -1,3 +1,2 @@ config X_PL011_RUST bool -default y if HAVE_RUST -- 2.47.0 (Do I need someone else reviewing this before picking this up in my tree?) Acked-by: Manos Pitsidianakis Oh, I wasn't aware that you were setting up a Rust tree, since you had asked me to include the first round of patches. Generally "acked-by" means that you are *not* going to include it in your pull requests but are delegating this to someone else; which would work for me because I have this patch included in my next pull request, which I plan on sending out tomorrow. Paolo
Re: [PATCH v2 3/3] tests/qtest/tpm: add unit test to tis-spi
On 10/25/24 4:12 PM, dan tan wrote:
Add qtest cases to exercise main TPM locality functionality
The TPM device emulation is provided by swtpm, which is TCG
TPM 2.0, and TCG TPM TIS compliant. See
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_TPM_Profile_PTP_2.0_r1.03_v22.pdf
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientTPMInterfaceSpecification_TIS__1-3_27_03212013.pdf
The SPI registers are specific to the PowerNV platform
architecture
Signed-off-by: dan tan
---
;
+
+static inline void tpm_reg_writeb(const PnvChip *c,
+ int locl,
+ uint8_t reg,
+ uint8_t val)
+{
+uint32_t tpm_reg_locl = SPI_TPM_TIS_ADDR | (locl <<
TPM_TIS_LOCALITY_SHIFT);
+
+spi_access_start(c, false, 1, TPM_WRITE_OP, tpm_reg_locl | reg);
+spi_write_reg(c, bswap64(val));
spi_write_reg(c, (uint64_t)val << 56);
A #define for the 56 would be good.
+
+static void spi_access_start(const PnvChip *chip,
+ bool n2,
+ uint8_t bytes,
+ uint8_t tpm_op,
+ uint32_t tpm_reg)
+{
+uint64_t cfg_reg;
+uint64_t reg_op;
+uint64_t seq_op = SEQ_OP_REG_BASIC;
+
+cfg_reg = pnv_spi_tpm_read(chip, SPI_CLK_CFG_REG);
+if (cfg_reg != CFG_COUNT_COMPARE_1) {
+pnv_spi_tpm_write(chip, SPI_CLK_CFG_REG, CFG_COUNT_COMPARE_1);
+}
+/* bytes - sequencer operation register bits 24:31 */
+if (n2) {
+seq_op |= SPI_SHIFT_COUNTER_N2 | (bytes << 0x18);
+} else {
+seq_op |= SPI_SHIFT_COUNTER_N1 | (bytes << 0x18);
+}
+pnv_spi_tpm_write(chip, SPI_SEQ_OP_REG, seq_op);
+pnv_spi_tpm_write(chip, SPI_MM_REG, MM_REG_RDR_MATCH);
+pnv_spi_tpm_write(chip, SPI_CTR_CFG_REG, (uint64_t)0);
+reg_op = bswap64(tpm_op) | ((uint64_t)tpm_reg << 0x20);
Same #define to use here, maybe called SPI_XMIT_DATA_OP_SHIFT. And one
for the 0x20, maybe called SPI_XMIT_DATA_ADDR_SHIFT. Any reference to a
spec?
(uint64_t)tmp_op << SPI_XMIT_DATA_OP_SHIFT | (uint64_t)(tpm_reg &
0xff) << SPI_XMIT_DATA_ADDR_SHIFT;
+pnv_spi_tpm_write(chip, SPI_XMIT_DATA_REG, reg_op);
+}
+
Re: [PATCH 03/11] rust/qemu-api-macros: introduce Device proc macro
Hello,
here is my second attempt to review this, this time placing the remarks
as close as possible to the code that is affected. However, the meat is
the same as in my previous replies to the 03/11 thread.
I hope this shows that I have practical concerns about the patch and
it's not just FUD that it's not acceptable.
On 10/24/24 16:03, Manos Pitsidianakis wrote:
Add a new derive procedural macro to declare device models. Add
corresponding DeviceImpl trait after already existing ObjectImpl trait.
At the same time, add instance_init, instance_post_init,
instance_finalize methods to the ObjectImpl trait and call them from the
ObjectImplUnsafe trait, which is generated by the procedural macro.
This allows all the boilerplate device model registration to be handled
by macros, and all pertinent details to be declared through proc macro
attributes or trait associated constants and methods.
The device class can now be generated automatically and the name can be
optionally overridden:
>8
#[repr(C)]
#[derive(Debug, qemu_api_macros::Object, qemu_api_macros::Device)]
#[device(class_name_override = PL011Class)]
/// PL011 Device Model in QEMU
pub struct PL011State {
The first design issue is already visible here in this example. I could
place the same comment when the code appears in rust/hw/char/pl011, but
it's easier to do it here.
You have two derive macros, Object and Device. Object is derived by all
objects (even if right now we have only devices), Device is derived by
devices only.
The class name is a property of any object, not just devices. It should
not be part of the #[device()] attribute. #[derive(Device)] and
#[device()] instead should take care of properties and categories (and
possibly vmstate, but I'm not sure about that and there's already enough
to say about this patch).
You also have no documentation, which means that users will have no idea
of what are the other sub-attributes of #[device()], including the
difference between class_name and class_name_override, or how categories
are defined.
Even if we don't have support for rustdoc yet in tree, we should have
all the needed documentation as soon as the API moves from "ad hoc usage
of C symbols" to idiomatic.
Object methods (instance_init, etc) methods are now trait methods:
>8
/// Trait a type must implement to be registered with QEMU.
pub trait ObjectImpl {
type Class: ClassImpl;
const TYPE_NAME: &'static CStr;
const PARENT_TYPE_NAME: Option<&'static CStr>;
const ABSTRACT: bool;
Class, TYPE_NAME, PARENT_TYPE_NAME, ABSTRACT should be defined via
#[object()].
But actually, there is already room for defining a separate trait:
/// # Safety
///
/// - the first field of the struct must be of `Object` type,
/// or derived from it
///
/// - `TYPE` must match the type name used in the `TypeInfo` (no matter
/// if it is defined in C or Rust).
///
/// - the struct must be `#[repr(C)]`
pub unsafe trait ObjectType {
type Class: ClassImpl;
const TYPE_NAME: &'static CStr;
}
... because you can implement it even for classes that are defined in C
code. Then #[derive(Object)] can find the TYPE_NAME directly from the
first field of the struct, i.e.
parent_obj: SysBusDevice;
becomes
const PARENT_TYPE_NAME: Option<&'static CStr> =
Some(::TYPE_NAME);
while #[object()] would be just
#[object(class_type = PL011Class, type_name = "pl011")]
Accessing the type of the first field is easy using the get_fields()
function that Junjie added at
https://lore.kernel.org/qemu-devel/20241025160209.194307-16-pbonz...@redhat.com/
This shows another reason why I prefer to get CI to work first. Having
to do simple, but still non-trivial work, often provides code that can
be reused in more complex setups.
unsafe fn instance_init(&mut self) {}
fn instance_post_init(&mut self) {}
fn instance_finalize(&mut self) {}
}
In the trait, having a default implementation that is empty works
(unlike for realize/reset, as we'll see later). So this is a bit
simpler. However, instance_finalize should have a non-empty default
implementation:
std::ptr::drop_in_place(self);
which should be okay for most devices.
Alternatively, leave out instance_post_init() and instance_finalize()
until we need them, and put the drop_in_place() call directly in the
unsafe function that goes in the TypeInfo.
>8
Device methods (realize/reset etc) are now safe idiomatic trait methods:
>8
/// Implementation methods for device types.
pub trait DeviceImpl: ObjectImpl {
fn realize(&mut self) {}
fn reset(&mut self) {}
}
>8
This is an incorrect definition of the trait. T
Re: [PATCH 03/36] next-cube: remove overlap between next.dma and next.mmio memory regions
Am Sat, 26 Oct 2024 22:13:25 +0100
schrieb Mark Cave-Ayland :
> On 26/10/2024 08:56, Thomas Huth wrote:
>
> > Am Wed, 23 Oct 2024 09:58:19 +0100
> > schrieb Mark Cave-Ayland :
> >
> >> Change the start of the next.mmio memory region so that it follows on
> >> directly
> >> after the next.dma memory region, adjusting the address offsets in
> >> next_mmio_read() and next_mmio_write() accordingly.
> >>
> >> Signed-off-by: Mark Cave-Ayland
> >> ---
> >> hw/m68k/next-cube.c | 28 ++--
> >> 1 file changed, 14 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
> >> index 4e8e55a8bd..e1d94c1ce0 100644
> >> --- a/hw/m68k/next-cube.c
> >> +++ b/hw/m68k/next-cube.c
> >> @@ -266,23 +266,23 @@ static uint64_t next_mmio_read(void *opaque, hwaddr
> >> addr, unsigned size)
> >> uint64_t val;
> >>
> >> switch (addr) {
> >> -case 0x7000:
> >> +case 0x2000:
> >> /* DPRINTF("Read INT status: %x\n", s->int_status); */
> >> val = s->int_status;
> >> break;
> >>
> >> -case 0x7800:
> >> +case 0x2800:
> >> DPRINTF("MMIO Read INT mask: %x\n", s->int_mask);
> >> val = s->int_mask;
> >> break;
> >>
> >> -case 0xc000 ... 0xc003:
> >> -val = extract32(s->scr1, (4 - (addr - 0xc000) - size) << 3,
> >> +case 0x7000 ... 0x7003:
> >> +val = extract32(s->scr1, (4 - (addr - 0x7000) - size) << 3,
> >> size << 3);
> >> break;
> >>
> >> -case 0xd000 ... 0xd003:
> >> -val = extract32(s->scr2, (4 - (addr - 0xd000) - size) << 3,
> >> +case 0x8000 ... 0x8003:
> >> +val = extract32(s->scr2, (4 - (addr - 0x8000) - size) << 3,
> >> size << 3);
> >> break;
> >>
> >> @@ -301,25 +301,25 @@ static void next_mmio_write(void *opaque, hwaddr
> >> addr, uint64_t val,
> >> NeXTPC *s = NEXT_PC(opaque);
> >>
> >> switch (addr) {
> >> -case 0x7000:
> >> +case 0x2000:
> >> DPRINTF("INT Status old: %x new: %x\n", s->int_status,
> >> (unsigned int)val);
> >> s->int_status = val;
> >> break;
> >>
> >> -case 0x7800:
> >> +case 0x2800:
> >> DPRINTF("INT Mask old: %x new: %x\n", s->int_mask, (unsigned
> >> int)val);
> >> s->int_mask = val;
> >> break;
> >
> > Could you please add comments at the end of the "case" lines, stating which
> > mmio addresses are handled in each case? Otherwise, it's harder to grep for
> > certain addresses later. E.g:
> >
> > case 0x2800: /* 0x2007800 */
>
> If you think it will help? Presumably this is to aid with comparing with
> other source
> code/documentation?
Yes, it will help with 1) debugging code that is running in the guest (so
you can find IO addresses that it is accessing more easily) and 2) help
when comparing the code with "Previous":
https://sourceforge.net/p/previous/code/HEAD/tree/trunk/src/ioMemTabNEXT.c#l36
> >> @@ -1000,7 +1000,7 @@ static void next_cube_init(MachineState *machine)
> >> sysbus_create_simple(TYPE_NEXTFB, 0x0B00, NULL);
> >>
> >> /* MMIO */
> >> -sysbus_mmio_map(SYS_BUS_DEVICE(pcdev), 0, 0x0200);
> >> +sysbus_mmio_map(SYS_BUS_DEVICE(pcdev), 0, 0x02005000);
> >
> > Why 0x02005000 and not directly 0x02007000 ?
>
> Before this patch the output of "info mtree" shows as follows:
>
> (qemu) info mtree
> address-space: cpu-memory-0
> address-space: memory
>- (prio 0, i/o): system
> -0001 (prio 0, rom): alias next.rom2
> @next.rom
> -0001
> 0100-0101 (prio 0, rom): next.rom
> 0200-02004fff (prio 0, i/o): next.dma
> 0200-020c (prio 0, i/o): next.mmio
> 0200e000-0200efff (prio 0, i/o): next.kbd
> 020c-020c003f (prio 0, ram): next.bmapmem
> 0210-0211 (prio 0, i/o): next.scr
> 02114000-0211400f (prio 0, i/o): esp-regs
> 02118000-02118003 (prio 0, i/o): escc
> 0400-07ff (prio 0, ram): next.ram
> 0b00-0b1cb0ff (prio 0, ram): next-video
> 820c-820c003f (prio 0, ram): alias next.bmapmem2
> @next.bmapmem -003f
>
> All this patch does is move the start of next.mmio to 0x2005000 to avoid the
> overlap.
>
> > I think there is another range at 0x02006000 related to the ethernet
> > controller, so directly going with 0x02007000 might cause less friction
> > later when we add the NIC?
>
> By the end of the series, everything except the "global" registers in
> next.mmio have
> their own memory region (or empty-slot if the target is unknown) so that
> "inf
Re: [PATCH 08/36] next-cube: introduce next-scsi device
Am Wed, 23 Oct 2024 09:58:24 +0100 schrieb Mark Cave-Ayland : > This device is intended to hold the ESP SCSI controller and the NeXT SCSI > CSRs. > Start by creating the device and moving the ESP SCSI controller to be an > embedded child device. > > Signed-off-by: Mark Cave-Ayland > --- > hw/m68k/next-cube.c | 93 - > 1 file changed, 74 insertions(+), 19 deletions(-) Reviewed-by: Thomas Huth
Re: [PATCH v3 00/23] rust: fix CI + allow older versions of rustc and bindgen
Il dom 27 ott 2024, 10:43 Michael Tokarev ha scritto: > 27.10.2024 12:38, Michael Tokarev wrote: > > 27.10.2024 11:00, Paolo Bonzini wrpte: > > > > [rustc-web] > > > >> Thanks for pointing it out! It is indeed better, however it does not > >> support mipsel. > > > > mipsel? do you mean mips64el? > > Ah. I see what you mean. > https://buildd.debian.org/status/package.php?p=rustc-web&suite=bookworm > > FWIW, mipsel has been removed for the next debian, it isn't supported > anymore in sid or testing (trixie). > Oh, then we need to deprecate it! But for now we're a bit stuck with it. Paolo > /mjt > >
[PATCH qemu.git v2 2/2] docs/devel/reset: add plural 's'
From: Axel Heider Suggested-by: Manos Pitsidianakis Signed-off-by: Axel Heider --- docs/devel/reset.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst index 3e64a7259f..adefd59ef9 100644 --- a/docs/devel/reset.rst +++ b/docs/devel/reset.rst @@ -287,7 +287,7 @@ resettable too. Additional parameters (a reset type and an opaque pointer) must be passed to the callback too. In ``DeviceClass`` and ``BusClass`` the ``ResettableState`` is located in the -``DeviceState`` and ``BusState`` structure. ``child_foreach()`` is implemented +``DeviceState`` and ``BusState`` structures. ``child_foreach()`` is implemented to follow the bus hierarchy; for a bus, it calls the function on every child device; for a device, it calls the function on every bus child. When we reset the main system bus, we reset the whole machine bus tree. -- 2.45.2
[PATCH qemu.git v2 1/2] docs/devel/reset: add missing words
From: Axel Heider Signed-off-by: Axel Heider --- docs/devel/reset.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst index 74c7c0171a..3e64a7259f 100644 --- a/docs/devel/reset.rst +++ b/docs/devel/reset.rst @@ -286,7 +286,7 @@ every reset child of the given resettable object. All children must be resettable too. Additional parameters (a reset type and an opaque pointer) must be passed to the callback too. -In ``DeviceClass`` and ``BusClass`` the ``ResettableState`` is located +In ``DeviceClass`` and ``BusClass`` the ``ResettableState`` is located in the ``DeviceState`` and ``BusState`` structure. ``child_foreach()`` is implemented to follow the bus hierarchy; for a bus, it calls the function on every child device; for a device, it calls the function on every bus child. When we reset -- 2.45.2
[PATCH qemu.git v2 0/2] docs/devel/reset: add missing words
v1: - Add missing words in documentation v2: - add plural 's' Axel Heider (2): docs/devel/reset: add missing words docs/devel/reset: add plural 's' docs/devel/reset.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.45.2
Re: [PATCH 0/2] arm: Add collie and sx functional tests
On 10/26/24 17:32, Guenter Roeck wrote:
On 10/26/24 03:02, Cédric Le Goater wrote:
[ ... ]
I don't mind a single file. What bothers me is that the partitioning is made
mandatory for ast2600 even if not used.
Our only use case, in 2019, was to boot QEMU ast2600 machines from an
eMMC device using an OpenBMC FW image like the ones we find on IBM
Power10 Rainier systems. I agree we only focused on this scenario.
Most of the support should be there for other use cases, and it's now
a question of finding the right tunables for the user.
I did a quick experiment using 2 patches,
one on hw/sd/sd.c to fix c8cb19876d3e ("hw/sd/sdcard: Support boot
area in emmc image")
@@ -826,7 +826,9 @@ static void sd_reset(DeviceState *dev)
sect = 0;
}
size = sect << HWBLOCK_SHIFT;
- size -= sd_bootpart_offset(sd);
+ if (sd_is_emmc(sd)) {
+ size -= sd->boot_part_size * 2;
+ }
sect = sd_addr_to_wpnum(size) + 1;
and another on hw/arm/aspeed.c to remove the setting of the eMMC
device properties when it is not bootable :
@@ -338,7 +338,7 @@ static void sdhci_attach_drive(SDHCIStat
return;
}
card = qdev_new(emmc ? TYPE_EMMC : TYPE_SD_CARD);
- if (emmc) {
+ if (emmc && boot_emmc) {
qdev_prop_set_uint64(card, "boot-partition-size", 1 * MiB);
qdev_prop_set_uint8(card, "boot-config",
boot_emmc ? 0x1 << 3 : 0x0);
(I am not saying this is correct)
Works for me, though, and it is much better than mandating the existence
of boot partitions.
Yes. However, if the emmc device was user creatable, we could use :
-blockdev node-name=emmc0,driver=file,filename=mmc-ast2600-evb-noboot.raw \
-device emmc,bus=sdhci-bus.2,drive=emmc0
and with boot partitions:
-M boot-emmc=true \
-blockdev node-name=emmc0,driver=file,filename=mmc-ast2600-evb.raw \
-device
emmc,bus=sdhci-bus.2,drive=emmc0,boot-partition-size=1048576,boot-config=8
The above would be my preferred approach if acceptable. The "sd-bus"
bus identifier should be changed in other machines tough.
If you end up submitting those patches, please feel free to add
Tested-by: Guenter Roeck
I should send these fixes for QEMU 9.2.
Thanks,
C.
Re: [PATCH 0/2] arm: Add collie and sx functional tests
On 10/27/24 14:13, Cédric Le Goater wrote:
On 10/26/24 17:32, Guenter Roeck wrote:
On 10/26/24 03:02, Cédric Le Goater wrote:
[ ... ]
I don't mind a single file. What bothers me is that the partitioning is made
mandatory for ast2600 even if not used.
Our only use case, in 2019, was to boot QEMU ast2600 machines from an
eMMC device using an OpenBMC FW image like the ones we find on IBM
Power10 Rainier systems. I agree we only focused on this scenario.
Most of the support should be there for other use cases, and it's now
a question of finding the right tunables for the user.
I did a quick experiment using 2 patches,
one on hw/sd/sd.c to fix c8cb19876d3e ("hw/sd/sdcard: Support boot
area in emmc image")
@@ -826,7 +826,9 @@ static void sd_reset(DeviceState *dev)
sect = 0;
}
size = sect << HWBLOCK_SHIFT;
- size -= sd_bootpart_offset(sd);
+ if (sd_is_emmc(sd)) {
+ size -= sd->boot_part_size * 2;
+ }
sect = sd_addr_to_wpnum(size) + 1;
and another on hw/arm/aspeed.c to remove the setting of the eMMC
device properties when it is not bootable :
@@ -338,7 +338,7 @@ static void sdhci_attach_drive(SDHCIStat
return;
}
card = qdev_new(emmc ? TYPE_EMMC : TYPE_SD_CARD);
- if (emmc) {
+ if (emmc && boot_emmc) {
qdev_prop_set_uint64(card, "boot-partition-size", 1 * MiB);
qdev_prop_set_uint8(card, "boot-config",
boot_emmc ? 0x1 << 3 : 0x0);
(I am not saying this is correct)
Works for me, though, and it is much better than mandating the existence
of boot partitions.
Yes. However, if the emmc device was user creatable, we could use :
-blockdev node-name=emmc0,driver=file,filename=mmc-ast2600-evb-noboot.raw \
-device emmc,bus=sdhci-bus.2,drive=emmc0
and with boot partitions:
-M boot-emmc=true \
-blockdev node-name=emmc0,driver=file,filename=mmc-ast2600-evb.raw \
-device
emmc,bus=sdhci-bus.2,drive=emmc0,boot-partition-size=1048576,boot-config=8
The above would be my preferred approach if acceptable. The "sd-bus"
bus identifier should be changed in other machines tough.
No real preference here, though my understanding is that emmc devices
are by definition built-in, and that is what emmc_class_init() says as well.
Also, there does not seem to be an sdhci-bus, only sd-bus, and that does
not support any index values. That may be just my lack of knowledge, though.
Guenter
If you end up submitting those patches, please feel free to add
Tested-by: Guenter Roeck
I should send these fixes for QEMU 9.2.
Thanks,
C.
