Re: [RFC] virt/acpi: set PSCI flag even when psci_conduit is disabled
在 2020/7/3 下午6:37, Peter Maydell 写道: On Fri, 3 Jul 2020 at 10:44, Heyi Guo wrote: vms->psci_conduit being disabled only means PSCI is not implemented by qemu; it doesn't mean PSCI is not supported on this virtual machine. Actually vms->psci_conduit is set to disabled when vms->secure and firmware_loaded are both set, which means we will run ARM trusted firmware, which will definitely provide PSCI. The issue can be reproduced when running qemu in TCG mode with secure enabled, while using ARM trusted firmware + qemu virt UEFI as firmware binaries, and we can see secondary cores will not be waken up. If you're using a real EL3 guest firmware then it's the job of the guest firmware to provide a DTB to the guest EL2/EL1 that says "and I support PSCI" if it supports PSCI, surely? QEMU can't tell whether the EL3 code does or doesn't do that... Thanks, Peter. Does that mean the ACPI tables generated in qemu are only templates and firmware should update them if necessary? Heyi thanks -- PMM
[RFC] virt/acpi: set PSCI flag even when psci_conduit is disabled
vms->psci_conduit being disabled only means PSCI is not implemented by qemu; it doesn't mean PSCI is not supported on this virtual machine. Actually vms->psci_conduit is set to disabled when vms->secure and firmware_loaded are both set, which means we will run ARM trusted firmware, which will definitely provide PSCI. The issue can be reproduced when running qemu in TCG mode with secure enabled, while using ARM trusted firmware + qemu virt UEFI as firmware binaries, and we can see secondary cores will not be waken up. Signed-off-by: Heyi Guo --- Cc: Shannon Zhao Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Peter Maydell Cc: qemu-...@nongnu.org --- hw/arm/virt-acpi-build.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 1384a2c..7622b97 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -728,13 +728,16 @@ static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker, }; switch (vms->psci_conduit) { -case QEMU_PSCI_CONDUIT_DISABLED: -fadt.arm_boot_arch = 0; -break; case QEMU_PSCI_CONDUIT_HVC: fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT | ACPI_FADT_ARM_PSCI_USE_HVC; break; +/* + * QEMU_PSCI_CONDUIT_DISABLED only means PSCI is not implemented by qemu, + * but typically it will still be provided by secure firmware, and it should + * use SMC as PSCI conduit. + */ +case QEMU_PSCI_CONDUIT_DISABLED: case QEMU_PSCI_CONDUIT_SMC: fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT; break; -- 2.7.4
Re: [PATCH v4 0/2] add new options to set smbios type 4 fields
On 2020/3/19 22:46, Igor Mammedov wrote: On Wed, 18 Mar 2020 14:48:18 +0800 Heyi Guo wrote: Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. it's probably too late for this series due to soft-freeze, pls repost once 5.0 is released Ah, I didn't pay enough attention to the merge window. When will the soft-freeze be ended? Will it be announced in the mailing list? Thanks, Heyi v3 -> v4: - Fix the default value when not specifying "-smbios type=4" option; it would be 0 instead of 2000 in previous versions - Use uint64_t type to check value overflow - Add test case to check smbios type 4 CPU speed Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Philippe Mathieu-Daudé Cc: Thomas Huth Cc: Laurent Vivier Cc: Paolo Bonzini Heyi Guo (2): hw/smbios: add options for type 4 max-speed and current-speed tests/bios-tables-test: add smbios cpu speed test hw/smbios/smbios.c | 36 + qemu-options.hx| 3 ++- tests/qtest/bios-tables-test.c | 42 ++ 3 files changed, 76 insertions(+), 5 deletions(-) .
[PATCH v4 0/2] add new options to set smbios type 4 fields
Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. v3 -> v4: - Fix the default value when not specifying "-smbios type=4" option; it would be 0 instead of 2000 in previous versions - Use uint64_t type to check value overflow - Add test case to check smbios type 4 CPU speed Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Philippe Mathieu-Daudé Cc: Thomas Huth Cc: Laurent Vivier Cc: Paolo Bonzini Heyi Guo (2): hw/smbios: add options for type 4 max-speed and current-speed tests/bios-tables-test: add smbios cpu speed test hw/smbios/smbios.c | 36 + qemu-options.hx| 3 ++- tests/qtest/bios-tables-test.c | 42 ++ 3 files changed, 76 insertions(+), 5 deletions(-) -- 2.19.1
[PATCH v4 1/2] hw/smbios: add options for type 4 max-speed and current-speed
Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. Strictly speaking, the "max speed" and "current speed" in type 4 are not really for the max speed and current speed of processor, for "max speed" identifies a capability of the system, and "current speed" identifies the processor's speed at boot (see smbios spec), but some applications do not tell the differences. Signed-off-by: Heyi Guo --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Philippe Mathieu-Daudé v3 -> v4: - Fix the default value when not specifying "-smbios type=4" option; it would be 0 instead of 2000 in previous versions - Use uint64_t type to check value overflow v2 -> v3: - Refine comments per Igor's suggestion. v1 -> v2: - change "_" in option names to "-" - check if option value is too large to fit in SMBIOS type 4 speed fields. --- hw/smbios/smbios.c | 36 qemu-options.hx| 3 ++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index ffd98727ee..b95de935e8 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -92,9 +92,21 @@ static struct { const char *manufacturer, *version, *serial, *asset, *sku; } type3; +/* + * SVVP requires max_speed and current_speed to be set and not being + * 0 which counts as unknown (SMBIOS 3.1.0/Table 21). Set the + * default value to 2000MHz as we did before. + */ +#define DEFAULT_CPU_SPEED 2000 + static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; -} type4; +uint64_t max_speed; +uint64_t current_speed; +} type4 = { +.max_speed = DEFAULT_CPU_SPEED, +.current_speed = DEFAULT_CPU_SPEED +}; static struct { size_t nvalues; @@ -272,6 +284,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { .name = "version", .type = QEMU_OPT_STRING, .help = "version number", +},{ +.name = "max-speed", +.type = QEMU_OPT_NUMBER, +.help = "max speed in MHz", +},{ +.name = "current-speed", +.type = QEMU_OPT_NUMBER, +.help = "speed at system boot in MHz", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -586,9 +606,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); t->voltage = 0; t->external_clock = cpu_to_le16(0); /* Unknown */ -/* SVVP requires max_speed and current_speed to not be unknown. */ -t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ -t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ +t->max_speed = cpu_to_le16(type4.max_speed); +t->current_speed = cpu_to_le16(type4.current_speed); t->status = 0x41; /* Socket populated, CPU enabled */ t->processor_upgrade = 0x01; /* Other */ t->l1_cache_handle = cpu_to_le16(0x); /* N/A */ @@ -1129,6 +1148,15 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) save_opt(, opts, "serial"); save_opt(, opts, "asset"); save_opt(, opts, "part"); +type4.max_speed = qemu_opt_get_number(opts, "max-speed", + DEFAULT_CPU_SPEED); +type4.current_speed = qemu_opt_get_number(opts, "current-speed", + DEFAULT_CPU_SPEED); +if (type4.max_speed > UINT16_MAX || +type4.current_speed > UINT16_MAX) { +error_setg(errp, "SMBIOS CPU speed is too large (> %d)", + UINT16_MAX); +} return; case 11: qemu_opts_validate(opts, qemu_smbios_type11_opts, ); diff --git a/qemu-options.hx b/qemu-options.hx index 962a5ebaa6..7665addc78 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2277,6 +2277,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios, "specify SMBIOS type 3 fields\n" "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n" " [,asset=str][,part=str]\n" +" [,max-speed=%d][,current-speed=%d]\n" "specify SMBIOS type 4 fields\n" "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" " [,asset=str][,part=str][,speed=%d]\n" @@ -2298,7 +2299,7 @@ SRST ``-smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str][,sku=str]`` Specify SMBIOS type 3 fields -``-smb
[PATCH v4 2/2] tests/bios-tables-test: add smbios cpu speed test
Add smbios type 4 CPU speed check for we added new options to set smbios type 4 "max speed" and "current speed". The default value should be 2000 when no option is specified, just as the old version did. We add the test case to one machine of each architecture, though it doesn't really run on aarch64 platform for smbios test can't run on uefi only platform yet. Signed-off-by: Heyi Guo --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Thomas Huth Cc: Laurent Vivier Cc: Paolo Bonzini --- tests/qtest/bios-tables-test.c | 42 ++ 1 file changed, 42 insertions(+) diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index 0a597bbacf..f2d2e97b4a 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -77,6 +77,8 @@ typedef struct { GArray *tables; uint32_t smbios_ep_addr; struct smbios_21_entry_point smbios_ep_table; +uint16_t smbios_cpu_max_speed; +uint16_t smbios_cpu_curr_speed; uint8_t *required_struct_types; int required_struct_types_len; QTestState *qts; @@ -560,6 +562,31 @@ static inline bool smbios_single_instance(uint8_t type) } } +static bool smbios_cpu_test(test_data *data, uint32_t addr) +{ +uint16_t expect_speed[2]; +uint16_t real; +int offset[2]; +int i; + +/* Check CPU speed for backward compatibility */ +offset[0] = offsetof(struct smbios_type_4, max_speed); +offset[1] = offsetof(struct smbios_type_4, current_speed); +expect_speed[0] = data->smbios_cpu_max_speed ? : 2000; +expect_speed[1] = data->smbios_cpu_curr_speed ? : 2000; + +for (i = 0; i < 2; i++) { +real = qtest_readw(data->qts, addr + offset[i]); +if (real != expect_speed[i]) { +fprintf(stderr, "Unexpected SMBIOS CPU speed: real %u expect %u\n", +real, expect_speed[i]); +return false; +} +} + +return true; +} + static void test_smbios_structs(test_data *data) { DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 }; @@ -582,6 +609,10 @@ static void test_smbios_structs(test_data *data) } set_bit(type, struct_bitmap); +if (type == 4) { +g_assert(smbios_cpu_test(data, addr)); +} + /* seek to end of unformatted string area of this struct ("\0\0") */ prv = crt = 1; while (prv || crt) { @@ -716,6 +747,11 @@ static void test_acpi_q35_tcg(void) data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types); test_acpi_one(NULL, ); free_test_data(); + +data.smbios_cpu_max_speed = 3000; +data.smbios_cpu_curr_speed = 2600; +test_acpi_one("-smbios type=4,max-speed=3000,current-speed=2600", ); +free_test_data(); } static void test_acpi_q35_tcg_bridge(void) @@ -1017,6 +1053,12 @@ static void test_acpi_virt_tcg(void) test_acpi_one("-cpu cortex-a57", ); free_test_data(); + +data.smbios_cpu_max_speed = 2900; +data.smbios_cpu_curr_speed = 2700; +test_acpi_one("-cpu cortex-a57 " + "-smbios type=4,max-speed=2900,current-speed=2700", ); +free_test_data(); } int main(int argc, char *argv[]) -- 2.19.1
Re: [PATCH v3] hw/smbios: add options for type 4 max-speed and current-speed
On 2020/3/3 16:33, Igor Mammedov wrote: On Tue, 3 Mar 2020 11:18:56 +0800 Heyi Guo wrote: One comment from myself after going through the code... On 2020/3/3 9:01, Heyi Guo wrote: Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. Strictly speaking, the "max speed" and "current speed" in type 4 are not really for the max speed and current speed of processor, for "max speed" identifies a capability of the system, and "current speed" identifies the processor's speed at boot (see smbios spec), but some applications do not tell the differences. Signed-off-by: Heyi Guo Reviewed-by: Igor Mammedov --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Philippe Mathieu-Daudé v2 -> v3: - Refine comments per Igor's suggestion. v1 -> v2: - change "_" in option names to "-" - check if option value is too large to fit in SMBIOS type 4 speed fields. --- hw/smbios/smbios.c | 29 ++--- qemu-options.hx| 3 ++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index ffd98727ee..4c5992241c 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -94,6 +94,8 @@ static struct { static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; +uint32_t max_speed; +uint32_t current_speed; How about defining these two fields as uint16_t, just like "speed" in type17? Then we can also drop the range check against UIN16_MAX. Well, someone should check if values provided by user make sense anyway so check should be there but in some other form. Shall we define the temporal variables as uint64_t, and check the values no larger than UINT16_MAX after invoking qemu_opt_get_number()? So any value larger than uint64_max will be blocked by qemu option parser, and value in (uint16_max, uint64_max] will be blocked by ourselves. Thanks, Heyi Please advise. Thanks, Heyi } type4; static struct { @@ -272,6 +274,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { .name = "version", .type = QEMU_OPT_STRING, .help = "version number", +},{ +.name = "max-speed", +.type = QEMU_OPT_NUMBER, +.help = "max speed in MHz", +},{ +.name = "current-speed", +.type = QEMU_OPT_NUMBER, +.help = "speed at system boot in MHz", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -586,9 +596,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); t->voltage = 0; t->external_clock = cpu_to_le16(0); /* Unknown */ -/* SVVP requires max_speed and current_speed to not be unknown. */ -t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ -t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ +t->max_speed = cpu_to_le16(type4.max_speed); +t->current_speed = cpu_to_le16(type4.current_speed); t->status = 0x41; /* Socket populated, CPU enabled */ t->processor_upgrade = 0x01; /* Other */ t->l1_cache_handle = cpu_to_le16(0x); /* N/A */ @@ -1129,6 +1138,20 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) save_opt(, opts, "serial"); save_opt(, opts, "asset"); save_opt(, opts, "part"); +/* + * SVVP requires max_speed and current_speed to be set and not being + * 0 which counts as unknown (SMBIOS 3.1.0/Table 21). Set the + * default value to 2000MHz as we did before. + */ +type4.max_speed = qemu_opt_get_number(opts, "max-speed", 2000); +type4.current_speed = qemu_opt_get_number(opts, "current-speed", + 2000); +if (type4.max_speed > UINT16_MAX || +type4.current_speed > UINT16_MAX) { +error_setg(errp, "SMBIOS CPU speed is too large (> %d)", + UINT16_MAX); +} + return; case 11: qemu_opts_validate(opts, qemu_smbios_type11_opts, ); diff --git a/qemu-options.hx b/qemu-options.hx index ac315c1ac4..7a2f7c1f66 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2233,6 +2233,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios, "specify SMBIOS type 3 fields\n" "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n" " [,asset=str][,part=str]\n&quo
Re: [PATCH v3] hw/smbios: add options for type 4 max-speed and current-speed
One comment from myself after going through the code... On 2020/3/3 9:01, Heyi Guo wrote: Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. Strictly speaking, the "max speed" and "current speed" in type 4 are not really for the max speed and current speed of processor, for "max speed" identifies a capability of the system, and "current speed" identifies the processor's speed at boot (see smbios spec), but some applications do not tell the differences. Signed-off-by: Heyi Guo Reviewed-by: Igor Mammedov --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Philippe Mathieu-Daudé v2 -> v3: - Refine comments per Igor's suggestion. v1 -> v2: - change "_" in option names to "-" - check if option value is too large to fit in SMBIOS type 4 speed fields. --- hw/smbios/smbios.c | 29 ++--- qemu-options.hx| 3 ++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index ffd98727ee..4c5992241c 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -94,6 +94,8 @@ static struct { static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; +uint32_t max_speed; +uint32_t current_speed; How about defining these two fields as uint16_t, just like "speed" in type17? Then we can also drop the range check against UIN16_MAX. Please advise. Thanks, Heyi } type4; static struct { @@ -272,6 +274,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { .name = "version", .type = QEMU_OPT_STRING, .help = "version number", +},{ +.name = "max-speed", +.type = QEMU_OPT_NUMBER, +.help = "max speed in MHz", +},{ +.name = "current-speed", +.type = QEMU_OPT_NUMBER, +.help = "speed at system boot in MHz", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -586,9 +596,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); t->voltage = 0; t->external_clock = cpu_to_le16(0); /* Unknown */ -/* SVVP requires max_speed and current_speed to not be unknown. */ -t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ -t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ +t->max_speed = cpu_to_le16(type4.max_speed); +t->current_speed = cpu_to_le16(type4.current_speed); t->status = 0x41; /* Socket populated, CPU enabled */ t->processor_upgrade = 0x01; /* Other */ t->l1_cache_handle = cpu_to_le16(0x); /* N/A */ @@ -1129,6 +1138,20 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) save_opt(, opts, "serial"); save_opt(, opts, "asset"); save_opt(, opts, "part"); +/* + * SVVP requires max_speed and current_speed to be set and not being + * 0 which counts as unknown (SMBIOS 3.1.0/Table 21). Set the + * default value to 2000MHz as we did before. + */ +type4.max_speed = qemu_opt_get_number(opts, "max-speed", 2000); +type4.current_speed = qemu_opt_get_number(opts, "current-speed", + 2000); +if (type4.max_speed > UINT16_MAX || +type4.current_speed > UINT16_MAX) { +error_setg(errp, "SMBIOS CPU speed is too large (> %d)", + UINT16_MAX); +} + return; case 11: qemu_opts_validate(opts, qemu_smbios_type11_opts, ); diff --git a/qemu-options.hx b/qemu-options.hx index ac315c1ac4..7a2f7c1f66 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2233,6 +2233,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios, "specify SMBIOS type 3 fields\n" "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n" " [,asset=str][,part=str]\n" +" [,max-speed=%d][,current-speed=%d]\n" "specify SMBIOS type 4 fields\n" "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" " [,asset=str][,part=str][,speed=%d]\n" @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields @item -smbios type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] Specify SMBIOS type 3 fields -@item -smbios type=4[,sock_pfx=@var{str}
Re: [PATCH v2] hw/smbios: add options for type 4 max-speed and current-speed
On 2020/3/2 21:52, Igor Mammedov wrote: On Mon, 2 Mar 2020 17:29:10 +0800 Heyi Guo wrote: Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. Strictly speaking, the "max speed" and "current speed" in type 4 are not really for the max speed and current speed of processor, for "max speed" identifies a capability of the system, and "current speed" identifies the processor's speed at boot (see smbios spec), but some applications do not tell the differences. Signed-off-by: Heyi Guo With comment fixed as suggested below: Reviewed-by: Igor Mammedov Thanks; I've fixed that in v3 and add your R-b :) PS: pls add/extend test cases for these options (default and custom settings). (see test_smbios_structs() for example) I'd do it as an additional patch. Let me take some time to learn how to write such test cases :) Thanks, Heyi --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Philippe Mathieu-Daudé v1 -> v2: - change "_" in option names to "-" - check if option value is too large to fit in SMBIOS type 4 speed fields. --- hw/smbios/smbios.c | 28 +--- qemu-options.hx| 3 ++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index ffd98727ee..77135a1eca 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -94,6 +94,8 @@ static struct { static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; +uint32_t max_speed; +uint32_t current_speed; } type4; static struct { @@ -272,6 +274,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { .name = "version", .type = QEMU_OPT_STRING, .help = "version number", +},{ +.name = "max-speed", +.type = QEMU_OPT_NUMBER, +.help = "max speed in MHz", +},{ +.name = "current-speed", +.type = QEMU_OPT_NUMBER, +.help = "speed at system boot in MHz", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -586,9 +596,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); t->voltage = 0; t->external_clock = cpu_to_le16(0); /* Unknown */ -/* SVVP requires max_speed and current_speed to not be unknown. */ -t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ -t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ +t->max_speed = cpu_to_le16(type4.max_speed); +t->current_speed = cpu_to_le16(type4.current_speed); t->status = 0x41; /* Socket populated, CPU enabled */ t->processor_upgrade = 0x01; /* Other */ t->l1_cache_handle = cpu_to_le16(0x); /* N/A */ @@ -1129,6 +1138,19 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) save_opt(, opts, "serial"); save_opt(, opts, "asset"); save_opt(, opts, "part"); +/* + * SVVP requires max_speed and current_speed to not be unknown, and double negation is hard to read, so s/ to not be unknown / to be set and not being 0 which counts as unknown (SMBIOS 3.1.0/Table 21) / + * we set the default value to 2000MHz as we did before. s/,and we set/. Set/ + */ +type4.max_speed = qemu_opt_get_number(opts, "max-speed", 2000); +type4.current_speed = qemu_opt_get_number(opts, "current-speed", + 2000); +if (type4.max_speed > UINT16_MAX || +type4.current_speed > UINT16_MAX) { +error_setg(errp, "SMBIOS CPU speed is too large (> %d)", + UINT16_MAX); +} + return; case 11: qemu_opts_validate(opts, qemu_smbios_type11_opts, ); diff --git a/qemu-options.hx b/qemu-options.hx index ac315c1ac4..7a2f7c1f66 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2233,6 +2233,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios, "specify SMBIOS type 3 fields\n" "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n" " [,asset=str][,part=str]\n" +" [,max-speed=%d][,current-speed=%d]\n" "specify SMBIOS type 4 fields\n" "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" " [,asset=str][,part=str][,speed=%d]\n" @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields
[PATCH v3] hw/smbios: add options for type 4 max-speed and current-speed
Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. Strictly speaking, the "max speed" and "current speed" in type 4 are not really for the max speed and current speed of processor, for "max speed" identifies a capability of the system, and "current speed" identifies the processor's speed at boot (see smbios spec), but some applications do not tell the differences. Signed-off-by: Heyi Guo Reviewed-by: Igor Mammedov --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Philippe Mathieu-Daudé v2 -> v3: - Refine comments per Igor's suggestion. v1 -> v2: - change "_" in option names to "-" - check if option value is too large to fit in SMBIOS type 4 speed fields. --- hw/smbios/smbios.c | 29 ++--- qemu-options.hx| 3 ++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index ffd98727ee..4c5992241c 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -94,6 +94,8 @@ static struct { static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; +uint32_t max_speed; +uint32_t current_speed; } type4; static struct { @@ -272,6 +274,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { .name = "version", .type = QEMU_OPT_STRING, .help = "version number", +},{ +.name = "max-speed", +.type = QEMU_OPT_NUMBER, +.help = "max speed in MHz", +},{ +.name = "current-speed", +.type = QEMU_OPT_NUMBER, +.help = "speed at system boot in MHz", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -586,9 +596,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); t->voltage = 0; t->external_clock = cpu_to_le16(0); /* Unknown */ -/* SVVP requires max_speed and current_speed to not be unknown. */ -t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ -t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ +t->max_speed = cpu_to_le16(type4.max_speed); +t->current_speed = cpu_to_le16(type4.current_speed); t->status = 0x41; /* Socket populated, CPU enabled */ t->processor_upgrade = 0x01; /* Other */ t->l1_cache_handle = cpu_to_le16(0x); /* N/A */ @@ -1129,6 +1138,20 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) save_opt(, opts, "serial"); save_opt(, opts, "asset"); save_opt(, opts, "part"); +/* + * SVVP requires max_speed and current_speed to be set and not being + * 0 which counts as unknown (SMBIOS 3.1.0/Table 21). Set the + * default value to 2000MHz as we did before. + */ +type4.max_speed = qemu_opt_get_number(opts, "max-speed", 2000); +type4.current_speed = qemu_opt_get_number(opts, "current-speed", + 2000); +if (type4.max_speed > UINT16_MAX || +type4.current_speed > UINT16_MAX) { +error_setg(errp, "SMBIOS CPU speed is too large (> %d)", + UINT16_MAX); +} + return; case 11: qemu_opts_validate(opts, qemu_smbios_type11_opts, ); diff --git a/qemu-options.hx b/qemu-options.hx index ac315c1ac4..7a2f7c1f66 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2233,6 +2233,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios, "specify SMBIOS type 3 fields\n" "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n" " [,asset=str][,part=str]\n" +" [,max-speed=%d][,current-speed=%d]\n" "specify SMBIOS type 4 fields\n" "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" " [,asset=str][,part=str][,speed=%d]\n" @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields @item -smbios type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] Specify SMBIOS type 3 fields -@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] +@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max-speed=@var{%d}][,current-speed=@var{%d}] Specify SMBIOS type 4 fields @item -smbios type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}] -- 2.19.1
[PATCH v2] hw/smbios: add options for type 4 max-speed and current-speed
Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. Strictly speaking, the "max speed" and "current speed" in type 4 are not really for the max speed and current speed of processor, for "max speed" identifies a capability of the system, and "current speed" identifies the processor's speed at boot (see smbios spec), but some applications do not tell the differences. Signed-off-by: Heyi Guo --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Philippe Mathieu-Daudé v1 -> v2: - change "_" in option names to "-" - check if option value is too large to fit in SMBIOS type 4 speed fields. --- hw/smbios/smbios.c | 28 +--- qemu-options.hx| 3 ++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index ffd98727ee..77135a1eca 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -94,6 +94,8 @@ static struct { static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; +uint32_t max_speed; +uint32_t current_speed; } type4; static struct { @@ -272,6 +274,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { .name = "version", .type = QEMU_OPT_STRING, .help = "version number", +},{ +.name = "max-speed", +.type = QEMU_OPT_NUMBER, +.help = "max speed in MHz", +},{ +.name = "current-speed", +.type = QEMU_OPT_NUMBER, +.help = "speed at system boot in MHz", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -586,9 +596,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); t->voltage = 0; t->external_clock = cpu_to_le16(0); /* Unknown */ -/* SVVP requires max_speed and current_speed to not be unknown. */ -t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ -t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ +t->max_speed = cpu_to_le16(type4.max_speed); +t->current_speed = cpu_to_le16(type4.current_speed); t->status = 0x41; /* Socket populated, CPU enabled */ t->processor_upgrade = 0x01; /* Other */ t->l1_cache_handle = cpu_to_le16(0x); /* N/A */ @@ -1129,6 +1138,19 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) save_opt(, opts, "serial"); save_opt(, opts, "asset"); save_opt(, opts, "part"); +/* + * SVVP requires max_speed and current_speed to not be unknown, and + * we set the default value to 2000MHz as we did before. + */ +type4.max_speed = qemu_opt_get_number(opts, "max-speed", 2000); +type4.current_speed = qemu_opt_get_number(opts, "current-speed", + 2000); +if (type4.max_speed > UINT16_MAX || +type4.current_speed > UINT16_MAX) { +error_setg(errp, "SMBIOS CPU speed is too large (> %d)", + UINT16_MAX); +} + return; case 11: qemu_opts_validate(opts, qemu_smbios_type11_opts, ); diff --git a/qemu-options.hx b/qemu-options.hx index ac315c1ac4..7a2f7c1f66 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2233,6 +2233,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios, "specify SMBIOS type 3 fields\n" "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n" " [,asset=str][,part=str]\n" +" [,max-speed=%d][,current-speed=%d]\n" "specify SMBIOS type 4 fields\n" "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" " [,asset=str][,part=str][,speed=%d]\n" @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields @item -smbios type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] Specify SMBIOS type 3 fields -@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] +@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max-speed=@var{%d}][,current-speed=@var{%d}] Specify SMBIOS type 4 fields @item -smbios type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}] -- 2.19.1
Re: [PATCH] hw/smbios: add options for type 4 max_speed and current_speed
On 2020/3/2 16:20, Igor Mammedov wrote: On Sat, 29 Feb 2020 08:17:48 +0800 Heyi Guo wrote: Hi Igor, On 2020/2/28 17:39, Igor Mammedov wrote: On Thu, 27 Feb 2020 17:12:21 +0800 Heyi Guo wrote: On 2020/2/25 17:24, Philippe Mathieu-Daudé wrote: On 2/25/20 8:50 AM, Heyi Guo wrote: Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. Strictly speaking, the "max speed" and "current speed" in type 4 are not really for the max speed and current speed of processor, for "max speed" identifies a capability of the system, and "current speed" identifies the processor's speed at boot (see smbios spec), but some applications do not tell the differences. Signed-off-by: Heyi Guo --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov --- hw/smbios/smbios.c | 22 +++--- qemu-options.hx | 3 ++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index ffd98727ee..1d5439643d 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -94,6 +94,8 @@ static struct { static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; + uint32_t max_speed; + uint32_t current_speed; } type4; static struct { @@ -272,6 +274,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { .name = "version", .type = QEMU_OPT_STRING, .help = "version number", + },{ + .name = "max_speed", I'd suggest to use - instead of _ in option name Thanks for your comments. However I can see other options like "sock_pfx" and "loc_pfx" also use "_" in option names. Should we keep consistent with the context? For new options one should use '-', that way we won't have to build wrappers around it if it becomes a qom property in the future. Thanks for the explanation. I'll fix that in v2. Thanks, Heyi Thanks, Heyi + .type = QEMU_OPT_NUMBER, + .help = "max speed in MHz", + },{ + .name = "current_speed", ditto + .type = QEMU_OPT_NUMBER, + .help = "speed at system boot in MHz", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -586,9 +596,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); t->voltage = 0; t->external_clock = cpu_to_le16(0); /* Unknown */ - /* SVVP requires max_speed and current_speed to not be unknown. */ - t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ - t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ + t->max_speed = cpu_to_le16(type4.max_speed); + t->current_speed = cpu_to_le16(type4.current_speed); t->status = 0x41; /* Socket populated, CPU enabled */ t->processor_upgrade = 0x01; /* Other */ t->l1_cache_handle = cpu_to_le16(0x); /* N/A */ @@ -1129,6 +1138,13 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) save_opt(, opts, "serial"); save_opt(, opts, "asset"); save_opt(, opts, "part"); + /* + * SVVP requires max_speed and current_speed to not be unknown, and + * we set the default value to 2000MHz as we did before. + */ + type4.max_speed = qemu_opt_get_number(opts, "max_speed", 2000); + type4.current_speed = qemu_opt_get_number(opts, "current_speed", + 2000); Maybe check speeds are <= UINT16_MAX else set errp? OK; I can do that in the v2. But I would wait for the maintainers to provide more comments :) Thanks, Heyi return; case 11: qemu_opts_validate(opts, qemu_smbios_type11_opts, ); diff --git a/qemu-options.hx b/qemu-options.hx index ac315c1ac4..bc9ef0fda8 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2233,6 +2233,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios, " specify SMBIOS type 3 fields\n" "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n" " [,asset=str][,part=str]\n" + " [,max_speed=%d][,current_speed=%d]\n" " specify SMBIOS type 4 fields\n" "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" " [,asset=str][,part=str][,speed=%d]\n" @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields @item -smbios type=3[,manufacturer=@var{str}][,versio
Re: [PATCH] hw/smbios: add options for type 4 max_speed and current_speed
Hi Igor, On 2020/2/28 17:39, Igor Mammedov wrote: On Thu, 27 Feb 2020 17:12:21 +0800 Heyi Guo wrote: On 2020/2/25 17:24, Philippe Mathieu-Daudé wrote: On 2/25/20 8:50 AM, Heyi Guo wrote: Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. Strictly speaking, the "max speed" and "current speed" in type 4 are not really for the max speed and current speed of processor, for "max speed" identifies a capability of the system, and "current speed" identifies the processor's speed at boot (see smbios spec), but some applications do not tell the differences. Signed-off-by: Heyi Guo --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov --- hw/smbios/smbios.c | 22 +++--- qemu-options.hx | 3 ++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index ffd98727ee..1d5439643d 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -94,6 +94,8 @@ static struct { static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; + uint32_t max_speed; + uint32_t current_speed; } type4; static struct { @@ -272,6 +274,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { .name = "version", .type = QEMU_OPT_STRING, .help = "version number", + },{ + .name = "max_speed", I'd suggest to use - instead of _ in option name Thanks for your comments. However I can see other options like "sock_pfx" and "loc_pfx" also use "_" in option names. Should we keep consistent with the context? Thanks, Heyi + .type = QEMU_OPT_NUMBER, + .help = "max speed in MHz", + },{ + .name = "current_speed", ditto + .type = QEMU_OPT_NUMBER, + .help = "speed at system boot in MHz", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -586,9 +596,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); t->voltage = 0; t->external_clock = cpu_to_le16(0); /* Unknown */ - /* SVVP requires max_speed and current_speed to not be unknown. */ - t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ - t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ + t->max_speed = cpu_to_le16(type4.max_speed); + t->current_speed = cpu_to_le16(type4.current_speed); t->status = 0x41; /* Socket populated, CPU enabled */ t->processor_upgrade = 0x01; /* Other */ t->l1_cache_handle = cpu_to_le16(0x); /* N/A */ @@ -1129,6 +1138,13 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) save_opt(, opts, "serial"); save_opt(, opts, "asset"); save_opt(, opts, "part"); + /* + * SVVP requires max_speed and current_speed to not be unknown, and + * we set the default value to 2000MHz as we did before. + */ + type4.max_speed = qemu_opt_get_number(opts, "max_speed", 2000); + type4.current_speed = qemu_opt_get_number(opts, "current_speed", + 2000); Maybe check speeds are <= UINT16_MAX else set errp? OK; I can do that in the v2. But I would wait for the maintainers to provide more comments :) Thanks, Heyi return; case 11: qemu_opts_validate(opts, qemu_smbios_type11_opts, ); diff --git a/qemu-options.hx b/qemu-options.hx index ac315c1ac4..bc9ef0fda8 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2233,6 +2233,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios, " specify SMBIOS type 3 fields\n" "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n" " [,asset=str][,part=str]\n" + " [,max_speed=%d][,current_speed=%d]\n" " specify SMBIOS type 4 fields\n" "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" " [,asset=str][,part=str][,speed=%d]\n" @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields @item -smbios type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] Specify SMBIOS type 3 fields -@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] +@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max_speed=@var{%d}][,current_speed=@var{%d}] Specify SMBIOS type 4 fields @item -smbios type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}] . .
Re: [PATCH] hw/smbios: add options for type 4 max_speed and current_speed
On 2020/2/25 17:24, Philippe Mathieu-Daudé wrote: On 2/25/20 8:50 AM, Heyi Guo wrote: Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. Strictly speaking, the "max speed" and "current speed" in type 4 are not really for the max speed and current speed of processor, for "max speed" identifies a capability of the system, and "current speed" identifies the processor's speed at boot (see smbios spec), but some applications do not tell the differences. Signed-off-by: Heyi Guo --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov --- hw/smbios/smbios.c | 22 +++--- qemu-options.hx | 3 ++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index ffd98727ee..1d5439643d 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -94,6 +94,8 @@ static struct { static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; + uint32_t max_speed; + uint32_t current_speed; } type4; static struct { @@ -272,6 +274,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { .name = "version", .type = QEMU_OPT_STRING, .help = "version number", + },{ + .name = "max_speed", + .type = QEMU_OPT_NUMBER, + .help = "max speed in MHz", + },{ + .name = "current_speed", + .type = QEMU_OPT_NUMBER, + .help = "speed at system boot in MHz", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -586,9 +596,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); t->voltage = 0; t->external_clock = cpu_to_le16(0); /* Unknown */ - /* SVVP requires max_speed and current_speed to not be unknown. */ - t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ - t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ + t->max_speed = cpu_to_le16(type4.max_speed); + t->current_speed = cpu_to_le16(type4.current_speed); t->status = 0x41; /* Socket populated, CPU enabled */ t->processor_upgrade = 0x01; /* Other */ t->l1_cache_handle = cpu_to_le16(0x); /* N/A */ @@ -1129,6 +1138,13 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) save_opt(, opts, "serial"); save_opt(, opts, "asset"); save_opt(, opts, "part"); + /* + * SVVP requires max_speed and current_speed to not be unknown, and + * we set the default value to 2000MHz as we did before. + */ + type4.max_speed = qemu_opt_get_number(opts, "max_speed", 2000); + type4.current_speed = qemu_opt_get_number(opts, "current_speed", + 2000); Maybe check speeds are <= UINT16_MAX else set errp? OK; I can do that in the v2. But I would wait for the maintainers to provide more comments :) Thanks, Heyi return; case 11: qemu_opts_validate(opts, qemu_smbios_type11_opts, ); diff --git a/qemu-options.hx b/qemu-options.hx index ac315c1ac4..bc9ef0fda8 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2233,6 +2233,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios, " specify SMBIOS type 3 fields\n" "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n" " [,asset=str][,part=str]\n" + " [,max_speed=%d][,current_speed=%d]\n" " specify SMBIOS type 4 fields\n" "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" " [,asset=str][,part=str][,speed=%d]\n" @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields @item -smbios type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] Specify SMBIOS type 3 fields -@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] +@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max_speed=@var{%d}][,current_speed=@var{%d}] Specify SMBIOS type 4 fields @item -smbios type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}] .
[PATCH] hw/smbios: add options for type 4 max_speed and current_speed
Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. Strictly speaking, the "max speed" and "current speed" in type 4 are not really for the max speed and current speed of processor, for "max speed" identifies a capability of the system, and "current speed" identifies the processor's speed at boot (see smbios spec), but some applications do not tell the differences. Signed-off-by: Heyi Guo --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov --- hw/smbios/smbios.c | 22 +++--- qemu-options.hx| 3 ++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index ffd98727ee..1d5439643d 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -94,6 +94,8 @@ static struct { static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; +uint32_t max_speed; +uint32_t current_speed; } type4; static struct { @@ -272,6 +274,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { .name = "version", .type = QEMU_OPT_STRING, .help = "version number", +},{ +.name = "max_speed", +.type = QEMU_OPT_NUMBER, +.help = "max speed in MHz", +},{ +.name = "current_speed", +.type = QEMU_OPT_NUMBER, +.help = "speed at system boot in MHz", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -586,9 +596,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); t->voltage = 0; t->external_clock = cpu_to_le16(0); /* Unknown */ -/* SVVP requires max_speed and current_speed to not be unknown. */ -t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ -t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ +t->max_speed = cpu_to_le16(type4.max_speed); +t->current_speed = cpu_to_le16(type4.current_speed); t->status = 0x41; /* Socket populated, CPU enabled */ t->processor_upgrade = 0x01; /* Other */ t->l1_cache_handle = cpu_to_le16(0x); /* N/A */ @@ -1129,6 +1138,13 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) save_opt(, opts, "serial"); save_opt(, opts, "asset"); save_opt(, opts, "part"); +/* + * SVVP requires max_speed and current_speed to not be unknown, and + * we set the default value to 2000MHz as we did before. + */ +type4.max_speed = qemu_opt_get_number(opts, "max_speed", 2000); +type4.current_speed = qemu_opt_get_number(opts, "current_speed", + 2000); return; case 11: qemu_opts_validate(opts, qemu_smbios_type11_opts, ); diff --git a/qemu-options.hx b/qemu-options.hx index ac315c1ac4..bc9ef0fda8 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2233,6 +2233,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios, "specify SMBIOS type 3 fields\n" "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n" " [,asset=str][,part=str]\n" +" [,max_speed=%d][,current_speed=%d]\n" "specify SMBIOS type 4 fields\n" "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" " [,asset=str][,part=str][,speed=%d]\n" @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields @item -smbios type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] Specify SMBIOS type 3 fields -@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] +@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max_speed=@var{%d}][,current_speed=@var{%d}] Specify SMBIOS type 4 fields @item -smbios type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}] -- 2.19.1
Re: [PATCH v3 0/7] Some cleanup in arm/virt/acpi
Hi Peter, Do you have any other comments? If not, could you help to merge into your tree? Thanks, Heyi On 2020/2/4 9:43, Heyi Guo wrote: Remove conflict _ADR objects, and fix and refine PCI device definition in ACPI/DSDT. History: v3 -> v2: - update commit message for patch 4/7. - remove diff keywords in commit message of patch 7/7 to avoid applying patch failure. v1 -> v2: - flow the work flow in tests/qtest/bios-table-test.c to post ACPI related patches. - update commit messages for removing "RP0" and "_ADR". - add 3 more cleanup patches. Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org Heyi Guo (7): bios-tables-test: prepare to change ARM virt ACPI DSDT arm/virt/acpi: remove meaningless sub device "RP0" from PCI0 arm/virt/acpi: remove _ADR from devices identified by _HID arm/acpi: fix PCI _PRT definition arm/acpi: fix duplicated _UID of PCI interrupt link devices arm/acpi: simplify the description of PCI _CRS virt/acpi: update golden masters for DSDT update hw/arm/virt-acpi-build.c | 25 ++--- tests/data/acpi/virt/DSDT | Bin 18462 -> 5307 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19799 -> 6644 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 5307 bytes 4 files changed, 6 insertions(+), 19 deletions(-)
Re: [RFC v2 00/14] Add SDEI support for arm64
On 2020/2/7 18:52, James Morse wrote: Hi guys, On 06/02/2020 17:30, Marc Zyngier wrote: On 2020-02-06 01:20, Heyi Guo wrote: On 2020/2/5 21:15, Marc Zyngier wrote: My concern is that SDEI implies having EL3. EL3 not being virtualizable with KVM, you end-up baking SDEI in *hardware*. Of course, this hardware is actually software (it is QEMU), but this isn't the way it was intended. It's not the first time we've done that (PSCI is another example), but the logic behind SDEI looks much more invasive. Thanks for your comments. Thinking about them for quite a while, below is my understanding, please correct me if I'm wrong: So should the KVM based virtual machine be treated as one with CPUs only having NS-EL1 and NS-EL0, ideally? And SDEI messes up this model, isn't it? Well, that's exactly what it is (until we have nested virt, in which case you will be able to add NS-EL2 to the mix). PSCI only contains some one-shot operations, so it is much less invasive than SDEI. Is there an established pattern for how Qemu 'gets' things that are done in secure-world? For PSCI the kernel does it, but this obviously doesn't scale to something like OP-TEE. Ideally we'd get the reference implementation (from ATF) in some form that is easy to use... I've another question. The origin of "virtual" SDEI requirement comes from the lack of hard lockup detector in VM. (this is your use case. Its origin was just symmetry with EL3<->EL2) Sure. But nothing guarantees that the guest is going to register a SDEI entry point anyway. We can have some kind of watchdog, but how can the watchdog trigger the VM OS to panic and run kdump, even in irq-off state? Nothing. All the events, including SDEI, are maskable, one way or another. Gavin's approach to inject a SError is probably OK for Linux, given that it tends to run with PSTATE.A==0. But that's not a guarantee either (if you take a recursive exception, SError won't be delivered). Or get stuck in debug-state (for which we mask SError), power-management, the vectors or somewhere weird, like KVM's world-switch. If you just want to kill the OS if its sort-of-alive, there is another trick: Synchronous exceptions can't be masked because they are caused by the instruction pointed to by the ELR. You can't inject an emulated data-abort unless the ELR points to an instruction that accesses memory, but... synchronous external abort for instruction fetch is something that could happen at any time. If you have v8.2 you can make the severity uncontainable for extra points. On real hardware, this would be as if this instruction missed in the i-cache, then got an abort from the PoU-cache. The PoU-cache must have suffered some metadata corruption to report an uncontained error. On real hardware its very likely the next instruction would suffer the same fate, but linux should put up a good show of trying to panic(). Good idea. It seems to be able to cover all the possible the cases, while we only need to highlight this exception in the user document :) Thanks, Heyi The long and the short of it is that there is no way to do what you want with absolute guarantees on the ARM architecture. It just doesn't exist. Yes. By sort-of-alive it needs to be making some kind of progress. If the CPU is spinning through the vectors (because some joker unmapped them), all bets are off. Thanks, James .
Re: [RFC v2 00/14] Add SDEI support for arm64
On 2020/2/7 1:30, Marc Zyngier wrote: On 2020-02-06 01:20, Heyi Guo wrote: Hi Marc, On 2020/2/5 21:15, Marc Zyngier wrote: Hi Heyi, On 2020-02-04 08:26, Heyi Guo wrote: Update Marc's email address. +cc Gavin as he is posting a RFC for ARM NMI. Hi Marc, Really sorry for missing to update your email address, for the initial topic was raised long time ago and I forgot to update the Cc list in the commit message of the patches. Thanks Gavin for forwarding current discussion on ARM NMI to me. For you said SDEI is "horrible", does it mean we'd better never implement SDEI in virtual world? Or do you have any advice on how to implement it? My concern is that SDEI implies having EL3. EL3 not being virtualizable with KVM, you end-up baking SDEI in *hardware*. Of course, this hardware is actually software (it is QEMU), but this isn't the way it was intended. It's not the first time we've done that (PSCI is another example), but the logic behind SDEI looks much more invasive. Thanks for your comments. Thinking about them for quite a while, below is my understanding, please correct me if I'm wrong: So should the KVM based virtual machine be treated as one with CPUs only having NS-EL1 and NS-EL0, ideally? And SDEI messes up this model, isn't it? Well, that's exactly what it is (until we have nested virt, in which case you will be able to add NS-EL2 to the mix). PSCI only contains some one-shot operations, so it is much less invasive than SDEI. I've another question. The origin of "virtual" SDEI requirement comes from the lack of hard lockup detector in VM. Sure. But nothing guarantees that the guest is going to register a SDEI entry point anyway. We can have some kind of watchdog, but how can the watchdog trigger the VM OS to panic and run kdump, even in irq-off state? Nothing. All the events, including SDEI, are maskable, one way or another. Indeed... Gavin's approach to inject a SError is probably OK for Linux, given that it tends to run with PSTATE.A==0. But that's not a guarantee either (if you take a recursive exception, SError won't be delivered). The long and the short of it is that there is no way to do what you want with absolute guarantees on the ARM architecture. It just doesn't exist. Much appreciate your explanation and advice. Thanks, Heyi M.
Re: [RFC v2 00/14] Add SDEI support for arm64
Hi Marc, On 2020/2/5 21:15, Marc Zyngier wrote: Hi Heyi, On 2020-02-04 08:26, Heyi Guo wrote: Update Marc's email address. +cc Gavin as he is posting a RFC for ARM NMI. Hi Marc, Really sorry for missing to update your email address, for the initial topic was raised long time ago and I forgot to update the Cc list in the commit message of the patches. Thanks Gavin for forwarding current discussion on ARM NMI to me. For you said SDEI is "horrible", does it mean we'd better never implement SDEI in virtual world? Or do you have any advice on how to implement it? My concern is that SDEI implies having EL3. EL3 not being virtualizable with KVM, you end-up baking SDEI in *hardware*. Of course, this hardware is actually software (it is QEMU), but this isn't the way it was intended. It's not the first time we've done that (PSCI is another example), but the logic behind SDEI looks much more invasive. Thanks for your comments. Thinking about them for quite a while, below is my understanding, please correct me if I'm wrong: So should the KVM based virtual machine be treated as one with CPUs only having NS-EL1 and NS-EL0, ideally? And SDEI messes up this model, isn't it? PSCI only contains some one-shot operations, so it is much less invasive than SDEI. I've another question. The origin of "virtual" SDEI requirement comes from the lack of hard lockup detector in VM. We can have some kind of watchdog, but how can the watchdog trigger the VM OS to panic and run kdump, even in irq-off state? Thanks, Heyi M.
Re: [RFC v2 00/14] Add SDEI support for arm64
Update Marc's email address. +cc Gavin as he is posting a RFC for ARM NMI. Hi Marc, Really sorry for missing to update your email address, for the initial topic was raised long time ago and I forgot to update the Cc list in the commit message of the patches. Thanks Gavin for forwarding current discussion on ARM NMI to me. For you said SDEI is "horrible", does it mean we'd better never implement SDEI in virtual world? Or do you have any advice on how to implement it? Thanks, Heyi On 2019/12/23 16:20, Guoheyi wrote: Hi Peter, Really appreciate your comments. For other platforms/boards emulated in qemu, like omap, imx*, etc, are they just TCG platforms? Can we just enable security and EL3 emulation for these platforms instead of implementing copies of firmware interfaces in qemu? Also I think it is possible to optimize the code to support all KVM enabled virtual boards with one single copy of SDEI code, so at least the duplication of code inside qemu might be avoided. I can understand your concerns; the exsiting SDEI code in ARM Trusted Firmware also tempted me when I started to writing the code in qemu. I agree the ideal way is to use the existing firmware directly, but how can we achieve that? Either I don't think it is good to modify the firmware code too much, for firmware should be kept simple and reliable. Does James or Marc have any idea? Thanks, Heyi 在 2019/12/20 21:44, Peter Maydell 写道: On Tue, 5 Nov 2019 at 09:12, Heyi Guo wrote: SDEI is for ARM "Software Delegated Exception Interface". AS ARM64 doesn't have native non-maskable interrupt (NMI), we rely on higher privileged (larger exception level) software to change the execution flow of lower privileged (smaller exception level) software when certain events occur, to emulate NMI mechanism, and SDEI is the standard interfaces between the two levels of privileged software. It is based on SMC/HVC calls. The higher privileged software implements an SDEI dispatcher to handle SDEI related SMC/HVC calls and trigger SDEI events; the lower privileged software implements an SDEI client to request SDEI services and handle SDEI events. Hi; I read through these patches last week, but I didn't reply then because although there are some aspects to the design that I don't like, I don't have a clear idea of what a better approach to the problems it's trying to solve would be. However I didn't want to go home for the end of the year without providing at least some response. So I'm going to lay out the parts I have issues with and perhaps somebody else will have a good idea. The first part that I dislike here is that this is implementing an entire ABI which in real hardware is provided by firmware. I think that QEMU's design works best when QEMU provides emulation of hardware or hardware-like facilities, which guest code (either in the kernel, or firmware/bios running in the guest) can then make use of. Once we start getting into implementing firmware interfaces directly in QEMU this rapidly becomes a large amount of work and code, and it's unclear where it should stop. Should we implement also the equivalent of firmware for omap boards? For imx* boards? For the raspberry pi? For xilinx boards? Are we going to end up reimplementing more of ARM Trusted Firmware functionality inside QEMU? The code to implement firmware-equivalent ABIs in all these boards would I think quickly become a large part of the codebase. My second concern is that to do the things it wants to do, the implementation here does some pretty invasive things: * intercepting interrupt lines which ought to just be emulated hardware signals between devices and the GIC * capturing register values of other CPUs, and arbitrarily stopping those other CPUs and making them run other code at a later point in time I'm really uncomfortable with what's just an 'emulated firmware' interface for one specific board model doing this kind of thing. Finally, the stated rationale for the patchset ("we'd like an emulated NMI equivalent") doesn't really feel to me like it's strong enough to counterbalance the amount of code here and the degree to which it's moving us into a swamp I'd prefer it if we could stay out of. I'd be much happier with a design where QEMU provides simple facilities to the guest and the guest firmware and kernel deal with making use of them. I appreciate that it's not clear how that would work though, given that in real hardware this works by the firmware running at EL3 and KVM not providing a mechanism that allows guest code that runs at a higher (effective or emulated) privilege level than the guest kernel... thanks -- PMM .
[PATCH v3 1/7] bios-tables-test: prepare to change ARM virt ACPI DSDT
We are going to change ARM virt ACPI DSDT table, which will cause make check to fail, so temporarily add related golden masters to ignore list. Signed-off-by: Heyi Guo Reviewed-by: Michael S. Tsirkin --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- tests/qtest/bios-tables-test-allowed-diff.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..32a401ae35 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,4 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/DSDT", +"tests/data/acpi/virt/DSDT.memhp", +"tests/data/acpi/virt/DSDT.numamem", -- 2.19.1
[PATCH v3 2/7] arm/virt/acpi: remove meaningless sub device "RP0" from PCI0
The sub device "RP0" under PCI0 in ACPI/DSDT does not contain any method or property other than "_ADR", so it is safe to remove it. Signed-off-by: Heyi Guo Acked-by: "Michael S. Tsirkin" --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 4 1 file changed, 4 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index bd5f771e9b..9f4c7d1889 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -317,10 +317,6 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, aml_return(buf)); aml_append(dev, method); -Aml *dev_rp0 = aml_device("%s", "RP0"); -aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0))); -aml_append(dev, dev_rp0); - Aml *dev_res0 = aml_device("%s", "RES0"); aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02"))); crs = aml_resource_template(); -- 2.19.1
[PATCH v3 3/7] arm/virt/acpi: remove _ADR from devices identified by _HID
According to ACPI spec, _ADR should be used for device on a bus that has a standard enumeration algorithm, but not for device which is on system bus and must be enumerated by OSPM. And it is not recommended to contain both _HID and _ADR in a single device. See ACPI 6.3, section 6.1, top of page 343: A device object must contain either an _HID object or an _ADR object, but should not contain both. (https://uefi.org/sites/default/files/resources/ACPI_6_3_May16.pdf) Signed-off-by: Heyi Guo Acked-by: Igor Mammedov Acked-by: Michael S. Tsirkin --- Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 8 1 file changed, 8 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 9f4c7d1889..be752c0ad8 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -78,11 +78,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap, AML_EXCLUSIVE, _irq, 1)); aml_append(dev, aml_name_decl("_CRS", crs)); -/* The _ADR entry is used to link this device to the UART described - * in the SPCR table, i.e. SPCR.base_address.address == _ADR. - */ -aml_append(dev, aml_name_decl("_ADR", aml_int(uart_memmap->base))); - aml_append(scope, dev); } @@ -170,7 +165,6 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03"))); aml_append(dev, aml_name_decl("_SEG", aml_int(0))); aml_append(dev, aml_name_decl("_BBN", aml_int(0))); -aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_string("PCI0"))); aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device"))); aml_append(dev, aml_name_decl("_CCA", aml_int(1))); @@ -334,7 +328,6 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, { Aml *dev = aml_device("GPO0"); aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061"))); -aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_int(0))); Aml *crs = aml_resource_template(); @@ -364,7 +357,6 @@ static void acpi_dsdt_add_power_button(Aml *scope) { Aml *dev = aml_device(ACPI_POWER_BUTTON_DEVICE); aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C"))); -aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_int(0))); aml_append(scope, dev); } -- 2.19.1
[PATCH v3 6/7] arm/acpi: simplify the description of PCI _CRS
The original code defines a named object for the resource template but then returns the resource template object itself; the resulted output is like below: Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings { Name (RBUF, ResourceTemplate () { WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, 0x, // Granularity 0x, // Range Minimum 0x00FF, // Range Maximum 0x, // Translation Offset 0x0100, // Length ,, ) .. }) Return (ResourceTemplate () { WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, 0x, // Granularity 0x, // Range Minimum 0x00FF, // Range Maximum 0x, // Translation Offset 0x0100, // Length ,, ) .. }) } So the named object "RBUF" is actually useless. The more natural way is to return RBUF instead, or simply drop RBUF definition. Choose the latter one to simplify the code. Signed-off-by: Heyi Guo Reviewed-by: Michael S. Tsirkin --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index f3e340b172..fb4b166f82 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -236,7 +236,6 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, size_mmio_high)); } -aml_append(method, aml_name_decl("RBUF", rbuf)); aml_append(method, aml_return(rbuf)); aml_append(dev, method); -- 2.19.1
[PATCH v3 4/7] arm/acpi: fix PCI _PRT definition
The address field in each _PRT mapping package should be constructed with high word for device# and low word for function#, so it is wrong to use bus_no as the high word. The existing code adds a bunch useless entries with device #s above 31. Enumerate all possible slots (i.e. PCI_SLOT_MAX) instead. Signed-off-by: Heyi Guo Reviewed-by: Michael S. Tsirkin --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index be752c0ad8..5d157a9dd5 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -151,7 +151,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, { int ecam_id = VIRT_ECAM_ID(highmem_ecam); Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf; -int i, bus_no; +int i, slot_no; hwaddr base_mmio = memmap[VIRT_PCIE_MMIO].base; hwaddr size_mmio = memmap[VIRT_PCIE_MMIO].size; hwaddr base_pio = memmap[VIRT_PCIE_PIO].base; @@ -170,12 +170,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(dev, aml_name_decl("_CCA", aml_int(1))); /* Declare the PCI Routing Table. */ -Aml *rt_pkg = aml_varpackage(nr_pcie_buses * PCI_NUM_PINS); -for (bus_no = 0; bus_no < nr_pcie_buses; bus_no++) { +Aml *rt_pkg = aml_varpackage(PCI_SLOT_MAX * PCI_NUM_PINS); +for (slot_no = 0; slot_no < PCI_SLOT_MAX; slot_no++) { for (i = 0; i < PCI_NUM_PINS; i++) { -int gsi = (i + bus_no) % PCI_NUM_PINS; +int gsi = (i + slot_no) % PCI_NUM_PINS; Aml *pkg = aml_package(4); -aml_append(pkg, aml_int((bus_no << 16) | 0x)); +aml_append(pkg, aml_int((slot_no << 16) | 0x)); aml_append(pkg, aml_int(i)); aml_append(pkg, aml_name("GSI%d", gsi)); aml_append(pkg, aml_int(0)); -- 2.19.1
[PATCH v3 7/7] virt/acpi: update golden masters for DSDT update
// Granularity -0x1000, // Range Minimum -0x3EFE, // Range Maximum -0x, // Translation Offset -0x2EFF, // Length -,, , AddressRangeMemory, TypeStatic) -DWordIO (ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange, -0x, // Granularity -0x, // Range Minimum -0x, // Range Maximum -0x3EFF, // Translation Offset -0x0001, // Length -,, , TypeStatic, DenseTranslation) -QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite, -0x, // Granularity -0x0080, // Range Minimum -0x00FF, // Range Maximum -0x, // Translation Offset -0x0080, // Length -,, , AddressRangeMemory, TypeStatic) -}) Return (ResourceTemplate () { WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, @@ -9080,11 +1879,6 @@ DefinitionBlock ("", "DSDT", 2, "BOCHS ", "BXPCDSDT", 0x0001) }) } -Device (RP0) -{ -Name (_ADR, Zero) // _ADR: Address -} - Device (RES0) { Name (_HID, "PNP0C02" /* PNP Motherboard Resources */) // _HID: Hardware ID @@ -9131,7 +1925,6 @@ DefinitionBlock ("", "DSDT", 2, "BOCHS ", "BXPCDSDT", 0x0001) Device (PWRB) { Name (_HID, "PNP0C0C" /* Power Button Device */) // _HID: Hardware ID -Name (_ADR, Zero) // _ADR: Address Name (_UID, Zero) // _UID: Unique ID } } The differences between the two versions of DSDT.memhp are almost the same as the above, except for total length and checksum. DSDT.numamem binary is just the same with DSDT on virt machine, so we don't show the differences again. Signed-off-by: Heyi Guo Reviewed-by: Michael S. Tsirkin --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- tests/data/acpi/virt/DSDT | Bin 18462 -> 5307 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19799 -> 6644 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 5307 bytes tests/qtest/bios-tables-test-allowed-diff.h | 3 --- 4 files changed, 3 deletions(-) diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT index d0f3afeb134fdf1c11f64cd06dbcdd30be603b80..d6f5c617881c4247f55d4dcd06581f9693916b2f 100644 GIT binary patch delta 156 zcmbO?fpNDcmrJlq$Zin^2BwP>xulufJQ*iyC^K43^tIeLL4lLWeZ}O>oO+X=b6ThCi^VCNML{-I`=g)4(x7&}nM*`1qLQ zpz7@!<28CLD+q${e)zR$!Q7lFEy?PZ=GK1kva+(=mNE4;-Kye^^@Q!--tkj18UL_~9%E-FO@w(J16KWeK z3R0o1B%7*Y`C2Dl_1|lD%Il+5!;MwtOiE%K<|FWeGb6 z{$oU^;O`OT=@Hf8tEg~uW<;!0)QlXPQQ zMh}~@sm&b-2n<~}2OKP`xQ9er%Z7Cs|-KkXJZ zqo2*#(q}~Pr-e~7`rC}Hd`9$s+C6HvKga%M)ZDK95rJNn^EO6 zqW9AtQ8R|vjB1||y`T1snz6Rch}(>c=>4=LYR0-YqsC`M@29Ip%~;Q7)cTC*{j^uq zj16o?ozIBgPkTqr7-lm@`;6%Qv`^HGGMk})#ykFn3jb}Wh~7{8M$M?O8TCFRdOz(K zHDhC&-cMJLnz4z^*w$x6@2CBvW{k8MV|_;SemWp(#%4C7!DduO@23N!W^7?I z#`%os{j@Y{##T0Ce0s)$RoRX4`t%EF9M@P@RW?!wE^!@@rK1+v@6Zy48V| zZgqs#EnF{rvMEtq8tdN}#Dn@^_h3*^rvGYm@8Dp1u}1i8(wJ!KdGdwX`9V z{G9yu_F!~UBU1OXbiX|4Q4l^J>!hg2M7E+b=+P~wpuIgS2-nea=?d4O-UE!LUMRhsw%xopQW6jJf$PU6aGmB=Y*3_aMYbwJv z^@=*SqNBsqvgt}2I~LUeR9cxycXo!ebH_F_#YdGcR80#1WZ+^KYD zS2(-E*_BSEJ9FX8?N~GOEztp*JC*LtgHs3dsqbFLwVVOmN_TdLvpbyK=~TK?CsXDu zf>Q^W?o_(77|voii|JIlvj?0#;M4)BJC*M231?3@d(x?NX9=7oaOwcool1AE2Ip#U zu12TQoxR}f1*Z;p-KlhEZ#a9y*_%$KJ9VmQKhHjJ>HyiDN_X~!voD-|=~TM2ADsQ* z)B&|SmF`>}>uold1Y`@`8EP91=|Q|ZnDa1MZT0G&#
[PATCH v3 5/7] arm/acpi: fix duplicated _UID of PCI interrupt link devices
Using _UID of 0 for all PCI interrupt link devices absolutely violates the spec. Simply increase one by one. Signed-off-by: Heyi Guo Reviewed-by: Michael S. Tsirkin --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 5d157a9dd5..f3e340b172 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -189,7 +189,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, uint32_t irqs = irq + i; Aml *dev_gsi = aml_device("GSI%d", i); aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F"))); -aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0))); +aml_append(dev_gsi, aml_name_decl("_UID", aml_int(i))); crs = aml_resource_template(); aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, -- 2.19.1
[PATCH v3 0/7] Some cleanup in arm/virt/acpi
Remove conflict _ADR objects, and fix and refine PCI device definition in ACPI/DSDT. History: v3 -> v2: - update commit message for patch 4/7. - remove diff keywords in commit message of patch 7/7 to avoid applying patch failure. v1 -> v2: - flow the work flow in tests/qtest/bios-table-test.c to post ACPI related patches. - update commit messages for removing "RP0" and "_ADR". - add 3 more cleanup patches. Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org Heyi Guo (7): bios-tables-test: prepare to change ARM virt ACPI DSDT arm/virt/acpi: remove meaningless sub device "RP0" from PCI0 arm/virt/acpi: remove _ADR from devices identified by _HID arm/acpi: fix PCI _PRT definition arm/acpi: fix duplicated _UID of PCI interrupt link devices arm/acpi: simplify the description of PCI _CRS virt/acpi: update golden masters for DSDT update hw/arm/virt-acpi-build.c | 25 ++--- tests/data/acpi/virt/DSDT | Bin 18462 -> 5307 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19799 -> 6644 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 5307 bytes 4 files changed, 6 insertions(+), 19 deletions(-) -- 2.19.1
Re: [PATCH v2 0/7] Some cleanup in arm/virt/acpi
在 2020/2/3 22:03, Peter Maydell 写道: On Mon, 3 Feb 2020 at 13:33, Heyi Guo wrote: 在 2020/2/3 14:43, Michael S. Tsirkin 写道: On Mon, Feb 03, 2020 at 08:14:58AM +0800, Heyi Guo wrote: Remove conflict _ADR objects, and fix and refine PCI device definition in ACPI/DSDT. Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org Series Reviewed-by: Michael S. Tsirkin merge through ARM tree pls. Thanks, Michael :) Hi Peter, Do I need to send v3 to update the commit message of patch 4/7 as Michael suggested? This patchset seems to be corrupt somehow: e104462:bionic:qemu$ patches apply -s id:20200203001505.52573-1-guoh...@huawei.com Applying: bios-tables-test: prepare to change ARM virt ACPI DSDT Applying: arm/virt/acpi: remove meaningless sub device "RP0" from PCI0 Applying: arm/virt/acpi: remove _ADR from devices identified by _HID Applying: arm/acpi: fix PCI _PRT definition Applying: arm/acpi: fix duplicated _UID of PCI interrupt link devices Applying: arm/acpi: simplify the description of PCI _CRS Applying: virt/acpi: update golden masters for DSDT update error: corrupt patch at line 68 error: could not build fake ancestor hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0007 virt/acpi: update golden masters for DSDT update When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Patchew didn't like it either: https://patchew.org/QEMU/20200203001505.52573-1-guoh...@huawei.com/ I think the problem here is the quoting of the diff in the commit message of patch 7: git am and friends think that is part of the actual patch body and get confused. You might be able to avoid that by not putting the diff --git a/DSDT.dsl.orig b/DSDT.dsl index ed3e5f0fa9..10cf70c886 100644 --- a/DSDT.dsl.orig +++ b/DSDT.dsl part in the commit message, but I haven't tested that. So resending a v4 would probably be a good idea anyway. Sorry for trouble. I should have tried locally before sending the patch. Yes, removing the above 4 lines help git am success. I'll send a v3. Thanks, Heyi thanks -- PMM .
Re: [PATCH v2 0/7] Some cleanup in arm/virt/acpi
在 2020/2/3 14:43, Michael S. Tsirkin 写道: On Mon, Feb 03, 2020 at 08:14:58AM +0800, Heyi Guo wrote: Remove conflict _ADR objects, and fix and refine PCI device definition in ACPI/DSDT. Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org Series Reviewed-by: Michael S. Tsirkin merge through ARM tree pls. Thanks, Michael :) Hi Peter, Do I need to send v3 to update the commit message of patch 4/7 as Michael suggested? Thanks, Heyi v1 -> v2: - flow the work flow in tests/qtest/bios-table-test.c to post ACPI related patches. - update commit messages for removing "RP0" and "_ADR". - add 3 more cleanup patches. Heyi Guo (7): bios-tables-test: prepare to change ARM virt ACPI DSDT arm/virt/acpi: remove meaningless sub device "PR0" from PCI0 arm/virt/acpi: remove _ADR from devices identified by _HID arm/acpi: fix PCI _PRT definition arm/acpi: fix duplicated _UID of PCI interrupt link devices arm/acpi: simplify the description of PCI _CRS virt/acpi: update golden masters for DSDT update hw/arm/virt-acpi-build.c | 25 ++--- tests/data/acpi/virt/DSDT | Bin 18462 -> 5307 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19799 -> 6644 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 5307 bytes 4 files changed, 6 insertions(+), 19 deletions(-) -- 2.19.1
[PATCH v2 7/7] virt/acpi: update golden masters for DSDT update
oducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite, -0x, // Granularity -0x1000, // Range Minimum -0x3EFE, // Range Maximum -0x, // Translation Offset -0x2EFF, // Length -,, , AddressRangeMemory, TypeStatic) -DWordIO (ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange, -0x, // Granularity -0x, // Range Minimum -0x, // Range Maximum -0x3EFF, // Translation Offset -0x0001, // Length -,, , TypeStatic, DenseTranslation) -QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite, -0x, // Granularity -0x0080, // Range Minimum -0x00FF, // Range Maximum -0x, // Translation Offset -0x0080, // Length -,, , AddressRangeMemory, TypeStatic) -}) Return (ResourceTemplate () { WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, @@ -9080,11 +1879,6 @@ DefinitionBlock ("", "DSDT", 2, "BOCHS ", "BXPCDSDT", 0x0001) }) } -Device (RP0) -{ -Name (_ADR, Zero) // _ADR: Address -} - Device (RES0) { Name (_HID, "PNP0C02" /* PNP Motherboard Resources */) // _HID: Hardware ID @@ -9131,7 +1925,6 @@ DefinitionBlock ("", "DSDT", 2, "BOCHS ", "BXPCDSDT", 0x0001) Device (PWRB) { Name (_HID, "PNP0C0C" /* Power Button Device */) // _HID: Hardware ID -Name (_ADR, Zero) // _ADR: Address Name (_UID, Zero) // _UID: Unique ID } } The differences between the two versions of DSDT.memhp are almost the same as the above, except for total length and checksum. DSDT.numamem binary is just the same with DSDT on virt machine, so we don't show the differences again. Signed-off-by: Heyi Guo --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- tests/data/acpi/virt/DSDT | Bin 18462 -> 5307 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19799 -> 6644 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 5307 bytes tests/qtest/bios-tables-test-allowed-diff.h | 3 --- 4 files changed, 3 deletions(-) diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT index d0f3afeb134fdf1c11f64cd06dbcdd30be603b80..d6f5c617881c4247f55d4dcd06581f9693916b2f 100644 GIT binary patch delta 156 zcmbO?fpNDcmrJlq$Zin^2BwP>xulufJQ*iyC^K43^tIeLL4lLWeZ}O>oO+X=b6ThCi^VCNML{-I`=g)4(x7&}nM*`1qLQ zpz7@!<28CLD+q${e)zR$!Q7lFEy?PZ=GK1kva+(=mNE4;-Kye^^@Q!--tkj18UL_~9%E-FO@w(J16KWeK z3R0o1B%7*Y`C2Dl_1|lD%Il+5!;MwtOiE%K<|FWeGb6 z{$oU^;O`OT=@Hf8tEg~uW<;!0)QlXPQQ zMh}~@sm&b-2n<~}2OKP`xQ9er%Z7Cs|-KkXJZ zqo2*#(q}~Pr-e~7`rC}Hd`9$s+C6HvKga%M)ZDK95rJNn^EO6 zqW9AtQ8R|vjB1||y`T1snz6Rch}(>c=>4=LYR0-YqsC`M@29Ip%~;Q7)cTC*{j^uq zj16o?ozIBgPkTqr7-lm@`;6%Qv`^HGGMk})#ykFn3jb}Wh~7{8M$M?O8TCFRdOz(K zHDhC&-cMJLnz4z^*w$x6@2CBvW{k8MV|_;SemWp(#%4C7!DduO@23N!W^7?I z#`%os{j@Y{##T0Ce0s)$RoRX4`t%EF9M@P@RW?!wE^!@@rK1+v@6Zy48V| zZgqs#EnF{rvMEtq8tdN}#Dn@^_h3*^rvGYm@8Dp1u}1i8(wJ!KdGdwX`9V z{G9yu_F!~UBU1OXbiX|4Q4l^J>!hg2M7E+b=+P~wpuIgS2-nea=?d4O-UE!LUMRhsw%xopQW6jJf$PU6aGmB=Y*3_aMYbwJv z^@=*SqNBsqvgt}2I~LUeR9cxycXo!ebH_F_#YdGcR80#1WZ+^KYD zS2(-E*_BSEJ9FX8?N~GOEztp*JC*LtgHs3dsqbFLwVVOmN_TdLvpbyK=~TK?CsXDu zf>Q^W?o_(77|voii|JIlvj?0#;M4)BJC*M231?3@d(x?NX9=7oaOwcool1AE2Ip#U zu12TQoxR}f1*Z;p-KlhEZ#a9y*_%$KJ9VmQKhHjJ>HyiDN_X~!voD-|=~TM2ADsQ* z)B&|SmF`>}>uold1Y`@`8EP91=|Q|ZnDa1MZT0G&#
[PATCH v2 3/7] arm/virt/acpi: remove _ADR from devices identified by _HID
According to ACPI spec, _ADR should be used for device on a bus that has a standard enumeration algorithm, but not for device which is on system bus and must be enumerated by OSPM. And it is not recommended to contain both _HID and _ADR in a single device. See ACPI 6.3, section 6.1, top of page 343: A device object must contain either an _HID object or an _ADR object, but should not contain both. (https://uefi.org/sites/default/files/resources/ACPI_6_3_May16.pdf) Signed-off-by: Heyi Guo Acked-by: Igor Mammedov --- Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 8 1 file changed, 8 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 9f4c7d1889..be752c0ad8 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -78,11 +78,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap, AML_EXCLUSIVE, _irq, 1)); aml_append(dev, aml_name_decl("_CRS", crs)); -/* The _ADR entry is used to link this device to the UART described - * in the SPCR table, i.e. SPCR.base_address.address == _ADR. - */ -aml_append(dev, aml_name_decl("_ADR", aml_int(uart_memmap->base))); - aml_append(scope, dev); } @@ -170,7 +165,6 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03"))); aml_append(dev, aml_name_decl("_SEG", aml_int(0))); aml_append(dev, aml_name_decl("_BBN", aml_int(0))); -aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_string("PCI0"))); aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device"))); aml_append(dev, aml_name_decl("_CCA", aml_int(1))); @@ -334,7 +328,6 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, { Aml *dev = aml_device("GPO0"); aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061"))); -aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_int(0))); Aml *crs = aml_resource_template(); @@ -364,7 +357,6 @@ static void acpi_dsdt_add_power_button(Aml *scope) { Aml *dev = aml_device(ACPI_POWER_BUTTON_DEVICE); aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C"))); -aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_int(0))); aml_append(scope, dev); } -- 2.19.1
[PATCH v2 5/7] arm/acpi: fix duplicated _UID of PCI interrupt link devices
Using _UID of 0 for all PCI interrupt link devices absolutely violates the spec. Simply increase one by one. Signed-off-by: Heyi Guo --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 5d157a9dd5..f3e340b172 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -189,7 +189,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, uint32_t irqs = irq + i; Aml *dev_gsi = aml_device("GSI%d", i); aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F"))); -aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0))); +aml_append(dev_gsi, aml_name_decl("_UID", aml_int(i))); crs = aml_resource_template(); aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, -- 2.19.1
[PATCH v2 2/7] arm/virt/acpi: remove meaningless sub device "RP0" from PCI0
The sub device "RP0" under PCI0 in ACPI/DSDT does not contain any method or property other than "_ADR", so it is safe to remove it. Signed-off-by: Heyi Guo Acked-by: "Michael S. Tsirkin" --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 4 1 file changed, 4 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index bd5f771e9b..9f4c7d1889 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -317,10 +317,6 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, aml_return(buf)); aml_append(dev, method); -Aml *dev_rp0 = aml_device("%s", "RP0"); -aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0))); -aml_append(dev, dev_rp0); - Aml *dev_res0 = aml_device("%s", "RES0"); aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02"))); crs = aml_resource_template(); -- 2.19.1
[PATCH v2 1/7] bios-tables-test: prepare to change ARM virt ACPI DSDT
We are going to change ARM virt ACPI DSDT table, which will cause make check to fail, so temporarily add related golden masters to ignore list. Signed-off-by: Heyi Guo --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- tests/qtest/bios-tables-test-allowed-diff.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..32a401ae35 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,4 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/DSDT", +"tests/data/acpi/virt/DSDT.memhp", +"tests/data/acpi/virt/DSDT.numamem", -- 2.19.1
[PATCH v2 6/7] arm/acpi: simplify the description of PCI _CRS
The original code defines a named object for the resource template but then returns the resource template object itself; the resulted output is like below: Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings { Name (RBUF, ResourceTemplate () { WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, 0x, // Granularity 0x, // Range Minimum 0x00FF, // Range Maximum 0x, // Translation Offset 0x0100, // Length ,, ) .. }) Return (ResourceTemplate () { WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, 0x, // Granularity 0x, // Range Minimum 0x00FF, // Range Maximum 0x, // Translation Offset 0x0100, // Length ,, ) .. }) } So the named object "RBUF" is actually useless. The more natural way is to return RBUF instead, or simply drop RBUF definition. Choose the latter one to simplify the code. Signed-off-by: Heyi Guo --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index f3e340b172..fb4b166f82 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -236,7 +236,6 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, size_mmio_high)); } -aml_append(method, aml_name_decl("RBUF", rbuf)); aml_append(method, aml_return(rbuf)); aml_append(dev, method); -- 2.19.1
[PATCH v2 4/7] arm/acpi: fix PCI _PRT definition
The address field in each _PRT mapping package should be constructed with high word for device# and low word for function#, so it is wrong to use bus_no as the high word. Enumerate all possible slots (i.e. PCI_SLOT_MAX) instead. Signed-off-by: Heyi Guo --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index be752c0ad8..5d157a9dd5 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -151,7 +151,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, { int ecam_id = VIRT_ECAM_ID(highmem_ecam); Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf; -int i, bus_no; +int i, slot_no; hwaddr base_mmio = memmap[VIRT_PCIE_MMIO].base; hwaddr size_mmio = memmap[VIRT_PCIE_MMIO].size; hwaddr base_pio = memmap[VIRT_PCIE_PIO].base; @@ -170,12 +170,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(dev, aml_name_decl("_CCA", aml_int(1))); /* Declare the PCI Routing Table. */ -Aml *rt_pkg = aml_varpackage(nr_pcie_buses * PCI_NUM_PINS); -for (bus_no = 0; bus_no < nr_pcie_buses; bus_no++) { +Aml *rt_pkg = aml_varpackage(PCI_SLOT_MAX * PCI_NUM_PINS); +for (slot_no = 0; slot_no < PCI_SLOT_MAX; slot_no++) { for (i = 0; i < PCI_NUM_PINS; i++) { -int gsi = (i + bus_no) % PCI_NUM_PINS; +int gsi = (i + slot_no) % PCI_NUM_PINS; Aml *pkg = aml_package(4); -aml_append(pkg, aml_int((bus_no << 16) | 0x)); +aml_append(pkg, aml_int((slot_no << 16) | 0x)); aml_append(pkg, aml_int(i)); aml_append(pkg, aml_name("GSI%d", gsi)); aml_append(pkg, aml_int(0)); -- 2.19.1
[PATCH v2 0/7] Some cleanup in arm/virt/acpi
Remove conflict _ADR objects, and fix and refine PCI device definition in ACPI/DSDT. Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org v1 -> v2: - flow the work flow in tests/qtest/bios-table-test.c to post ACPI related patches. - update commit messages for removing "RP0" and "_ADR". - add 3 more cleanup patches. Heyi Guo (7): bios-tables-test: prepare to change ARM virt ACPI DSDT arm/virt/acpi: remove meaningless sub device "PR0" from PCI0 arm/virt/acpi: remove _ADR from devices identified by _HID arm/acpi: fix PCI _PRT definition arm/acpi: fix duplicated _UID of PCI interrupt link devices arm/acpi: simplify the description of PCI _CRS virt/acpi: update golden masters for DSDT update hw/arm/virt-acpi-build.c | 25 ++--- tests/data/acpi/virt/DSDT | Bin 18462 -> 5307 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19799 -> 6644 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 5307 bytes 4 files changed, 6 insertions(+), 19 deletions(-) -- 2.19.1
[PATCH] tests/qtest: update comments about bios-tables-test-allowed-diff.h
Update comments in tests/qtest/bios-tables-test.c to reflect the current path of bios-tables-test-allowed-diff.h, which is now under tests/qtest/ as well. Signed-off-by: Heyi Guo --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Thomas Huth Cc: Laurent Vivier Cc: Paolo Bonzini --- tests/qtest/bios-tables-test.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index 3ab4872bd7..b4752c644c 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -14,14 +14,14 @@ * How to add or update the tests: * Contributor: * 1. add empty files for new tables, if any, under tests/data/acpi - * 2. list any changed files in tests/bios-tables-test-allowed-diff.h + * 2. list any changed files in tests/qtest/bios-tables-test-allowed-diff.h * 3. commit the above *before* making changes that affect the tables * * Contributor or ACPI Maintainer (steps 4-7 need to be redone to resolve conflicts * in binary commit created in step 6): * * After 1-3 above tests will pass but ignore differences with the expected files. - * You will also notice that tests/bios-tables-test-allowed-diff.h lists + * You will also notice that tests/qtest/bios-tables-test-allowed-diff.h lists * a bunch of files. This is your hint that you need to do the below: * 4. Run * make check V=1 @@ -40,14 +40,14 @@ *in commit log. * 7. Before sending patches to the list (Contributor) *or before doing a pull request (Maintainer), make sure - *tests/bios-tables-test-allowed-diff.h is empty - this will ensure + *tests/qtest/bios-tables-test-allowed-diff.h is empty - this will ensure *following changes to ACPI tables will be noticed. * * The resulting patchset/pull request then looks like this: - * - patch 1: list changed files in tests/bios-tables-test-allowed-diff.h. + * - patch 1: list changed files in tests/qtest/bios-tables-test-allowed-diff.h. * - patches 2 - n: real changes, may contain multiple patches. * - patch n + 1: update golden master binaries and empty - * tests/bios-tables-test-allowed-diff.h + * tests/qtest/bios-tables-test-allowed-diff.h */ #include "qemu/osdep.h" -- 2.19.1
[PATCH 2/2] arm/virt/acpi: remove _ADR from devices identified by _HID
According to ACPI spec, _ADR should be used for device which is on a bus that has a standard enumeration algorithm. It does not make sense to have a _ADR object for devices which already have _HID and will be enumerated by OSPM. Signed-off-by: Heyi Guo --- Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 8 tests/data/acpi/virt/DSDT | Bin 18449 -> 18426 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19786 -> 19763 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18449 -> 18426 bytes 4 files changed, 8 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 9f4c7d1889..be752c0ad8 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -78,11 +78,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap, AML_EXCLUSIVE, _irq, 1)); aml_append(dev, aml_name_decl("_CRS", crs)); -/* The _ADR entry is used to link this device to the UART described - * in the SPCR table, i.e. SPCR.base_address.address == _ADR. - */ -aml_append(dev, aml_name_decl("_ADR", aml_int(uart_memmap->base))); - aml_append(scope, dev); } @@ -170,7 +165,6 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03"))); aml_append(dev, aml_name_decl("_SEG", aml_int(0))); aml_append(dev, aml_name_decl("_BBN", aml_int(0))); -aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_string("PCI0"))); aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device"))); aml_append(dev, aml_name_decl("_CCA", aml_int(1))); @@ -334,7 +328,6 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, { Aml *dev = aml_device("GPO0"); aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061"))); -aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_int(0))); Aml *crs = aml_resource_template(); @@ -364,7 +357,6 @@ static void acpi_dsdt_add_power_button(Aml *scope) { Aml *dev = aml_device(ACPI_POWER_BUTTON_DEVICE); aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C"))); -aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_int(0))); aml_append(scope, dev); } diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT index b5895cb22446860a0b9be3d32ec856feb388be4c..a759ff739a071d5fbf50519a6aea296e5e0f1e0c 100644 GIT binary patch delta 72 zcmbO@f$>*ABbQ6COUN_q{66S<_BT5Bh^tIeLL4lL8ZSqD=gU!!5x$Pt+ c1HyxxIO07#U3dfh0t}oDoEbRcLp@y>07w882mk;8 delta 94 zcmey>@cBbQ6CONgKc0|V26iCof5J#`b+RhV2^Ci+-%al|{i1o1F1FmP^cRp4ao tnY@hCfEg`7$S;oxFTNc#soEyoaX?Z-8HbfwO@#16Tu)4E1zj005fm7mWY_ diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp index 69ad844f65d047973a3e55198beecd45a35b8fce..6e5cc61977e4cd24f765fec0693f75a528c144c1 100644 GIT binary patch delta 72 zcmX>#i*fTTMlP3Nmk?uL1_q|eiCof5eHSLGt1wzk^tIeLL4lL8ZSqD=gU!!5U7RH) c1HyxxIO07#U3dfh0t}oDoEbRcLp@y>03)CjmjD0& delta 94 zcmdlyi}BPfMlP3Nmk=*s1_q}3iCof5t(PXMt1!8;O!Tqj;)r*23F2X3VBp-?s=&$E tGkF=O0W(l&^JPwVXL*ABbQ6COUN_q{66S<_BT5Bh^tIeLL4lL8ZSqD=gU!!5x$Pt+ c1HyxxIO07#U3dfh0t}oDoEbRcLp@y>07w882mk;8 delta 94 zcmey>@cBbQ6CONgKc0|V26iCof5J#`b+RhV2^Ci+-%al|{i1o1F1FmP^cRp4ao tnY@hCfEg`7$S;oxFTNc#soEyoaX?Z-8HbfwO@#16Tu)4E1zj005fm7mWY_ -- 2.19.1
[PATCH 0/2] Some cleanup in arm/virt/acpi
Remove useless device node and conflict _ADR objects in ACPI/DSDT. Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org Heyi Guo (2): arm/virt/acpi: remove meaningless sub device "PR0" from PCI0 arm/virt/acpi: remove _ADR from devices identified by _HID hw/arm/virt-acpi-build.c | 12 tests/data/acpi/virt/DSDT | Bin 18462 -> 18426 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19799 -> 19763 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 18426 bytes 4 files changed, 12 deletions(-) -- 2.19.1
[PATCH 1/2] arm/virt/acpi: remove meaningless sub device "PR0" from PCI0
The sub device "PR0" under PCI0 in ACPI/DSDT does not make any sense, so simply remote it. Signed-off-by: Heyi Guo --- Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Shannon Zhao Cc: qemu-...@nongnu.org Cc: qemu-devel@nongnu.org --- hw/arm/virt-acpi-build.c | 4 tests/data/acpi/virt/DSDT | Bin 18462 -> 18449 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19799 -> 19786 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 18449 bytes 4 files changed, 4 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index bd5f771e9b..9f4c7d1889 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -317,10 +317,6 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, aml_return(buf)); aml_append(dev, method); -Aml *dev_rp0 = aml_device("%s", "RP0"); -aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0))); -aml_append(dev, dev_rp0); - Aml *dev_res0 = aml_device("%s", "RES0"); aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02"))); crs = aml_resource_template(); diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT index d0f3afeb134fdf1c11f64cd06dbcdd30be603b80..b5895cb22446860a0b9be3d32ec856feb388be4c 100644 GIT binary patch delta 39 vcmbO?fpOvlMlP3Nmk>b@1_q`B6S<_Bdg?Z+cXBfI+}XT|v(|R9jr$`2@RSW) delta 50 zcmbO@fpOjhMlP3Nmk>D*1_q{tiCof5o%I{lJ2{y;?{412S!>J19TZ>?^tF5;R%I G{V4!>hYx%J diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp index 41ccc6431b917252bcbaac86c33b340c796be5ce..69ad844f65d047973a3e55198beecd45a35b8fce 100644 GIT binary patch delta 40 wcmcaUi}BPfMlP3Nmk=*s1_q}3iCof5t(P{ccXBfI+}XT|v(|RAjk`1(02g)*ivR!s delta 51 zcmX>#i}Cs_MlP3NmymE@1_mbiiCof5O_w*ScXBdy-rc;3v(}c2J1D>)o+IATC1|sb HyBr$;t7;Fc diff --git a/tests/data/acpi/virt/DSDT.numamem b/tests/data/acpi/virt/DSDT.numamem index d0f3afeb134fdf1c11f64cd06dbcdd30be603b80..b5895cb22446860a0b9be3d32ec856feb388be4c 100644 GIT binary patch delta 39 vcmbO?fpOvlMlP3Nmk>b@1_q`B6S<_Bdg?Z+cXBfI+}XT|v(|R9jr$`2@RSW) delta 50 zcmbO@fpOjhMlP3Nmk>D*1_q{tiCof5o%I{lJ2{y;?{412S!>J19TZ>?^tF5;R%I G{V4!>hYx%J -- 2.19.1
[PATCH v6 1/2] hw/arm/acpi: simplify AML bit and/or statement
The last argument of AML bit and/or statement is the target variable, so we don't need to use a NULL target and then an additional store operation; using just aml_and() or aml_or() statement is enough. Also update tests/data/acpi/virt/DSDT* to pass "make check". Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Suggested-by: Igor Mammedov Reviewed-by: Igor Mammedov Signed-off-by: Heyi Guo --- hw/arm/virt-acpi-build.c | 16 tests/data/acpi/virt/DSDT | Bin 18470 -> 18462 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19807 -> 19799 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18470 -> 18462 bytes 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 4cd50175e0..51b293e0a1 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -267,17 +267,17 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); -aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL), -aml_name("CTRL"))); +aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL), - aml_name("CDW1"))); +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL), - aml_name("CDW1"))); +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3"))); @@ -285,8 +285,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, ifctx); elsectx = aml_else(); -aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL), - aml_name("CDW1"))); +aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4), + aml_name("CDW1"))); aml_append(elsectx, aml_return(aml_arg(3))); aml_append(method, elsectx); aml_append(dev, method); diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT index bce76e3d23e99e6c5ef64c94c770282dd30ecdd0..05bcfc8a912f58f266aa906563ea01c24906717e 100644 GIT binary patch delta 133 zcmZ2BfpOjhMlP3Nmk>D*1_q|2iCof5o%I{lJ2{y;?{412x!p#mARjJS5V=5L(Nq{%?q7$gZ1761tsfcPNsCD{x4 MAmS{W8QoPG0j8@bzW@LL delta 141 zcmbO?fpOUcMlP3Nmk>1%1_q`n6S<_B8XGpMcXBc{-rKy1bGwazA7{LOuro_nHiNTE zxZwi7$(3%F{sq;}AwfP|vJ4<! diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp index b4b153fcdc30d211237fced6be1e99d3500bd276..c041a910fdf272cb89263bb636239ae3a5e1708d 100644 GIT binary patch delta 132 zcmcaVi}Cs_MlP3NmymE@1_ma@iCof*O^IGH-{Zr;SX-A2HTGu}VgnWZb6!PzC; zaDm6_#p8m*$ep~ L;w+mP-Q(B*s{AMU delta 140 zcmcaUi}C&}MlP3Nmymd01_maViCof*T^rT9IGGynZQjJW-A2HVGu}VgnWZb6!PzC; zaDm_CN;gaYf@D*1_q|2iCof5o%I{lJ2{y;?{412x!p#mARjJS5V=5L(Nq{%?q7$gZ1761tsfcPNsCD{x4 MAmS{W8QoPG0j8@bzW@LL delta 141 zcmbO?fpOUcMlP3Nmk>1%1_q`n6S<_B8XGpMcXBc{-rKy1bGwazA7{LOuro_nHiNTE zxZwi7$(3%F{sq;}AwfP|vJ4<! -- 2.19.1
[PATCH v6 0/2] arm/acpi: simplify aml code and enable SHPC
After the introduction of generic PCIe root port and PCIe-PCI bridge, we will also have SHPC controller on ARM, and we don't support ACPI hot plug, so just enable SHPC native hot plug. Igor also spotted the store operation outside of bit and/or is not necessary, so simply the code at first. v6: - Fix "make check" errors by updating tests/data/acpi/virt/DSDT*. v5: - Refine commit message of patch 1/2 v4: - Improve the code indention. Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Heyi Guo (2): hw/arm/acpi: simplify AML bit and/or statement hw/arm/acpi: enable SHPC native hot plug hw/arm/virt-acpi-build.c | 21 + tests/data/acpi/virt/DSDT | Bin 18470 -> 18462 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19807 -> 19799 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18470 -> 18462 bytes 4 files changed, 13 insertions(+), 8 deletions(-) -- 2.19.1
[PATCH v6 2/2] hw/arm/acpi: enable SHPC native hot plug
After the introduction of generic PCIe root port and PCIe-PCI bridge, we will also have SHPC controller on ARM, so just enable SHPC native hot plug. Also update tests/data/acpi/virt/DSDT* to pass "make check". Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Reviewed-by: Michael S. Tsirkin Reviewed-by: Igor Mammedov Signed-off-by: Heyi Guo --- hw/arm/virt-acpi-build.c | 7 ++- tests/data/acpi/virt/DSDT | Bin 18462 -> 18462 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19799 -> 19799 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 18462 bytes 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 51b293e0a1..bd5f771e9b 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -267,7 +267,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); -aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + +/* + * Allow OS control for all 5 features: + * PCIeHotplug SHPCHotplug PME AER PCIeCapability. + */ +aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1F), aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1; diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT index 05bcfc8a912f58f266aa906563ea01c24906717e..d0f3afeb134fdf1c11f64cd06dbcdd30be603b80 100644 GIT binary patch delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q{tja=*8809zbbW3Ff0C~9xM*si- delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q|2ja=*87-cu_bW3Ff0C~j-M*si- diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp index c041a910fdf272cb89263bb636239ae3a5e1708d..41ccc6431b917252bcbaac86c33b340c796be5ce 100644 GIT binary patch delta 28 kcmcaUi}Cs_MlP3NmymE@1_mbija=*8809zbbeqQp0Eq|*2mk;8 delta 28 kcmcaUi}Cs_MlP3NmymE@1_ma@ja=*87-cu_beqQp0ErX{2mk;8 diff --git a/tests/data/acpi/virt/DSDT.numamem b/tests/data/acpi/virt/DSDT.numamem index 05bcfc8a912f58f266aa906563ea01c24906717e..d0f3afeb134fdf1c11f64cd06dbcdd30be603b80 100644 GIT binary patch delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q{tja=*8809zbbW3Ff0C~9xM*si- delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q|2ja=*87-cu_bW3Ff0C~j-M*si- -- 2.19.1
[RFC v2 03/14] arm/sdei: add virtual device framework
SDEI is useful to emulate NMI on arm64 platforms. To support SDEI in virtual machine with KVM enabled, we choose to implement SDEI interfaces in qemu. It is targeted for KVM mode only, for the full user space emulation can also emulate secure world and have ARM Trusted Firmware to run on emulated EL3. - We create a logical SDEI device to hold the states of SDEI services, to support VM migration. - Only one SDEI virtual device is allowed in the whole VM to provide SDEI services. - We create struct QemuSDE to hold states of each SDEI event, and private events with the same ID on different CPUs have their own QemuSDE instance. - We create struct QemuSDEProp to hold properties of each SDEI event, so all private instances with the same ID will pointed to the same QemuSDEProp. - We create struct QemuSDECpu to hold CPU/PE states, including the interrupted CPU context. - Slot numbers for private and shared event are fixed, for guests cannot request more interrupt binds than BIND_SLOTS in SDEI_FEATURES call. - The first PRIVATE_SLOT_COUNT slots in property array are for private events, and the next SHARED_SLOT_COUNT slots are for shared events. - We use property slot index as lower bit for each allocated event number, so that we can get property easily from valid input event number, as well as the QemuSDE instance. Signed-off-by: Heyi Guo Signed-off-by: Jingyi Wang Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 344 ++ target/arm/sdei_int.h | 118 +++ 2 files changed, 462 insertions(+) create mode 100644 target/arm/sdei.c create mode 100644 target/arm/sdei_int.h diff --git a/target/arm/sdei.c b/target/arm/sdei.c new file mode 100644 index 00..931e46923a --- /dev/null +++ b/target/arm/sdei.c @@ -0,0 +1,344 @@ +/* + * ARM SDEI emulation for ARM64 virtual machine with KVM + * + * Copyright (c) Huawei Technologies Co., Ltd. 2019. All rights reserved. + * + * Authors: + *Heyi Guo + *Jingyi Wang + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2 or later, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "arm-powerctl.h" +#include "qemu/timer.h" +#include "sysemu/kvm.h" +#include "sysemu/kvm_int.h" +#include "sysemu/sysemu.h" +#include "sysemu/reset.h" +#include "qemu/error-report.h" +#include "sdei_int.h" +#include "internals.h" +#include "hw/boards.h" +#include "hw/intc/arm_gicv3.h" +#include "hw/intc/arm_gic.h" +#include "hw/irq.h" +#include "hw/sysbus.h" +#include "migration/vmstate.h" +#include "qom/object.h" + +#define TYPE_ARM_SDEI "arm_sdei" +#define QEMU_SDEI(obj) OBJECT_CHECK(QemuSDEState, (obj), TYPE_ARM_SDEI) + +static QemuSDEState *sde_state; + +static void qemu_sde_prop_init(QemuSDEState *s) +{ +QemuSDEProp *sde_props = s->sde_props_state; +int i; +for (i = 0; i < ARRAY_SIZE(s->sde_props_state); i++) { +sde_props[i].event_id = SDEI_INVALID_EVENT_ID; +sde_props[i].interrupt = SDEI_INVALID_INTERRUPT; +sde_props[i].sde_index = i >= PRIVATE_SLOT_COUNT ? + i - PRIVATE_SLOT_COUNT : i; + +qemu_mutex_init(&(sde_props[i].lock)); +sde_props[i].refcount = 0; +} +sde_props[0].event_id = SDEI_STD_EVT_SOFTWARE_SIGNAL; +sde_props[0].interrupt = SDEI_INVALID_INTERRUPT; +sde_props[0].is_shared = false; +sde_props[0].is_critical = false; + +for (i = 0; i < ARRAY_SIZE(s->irq_map); i++) { +s->irq_map[i] = SDEI_INVALID_EVENT_ID; +} + +qemu_mutex_init(>sdei_interrupt_bind_lock); +} + +static void qemu_sde_cpu_init(QemuSDEState *s) +{ +int i; +QemuSDECpu *sde_cpus; + +s->sdei_max_cpus = current_machine->smp.max_cpus; +s->sde_cpus = g_new0(QemuSDECpu, s->sdei_max_cpus); +sde_cpus = s->sde_cpus; +for (i = 0; i < s->sdei_max_cpus; i++) { +sde_cpus[i].masked = true; +sde_cpus[i].critical_running_event = SDEI_INVALID_EVENT_ID; +sde_cpus[i].normal_running_event = SDEI_INVALID_EVENT_ID; +} +} + +static bool is_valid_event_number(int32_t event) +{ +int32_t slot_id; + +if (event <
[RFC v2 12/14] arm/sdei: add stub to fix build failure when SDEI is not enabled
Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/Makefile.objs | 2 ++ target/arm/sdei-stub.c | 49 2 files changed, 51 insertions(+) create mode 100644 target/arm/sdei-stub.c diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs index 72e01d08dc..90235af8ec 100644 --- a/target/arm/Makefile.objs +++ b/target/arm/Makefile.objs @@ -12,6 +12,8 @@ obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o obj-$(CONFIG_SDEI) += sdei.o +obj-$(call lnot,$(CONFIG_SDEI)) += sdei-stub.o + DECODETREE = $(SRC_PATH)/scripts/decodetree.py diff --git a/target/arm/sdei-stub.c b/target/arm/sdei-stub.c new file mode 100644 index 00..4eaf365de7 --- /dev/null +++ b/target/arm/sdei-stub.c @@ -0,0 +1,49 @@ +/* + * QEMU ARM SDEI specific function stubs + * + * Copyright (c) Huawei Technologies Co., Ltd. 2019. All rights reserved. + * + * Author: Heyi Guo + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#include "qemu/osdep.h" +#include "sdei.h" + +bool sdei_enabled; + +void sdei_handle_request(CPUState *cs, struct kvm_run *run) +{ +run->hypercall.args[0] = SDEI_NOT_SUPPORTED; +return; +} + +/* + * Trigger an SDEI event bound to an interrupt. + * Return true if event has been triggered successfully. + * Return false if event has not been triggered for some reason. + */ +bool trigger_sdei_by_irq(int cpu, int irq) +{ +return false; +} + +/* + * Register a notify callback for a specific interrupt bind operation; the + * client will be both notified by bind and unbind operation. + */ +void qemu_register_sdei_bind_notifier(QemuSDEIBindNotify *func, + void *opaque, int irq) +{ +} + +/* + * Unregister a notify callback for a specific interrupt bind operation. + */ +void qemu_unregister_sdei_bind_notifier(QemuSDEIBindNotify *func, +void *opaque, int irq) +{ +} -- 2.19.1
[RFC v2 09/14] arm/sdei: override qemu_irq handler when binding interrupt
Override qemu_irq handler to support trigger SDEI event transparently after guest binds interrupt to SDEI event. We don't have good way to get GIC device and to guarantee SDEI device is initialized after GIC, so we search GIC in system bus when the first SDEI request happens or in VMSTATE post_load(). Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 130 +- target/arm/sdei_int.h | 3 + 2 files changed, 132 insertions(+), 1 deletion(-) diff --git a/target/arm/sdei.c b/target/arm/sdei.c index 713ac97775..529a06c1f6 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -88,6 +88,24 @@ static void qemu_sde_cpu_init(QemuSDEState *s) } } +static int gic_int_to_irq(int num_irq, int intid, int cpu) +{ +if (intid >= GIC_INTERNAL) { +return intid - GIC_INTERNAL; +} +return num_irq - GIC_INTERNAL + cpu * GIC_INTERNAL + intid; +} + +static int irq_to_gic_int(int num_irq, int irq, int *cpu) +{ +if (irq < num_irq - GIC_INTERNAL) { +return irq + GIC_INTERNAL; +} +irq -= num_irq - GIC_INTERNAL; +*cpu = irq / GIC_INTERNAL; +return irq % GIC_INTERNAL; +} + static inline QemuSDECpu *get_sde_cpu(QemuSDEState *s, CPUState *cs) { if (cs->cpu_index >= s->sdei_max_cpus) { @@ -410,6 +428,74 @@ static void dispatch_cpu(QemuSDEState *s, CPUState *cs, bool is_critical) } } +static void qemu_sdei_irq_handler(void *opaque, int irq, int level) +{ +int cpu = 0; + +irq = irq_to_gic_int(sde_state->num_irq, irq, ); +trigger_sdei_by_irq(cpu, irq); +} + +static void override_qemu_irq(QemuSDEState *s, int32_t event, uint32_t intid) +{ +qemu_irq irq; +QemuSDE *sde; +CPUState *cs; + +/* SPI */ +if (intid >= GIC_INTERNAL) { +cs = first_cpu; +irq = qdev_get_gpio_in(s->gic_dev, + gic_int_to_irq(s->num_irq, intid, 0)); +if (irq) { +qemu_irq_intercept_in(, qemu_sdei_irq_handler, 1); +} +sde = get_sde_no_check(s, event, cs); +sde->irq = irq; +put_sde(sde, cs); +return; +} +/* PPI */ +CPU_FOREACH(cs) { +irq = qdev_get_gpio_in( +s->gic_dev, +gic_int_to_irq(s->num_irq, intid, cs->cpu_index)); +if (irq) { +qemu_irq_intercept_in(, qemu_sdei_irq_handler, 1); +} +sde = get_sde_no_check(s, event, cs); +sde->irq = irq; +put_sde(sde, cs); +} +} + +static void restore_qemu_irq(QemuSDEState *s, int32_t event, uint32_t intid) +{ +QemuSDE *sde; +CPUState *cs; + +/* SPI */ +if (intid >= GIC_INTERNAL) { +cs = first_cpu; +sde = get_sde_no_check(s, event, cs); +if (sde->irq) { +qemu_irq_remove_intercept(>irq, 1); +sde->irq = NULL; +} +put_sde(sde, cs); +return; +} +/* PPI */ +CPU_FOREACH(cs) { +sde = get_sde_no_check(s, event, cs); +if (sde->irq) { +qemu_irq_remove_intercept(>irq, 1); +sde->irq = NULL; +} +put_sde(sde, cs); +} +} + static int32_t sdei_alloc_event_num(QemuSDEState *s, bool is_critical, bool is_shared, int intid) { @@ -443,6 +529,7 @@ static int32_t sdei_alloc_event_num(QemuSDEState *s, bool is_critical, sde_props[index].interrupt = intid; sde_props[index].is_shared = is_shared; sde_props[index].is_critical = is_critical; +override_qemu_irq(s, event, intid); s->irq_map[intid] = event; qemu_mutex_unlock(_props[index].lock); qemu_mutex_unlock(>sdei_interrupt_bind_lock); @@ -460,6 +547,7 @@ static int32_t sdei_free_event_num_locked(QemuSDEState *s, QemuSDEProp *prop) return SDEI_DENIED; } +restore_qemu_irq(s, prop->event_id, prop->interrupt); s->irq_map[prop->interrupt] = SDEI_INVALID_EVENT_ID; prop->event_id = SDEI_INVALID_EVENT_ID; prop->interrupt = SDEI_INVALID_INTERRUPT; @@ -992,13 +1080,33 @@ static int64_t sdei_event_pe_unmask(QemuSDEState *s, CPUState *cs, return SDEI_SUCCESS; } +static int dev_walkerfn(DeviceState *dev, void *opaque) +{ +QemuSDEState *s = opaque; + +if (object_dynamic_cast(OBJECT(dev), TYPE_ARM_GICV3_COMMON)) { +GICv3State *gic = ARM_GICV3_COMMON(dev); +s->num_irq = gic->num_irq; +s->gic_dev = dev; +return -1; +} + +if (object_dynamic_cast(OBJECT(dev), TYPE_ARM_GIC_COMMON)) { +GICState *gic = ARM_GIC_COMMON(dev); +s->num_irq = gic->num_irq; +s->gic_dev = dev; +return -1; +} +return 0; +} + static int64_t sdei_event_interrupt_bind(QemuSDEState *s, CPUState *cs,
[RFC v2 11/14] linux-headers/kvm.h: add capability to forward hypercall
To keep backward compatibility, we add new KVM capability "KVM_CAP_FORWARD_HYPERCALL" to probe whether KVM supports forwarding hypercall to userspace. The capability should be enabled explicitly, for we don't want user space application to deal with unexpected hypercall exits. After enabling this cap, all HVC calls unhandled by kvm will be forwarded to user space. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Cornelia Huck Cc: Paolo Bonzini Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- linux-headers/linux/kvm.h | 1 + target/arm/sdei.c | 16 target/arm/sdei.h | 2 ++ 3 files changed, 19 insertions(+) diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h index 3d9b18f7f8..36c9b3859f 100644 --- a/linux-headers/linux/kvm.h +++ b/linux-headers/linux/kvm.h @@ -1000,6 +1000,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_PMU_EVENT_FILTER 173 #define KVM_CAP_ARM_IRQ_LINE_LAYOUT_2 174 #define KVM_CAP_HYPERV_DIRECT_TLBFLUSH 175 +#define KVM_CAP_FORWARD_HYPERCALL 176 #ifdef KVM_CAP_IRQ_ROUTING diff --git a/target/arm/sdei.c b/target/arm/sdei.c index 4cc68e4acf..56e6874f6f 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -46,6 +46,7 @@ #define SMCCC_RETURN_REG_COUNT 4 #define PSTATE_M_EL_SHIFT 2 +bool sdei_enabled; static QemuSDEState *sde_state; typedef struct QemuSDEIBindNotifyEntry { @@ -1524,6 +1525,7 @@ static const VMStateDescription vmstate_sde_state = { static void sdei_initfn(Object *obj) { QemuSDEState *s = QEMU_SDEI(obj); +KVMState *kvm = KVM_STATE(current_machine->accelerator); if (sde_state) { error_report("Only one SDEI dispatcher is allowed!"); @@ -1533,6 +1535,20 @@ static void sdei_initfn(Object *obj) qemu_sde_init(s); qemu_register_reset(qemu_sde_reset, s); + +if (kvm_check_extension(kvm, KVM_CAP_FORWARD_HYPERCALL)) { +int ret; +ret = kvm_vm_enable_cap(kvm, KVM_CAP_FORWARD_HYPERCALL, 0, 0); +if (ret < 0) { +error_report("Enable hypercall forwarding failed: %s", + strerror(-ret)); +abort(); +} +sdei_enabled = true; +info_report("qemu sdei enabled"); +} else { +info_report("KVM does not support forwarding hypercall."); +} } static void qemu_sde_class_init(ObjectClass *klass, void *data) diff --git a/target/arm/sdei.h b/target/arm/sdei.h index 9c15cf3186..9f683ca2a0 100644 --- a/target/arm/sdei.h +++ b/target/arm/sdei.h @@ -29,6 +29,8 @@ #define SDEI_MAX_REQSDEI_1_0_FN(0x12) +extern bool sdei_enabled; + void sdei_handle_request(CPUState *cs, struct kvm_run *run); /* -- 2.19.1
[RFC v2 02/14] standard-headers: import arm_sdei.h
Import Linux header file include/uapi/linux/arm_sdei.h from kernel v5.4-rc5. This is to prepare for qemu SDEI emulation. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse Cc: "Michael S. Tsirkin" Cc: Cornelia Huck Cc: Paolo Bonzini --- Notes: v2: - Import arm_sdei.h by running update-linux-headers.sh include/standard-headers/linux/arm_sdei.h | 73 +++ 1 file changed, 73 insertions(+) create mode 100644 include/standard-headers/linux/arm_sdei.h diff --git a/include/standard-headers/linux/arm_sdei.h b/include/standard-headers/linux/arm_sdei.h new file mode 100644 index 00..568d971915 --- /dev/null +++ b/include/standard-headers/linux/arm_sdei.h @@ -0,0 +1,73 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* Copyright (C) 2017 Arm Ltd. */ +#ifndef _LINUX_ARM_SDEI_H +#define _LINUX_ARM_SDEI_H + +#define SDEI_1_0_FN_BASE 0xC420 +#define SDEI_1_0_MASK 0xFFE0 +#define SDEI_1_0_FN(n) (SDEI_1_0_FN_BASE + (n)) + +#define SDEI_1_0_FN_SDEI_VERSION SDEI_1_0_FN(0x00) +#define SDEI_1_0_FN_SDEI_EVENT_REGISTER SDEI_1_0_FN(0x01) +#define SDEI_1_0_FN_SDEI_EVENT_ENABLE SDEI_1_0_FN(0x02) +#define SDEI_1_0_FN_SDEI_EVENT_DISABLE SDEI_1_0_FN(0x03) +#define SDEI_1_0_FN_SDEI_EVENT_CONTEXT SDEI_1_0_FN(0x04) +#define SDEI_1_0_FN_SDEI_EVENT_COMPLETE SDEI_1_0_FN(0x05) +#define SDEI_1_0_FN_SDEI_EVENT_COMPLETE_AND_RESUME SDEI_1_0_FN(0x06) +#define SDEI_1_0_FN_SDEI_EVENT_UNREGISTER SDEI_1_0_FN(0x07) +#define SDEI_1_0_FN_SDEI_EVENT_STATUS SDEI_1_0_FN(0x08) +#define SDEI_1_0_FN_SDEI_EVENT_GET_INFO SDEI_1_0_FN(0x09) +#define SDEI_1_0_FN_SDEI_EVENT_ROUTING_SET SDEI_1_0_FN(0x0A) +#define SDEI_1_0_FN_SDEI_PE_MASK SDEI_1_0_FN(0x0B) +#define SDEI_1_0_FN_SDEI_PE_UNMASK SDEI_1_0_FN(0x0C) +#define SDEI_1_0_FN_SDEI_INTERRUPT_BIND SDEI_1_0_FN(0x0D) +#define SDEI_1_0_FN_SDEI_INTERRUPT_RELEASE SDEI_1_0_FN(0x0E) +#define SDEI_1_0_FN_SDEI_PRIVATE_RESET SDEI_1_0_FN(0x11) +#define SDEI_1_0_FN_SDEI_SHARED_RESET SDEI_1_0_FN(0x12) + +#define SDEI_VERSION_MAJOR_SHIFT 48 +#define SDEI_VERSION_MAJOR_MASK0x7fff +#define SDEI_VERSION_MINOR_SHIFT 32 +#define SDEI_VERSION_MINOR_MASK0x +#define SDEI_VERSION_VENDOR_SHIFT 0 +#define SDEI_VERSION_VENDOR_MASK 0x + +#define SDEI_VERSION_MAJOR(x) (x>>SDEI_VERSION_MAJOR_SHIFT & SDEI_VERSION_MAJOR_MASK) +#define SDEI_VERSION_MINOR(x) (x>>SDEI_VERSION_MINOR_SHIFT & SDEI_VERSION_MINOR_MASK) +#define SDEI_VERSION_VENDOR(x) (x>>SDEI_VERSION_VENDOR_SHIFT & SDEI_VERSION_VENDOR_MASK) + +/* SDEI return values */ +#define SDEI_SUCCESS 0 +#define SDEI_NOT_SUPPORTED -1 +#define SDEI_INVALID_PARAMETERS-2 +#define SDEI_DENIED-3 +#define SDEI_PENDING -5 +#define SDEI_OUT_OF_RESOURCE -10 + +/* EVENT_REGISTER flags */ +#define SDEI_EVENT_REGISTER_RM_ANY 0 +#define SDEI_EVENT_REGISTER_RM_PE 1 + +/* EVENT_STATUS return value bits */ +#define SDEI_EVENT_STATUS_RUNNING 2 +#define SDEI_EVENT_STATUS_ENABLED 1 +#define SDEI_EVENT_STATUS_REGISTERED 0 + +/* EVENT_COMPLETE status values */ +#define SDEI_EV_HANDLED0 +#define SDEI_EV_FAILED 1 + +/* GET_INFO values */ +#define SDEI_EVENT_INFO_EV_TYPE0 +#define SDEI_EVENT_INFO_EV_SIGNALED1 +#define SDEI_EVENT_INFO_EV_PRIORITY2 +#define SDEI_EVENT_INFO_EV_ROUTING_MODE3 +#define SDEI_EVENT_INFO_EV_ROUTING_AFF 4 + +/* and their results */ +#define SDEI_EVENT_TYPE_PRIVATE0 +#define SDEI_EVENT_TYPE_SHARED 1 +#define SDEI_EVENT_PRIORITY_NORMAL 0 +#define SDEI_EVENT_PRIORITY_CRITICAL 1 + +#endif /* _LINUX_ARM_SDEI_H */ -- 2.19.1
[RFC v2 10/14] arm/sdei: add support to register interrupt bind notifier
Other qemu modules related with the interrupt bind operation may want to be notified when guest requests to bind interrupt to sdei event, so we add register and unregister interfaces. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 48 +++ target/arm/sdei.h | 17 + 2 files changed, 65 insertions(+) diff --git a/target/arm/sdei.c b/target/arm/sdei.c index 529a06c1f6..4cc68e4acf 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -48,6 +48,52 @@ static QemuSDEState *sde_state; +typedef struct QemuSDEIBindNotifyEntry { +QTAILQ_ENTRY(QemuSDEIBindNotifyEntry) entry; +QemuSDEIBindNotify *func; +void *opaque; +int irq; +} QemuSDEIBindNotifyEntry; + +static QTAILQ_HEAD(, QemuSDEIBindNotifyEntry) bind_notifiers = +QTAILQ_HEAD_INITIALIZER(bind_notifiers); + +void qemu_register_sdei_bind_notifier(QemuSDEIBindNotify *func, + void *opaque, int irq) +{ +QemuSDEIBindNotifyEntry *be = g_new0(QemuSDEIBindNotifyEntry, 1); + +be->func = func; +be->opaque = opaque; +be->irq = irq; +QTAILQ_INSERT_TAIL(_notifiers, be, entry); +} + +void qemu_unregister_sdei_bind_notifier(QemuSDEIBindNotify *func, +void *opaque, int irq) +{ +QemuSDEIBindNotifyEntry *be; + +QTAILQ_FOREACH(be, _notifiers, entry) { +if (be->func == func && be->opaque == opaque && be->irq == irq) { +QTAILQ_REMOVE(_notifiers, be, entry); +g_free(be); +return; +} +} +} + +static void sdei_notify_bind(int irq, int32_t event, bool bind) +{ +QemuSDEIBindNotifyEntry *be, *nbe; + +QTAILQ_FOREACH_SAFE(be, _notifiers, entry, nbe) { +if (be->irq == irq) { +be->func(be->opaque, irq, event, bind); +} +} +} + static void qemu_sde_prop_init(QemuSDEState *s) { QemuSDEProp *sde_props = s->sde_props_state; @@ -529,6 +575,7 @@ static int32_t sdei_alloc_event_num(QemuSDEState *s, bool is_critical, sde_props[index].interrupt = intid; sde_props[index].is_shared = is_shared; sde_props[index].is_critical = is_critical; +sdei_notify_bind(intid, event, true); override_qemu_irq(s, event, intid); s->irq_map[intid] = event; qemu_mutex_unlock(_props[index].lock); @@ -547,6 +594,7 @@ static int32_t sdei_free_event_num_locked(QemuSDEState *s, QemuSDEProp *prop) return SDEI_DENIED; } +sdei_notify_bind(prop->interrupt, prop->event_id, false); restore_qemu_irq(s, prop->event_id, prop->interrupt); s->irq_map[prop->interrupt] = SDEI_INVALID_EVENT_ID; prop->event_id = SDEI_INVALID_EVENT_ID; diff --git a/target/arm/sdei.h b/target/arm/sdei.h index 5ecc32d667..9c15cf3186 100644 --- a/target/arm/sdei.h +++ b/target/arm/sdei.h @@ -38,4 +38,21 @@ void sdei_handle_request(CPUState *cs, struct kvm_run *run); */ bool trigger_sdei_by_irq(int cpu, int irq); +/* + * Notify callback prototype; the argument "bind" tells whether it is a bind + * operation or unbind one. + */ +typedef void QemuSDEIBindNotify(void *opaque, int irq, +int32_t event, bool bind); +/* + * Register a notify callback for a specific interrupt bind operation; the + * client will be both notified by bind and unbind operation. + */ +void qemu_register_sdei_bind_notifier(QemuSDEIBindNotify *func, + void *opaque, int irq); +/* + * Unregister a notify callback for a specific interrupt bind operation. + */ +void qemu_unregister_sdei_bind_notifier(QemuSDEIBindNotify *func, +void *opaque, int irq); #endif -- 2.19.1
[RFC v2 13/14] arm/kvm: handle guest exit of hypercall
Add support to handle guest exit of hypercall, and forward to SDEI dispatcher if SDEI is enabled and it is an SDEI request. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/kvm.c | 17 + 1 file changed, 17 insertions(+) diff --git a/target/arm/kvm.c b/target/arm/kvm.c index b473c63edb..035a39e49f 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -30,6 +30,7 @@ #include "hw/boards.h" #include "hw/irq.h" #include "qemu/log.h" +#include "sdei.h" const KVMCapabilityInfo kvm_arch_required_capabilities[] = { KVM_CAP_LAST_INFO @@ -676,6 +677,19 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run) } +static void kvm_arm_handle_hypercall(CPUState *cs, struct kvm_run *run) +{ +uint32_t func_id = run->hypercall.args[0]; + +if (sdei_enabled && +func_id >= SDEI_1_0_FN_BASE && func_id <= SDEI_MAX_REQ) { +sdei_handle_request(cs, run); +return; +} + +run->hypercall.args[0] = -1; +} + int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) { int ret = 0; @@ -686,6 +700,9 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) ret = EXCP_DEBUG; } /* otherwise return to guest */ break; +case KVM_EXIT_HYPERCALL: +kvm_arm_handle_hypercall(cs, run); +break; default: qemu_log_mask(LOG_UNIMP, "%s: un-handled exit reason %d\n", __func__, run->exit_reason); -- 2.19.1
[RFC v2 01/14] update-linux-headers.sh: import linux/arm_sdei.h to standard-headers
This is to prepare for qemu SDEI emulation. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse Cc: "Michael S. Tsirkin" Cc: Cornelia Huck Cc: Paolo Bonzini --- Notes: v2: - Update update-linux-headers.sh first and then import the header file - Import arm_sdei.h to include/standard-headers/linux/ 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 f76d77363b..84d15c18f2 100755 --- a/scripts/update-linux-headers.sh +++ b/scripts/update-linux-headers.sh @@ -191,6 +191,7 @@ for i in "$tmpdir"/include/linux/*virtio*.h \ "$tmpdir/include/linux/pci_regs.h" \ "$tmpdir/include/linux/ethtool.h" "$tmpdir/include/linux/kernel.h" \ "$tmpdir/include/linux/vhost_types.h" \ + "$tmpdir/include/linux/arm_sdei.h" \ "$tmpdir/include/linux/sysinfo.h"; do cp_portable "$i" "$output/include/standard-headers/linux" done -- 2.19.1
[RFC v2 05/14] arm/sdei: add support to handle SDEI requests from guest
Add support for all interfaces defined in ARM SDEI 1.0 spec. http://infocenter.arm.com/help/topic/com.arm.doc.den0054a/ARM_DEN0054A_Software_Delegated_Exception_Interface.pdf The exit reason KVM_EXIT_HYPERCALL is used to indicate it is an HVC/SMC forward, and the structure kvm_run->hypercall is used to pass arguments and return values between KVM and qemu: Input: nr: the immediate value of SMC/HVC calls; not really used today. args[6]: x0..x5 (This is not fully conform with SMCCC which requires x6 as argument as well, but we can use GET_ONE_REG ioctl for such rare case). Return: args[0..3]: x0..x3 as defined in SMCCC. We rely on KVM to extract args[0..3] and write them to x0..x3 when hypercall exit returns. Signed-off-by: Heyi Guo Signed-off-by: Jingyi Wang Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 982 ++ target/arm/sdei.h | 34 ++ 2 files changed, 1016 insertions(+) create mode 100644 target/arm/sdei.h diff --git a/target/arm/sdei.c b/target/arm/sdei.c index 931e46923a..0c0212bfa8 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -29,6 +29,7 @@ #include "sysemu/sysemu.h" #include "sysemu/reset.h" #include "qemu/error-report.h" +#include "sdei.h" #include "sdei_int.h" #include "internals.h" #include "hw/boards.h" @@ -42,6 +43,9 @@ #define TYPE_ARM_SDEI "arm_sdei" #define QEMU_SDEI(obj) OBJECT_CHECK(QemuSDEState, (obj), TYPE_ARM_SDEI) +#define SMCCC_RETURN_REG_COUNT 4 +#define PSTATE_M_EL_SHIFT 2 + static QemuSDEState *sde_state; static void qemu_sde_prop_init(QemuSDEState *s) @@ -84,6 +88,16 @@ static void qemu_sde_cpu_init(QemuSDEState *s) } } +static inline QemuSDECpu *get_sde_cpu(QemuSDEState *s, CPUState *cs) +{ +if (cs->cpu_index >= s->sdei_max_cpus) { +error_report("BUG: cpu index %d >= max_cpus %d", + cs->cpu_index, s->sdei_max_cpus); +return NULL; +} +return >sde_cpus[cs->cpu_index]; +} + static bool is_valid_event_number(int32_t event) { int32_t slot_id; @@ -122,6 +136,974 @@ static QemuSDEProp *get_sde_prop_no_lock(QemuSDEState *s, int32_t event) return >sde_props_state[SDEI_EVENT_TO_SLOT(event)]; } +static QemuSDEProp *get_sde_prop(QemuSDEState *s, int32_t event) +{ +QemuSDEProp *sde_props = s->sde_props_state; + +if (!is_valid_event_number(event)) { +return NULL; +} + +event = SDEI_EVENT_TO_SLOT(event); + +qemu_mutex_lock(_props[event].lock); +if (sde_props[event].event_id < 0) { +qemu_mutex_unlock(_props[event].lock); +return NULL; +} +return _props[event]; +} + +static void put_sde_prop(QemuSDEProp *prop) +{ +qemu_mutex_unlock(>lock); +} + +static void sde_slot_lock(QemuSDE *sde, CPUState *cs) +{ +qemu_mutex_lock(>lock); +} + +static void sde_slot_unlock(QemuSDE *sde, CPUState *cs) +{ +qemu_mutex_unlock(>lock); +} + +/* + * It will always return a pointer to a preallocated sde; event number must be + * validated before calling this function. + */ +static QemuSDE *get_sde_no_check(QemuSDEState *s, int32_t event, CPUState *cs) +{ +QemuSDE **array = s->sde_cpus[cs->cpu_index].private_sde_array; +int32_t sde_index = SDEI_EVENT_TO_SLOT(event); +QemuSDE *sde; + +if (SDEI_IS_SHARED_EVENT(event)) { +array = s->shared_sde_array; +sde_index -= PRIVATE_SLOT_COUNT; +} + +sde = array[sde_index]; +sde_slot_lock(sde, cs); +return sde; +} + +static void put_sde(QemuSDE *sde, CPUState *cs) +{ +sde_slot_unlock(sde, cs); +} + +static inline bool is_sde_nested(QemuSDECpu *sde_cpu) +{ +return sde_cpu->critical_running_event >= 0 && + sde_cpu->normal_running_event >= 0; +} + +static int32_t get_running_sde(QemuSDEState *s, CPUState *cs) +{ +QemuSDECpu *sde_cpu = get_sde_cpu(s, cs); + +if (!sde_cpu) { +return SDEI_INVALID_EVENT_ID; +} + +if (sde_cpu->critical_running_event >= 0) { +return sde_cpu->critical_running_event; +} +return sde_cpu->normal_running_event; +} + +static void override_return_value(CPUState *cs, uint64_t *args) +{ +CPUARMState *env = _CPU(cs)->env; +int i; + +for (i = 0; i < SMCCC_RETURN_REG_COUNT; i++) { +args[i] = env->xregs[i]; +} +} + +static void sde_save_cpu_ctx(CPUState *cs, QemuSDECpu *sde_cpu, bool critical) +{ +CPUARMState *env = _CPU(cs)->env; +QemuSDECpuCtx *ctx = _cpu->ctx[critical ? 1 : 0]; + +memcpy(ctx->xregs, env->xregs, sizeof(ctx->xregs)); +ctx->pc = env->pc; +ctx->pstate = pstate_read(env); +} + +static void sde_restore_cpu_ctx(QemuSDEState *s, CPUState *cs, bool c
[RFC v2 00/14] Add SDEI support for arm64
SDEI is for ARM "Software Delegated Exception Interface". AS ARM64 doesn't have native non-maskable interrupt (NMI), we rely on higher privileged (larger exception level) software to change the execution flow of lower privileged (smaller exception level) software when certain events occur, to emulate NMI mechanism, and SDEI is the standard interfaces between the two levels of privileged software. It is based on SMC/HVC calls. The higher privileged software implements an SDEI dispatcher to handle SDEI related SMC/HVC calls and trigger SDEI events; the lower privileged software implements an SDEI client to request SDEI services and handle SDEI events. Core interfaces provided by SDEI include: 1. interrupt bind: client can request to bind an interrupt to an SDEI event, so the interrupt will be a non-maskable event and the event number will be returned to the caller. Only PPI and SPI can be bound to SDEI events. 2. register: client can request to register a handler to an SDEI event, so dispatcher will change PC of lower privileged software to this handler when certain event occurs. 3. complete: client notifies dispatcher that it has completed the event handling, so dispatcher will restore the context of guest when it is interrupted. In virtualization situation, guest OS is the lower privileged software and hypervisor is the higher one. KVM is supposed to pass SMC/HVC calls to qemu, and qemu will emulate an SDEI dispatcher to serve the SDEI requests and trigger the events. If an interrupt is requested to be bound to an event, qemu should not inject the interrupt to guest any more; instead, it should save the context of VCPU and change the PC to event handler which is registered by guest, and then return to guest. To make the conversion of interrupt to SDEI event transparent to other modules in qemu, we used qemu_irq and qemu_irq_intercept_in() to override the default irq handler with SDEI event trigger. I saw qemu_irq_intercept_in() should be only used in qemu MST, but it seemed fit to override interrupt injection with event trigger after guest requests to bind interrupt to SDEI event. This patchset is trying to implement the whole SDEI framework in qemu with KVM enabled, including all SDEI v1.0 interfaces, as well as event trigger conduit from other qemu devices after interrupt binding. Key points: - We propose to only support kvm enabled arm64 virtual machines, for non-kvm VMs can emulate EL3 and have Trusted Firmware run on it, which has a builtin SDEI dispatcher. - New kvm capability KVM_CAP_FORWARD_HYPERCALL is added to probe if kvm supports forwarding hypercalls, and the capability should be enabled explicitly. - We make the dispatcher as a logical device, to save the states during migration or save/restore operation; only one instance is allowed in one VM. - We use qemu_irq as the bridge for other qemu modules to switch from irq injection to SDEI event trigger after VM binds the interrupt to SDEI event. We use qemu_irq_intercept_in() to override qemu_irq handler with SDEI event trigger, and a new interface qemu_irq_remove_intercept() is added to restore the handler to default one (i.e. ARM GIC). More details are in the commit message of each patch. Basic tests are done by emulating a watchdog timer and triggering SDEI event in every 10s. Please focus on the interfaces and framework first. We can refine the code for several rounds after the big things have been determined. Any comment or suggestion is welcome. Thanks, HG Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse Cc: "Michael S. Tsirkin" Cc: Cornelia Huck Cc: Paolo Bonzini Cc: Shannon Zhao Cc: Igor Mammedov v2: - Import import linux/arm_sdei.h to standard-headers - Drop SDEI table definition and add comments - Some bugfix and code refinement Heyi Guo (14): update-linux-headers.sh: import linux/arm_sdei.h to standard-headers standard-headers: import arm_sdei.h arm/sdei: add virtual device framework arm: add CONFIG_SDEI build flag arm/sdei: add support to handle SDEI requests from guest arm/sdei: add system reset callback arm/sdei: add support to trigger event by GIC interrupt ID core/irq: add qemu_irq_remove_intercept interface arm/sdei: override qemu_irq handler when binding interrupt arm/sdei: add support to register interrupt bind notifier linux-headers/kvm.h: add capability to forward hypercall arm/sdei: add stub to fix build failure when SDEI is not enabled arm/kvm: handle guest exit of hypercall virt/acpi: add SDEI table if SDEI is enabled default-configs/arm-softmmu.mak |1 + hw/arm/Kconfig|4 + hw/arm/virt-acpi-build.c | 26 + hw/core/irq.c | 11 + include/hw/irq.h |8 +- include/standard-headers/linux/arm_sdei.h | 73 + linux-headers/linux/kvm.h |1 +
[RFC v2 08/14] core/irq: add qemu_irq_remove_intercept interface
We use qemu_irq as the bridge for other qemu modules to switch from irq injection to SDEI event trigger after VM binds the interrupt to SDEI event. We use qemu_irq_intercept_in() to override qemu_irq handler with SDEI event trigger, so we also need a corresponding interface to restore the handler to default one (i.e. ARM GIC). qemu_irq_remove_intercept() is the new interface to do the above job. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- hw/core/irq.c| 11 +++ include/hw/irq.h | 8 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/hw/core/irq.c b/hw/core/irq.c index 7cc0295d0e..114bce6c21 100644 --- a/hw/core/irq.c +++ b/hw/core/irq.c @@ -145,6 +145,17 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n) } } +void qemu_irq_remove_intercept(qemu_irq *gpio_in, int n) +{ +int i; +qemu_irq *old_irqs = gpio_in[0]->opaque; +for (i = 0; i < n; i++) { +gpio_in[i]->handler = old_irqs[i]->handler; +gpio_in[i]->opaque = old_irqs[i]->opaque; +} +qemu_free_irqs(old_irqs, n); +} + static const TypeInfo irq_type_info = { .name = TYPE_IRQ, .parent = TYPE_OBJECT, diff --git a/include/hw/irq.h b/include/hw/irq.h index fe527f6f51..1af1db93bb 100644 --- a/include/hw/irq.h +++ b/include/hw/irq.h @@ -56,8 +56,12 @@ qemu_irq qemu_irq_split(qemu_irq irq1, qemu_irq irq2); */ qemu_irq *qemu_irq_proxy(qemu_irq **target, int n); -/* For internal use in qtest. Similar to qemu_irq_split, but operating - on an existing vector of qemu_irq. */ +/* + * Similar to qemu_irq_split, but operating on an existing vector of qemu_irq. + */ void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n); +/* Restore the irq handler intercepted by qemu_irq_intercept_in() */ +void qemu_irq_remove_intercept(qemu_irq *gpio_in, int n); + #endif -- 2.19.1
[RFC v2 06/14] arm/sdei: add system reset callback
For this is a logical device which is not attached to system bus, we cannot use DeviceClass->reset interface directly. Instead we register our own reset callback to reset SDEI services when system resets. Signed-off-by: Heyi Guo Signed-off-by: Jingyi Wang Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 21 + 1 file changed, 21 insertions(+) diff --git a/target/arm/sdei.c b/target/arm/sdei.c index 0c0212bfa8..6af4a9044b 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -1147,6 +1147,26 @@ static void qemu_sde_init(QemuSDEState *s) qemu_private_sde_init(s); } +static void qemu_sde_reset(void *opaque) +{ +int64_t ret = 0; +CPUState*cs; +QemuSDEState*s = opaque; + +CPU_FOREACH(cs) { +QemuSDECpu *sde_cpu = get_sde_cpu(s, cs); +ret |= sdei_private_reset_common(s, cs, true); +sde_cpu->masked = true; +sde_cpu->critical_running_event = SDEI_INVALID_EVENT_ID; +sde_cpu->normal_running_event = SDEI_INVALID_EVENT_ID; +} + +ret |= sdei_shared_reset_common(s, first_cpu, true); +if (ret) { +error_report("SDEI system reset failed: 0x%lx", ret); +} +} + static void sde_array_save(QemuSDE **array, int count) { int i; @@ -1299,6 +1319,7 @@ static void sdei_initfn(Object *obj) sde_state = s; qemu_sde_init(s); +qemu_register_reset(qemu_sde_reset, s); } static void qemu_sde_class_init(ObjectClass *klass, void *data) -- 2.19.1
[RFC v2 14/14] virt/acpi: add SDEI table if SDEI is enabled
Add SDEI table if SDEI is enabled, so that guest OS can get aware and utilize the interfaces. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse Cc: Shannon Zhao Cc: "Michael S. Tsirkin" Cc: Igor Mammedov --- Notes: v2: - Drop SDEI table definition and add comments hw/arm/virt-acpi-build.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 4cd50175e0..73d3f8cd15 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -32,6 +32,7 @@ #include "trace.h" #include "hw/core/cpu.h" #include "target/arm/cpu.h" +#include "target/arm/sdei.h" #include "hw/acpi/acpi-defs.h" #include "hw/acpi/acpi.h" #include "hw/nvram/fw_cfg.h" @@ -475,6 +476,26 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) "IORT", table_data->len - iort_start, 0, NULL, NULL); } +/* + * ACPI spec 6.2 Software Delegated Exception Interface (SDEI). + * (Revision 1.0) + * "SDEI" was reserved in ACPI 6.2. See "Links to ACPI-Related Documents" + * (http://uefi.org/acpi) under the heading "Software + * Delegated Exceptions Interface." The definition is under + * "10 Appendix C: ACPI table definitions for SDEI" in the linked document. + * + * This is a dummy table to expose platform SDEI capbility to OS. + */ +static void +build_sdei(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) +{ +int sdei_start = table_data->len; + +(void)acpi_data_push(table_data, sizeof(AcpiTableHeader)); +build_header(linker, table_data, (void *)(table_data->data + sdei_start), + "SDEI", table_data->len - sdei_start, 1, NULL, NULL); +} + static void build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) { @@ -825,6 +846,11 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) acpi_add_table(table_offsets, tables_blob); build_spcr(tables_blob, tables->linker, vms); +if (sdei_enabled) { +acpi_add_table(table_offsets, tables_blob); +build_sdei(tables_blob, tables->linker, vms); +} + if (ms->numa_state->num_nodes > 0) { acpi_add_table(table_offsets, tables_blob); build_srat(tables_blob, tables->linker, vms); -- 2.19.1
[RFC v2 07/14] arm/sdei: add support to trigger event by GIC interrupt ID
Add an external interface to trigger an SDEI event bound to an interrupt by providing GIC interrupt ID. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 37 + target/arm/sdei.h | 7 +++ 2 files changed, 44 insertions(+) diff --git a/target/arm/sdei.c b/target/arm/sdei.c index 6af4a9044b..713ac97775 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -476,6 +476,28 @@ static int64_t sdei_version(QemuSDEState *s, CPUState *cs, struct kvm_run *run) (0ULL << SDEI_VERSION_MINOR_SHIFT); } +static bool inject_event(QemuSDEState *s, CPUState *cs, int32_t event, int irq) +{ +QemuSDE *sde; + +if (event < 0) { +return false; +} +sde = get_sde_no_check(s, event, cs); +if (sde->event_id == SDEI_INVALID_EVENT_ID) { +put_sde(sde, cs); +return false; +} +if (irq > 0 && sde->prop->interrupt != irq) { +/* Someone unbinds the interrupt! */ +put_sde(sde, cs); +return false; +} +sde->pending = true; +dispatch_single(s, sde, cs); +return true; +} + static int64_t unregister_single_sde(QemuSDEState *s, int32_t event, CPUState *cs, bool force) { @@ -1104,6 +1126,21 @@ void sdei_handle_request(CPUState *cs, struct kvm_run *run) } } +bool trigger_sdei_by_irq(int cpu, int irq) +{ +QemuSDEState *s = sde_state; + +if (!s || irq >= ARRAY_SIZE(s->irq_map)) { +return false; +} + +if (s->irq_map[irq] == SDEI_INVALID_EVENT_ID) { +return false; +} + +return inject_event(s, qemu_get_cpu(cpu), s->irq_map[irq], irq); +} + static void sde_array_init(QemuSDE **array, int count) { int i; diff --git a/target/arm/sdei.h b/target/arm/sdei.h index 828f70bbf1..5ecc32d667 100644 --- a/target/arm/sdei.h +++ b/target/arm/sdei.h @@ -31,4 +31,11 @@ void sdei_handle_request(CPUState *cs, struct kvm_run *run); +/* + * Trigger an SDEI event bound to an interrupt. + * Return true if event has been triggered successfully. + * Return false if event has not been triggered for some reason. + */ +bool trigger_sdei_by_irq(int cpu, int irq); + #endif -- 2.19.1
[RFC v2 04/14] arm: add CONFIG_SDEI build flag
Integrate SDEI support for arm/aarch64 targets by default, if KVM is enabled. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- default-configs/arm-softmmu.mak | 1 + hw/arm/Kconfig | 4 target/arm/Makefile.objs| 2 ++ 3 files changed, 7 insertions(+) diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index 1f2e0e7fde..fc1f2b2ead 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -40,3 +40,4 @@ CONFIG_FSL_IMX25=y CONFIG_FSL_IMX7=y CONFIG_FSL_IMX6UL=y CONFIG_SEMIHOSTING=y +CONFIG_SDEI=y diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index c6e7782580..472bc3a75b 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -469,3 +469,7 @@ config ARMSSE_CPUID config ARMSSE_MHU bool + +config SDEI +bool +depends on KVM diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs index cf26c16f5f..72e01d08dc 100644 --- a/target/arm/Makefile.objs +++ b/target/arm/Makefile.objs @@ -11,6 +11,8 @@ obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o +obj-$(CONFIG_SDEI) += sdei.o + DECODETREE = $(SRC_PATH)/scripts/decodetree.py target/arm/decode-sve.inc.c: $(SRC_PATH)/target/arm/sve.decode $(DECODETREE) -- 2.19.1
[RFC PATCH 03/12] arm/sdei: add support to handle SDEI requests from guest
Add support for all interfaces defined in ARM SDEI 1.0 spec. http://infocenter.arm.com/help/topic/com.arm.doc.den0054a/ARM_DEN0054A_Software_Delegated_Exception_Interface.pdf The exit reason KVM_EXIT_HYPERCALL is used to indicate it is an HVC/SMC forward, and the structure kvm_run->hypercall is used to pass arguments and return values between KVM and qemu: Input: nr: the immediate value of SMC/HVC calls; not really used today. args[6]: x0..x5 (This is not fully conform with SMCCC which requires x6 as argument as well, but we can use GET_ONE_REG ioctl for such rare case). Return: args[0..3]: x0..x3 as defined in SMCCC. We rely on KVM to extract args[0..3] and write them to x0..x3 when hypercall exit returns. Signed-off-by: Heyi Guo Signed-off-by: Jingyi Wang Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 911 ++ target/arm/sdei.h | 34 ++ 2 files changed, 945 insertions(+) create mode 100644 target/arm/sdei.h diff --git a/target/arm/sdei.c b/target/arm/sdei.c index 7f12d69..b40fa36 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -29,6 +29,7 @@ #include "sysemu/sysemu.h" #include "sysemu/reset.h" #include "qemu/error-report.h" +#include "sdei.h" #include "sdei_int.h" #include "internals.h" #include "hw/boards.h" @@ -84,6 +85,12 @@ static void qemu_sde_cpu_init(QemuSDEState *s) } } +static inline QemuSDECpu *get_sde_cpu(QemuSDEState *s, CPUState *cs) +{ +assert(cs->cpu_index < s->sdei_max_cpus); +return >sde_cpus[cs->cpu_index]; +} + static bool is_valid_event_number(int32_t event) { int32_t slot_id; @@ -122,6 +129,910 @@ static QemuSDEProp *get_sde_prop_no_lock(QemuSDEState *s, int32_t event) return >sde_props_state[SDEI_EVENT_TO_SLOT(event)]; } +static QemuSDEProp *get_sde_prop(QemuSDEState *s, int32_t event) +{ +QemuSDEProp *sde_props = s->sde_props_state; + +if (!is_valid_event_number(event)) { +return NULL; +} + +event = SDEI_EVENT_TO_SLOT(event); + +qemu_mutex_lock(_props[event].lock); +if (sde_props[event].event_id < 0) { +qemu_mutex_unlock(_props[event].lock); +return NULL; +} +return _props[event]; +} + +static void put_sde_prop(QemuSDEProp *prop) +{ +qemu_mutex_unlock(>lock); +} + +static void sde_slot_lock(QemuSDE *sde, CPUState *cs) +{ +qemu_mutex_lock(>lock); +} + +static void sde_slot_unlock(QemuSDE *sde, CPUState *cs) +{ +qemu_mutex_unlock(>lock); +} + +/* + * It will always return a pointer to a preallocated sde; event number must be + * validated before calling this function. + */ +static QemuSDE *get_sde_no_check(QemuSDEState *s, int32_t event, CPUState *cs) +{ +QemuSDE **array = s->sde_cpus[cs->cpu_index].private_sde_array; +int32_t sde_index = SDEI_EVENT_TO_SLOT(event); +QemuSDE *sde; + +if (SDEI_IS_SHARED_EVENT(event)) { +array = s->shared_sde_array; +sde_index -= PRIVATE_SLOT_COUNT; +} + +sde = array[sde_index]; +sde_slot_lock(sde, cs); +return sde; +} + +static void put_sde(QemuSDE *sde, CPUState *cs) +{ +sde_slot_unlock(sde, cs); +} + +static inline bool is_sde_nested(QemuSDECpu *sde_cpu) +{ +return sde_cpu->critical_running_event >= 0 && + sde_cpu->normal_running_event >= 0; +} + +static int32_t get_running_sde(QemuSDEState *s, CPUState *cs) +{ +QemuSDECpu *sde_cpu = get_sde_cpu(s, cs); + +if (sde_cpu->critical_running_event >= 0) { +return sde_cpu->critical_running_event; +} +return sde_cpu->normal_running_event; +} + +static void override_return_value(CPUState *cs, uint64_t *args) +{ +CPUARMState *env = _CPU(cs)->env; +int i; + +for (i = 0; i < 4; i++) { +args[i] = env->xregs[i]; +} +} + +static void sde_save_cpu_ctx(CPUState *cs, QemuSDECpu *sde_cpu, bool critical) +{ +CPUARMState *env = _CPU(cs)->env; +QemuSDECpuCtx *ctx = _cpu->ctx[critical ? 1 : 0]; + +memcpy(ctx->xregs, env->xregs, sizeof(ctx->xregs)); +ctx->pc = env->pc; +ctx->pstate = pstate_read(env); +} + +static void sde_restore_cpu_ctx(QemuSDEState *s, CPUState *cs, bool critical) +{ +CPUARMState *env = _CPU(cs)->env; +QemuSDECpu *sde_cpu = get_sde_cpu(s, cs); +QemuSDECpuCtx *ctx = _cpu->ctx[critical ? 1 : 0]; + +/* + * TODO: we need to optimize to only restore affected registers by calling + * ioctl individialy + */ +kvm_arch_get_registers(cs); + +env->aarch64 = ((ctx->pstate & PSTATE_nRW) == 0); +memcpy(env->xregs, ctx->xregs, sizeof(ctx->xregs)); +env->pc = ctx->pc; +pstate_write(env, ctx->pstate); +aarch64_restore_
[RFC PATCH 08/12] arm/sdei: add support to register interrupt bind notifier
Other qemu modules related with the interrupt bind operation may want to be notified when guest requests to bind interrupt to sdei event, so we add register and unregister interfaces. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 49 + target/arm/sdei.h | 17 + 2 files changed, 66 insertions(+) diff --git a/target/arm/sdei.c b/target/arm/sdei.c index 9ceb131..efdb681 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -45,6 +45,52 @@ static QemuSDEState *sde_state; +typedef struct QemuSDEIBindNotifyEntry { +QTAILQ_ENTRY(QemuSDEIBindNotifyEntry) entry; +QemuSDEIBindNotify *func; +void *opaque; +int irq; +} QemuSDEIBindNotifyEntry; + +static QTAILQ_HEAD(, QemuSDEIBindNotifyEntry) bind_notifiers = +QTAILQ_HEAD_INITIALIZER(bind_notifiers); + +void qemu_register_sdei_bind_notifier(QemuSDEIBindNotify *func, + void *opaque, int irq) +{ +QemuSDEIBindNotifyEntry *be = g_new0(QemuSDEIBindNotifyEntry, 1); + +be->func = func; +be->opaque = opaque; +be->irq = irq; +QTAILQ_INSERT_TAIL(_notifiers, be, entry); +} + +void qemu_unregister_sdei_bind_notifier(QemuSDEIBindNotify *func, +void *opaque, int irq) +{ +QemuSDEIBindNotifyEntry *be; + +QTAILQ_FOREACH(be, _notifiers, entry) { +if (be->func == func && be->opaque == opaque && be->irq == irq) { +QTAILQ_REMOVE(_notifiers, be, entry); +g_free(be); +return; +} +} +} + +static void sdei_notify_bind(int irq, int32_t event, bool bind) +{ +QemuSDEIBindNotifyEntry *be, *nbe; + +QTAILQ_FOREACH_SAFE(be, _notifiers, entry, nbe) { +if (be->irq == irq) { +be->func(be->opaque, irq, event, bind); +} +} +} + static void qemu_sde_prop_init(QemuSDEState *s) { QemuSDEProp *sde_props = s->sde_props_state; @@ -502,6 +548,7 @@ static int32_t sdei_alloc_event_num(QemuSDEState *s, bool is_critical, sde_props[index].interrupt = intid; sde_props[index].is_shared = is_shared; sde_props[index].is_critical = is_critical; +sdei_notify_bind(intid, event, true); override_qemu_irq(s, event, intid); s->irq_map[intid] = event; qemu_mutex_unlock(_props[index].lock); @@ -522,6 +569,7 @@ static int32_t sdei_free_event_num_locked(QemuSDEState *s, QemuSDEProp *prop) goto unlock_return; } +sdei_notify_bind(prop->interrupt, prop->event_id, false); restore_qemu_irq(s, prop->event_id, prop->interrupt); s->irq_map[prop->interrupt] = SDEI_INVALID_EVENT_ID; prop->event_id = SDEI_INVALID_EVENT_ID; @@ -1331,6 +1379,7 @@ static int qemu_sdei_post_load(void *opaque, int version_id) int intid = sde_props[i].interrupt; if (intid != SDEI_INVALID_INTERRUPT) { s->irq_map[intid] = sde_props[i].event_id; +sdei_notify_bind(intid, sde_props[i].event_id, true); override_qemu_irq(s, sde_props[i].event_id, intid); } } diff --git a/target/arm/sdei.h b/target/arm/sdei.h index a61e788..feaaf1a 100644 --- a/target/arm/sdei.h +++ b/target/arm/sdei.h @@ -38,4 +38,21 @@ void sdei_handle_request(CPUState *cs, struct kvm_run *run); */ bool trigger_sdei_by_irq(int cpu, int irq); +/* + * Notify callback prototype; the argument "bind" tells whether it is a bind + * operation or unbind one. + */ +typedef void QemuSDEIBindNotify(void *opaque, int irq, +int32_t event, bool bind); +/* + * Register a notify callback for a specific interrupt bind operation; the + * client be both notified by bind and unbind operation. + */ +void qemu_register_sdei_bind_notifier(QemuSDEIBindNotify *func, + void *opaque, int irq); +/* + * Unregister a notify callback for a specific interrupt bind operation. + */ +void qemu_unregister_sdei_bind_notifier(QemuSDEIBindNotify *func, +void *opaque, int irq); #endif -- 1.8.3.1
[RFC PATCH 07/12] arm/sdei: override qemu_irq handler when binding interrupt
Override qemu_irq handler to support trigger SDEI event transparently after guest binds interrupt to SDEI event. We don't have good way to get GIC device and to guarantee SDEI device is initialized after GIC, so we search GIC in system bus when the first SDEI request happens or in VMSTATE post_load(). Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 137 -- target/arm/sdei_int.h | 3 ++ 2 files changed, 137 insertions(+), 3 deletions(-) diff --git a/target/arm/sdei.c b/target/arm/sdei.c index 088ed76..9ceb131 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -85,6 +85,24 @@ static void qemu_sde_cpu_init(QemuSDEState *s) } } +static int gic_int_to_irq(int num_irq, int intid, int cpu) +{ +if (intid >= GIC_INTERNAL) { +return intid - GIC_INTERNAL; +} +return num_irq - GIC_INTERNAL + cpu * GIC_INTERNAL + intid; +} + +static int irq_to_gic_int(int num_irq, int irq, int *cpu) +{ +if (irq < num_irq - GIC_INTERNAL) { +return irq + GIC_INTERNAL; +} +irq -= num_irq - GIC_INTERNAL; +*cpu = irq / GIC_INTERNAL; +return irq % GIC_INTERNAL; +} + static inline QemuSDECpu *get_sde_cpu(QemuSDEState *s, CPUState *cs) { assert(cs->cpu_index < s->sdei_max_cpus); @@ -381,6 +399,76 @@ static void dispatch_cpu(QemuSDEState *s, CPUState *cs, bool is_critical) } } +static void qemu_sdei_irq_handler(void *opaque, int irq, int level) +{ +int cpu = 0; +irq = irq_to_gic_int(sde_state->num_irq, irq, ); +trigger_sdei_by_irq(cpu, irq); +} + +static void override_qemu_irq(QemuSDEState *s, int32_t event, uint32_t intid) +{ +qemu_irq irq; +QemuSDE *sde; +CPUState *cs; +int cpu; + +/* SPI */ +if (intid >= GIC_INTERNAL) { +cs = arm_get_cpu_by_id(0); +irq = qdev_get_gpio_in(s->gic_dev, + gic_int_to_irq(s->num_irq, intid, 0)); +if (irq) { +qemu_irq_intercept_in(, qemu_sdei_irq_handler, 1); +} +sde = get_sde_no_check(s, event, cs); +sde->irq = irq; +put_sde(sde, cs); +return; +} +/* PPI */ +for (cpu = 0; cpu < s->sdei_max_cpus; cpu++) { +cs = arm_get_cpu_by_id(cpu); +irq = qdev_get_gpio_in(s->gic_dev, + gic_int_to_irq(s->num_irq, intid, cpu)); +if (irq) { +qemu_irq_intercept_in(, qemu_sdei_irq_handler, 1); +} +sde = get_sde_no_check(s, event, cs); +sde->irq = irq; +put_sde(sde, cs); +} +} + +static void restore_qemu_irq(QemuSDEState *s, int32_t event, uint32_t intid) +{ +QemuSDE *sde; +CPUState *cs; +int cpu; + +/* SPI */ +if (intid >= GIC_INTERNAL) { +cs = arm_get_cpu_by_id(0); +sde = get_sde_no_check(s, event, cs); +if (sde->irq) { +qemu_irq_remove_intercept(>irq, 1); +sde->irq = NULL; +} +put_sde(sde, cs); +return; +} +/* PPI */ +for (cpu = 0; cpu < s->sdei_max_cpus; cpu++) { +cs = arm_get_cpu_by_id(cpu); +sde = get_sde_no_check(s, event, cs); +if (sde->irq) { +qemu_irq_remove_intercept(>irq, 1); +sde->irq = NULL; +} +put_sde(sde, cs); +} +} + static int32_t sdei_alloc_event_num(QemuSDEState *s, bool is_critical, bool is_shared, int intid) { @@ -414,6 +502,7 @@ static int32_t sdei_alloc_event_num(QemuSDEState *s, bool is_critical, sde_props[index].interrupt = intid; sde_props[index].is_shared = is_shared; sde_props[index].is_critical = is_critical; +override_qemu_irq(s, event, intid); s->irq_map[intid] = event; qemu_mutex_unlock(_props[index].lock); qemu_mutex_unlock(>sdei_interrupt_bind_lock); @@ -433,6 +522,7 @@ static int32_t sdei_free_event_num_locked(QemuSDEState *s, QemuSDEProp *prop) goto unlock_return; } +restore_qemu_irq(s, prop->event_id, prop->interrupt); s->irq_map[prop->interrupt] = SDEI_INVALID_EVENT_ID; prop->event_id = SDEI_INVALID_EVENT_ID; prop->interrupt = SDEI_INVALID_INTERRUPT; @@ -929,13 +1019,33 @@ static int64_t sdei_event_pe_unmask(QemuSDEState *s, CPUState *cs, return SDEI_SUCCESS; } +static int dev_walkerfn(DeviceState *dev, void *opaque) +{ +QemuSDEState *s = opaque; + +if (object_dynamic_cast(OBJECT(dev), TYPE_ARM_GICV3_COMMON)) { +GICv3State *gic = ARM_GICV3_COMMON(dev); +s->num_irq = gic->num_irq; +s->gic_dev = dev; +return -1; +} + +if (object_dynamic_cast(OBJECT(dev), TYPE_ARM_GIC_COMMON)) { +GICState *gic = ARM_GIC_COMMON(dev)
[RFC PATCH 04/12] arm/sdei: add system reset callback
For this is a logical device which is not attached to system bus, we cannot use DeviceClass->reset interface directly. Instead we register our own reset callback to reset SDEI services when system resets. Signed-off-by: Heyi Guo Signed-off-by: Jingyi Wang Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 21 + 1 file changed, 21 insertions(+) diff --git a/target/arm/sdei.c b/target/arm/sdei.c index b40fa36..f9a1208 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -1083,6 +1083,26 @@ static void qemu_sde_init(QemuSDEState *s) qemu_private_sde_init(s); } +static void qemu_sde_reset(void *opaque) +{ +int64_t ret; +CPUState*cs; +QemuSDEState*s = opaque; + +CPU_FOREACH(cs) { +QemuSDECpu *sde_cpu = get_sde_cpu(s, cs); +sdei_private_reset_common(s, cs, true); +sde_cpu->masked = true; +sde_cpu->critical_running_event = SDEI_INVALID_EVENT_ID; +sde_cpu->normal_running_event = SDEI_INVALID_EVENT_ID; +} + +ret = sdei_shared_reset_common(s, first_cpu, true); +if (ret) { +error_report("SDEI system reset failed: 0x%lx", ret); +} +} + static int qemu_sdei_pre_save(void *opaque) { QemuSDEState *s = opaque; @@ -1235,6 +1255,7 @@ static void sdei_initfn(Object *obj) sde_state = s; qemu_sde_init(s); +qemu_register_reset(qemu_sde_reset, s); } static void qemu_sde_class_init(ObjectClass *klass, void *data) -- 1.8.3.1
[RFC PATCH 12/12] virt/acpi: add SDEI table if SDEI is enabled
Add SDEI table if SDEI is enabled, so that guest OS can get aware and utilize the interfaces. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse Cc: Shannon Zhao Cc: "Michael S. Tsirkin" Cc: Igor Mammedov --- hw/arm/virt-acpi-build.c| 16 include/hw/acpi/acpi-defs.h | 5 + 2 files changed, 21 insertions(+) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 6cdf156..1088214 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -32,6 +32,7 @@ #include "trace.h" #include "hw/core/cpu.h" #include "target/arm/cpu.h" +#include "target/arm/sdei.h" #include "hw/acpi/acpi-defs.h" #include "hw/acpi/acpi.h" #include "hw/nvram/fw_cfg.h" @@ -475,6 +476,16 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) } static void +build_sdei(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) +{ +int sdei_start = table_data->len; + +(void)acpi_data_push(table_data, sizeof(AcpiSdei)); +build_header(linker, table_data, (void *)(table_data->data + sdei_start), + "SDEI", table_data->len - sdei_start, 1, NULL, NULL); +} + +static void build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) { AcpiSerialPortConsoleRedirection *spcr; @@ -796,6 +807,11 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) acpi_add_table(table_offsets, tables_blob); build_spcr(tables_blob, tables->linker, vms); +if (sdei_enabled) { +acpi_add_table(table_offsets, tables_blob); +build_sdei(tables_blob, tables->linker, vms); +} + if (ms->numa_state->num_nodes > 0) { acpi_add_table(table_offsets, tables_blob); build_srat(tables_blob, tables->linker, vms); diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index 57a3f58..0a2265d 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -634,4 +634,9 @@ struct AcpiIortRC { } QEMU_PACKED; typedef struct AcpiIortRC AcpiIortRC; +struct AcpiSdei { +ACPI_TABLE_HEADER_DEF /* ACPI common table header */ +} QEMU_PACKED; +typedef struct AcpiSdei AcpiSdei; + #endif -- 1.8.3.1
[RFC PATCH 06/12] core/irq: add qemu_irq_remove_intercept interface
We use qemu_irq as the bridge for other qemu modules to switch from irq injection to SDEI event trigger after VM binds the interrupt to SDEI event. We use qemu_irq_intercept_in() to override qemu_irq handler with SDEI event trigger, so we also need a corresponding interface to restore the handler to default one (i.e. ARM GIC). qemu_irq_remove_intercept() is the new interface to do the above job. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- hw/core/irq.c| 11 +++ include/hw/irq.h | 8 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/hw/core/irq.c b/hw/core/irq.c index 7cc0295..114bce6 100644 --- a/hw/core/irq.c +++ b/hw/core/irq.c @@ -145,6 +145,17 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n) } } +void qemu_irq_remove_intercept(qemu_irq *gpio_in, int n) +{ +int i; +qemu_irq *old_irqs = gpio_in[0]->opaque; +for (i = 0; i < n; i++) { +gpio_in[i]->handler = old_irqs[i]->handler; +gpio_in[i]->opaque = old_irqs[i]->opaque; +} +qemu_free_irqs(old_irqs, n); +} + static const TypeInfo irq_type_info = { .name = TYPE_IRQ, .parent = TYPE_OBJECT, diff --git a/include/hw/irq.h b/include/hw/irq.h index fe527f6..1af1db9 100644 --- a/include/hw/irq.h +++ b/include/hw/irq.h @@ -56,8 +56,12 @@ qemu_irq qemu_irq_split(qemu_irq irq1, qemu_irq irq2); */ qemu_irq *qemu_irq_proxy(qemu_irq **target, int n); -/* For internal use in qtest. Similar to qemu_irq_split, but operating - on an existing vector of qemu_irq. */ +/* + * Similar to qemu_irq_split, but operating on an existing vector of qemu_irq. + */ void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n); +/* Restore the irq handler intercepted by qemu_irq_intercept_in() */ +void qemu_irq_remove_intercept(qemu_irq *gpio_in, int n); + #endif -- 1.8.3.1
[RFC PATCH 00/12] Add SDEI support for arm64
As promised, this is the first RFC patch set for arm64 SDEI support. Key points: - We propose to only support kvm enabled arm64 virtual machines, for non-kvm VMs can emulate EL3 and have Trusted Firmware run on it, which has a builtin SDEI dispatcher. - New kvm capability KVM_CAP_FORWARD_HYPERCALL is added to probe if kvm supports forwarding hypercalls, and the capability should be enabled explicitly. PSCI can be set as exception for backward compatibility. - We make the dispatcher as a logical device, to save the states during migration or save/restore operation; only one instance is allowed in one VM. - We use qemu_irq as the bridge for other qemu modules to switch from irq injection to SDEI event trigger after VM binds the interrupt to SDEI event. We use qemu_irq_intercept_in() to override qemu_irq handler with SDEI event trigger, and a new interface qemu_irq_remove_intercept() is added to restore the handler to default one (i.e. ARM GIC). More details are in the commit message of each patch. Basic tests are done by emulating a watchdog timer and triggering SDEI event in every 10s. As this is the first rough RFC, please focus on the interfaces and framework first. We can refine the code for several rounds after the big things have been determined. Please give your comments and suggestions. Thanks, HG Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse Heyi Guo (12): linux-headers: import arm_sdei.h arm/sdei: add virtual device framework arm/sdei: add support to handle SDEI requests from guest arm/sdei: add system reset callback arm/sdei: add support to trigger event by GIC interrupt ID core/irq: add qemu_irq_remove_intercept interface arm/sdei: override qemu_irq handler when binding interrupt arm/sdei: add support to register interrupt bind notifier linux-headers/kvm.h: add capability to forward hypercall arm/sdei: check KVM cap and enable SDEI arm/kvm: handle guest exit of hypercall virt/acpi: add SDEI table if SDEI is enabled hw/arm/virt-acpi-build.c | 16 + hw/core/irq.c | 11 + include/hw/acpi/acpi-defs.h|5 + include/hw/irq.h |8 +- linux-headers/linux/arm_sdei.h | 73 ++ linux-headers/linux/kvm.h |3 + target/arm/Makefile.objs |1 + target/arm/kvm.c | 17 + target/arm/sdei.c | 1518 target/arm/sdei.h | 60 ++ target/arm/sdei_int.h | 109 +++ 11 files changed, 1819 insertions(+), 2 deletions(-) create mode 100644 linux-headers/linux/arm_sdei.h create mode 100644 target/arm/sdei.c create mode 100644 target/arm/sdei.h create mode 100644 target/arm/sdei_int.h -- 1.8.3.1
[RFC PATCH 02/12] arm/sdei: add virtual device framework
SDEI is useful to emulate NMI on arm64 platforms. To support SDEI in virtual machine with KVM enabled, we choose to implement SDEI interfaces in qemu. It is targeted for KVM mode only, for the full user space emulation can also emulate secure world and have ARM Trusted Firmware to run on emulated EL3. - We create a logical SDEI device to hold the states of SDEI services, to support VM migration. - Only one SDEI virtual device is allowed in the whole VM to provide SDEI services. - We create struct QemuSDE to hold states of each SDEI event, and private events with the same ID on different CPUs have their own QemuSDE instance. - We create struct QemuSDEProp to hold properties of each SDEI event, so all private instances with the same ID will pointed to the same QemuSDEProp. - We create struct QemuSDECpu to hold CPU/PE states, including the interrupted CPU context. - Slot numbers for private and shared event are fixed, for guests cannot request more interrupt binds than BIND_SLOTS in SDEI_FEATURES call. - The first PRIVATE_SLOT_COUNT slots in property array are for private events, and the next SHARED_SLOT_COUNT slots are for shared events. - We use property slot index as lower bit for each allocated event number, so that we can get property easily from valid input event number, as well as the QemuSDE instance. Signed-off-by: Heyi Guo Signed-off-by: Jingyi Wang Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/Makefile.objs | 1 + target/arm/sdei.c| 351 +++ target/arm/sdei_int.h| 106 ++ 3 files changed, 458 insertions(+) create mode 100644 target/arm/sdei.c create mode 100644 target/arm/sdei_int.h diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs index cf26c16..674109d 100644 --- a/target/arm/Makefile.objs +++ b/target/arm/Makefile.objs @@ -7,6 +7,7 @@ obj-$(CONFIG_SOFTMMU) += machine.o arch_dump.o monitor.o obj-$(CONFIG_SOFTMMU) += arm-powerctl.o obj-$(CONFIG_KVM) += kvm.o +obj-$(CONFIG_KVM) += sdei.o obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o diff --git a/target/arm/sdei.c b/target/arm/sdei.c new file mode 100644 index 000..7f12d69 --- /dev/null +++ b/target/arm/sdei.c @@ -0,0 +1,351 @@ +/* + * ARM SDEI emulation for ARM64 virtual machine with KVM + * + * Copyright (c) 2019 HUAWEI TECHNOLOGIES CO., LTD. + * + * Authors: + *Heyi Guo + *Jingyi Wang + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2 or later, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "arm-powerctl.h" +#include "qemu/timer.h" +#include "sysemu/kvm.h" +#include "sysemu/kvm_int.h" +#include "sysemu/sysemu.h" +#include "sysemu/reset.h" +#include "qemu/error-report.h" +#include "sdei_int.h" +#include "internals.h" +#include "hw/boards.h" +#include "hw/intc/arm_gicv3.h" +#include "hw/intc/arm_gic.h" +#include "hw/irq.h" +#include "hw/sysbus.h" +#include "migration/vmstate.h" +#include "qom/object.h" + +#define TYPE_QEMU_SDEI "qemu_sdei" +#define QEMU_SDEI(obj) OBJECT_CHECK(QemuSDEState, (obj), TYPE_QEMU_SDEI) + +static QemuSDEState *sde_state; + +static void qemu_sde_prop_init(QemuSDEState *s) +{ +QemuSDEProp *sde_props = s->sde_props_state; +int i; +for (i = 0; i < ARRAY_SIZE(s->sde_props_state); i++) { +sde_props[i].event_id = SDEI_INVALID_EVENT_ID; +sde_props[i].interrupt = SDEI_INVALID_INTERRUPT; +sde_props[i].sde_index = i >= PRIVATE_SLOT_COUNT ? + i - PRIVATE_SLOT_COUNT : i; + +qemu_mutex_init(&(sde_props[i].lock)); +sde_props[i].refcount = 0; +} +sde_props[0].event_id = SDEI_STD_EVT_SOFTWARE_SIGNAL; +sde_props[0].interrupt = SDEI_INVALID_INTERRUPT; +sde_props[0].is_shared = false; +sde_props[0].is_critical = false; + +for (i = 0; i < ARRAY_SIZE(s->irq_map); i++) { +s->irq_map[i] = SDEI_INVALID_EVENT_ID; +} + +qemu_mutex_init(>sdei_interrupt_bind_lock); +} + +static void qemu_sde_cpu_init(QemuSDEState *s) +
[RFC PATCH 11/12] arm/kvm: handle guest exit of hypercall
Add support to handle guest exit of hypercall, and forward to SDEI dispatcher if SDEI is enabled and it is an SDEI request. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/kvm.c | 17 + 1 file changed, 17 insertions(+) diff --git a/target/arm/kvm.c b/target/arm/kvm.c index b2eaa50..97a67b1 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -30,6 +30,7 @@ #include "hw/boards.h" #include "hw/irq.h" #include "qemu/log.h" +#include "sdei.h" const KVMCapabilityInfo kvm_arch_required_capabilities[] = { KVM_CAP_LAST_INFO @@ -668,6 +669,19 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run) } +static void kvm_arm_handle_hypercall(CPUState *cs, struct kvm_run *run) +{ +uint32_t func_id = run->hypercall.args[0]; + +if (sdei_enabled && +func_id >= SDEI_1_0_FN_BASE && func_id <= SDEI_MAX_REQ) { +sdei_handle_request(cs, run); +} else { +run->hypercall.args[0] = -1; +} +} + + int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) { int ret = 0; @@ -678,6 +692,9 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) ret = EXCP_DEBUG; } /* otherwise return to guest */ break; +case KVM_EXIT_HYPERCALL: +kvm_arm_handle_hypercall(cs, run); +break; default: qemu_log_mask(LOG_UNIMP, "%s: un-handled exit reason %d\n", __func__, run->exit_reason); -- 1.8.3.1
[RFC PATCH 05/12] arm/sdei: add support to trigger event by GIC interrupt ID
Add an external interface to trigger an SDEI event bound to an interrupt by providing GIC interrupt ID. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 38 ++ target/arm/sdei.h | 7 +++ 2 files changed, 45 insertions(+) diff --git a/target/arm/sdei.c b/target/arm/sdei.c index f9a1208..088ed76 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -453,6 +453,29 @@ static int64_t sdei_version(QemuSDEState *s, CPUState *cs, struct kvm_run *run) (0ULL << SDEI_VERSION_MINOR_SHIFT); } +static bool inject_event(QemuSDEState *s, CPUState *cs, + int32_t event, int irq) +{ +QemuSDE *sde; + +if (event < 0) { +return false; +} +sde = get_sde_no_check(s, event, cs); +if (sde->event_id == SDEI_INVALID_EVENT_ID) { +put_sde(sde, cs); +return false; +} +if (irq > 0 && sde->prop->interrupt != irq) { +/* Someone unbinds the interrupt! */ +put_sde(sde, cs); +return false; +} +sde->pending = true; +dispatch_single(s, sde, cs); +return true; +} + static int64_t unregister_single_sde(QemuSDEState *s, int32_t event, CPUState *cs, bool force) { @@ -1033,6 +1056,21 @@ void sdei_handle_request(CPUState *cs, struct kvm_run *run) } } +bool trigger_sdei_by_irq(int cpu, int irq) +{ +QemuSDEState *s = sde_state; + +if (!s || irq >= ARRAY_SIZE(s->irq_map)) { +return false; +} + +if (s->irq_map[irq] == SDEI_INVALID_EVENT_ID) { +return false; +} + +return inject_event(s, arm_get_cpu_by_id(cpu), s->irq_map[irq], irq); +} + static void qemu_shared_sde_init(QemuSDEState *s) { int i; diff --git a/target/arm/sdei.h b/target/arm/sdei.h index a69a0e4..a61e788 100644 --- a/target/arm/sdei.h +++ b/target/arm/sdei.h @@ -31,4 +31,11 @@ void sdei_handle_request(CPUState *cs, struct kvm_run *run); +/* + * Trigger an SDEI event bound to an interrupt. + * Return true if event has been triggered successfully. + * Return false if event has not been triggered for some reason. + */ +bool trigger_sdei_by_irq(int cpu, int irq); + #endif -- 1.8.3.1
[RFC PATCH 09/12] linux-headers/kvm.h: add capability to forward hypercall
To keep backward compatibility, we add new KVM capability "KVM_CAP_FORWARD_HYPERCALL" to probe whether KVM supports forwarding hypercall to userspace. The capability should be enabled explicitly, for we don't want user space application to deal with unexpected hypercall exits. We also use an additional argument to pass exception bit mask, to request KVM to forward all hypercalls except the classes specified in the bit mask. Currently only PSCI can be set as exception, so that we can still keep consistent with the original PSCI processing flow. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse Cc: "Michael S. Tsirkin" Cc: Cornelia Huck Cc: Paolo Bonzini --- linux-headers/linux/kvm.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h index 18892d6..20e8a68 100644 --- a/linux-headers/linux/kvm.h +++ b/linux-headers/linux/kvm.h @@ -711,6 +711,8 @@ struct kvm_enable_cap { __u8 pad[64]; }; +#define KVM_CAP_FORWARD_HYPERCALL_EXCL_PSCI (1 << 0) + /* for KVM_PPC_GET_PVINFO */ #define KVM_PPC_PVINFO_FLAGS_EV_IDLE (1<<0) @@ -995,6 +997,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_ARM_SVE 170 #define KVM_CAP_ARM_PTRAUTH_ADDRESS 171 #define KVM_CAP_ARM_PTRAUTH_GENERIC 172 +#define KVM_CAP_FORWARD_HYPERCALL 174 #ifdef KVM_CAP_IRQ_ROUTING -- 1.8.3.1
[RFC PATCH 10/12] arm/sdei: check KVM cap and enable SDEI
Check KVM hypercall forward capability and enable it, and set global flag "sdei_enabled" to true if everything works well. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse --- target/arm/sdei.c | 17 + target/arm/sdei.h | 2 ++ 2 files changed, 19 insertions(+) diff --git a/target/arm/sdei.c b/target/arm/sdei.c index efdb681..000545e 100644 --- a/target/arm/sdei.c +++ b/target/arm/sdei.c @@ -43,6 +43,7 @@ #define TYPE_QEMU_SDEI "qemu_sdei" #define QEMU_SDEI(obj) OBJECT_CHECK(QemuSDEState, (obj), TYPE_QEMU_SDEI) +bool sdei_enabled; static QemuSDEState *sde_state; typedef struct QemuSDEIBindNotifyEntry { @@ -1465,6 +1466,7 @@ static const VMStateDescription vmstate_sde_state = { static void sdei_initfn(Object *obj) { QemuSDEState *s = QEMU_SDEI(obj); +KVMState *kvm = KVM_STATE(current_machine->accelerator); if (sde_state) { error_report("Only one SDEI dispatcher is allowed!"); @@ -1474,6 +1476,21 @@ static void sdei_initfn(Object *obj) qemu_sde_init(s); qemu_register_reset(qemu_sde_reset, s); + +if (kvm_check_extension(kvm, KVM_CAP_FORWARD_HYPERCALL)) { +int ret; +ret = kvm_vm_enable_cap(kvm, KVM_CAP_FORWARD_HYPERCALL, 0, +KVM_CAP_FORWARD_HYPERCALL_EXCL_PSCI); +if (ret < 0) { +error_report("Enable hypercall forwarding failed: %s", + strerror(-ret)); +abort(); +} +sdei_enabled = true; +info_report("qemu sdei enabled"); +} else { +info_report("KVM does not support forwarding hypercall."); +} } static void qemu_sde_class_init(ObjectClass *klass, void *data) diff --git a/target/arm/sdei.h b/target/arm/sdei.h index feaaf1a..95e7d8d 100644 --- a/target/arm/sdei.h +++ b/target/arm/sdei.h @@ -29,6 +29,8 @@ #define SDEI_MAX_REQSDEI_1_0_FN(0x12) +extern bool sdei_enabled; + void sdei_handle_request(CPUState *cs, struct kvm_run *run); /* -- 1.8.3.1
[RFC PATCH 01/12] linux-headers: import arm_sdei.h
Import Linux header file include/uapi/linux/arm_sdei.h from kernel v5.3 release. This is to prepare for qemu SDEI emulation. Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse Cc: "Michael S. Tsirkin" Cc: Cornelia Huck Cc: Paolo Bonzini --- linux-headers/linux/arm_sdei.h | 73 ++ 1 file changed, 73 insertions(+) create mode 100644 linux-headers/linux/arm_sdei.h diff --git a/linux-headers/linux/arm_sdei.h b/linux-headers/linux/arm_sdei.h new file mode 100644 index 000..af0630b --- /dev/null +++ b/linux-headers/linux/arm_sdei.h @@ -0,0 +1,73 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* Copyright (C) 2017 Arm Ltd. */ +#ifndef _UAPI_LINUX_ARM_SDEI_H +#define _UAPI_LINUX_ARM_SDEI_H + +#define SDEI_1_0_FN_BASE 0xC420 +#define SDEI_1_0_MASK 0xFFE0 +#define SDEI_1_0_FN(n) (SDEI_1_0_FN_BASE + (n)) + +#define SDEI_1_0_FN_SDEI_VERSION SDEI_1_0_FN(0x00) +#define SDEI_1_0_FN_SDEI_EVENT_REGISTER SDEI_1_0_FN(0x01) +#define SDEI_1_0_FN_SDEI_EVENT_ENABLE SDEI_1_0_FN(0x02) +#define SDEI_1_0_FN_SDEI_EVENT_DISABLE SDEI_1_0_FN(0x03) +#define SDEI_1_0_FN_SDEI_EVENT_CONTEXT SDEI_1_0_FN(0x04) +#define SDEI_1_0_FN_SDEI_EVENT_COMPLETE SDEI_1_0_FN(0x05) +#define SDEI_1_0_FN_SDEI_EVENT_COMPLETE_AND_RESUME SDEI_1_0_FN(0x06) +#define SDEI_1_0_FN_SDEI_EVENT_UNREGISTER SDEI_1_0_FN(0x07) +#define SDEI_1_0_FN_SDEI_EVENT_STATUS SDEI_1_0_FN(0x08) +#define SDEI_1_0_FN_SDEI_EVENT_GET_INFO SDEI_1_0_FN(0x09) +#define SDEI_1_0_FN_SDEI_EVENT_ROUTING_SET SDEI_1_0_FN(0x0A) +#define SDEI_1_0_FN_SDEI_PE_MASK SDEI_1_0_FN(0x0B) +#define SDEI_1_0_FN_SDEI_PE_UNMASK SDEI_1_0_FN(0x0C) +#define SDEI_1_0_FN_SDEI_INTERRUPT_BIND SDEI_1_0_FN(0x0D) +#define SDEI_1_0_FN_SDEI_INTERRUPT_RELEASE SDEI_1_0_FN(0x0E) +#define SDEI_1_0_FN_SDEI_PRIVATE_RESET SDEI_1_0_FN(0x11) +#define SDEI_1_0_FN_SDEI_SHARED_RESET SDEI_1_0_FN(0x12) + +#define SDEI_VERSION_MAJOR_SHIFT 48 +#define SDEI_VERSION_MAJOR_MASK0x7fff +#define SDEI_VERSION_MINOR_SHIFT 32 +#define SDEI_VERSION_MINOR_MASK0x +#define SDEI_VERSION_VENDOR_SHIFT 0 +#define SDEI_VERSION_VENDOR_MASK 0x + +#define SDEI_VERSION_MAJOR(x) (x>>SDEI_VERSION_MAJOR_SHIFT & SDEI_VERSION_MAJOR_MASK) +#define SDEI_VERSION_MINOR(x) (x>>SDEI_VERSION_MINOR_SHIFT & SDEI_VERSION_MINOR_MASK) +#define SDEI_VERSION_VENDOR(x) (x>>SDEI_VERSION_VENDOR_SHIFT & SDEI_VERSION_VENDOR_MASK) + +/* SDEI return values */ +#define SDEI_SUCCESS 0 +#define SDEI_NOT_SUPPORTED -1 +#define SDEI_INVALID_PARAMETERS-2 +#define SDEI_DENIED-3 +#define SDEI_PENDING -5 +#define SDEI_OUT_OF_RESOURCE -10 + +/* EVENT_REGISTER flags */ +#define SDEI_EVENT_REGISTER_RM_ANY 0 +#define SDEI_EVENT_REGISTER_RM_PE 1 + +/* EVENT_STATUS return value bits */ +#define SDEI_EVENT_STATUS_RUNNING 2 +#define SDEI_EVENT_STATUS_ENABLED 1 +#define SDEI_EVENT_STATUS_REGISTERED 0 + +/* EVENT_COMPLETE status values */ +#define SDEI_EV_HANDLED0 +#define SDEI_EV_FAILED 1 + +/* GET_INFO values */ +#define SDEI_EVENT_INFO_EV_TYPE0 +#define SDEI_EVENT_INFO_EV_SIGNALED1 +#define SDEI_EVENT_INFO_EV_PRIORITY2 +#define SDEI_EVENT_INFO_EV_ROUTING_MODE3 +#define SDEI_EVENT_INFO_EV_ROUTING_AFF 4 + +/* and their results */ +#define SDEI_EVENT_TYPE_PRIVATE0 +#define SDEI_EVENT_TYPE_SHARED 1 +#define SDEI_EVENT_PRIORITY_NORMAL 0 +#define SDEI_EVENT_PRIORITY_CRITICAL 1 + +#endif /* _UAPI_LINUX_ARM_SDEI_H */ -- 1.8.3.1
Re: [Qemu-devel] Question: can we hot plug a PCIe switch on machine "virt"
Hi Eric and Michael, Appreciate your sharing of this information. But sorry I'm still a little confused due to my poor English... On 2019/4/11 20:19, Auger Eric wrote: Hi Heyi, On 4/11/19 1:30 PM, Heyi Guo wrote: Hi Eric, Could you help to confirm? Practically I have not tried anything else than hot-plugging the downstream port downstream to the upstream port. This looks not supported as documented. Do you mean you *have* tried hot-plugging the downstream port to the upstream port? Have you succeeded? Could you tell how to do that? Michael answered that up to him, the downstream port could only be plugged downto the upstream port. Is only code-plugging supported here, or hot-plugging is supported as well? Thanks, Heyi On my side I have not aware of any usage of the downstream port outside of the switch's scope. Thanks Eric Thanks, Heyi On 2019/4/7 9:59, Heyi Guo wrote: Hi Eric, My real interesting is about the hotplug of PCIe switch, which means we don't need to provide lots of PCIe root ports or PCIe down stream ports at the beginning, but we can extend the capacity by hot adding PCIe switches which can provide more hot-pluggable slots for endpoint devices. The document docs/pcie.txt says "PCI Express Downstream Ports can't be hot-plugged into an existing PCI Express Upstream Port" which confuses me. Does it actually mean Downstream Ports can't be hot-plugged? For they can't be hot-plugged into an existing Upstream Port as the doc says, either they can't be hot-plugged into an non-existing Upstream Port or another place... Thanks, Heyi On 2019/4/4 15:39, Auger Eric wrote: Hi Heyi, On 4/3/19 8:50 PM, Michael S. Tsirkin wrote: On Wed, Apr 03, 2019 at 03:32:09PM +0800, Heyi Guo wrote: Hi folks, In physical world, a PCIe switch including one upstream port and several downstream ports is a single physical device, however we treat each port as a device in qemu world. In qemu docs/pcie.txt, we have below statements: Line 230: Be aware that PCI Express Downstream Ports can't be hot-plugged into Line 231: an existing PCI Express Upstream Port. To my understanding, it implies PCIe downstream ports *can* be hot-plugged into something which is not an existing upstream port. If it is true, how can we do that? AFAIK monitor command device_add can only add one device at a time. Please help to show the truth. Thanks, Heyi afaik they can only be plugged into upstearm ports with or without hotplug. Hotplug on upstream port does not look supported, as mentionned in the doc: (QEMU) device_add driver=xio3130-downstream id=down0 bus=upstream_port1 {"error": {"class": "GenericError", "desc": "Bus 'upstream_port1' does not support hotplugging"}} Looks the std way to use the downstream port is the one documented in 2.2.3: 2.2.3 Plugging a PCI Express device into a Switch: -device ioh3420,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z] -device x3130-upstream,id=upstream_port1,bus=root_port1[,addr=x] -device xio3130-downstream,id=downstream_port1,bus=upstream_port1,chassis=x1,slot=y1[,addr=z1]] -device ,bus=downstream_port1 For my curiosity why do you want to hotplug a downstream port in another place than an upstream port? Thanks Eric . .
Re: [Qemu-devel] Question: can we hot plug a PCIe switch on machine "virt"
Hi Eric, Could you help to confirm? Thanks, Heyi On 2019/4/7 9:59, Heyi Guo wrote: Hi Eric, My real interesting is about the hotplug of PCIe switch, which means we don't need to provide lots of PCIe root ports or PCIe down stream ports at the beginning, but we can extend the capacity by hot adding PCIe switches which can provide more hot-pluggable slots for endpoint devices. The document docs/pcie.txt says "PCI Express Downstream Ports can't be hot-plugged into an existing PCI Express Upstream Port" which confuses me. Does it actually mean Downstream Ports can't be hot-plugged? For they can't be hot-plugged into an existing Upstream Port as the doc says, either they can't be hot-plugged into an non-existing Upstream Port or another place... Thanks, Heyi On 2019/4/4 15:39, Auger Eric wrote: Hi Heyi, On 4/3/19 8:50 PM, Michael S. Tsirkin wrote: On Wed, Apr 03, 2019 at 03:32:09PM +0800, Heyi Guo wrote: Hi folks, In physical world, a PCIe switch including one upstream port and several downstream ports is a single physical device, however we treat each port as a device in qemu world. In qemu docs/pcie.txt, we have below statements: Line 230: Be aware that PCI Express Downstream Ports can't be hot-plugged into Line 231: an existing PCI Express Upstream Port. To my understanding, it implies PCIe downstream ports *can* be hot-plugged into something which is not an existing upstream port. If it is true, how can we do that? AFAIK monitor command device_add can only add one device at a time. Please help to show the truth. Thanks, Heyi afaik they can only be plugged into upstearm ports with or without hotplug. Hotplug on upstream port does not look supported, as mentionned in the doc: (QEMU) device_add driver=xio3130-downstream id=down0 bus=upstream_port1 {"error": {"class": "GenericError", "desc": "Bus 'upstream_port1' does not support hotplugging"}} Looks the std way to use the downstream port is the one documented in 2.2.3: 2.2.3 Plugging a PCI Express device into a Switch: -device ioh3420,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z] -device x3130-upstream,id=upstream_port1,bus=root_port1[,addr=x] -device xio3130-downstream,id=downstream_port1,bus=upstream_port1,chassis=x1,slot=y1[,addr=z1]] -device ,bus=downstream_port1 For my curiosity why do you want to hotplug a downstream port in another place than an upstream port? Thanks Eric .
Re: [Qemu-devel] [RFC] arm/cpu: fix soft lockup panic after resuming from stop
Hi Steve, After reading kernel code about time keeping and something related, I've not got a clear picture of how we can use MSR_KVM_WALL_CLOCK_NEW to keep wall clock in guest VM. 1. On X86, MSR_KVM_WALL_CLOCK_NEW is only used by the callback of system suspend and resume; I didn't find it used for runtime wall clock reading. 2. To use the MSR for wall clock synchronization, shall we register KVM PV-clock as a higher rating clock source, so that it will be bound to tk_core.timekeeper and be read at each time of running update_wall_time() in each timer tick? 3. If the above is true, how can we keep the paravirtualized wall clock always updated? Is it always trapped to the hypervisor? I'm afraid this may cause performance loss. If there is no trap and the data is updated by the hypervisor periodically, how can we guarantee the accuracy? Meanwhile it seems easier to use KVM_KVMCLOCK_CTRL to get rid of false positive soft lock panic, and guest can rely on cntvct for wall clock updating as it does now, and it seems not difficult for the hypervisor to keep cntvct "always on" and "monotonic". Please let me know if I miss something. Thanks, Heyi On 2019/3/27 1:12, Steven Price wrote: Hi Heyi, On 26/03/2019 13:53, Heyi Guo wrote: I also tested save/restore operations, and observed that clock in guest would not jump after restoring either. If we consider guest clock not being synchronized with real wall clock as an issue, does it mean save/restore function has the same issue? Basically at the moment when the guest isn't running you have a choice of two behaviours: 1. Stop (i.e. save/restore) CNTVCT - this means that the guest sees no time occur. If the guest needs to have a concept of wall-clock time (e.g. it communicates with other systems over a network) then this can cause problems (e.g. timeouts might be wrong, certificates might start appearing to be in the future etc). 2. Leave CNTVCT running - the guest sees the time pass but interprets the vCPUs as effectively having locked up. Linux will trigger the soft lockup warning. There are two ways of solving this, which match the two behaviours above: 1. Provide the guest with a view of wall-clock time. The obvious way of doing this is with a pvclock implementation like MSR_KVM_WALL_CLOCK_NEW for x86. 2. Inform the guest to ignore the apparent "soft-lockup". There's already an ioctl for x86 for this: KVM_KVMCLOCK_CTRL My preference is for option 1 - as this gives the guest a good view of both the time that it is actually executing (useful for internal watchdog timers like the soft-lockup one in Linux) and maintains a view of wall-clock time (useful when communicating with other external services - e.g. the a server on the internet). Your patch to QEMU provides the first step of that, but as you mention there's much more to do. One thing I haven't investigated in great detail is how KVM handles the timer during various forms of suspend. In particular for suspend types like full hibernation the host's physical counter will jump (quite possibly backwards) - I haven't looked in detail how KVM presents this to the guest. Hopefully not by making it go backwards! I'm not sure how much time I'm going to have to look at this in the near future, but please keep me in the loop if you decide to tackle any of this. Thanks, Steve .
Re: [Qemu-devel] [RFC] arm/virt: add one more uart for UEFI runtime debug
On 2019/4/7 21:16, Peter Maydell wrote: On Sun, 7 Apr 2019 at 09:19, Heyi Guo wrote: This patch is based on the discussion in TianoCore edk2-devel mailing list: https://lists.01.org/pipermail/edk2-devel/2019-March/037986.html The conclusion is that we need an individual UART for UEFI runtime services to print debug message at runtime, to avoid conflicting with the system UART. We cannot use the secure UART either, for we may both have Trusted Firmware and UEFI runtime services running on the VM, and it is not easy to keep synchronization between the two components. I don't think it makes much sense to keep adding UARTs for every bit of the system software stack. We don't have an infinite supply of UARTs on real hardware either -- how is this handled there ? As you were in the edk2 mail thread, I supposed you also agreed to add one more UART. What did you mean by "I do still have a todo list item to add a 2nd UART"? How shall we do that? On our real hardware platforms, we don't have a separate serial port for UEFI runtime services; that's the reason for why I had wanted to have an adaptable approach for platform without one more serial port... Also adding new UARTs to this board is awkward because of the weird choices various bits of software have made in reading the dtb and picking a UART if there's more than one listed. @@ -713,13 +715,23 @@ static void create_uart(const VirtMachineState *vms, qemu_irq *pic, int uart, if (uart == VIRT_UART) { qemu_fdt_setprop_string(vms->fdt, "/chosen", "stdout-path", nodename); } else { +const char *status_string; + +if (uart == VIRT_SECURE_UART) { +status_string = "secure-status"; +} else { +status_string = "uefi-status"; Where does this status string come from? The "secure-status" one is documented in the device tree spec and has a well defined meaning. Sorry this came from my imagination, so I sent it as a "RFC" :) Thanks, Heyi thanks -- PMM .
[Qemu-devel] [RFC] arm/virt: add one more uart for UEFI runtime debug
This patch is based on the discussion in TianoCore edk2-devel mailing list: https://lists.01.org/pipermail/edk2-devel/2019-March/037986.html The conclusion is that we need an individual UART for UEFI runtime services to print debug message at runtime, to avoid conflicting with the system UART. We cannot use the secure UART either, for we may both have Trusted Firmware and UEFI runtime services running on the VM, and it is not easy to keep synchronization between the two components. To keep backward compatibility, UEFI UART is put behind the secure UART when secure world is enabled. Cc: Peter Maydell Cc: Laszlo Ersek Signed-off-by: Heyi Guo --- hw/arm/virt.c | 29 +++-- include/hw/arm/virt.h | 1 + 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index ce2664a..cbc5a66 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -131,6 +131,7 @@ static const MemMapEntry base_memmap[] = { [VIRT_GPIO] = { 0x0903, 0x1000 }, [VIRT_SECURE_UART] ={ 0x0904, 0x1000 }, [VIRT_SMMU] = { 0x0905, 0x0002 }, +[VIRT_UEFI_UART] = { 0x0907, 0x1000 }, [VIRT_MMIO] = { 0x0a00, 0x0200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ [VIRT_PLATFORM_BUS] = { 0x0c00, 0x0200 }, @@ -166,6 +167,7 @@ static const int a15irqmap[] = { [VIRT_PCIE] = 3, /* ... to 6 */ [VIRT_GPIO] = 7, [VIRT_SECURE_UART] = 8, +[VIRT_UEFI_UART] = 9, [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */ [VIRT_SMMU] = 74,/* ...to 74 + NUM_SMMU_IRQS - 1 */ @@ -713,13 +715,23 @@ static void create_uart(const VirtMachineState *vms, qemu_irq *pic, int uart, if (uart == VIRT_UART) { qemu_fdt_setprop_string(vms->fdt, "/chosen", "stdout-path", nodename); } else { +const char *status_string; + +if (uart == VIRT_SECURE_UART) { +status_string = "secure-status"; +} else { +status_string = "uefi-status"; +} + /* Mark as not usable by the normal world */ qemu_fdt_setprop_string(vms->fdt, nodename, "status", "disabled"); -qemu_fdt_setprop_string(vms->fdt, nodename, "secure-status", "okay"); +qemu_fdt_setprop_string(vms->fdt, nodename, status_string, "okay"); -qemu_fdt_add_subnode(vms->fdt, "/secure-chosen"); -qemu_fdt_setprop_string(vms->fdt, "/secure-chosen", "stdout-path", -nodename); +if (uart == VIRT_SECURE_UART) { +qemu_fdt_add_subnode(vms->fdt, "/secure-chosen"); +qemu_fdt_setprop_string(vms->fdt, "/secure-chosen", "stdout-path", +nodename); +} } g_free(nodename); @@ -1423,6 +1435,7 @@ static void machvirt_init(MachineState *machine) MemoryRegion *ram = g_new(MemoryRegion, 1); bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0); bool aarch64 = true; +int uart_index = 0; /* * In accelerated mode, the memory map is computed earlier in kvm_type() @@ -1616,13 +1629,17 @@ static void machvirt_init(MachineState *machine) fdt_add_pmu_nodes(vms); -create_uart(vms, pic, VIRT_UART, sysmem, serial_hd(0)); +create_uart(vms, pic, VIRT_UART, sysmem, serial_hd(uart_index++)); if (vms->secure) { create_secure_ram(vms, secure_sysmem); -create_uart(vms, pic, VIRT_SECURE_UART, secure_sysmem, serial_hd(1)); +create_uart(vms, pic, VIRT_SECURE_UART, secure_sysmem, +serial_hd(uart_index++)); } +/* Create UART for UEFI runtime services debug */ +create_uart(vms, pic, VIRT_UEFI_UART, sysmem, serial_hd(uart_index)); + vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64); create_rtc(vms, pic); diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index 507517c..565769f 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -77,6 +77,7 @@ enum { VIRT_GPIO, VIRT_SECURE_UART, VIRT_SECURE_MEM, +VIRT_UEFI_UART, VIRT_LOWMEMMAP_LAST, }; -- 1.8.3.1
Re: [Qemu-devel] Question: can we hot plug a PCIe switch on machine "virt"
Hi Eric, My real interesting is about the hotplug of PCIe switch, which means we don't need to provide lots of PCIe root ports or PCIe down stream ports at the beginning, but we can extend the capacity by hot adding PCIe switches which can provide more hot-pluggable slots for endpoint devices. The document docs/pcie.txt says "PCI Express Downstream Ports can't be hot-plugged into an existing PCI Express Upstream Port" which confuses me. Does it actually mean Downstream Ports can't be hot-plugged? For they can't be hot-plugged into an existing Upstream Port as the doc says, either they can't be hot-plugged into an non-existing Upstream Port or another place... Thanks, Heyi On 2019/4/4 15:39, Auger Eric wrote: Hi Heyi, On 4/3/19 8:50 PM, Michael S. Tsirkin wrote: On Wed, Apr 03, 2019 at 03:32:09PM +0800, Heyi Guo wrote: Hi folks, In physical world, a PCIe switch including one upstream port and several downstream ports is a single physical device, however we treat each port as a device in qemu world. In qemu docs/pcie.txt, we have below statements: Line 230: Be aware that PCI Express Downstream Ports can't be hot-plugged into Line 231: an existing PCI Express Upstream Port. To my understanding, it implies PCIe downstream ports *can* be hot-plugged into something which is not an existing upstream port. If it is true, how can we do that? AFAIK monitor command device_add can only add one device at a time. Please help to show the truth. Thanks, Heyi afaik they can only be plugged into upstearm ports with or without hotplug. Hotplug on upstream port does not look supported, as mentionned in the doc: (QEMU) device_add driver=xio3130-downstream id=down0 bus=upstream_port1 {"error": {"class": "GenericError", "desc": "Bus 'upstream_port1' does not support hotplugging"}} Looks the std way to use the downstream port is the one documented in 2.2.3: 2.2.3 Plugging a PCI Express device into a Switch: -device ioh3420,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z] -device x3130-upstream,id=upstream_port1,bus=root_port1[,addr=x] -device xio3130-downstream,id=downstream_port1,bus=upstream_port1,chassis=x1,slot=y1[,addr=z1]] -device ,bus=downstream_port1 For my curiosity why do you want to hotplug a downstream port in another place than an upstream port? Thanks Eric .
[Qemu-devel] Question: can we hot plug a PCIe switch on machine "virt"
Hi folks, In physical world, a PCIe switch including one upstream port and several downstream ports is a single physical device, however we treat each port as a device in qemu world. In qemu docs/pcie.txt, we have below statements: Line 230: Be aware that PCI Express Downstream Ports can't be hot-plugged into Line 231: an existing PCI Express Upstream Port. To my understanding, it implies PCIe downstream ports *can* be hot-plugged into something which is not an existing upstream port. If it is true, how can we do that? AFAIK monitor command device_add can only add one device at a time. Please help to show the truth. Thanks, Heyi
Re: [Qemu-devel] [RFC] arm/cpu: fix soft lockup panic after resuming from stop
I also tested save/restore operations, and observed that clock in guest would not jump after restoring either. If we consider guest clock not being synchronized with real wall clock as an issue, does it mean save/restore function has the same issue? Thanks, Heyi On 2019/3/26 19:54, Heyi Guo wrote: Hi Steven, Thanks for your explanation. Please see my comments below. On 2019/3/21 0:29, Steven Price wrote: On 19/03/2019 14:39, Heyi Guo wrote: Hi Christoffer and Steve, On 2019/3/15 16:59, Christoffer Dall wrote: Hi Steve, On Wed, Mar 13, 2019 at 10:11:30AM +, Steven Price wrote: Personally I think what we need is: * Either a patch like the one from Heyi Guo (save/restore CNTVCT_EL0) or alternatively hooking up KVM_KVMCLOCK_CTRL to prevent the watchdog firing when user space explicitly stops scheduling the guest for a while. If we save/restore CNTVCT_EL0 and the warning goes away, does the guest wall clock timekeeping get all confused and does it figure this out automagically somehow? What's the source for guest wall clock timekeeping in current ARM64 implementation? Is it the value from CNTP_TVAL_EL0? Will guest OS keep track of this? Or is the wall clock simply platform RTC? I tested the RFC change and did not see anything unusual. Did I miss someting? Are you running ARM or ARM64? I'm assuming ARM64 here... Yes, definitely :) Unless I'm mistaken you should see the time reported by the guest not progress when you have stopped it using the QEMU monitor console. Running something like "while /bin/true; do date; sleep 1; done" should allow you to see that without the patch time will jump in the guest (i.e. the time reflects wall-clock time). With the patch I believe it will not jump (i.e. the clock will be behind wall-clock time after the pause). I did the test and the result was exactly like you said. However, this behaviour does depend on the exact system being emulated. >From drivers/clocksource/arm_arch_timer.c: static void __init arch_counter_register(unsigned type) { u64 start_count; /* Register the CP15 based counter if we have one */ if (type & ARCH_TIMER_TYPE_CP15) { if ((IS_ENABLED(CONFIG_ARM64) && !is_hyp_mode_available()) || arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI) arch_timer_read_counter = arch_counter_get_cntvct; else arch_timer_read_counter = arch_counter_get_cntpct; clocksource_counter.archdata.vdso_direct = vdso_default; } else { arch_timer_read_counter = arch_counter_get_cntvct_mem; } So we can see here that there are three functions for reading the counter - there's the memory interface for CNTVCT, the "CP15" interface also for CNTVCT, and an interface for CNTPCT. CNTPCT is only used for !ARM64 or if hyp mode is available (not relevant until nested virtualisation is added). The upshot is that on arm64 we're using the virtual counter (CNTVCT). Does KVM_KVMCLOCK_CTRL solve that problem? It seems KVM_KVMCLOCK_CTRL is dedicated for guest pause/resume scenario, but it relies on pvclock both in KVM and Guest and right now only X86 supports it :( KVM_KVMCLOCK_CTRL simply provides a mechanism for the host to inform the guest that a vCPU hasn't been scheduled for "a while". The guest can then use that information to avoid triggering the watchdog timeout. As you note it is only currently implemented for X86. Could Steve provide more insights about the whole thing? I'll try - see below. Thanks, Heyi * KVM itself saving/restoring CNTVCT_EL0 during suspend/resume so the guest doesn't see time pass during a suspend. This smells like policy to me so I'd much prefer keeping as much functionality in user space as possible. If we already have the APIs we need from KVM, let's use them. The problem with suspend/resume is that user space doesn't get a look-in. There's no way (generically) for a user space application to save/restore registers during the suspend. User space simply sees time jump forwards - which is precisely what we're trying to hide from the guest (as it otherwise gets upset that it appears a vCPU has deadlocked for a while). So while I agree this is "policy", it's something we need the kernel to do on our behalf. Potentially we can put it behind an ioctl so that user space can opt-in to it. * Something equivalent to MSR_KVM_WALL_CLOCK_NEW for arm which allows the guest to query the wall clock time from the host and provides an offset between CNTVCT_EL0 to wall clock time which the KVM can update during suspend/resume. This means that during a suspend/resume the guest can observe that wall clock time has passed, without having to be bothered about CNTVCT_EL0 jumping forwards. Isn't the proper Arm architectural solution for this to read the physical counter for wall clock time keeping ? (Yes that will require a trap on physical counter reads after migration on current systems, but migratio
Re: [Qemu-devel] [RFC] arm/cpu: fix soft lockup panic after resuming from stop
Hi Steven, Thanks for your explanation. Please see my comments below. On 2019/3/21 0:29, Steven Price wrote: On 19/03/2019 14:39, Heyi Guo wrote: Hi Christoffer and Steve, On 2019/3/15 16:59, Christoffer Dall wrote: Hi Steve, On Wed, Mar 13, 2019 at 10:11:30AM +, Steven Price wrote: Personally I think what we need is: * Either a patch like the one from Heyi Guo (save/restore CNTVCT_EL0) or alternatively hooking up KVM_KVMCLOCK_CTRL to prevent the watchdog firing when user space explicitly stops scheduling the guest for a while. If we save/restore CNTVCT_EL0 and the warning goes away, does the guest wall clock timekeeping get all confused and does it figure this out automagically somehow? What's the source for guest wall clock timekeeping in current ARM64 implementation? Is it the value from CNTP_TVAL_EL0? Will guest OS keep track of this? Or is the wall clock simply platform RTC? I tested the RFC change and did not see anything unusual. Did I miss someting? Are you running ARM or ARM64? I'm assuming ARM64 here... Yes, definitely :) Unless I'm mistaken you should see the time reported by the guest not progress when you have stopped it using the QEMU monitor console. Running something like "while /bin/true; do date; sleep 1; done" should allow you to see that without the patch time will jump in the guest (i.e. the time reflects wall-clock time). With the patch I believe it will not jump (i.e. the clock will be behind wall-clock time after the pause). I did the test and the result was exactly like you said. However, this behaviour does depend on the exact system being emulated. >From drivers/clocksource/arm_arch_timer.c: static void __init arch_counter_register(unsigned type) { u64 start_count; /* Register the CP15 based counter if we have one */ if (type & ARCH_TIMER_TYPE_CP15) { if ((IS_ENABLED(CONFIG_ARM64) && !is_hyp_mode_available()) || arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI) arch_timer_read_counter = arch_counter_get_cntvct; else arch_timer_read_counter = arch_counter_get_cntpct; clocksource_counter.archdata.vdso_direct = vdso_default; } else { arch_timer_read_counter = arch_counter_get_cntvct_mem; } So we can see here that there are three functions for reading the counter - there's the memory interface for CNTVCT, the "CP15" interface also for CNTVCT, and an interface for CNTPCT. CNTPCT is only used for !ARM64 or if hyp mode is available (not relevant until nested virtualisation is added). The upshot is that on arm64 we're using the virtual counter (CNTVCT). Does KVM_KVMCLOCK_CTRL solve that problem? It seems KVM_KVMCLOCK_CTRL is dedicated for guest pause/resume scenario, but it relies on pvclock both in KVM and Guest and right now only X86 supports it :( KVM_KVMCLOCK_CTRL simply provides a mechanism for the host to inform the guest that a vCPU hasn't been scheduled for "a while". The guest can then use that information to avoid triggering the watchdog timeout. As you note it is only currently implemented for X86. Could Steve provide more insights about the whole thing? I'll try - see below. Thanks, Heyi * KVM itself saving/restoring CNTVCT_EL0 during suspend/resume so the guest doesn't see time pass during a suspend. This smells like policy to me so I'd much prefer keeping as much functionality in user space as possible. If we already have the APIs we need from KVM, let's use them. The problem with suspend/resume is that user space doesn't get a look-in. There's no way (generically) for a user space application to save/restore registers during the suspend. User space simply sees time jump forwards - which is precisely what we're trying to hide from the guest (as it otherwise gets upset that it appears a vCPU has deadlocked for a while). So while I agree this is "policy", it's something we need the kernel to do on our behalf. Potentially we can put it behind an ioctl so that user space can opt-in to it. * Something equivalent to MSR_KVM_WALL_CLOCK_NEW for arm which allows the guest to query the wall clock time from the host and provides an offset between CNTVCT_EL0 to wall clock time which the KVM can update during suspend/resume. This means that during a suspend/resume the guest can observe that wall clock time has passed, without having to be bothered about CNTVCT_EL0 jumping forwards. Isn't the proper Arm architectural solution for this to read the physical counter for wall clock time keeping ? (Yes that will require a trap on physical counter reads after migration on current systems, but migration sucks in terms of timekeeping already.) The physical counter is certainly tempting as it largely does what we want as a secondary counter. However I'm wary of using it because it starts to get "really interest
Re: [Qemu-devel] [RFC] arm/cpu: fix soft lockup panic after resuming from stop
Hi Christoffer and Steve, On 2019/3/15 16:59, Christoffer Dall wrote: Hi Steve, On Wed, Mar 13, 2019 at 10:11:30AM +, Steven Price wrote: Personally I think what we need is: * Either a patch like the one from Heyi Guo (save/restore CNTVCT_EL0) or alternatively hooking up KVM_KVMCLOCK_CTRL to prevent the watchdog firing when user space explicitly stops scheduling the guest for a while. If we save/restore CNTVCT_EL0 and the warning goes away, does the guest wall clock timekeeping get all confused and does it figure this out automagically somehow? What's the source for guest wall clock timekeeping in current ARM64 implementation? Is it the value from CNTP_TVAL_EL0? Will guest OS keep track of this? Or is the wall clock simply platform RTC? I tested the RFC change and did not see anything unusual. Did I miss someting? Does KVM_KVMCLOCK_CTRL solve that problem? It seems KVM_KVMCLOCK_CTRL is dedicated for guest pause/resume scenario, but it relies on pvclock both in KVM and Guest and right now only X86 supports it :( Could Steve provide more insights about the whole thing? Thanks, Heyi * KVM itself saving/restoring CNTVCT_EL0 during suspend/resume so the guest doesn't see time pass during a suspend. This smells like policy to me so I'd much prefer keeping as much functionality in user space as possible. If we already have the APIs we need from KVM, let's use them. * Something equivalent to MSR_KVM_WALL_CLOCK_NEW for arm which allows the guest to query the wall clock time from the host and provides an offset between CNTVCT_EL0 to wall clock time which the KVM can update during suspend/resume. This means that during a suspend/resume the guest can observe that wall clock time has passed, without having to be bothered about CNTVCT_EL0 jumping forwards. Isn't the proper Arm architectural solution for this to read the physical counter for wall clock time keeping ? (Yes that will require a trap on physical counter reads after migration on current systems, but migration sucks in terms of timekeeping already.) Thanks, Christoffer .
Re: [Qemu-devel] [RFC] arm/cpu: fix soft lockup panic after resuming from stop
Hi Peter and Steven, On 2019/3/13 18:11, Steven Price wrote: On 12/03/2019 14:12, Marc Zyngier wrote: Hi Peter, On 12/03/2019 10:08, Peter Maydell wrote: On Tue, 12 Mar 2019 at 06:10, Heyi Guo wrote: When we stop a VM for more than 30 seconds and then resume it, by qemu monitor command "stop" and "cont", Linux on VM will complain of "soft lockup - CPU#x stuck for xxs!" as below: [ 2783.809517] watchdog: BUG: soft lockup - CPU#3 stuck for 2395s! [ 2783.809559] watchdog: BUG: soft lockup - CPU#2 stuck for 2395s! [ 2783.809561] watchdog: BUG: soft lockup - CPU#1 stuck for 2395s! [ 2783.809563] Modules linked in... This is because Guest Linux uses generic timer virtual counter as a software watchdog, and CNTVCT_EL0 does not stop when VM is stopped by qemu. This patch is to fix this issue by saving the value of CNTVCT_EL0 when stopping and restoring it when resuming. An alternative way of fixing this particular issue ("stop"/"cont" commands in QEMU) would be to wire up KVM_KVMCLOCK_CTRL for arm to allow QEMU to signal to the guest that it was forcibly stopped for a while (and so the watchdog timeout can be ignored by the guest). Hi -- I know we have issues with the passage of time in Arm VMs running under KVM when the VM is suspended, but the topic is a tricky one, and it's not clear to me that this is the correct way to fix it. I would prefer to see us start with a discussion on the kvm-arm mailing list about the best approach to the problem. I've cc'd that list and a couple of the Arm KVM maintainers for their opinion. QEMU patch left below for context -- the brief summary is that it uses KVM_GET_ONE_REG/KVM_SET_ONE_REG on the timer CNT register to save it on VM pause and write that value back on VM resume. That's probably good enough for this particular use case, but I think there is more. I can get into similar trouble if I suspend my laptop, or suspend QEMU. It also has the slightly bizarre effect of skewing time, and this will affect timekeeping in the guest in ways that are much more subtle than just shouty CPUs. Indeed this is the bigger issue - user space doesn't get an opportunity to be involved when suspending/resuming, so saving/restoring (or using KVM_KVMCLOCK_CTRL) in user space won't fix these cases. Christoffer and Steve had some stuff regarding Live Physical Time, which should cover that, and other cases such as host system suspend, and QEMU being suspended. Live Physical Time (LPT) is only part of the solution - this handles the mess that otherwise would occur when moving to a new host with a different clock frequency. Personally I think what we need is: * Either a patch like the one from Heyi Guo (save/restore CNTVCT_EL0) or alternatively hooking up KVM_KVMCLOCK_CTRL to prevent the watchdog firing when user space explicitly stops scheduling the guest for a while. Per Steven's comments, the above change is still needed even when LPT is implemented, so do it make sense for us to apply this patch first? At least it fixes something and does not cause conflict with future solution. And do you think it is better to use KVM_KVMCLOCK_CTRL? Thanks, Heyi * KVM itself saving/restoring CNTVCT_EL0 during suspend/resume so the guest doesn't see time pass during a suspend. * Something equivalent to MSR_KVM_WALL_CLOCK_NEW for arm which allows the guest to query the wall clock time from the host and provides an offset between CNTVCT_EL0 to wall clock time which the KVM can update during suspend/resume. This means that during a suspend/resume the guest can observe that wall clock time has passed, without having to be bothered about CNTVCT_EL0 jumping forwards. Steve .
Re: [Qemu-devel] [RFC] arm/cpu: fix soft lockup panic after resuming from stop
Dear all, Really appreciate your comments and information. For "Live Physical Time", is there any document to tell the whole story? And may I know the timeline to make it happen? Thanks, Heyi On 2019/3/12 22:12, Marc Zyngier wrote: Hi Peter, On 12/03/2019 10:08, Peter Maydell wrote: On Tue, 12 Mar 2019 at 06:10, Heyi Guo wrote: When we stop a VM for more than 30 seconds and then resume it, by qemu monitor command "stop" and "cont", Linux on VM will complain of "soft lockup - CPU#x stuck for xxs!" as below: [ 2783.809517] watchdog: BUG: soft lockup - CPU#3 stuck for 2395s! [ 2783.809559] watchdog: BUG: soft lockup - CPU#2 stuck for 2395s! [ 2783.809561] watchdog: BUG: soft lockup - CPU#1 stuck for 2395s! [ 2783.809563] Modules linked in... This is because Guest Linux uses generic timer virtual counter as a software watchdog, and CNTVCT_EL0 does not stop when VM is stopped by qemu. This patch is to fix this issue by saving the value of CNTVCT_EL0 when stopping and restoring it when resuming. Hi -- I know we have issues with the passage of time in Arm VMs running under KVM when the VM is suspended, but the topic is a tricky one, and it's not clear to me that this is the correct way to fix it. I would prefer to see us start with a discussion on the kvm-arm mailing list about the best approach to the problem. I've cc'd that list and a couple of the Arm KVM maintainers for their opinion. QEMU patch left below for context -- the brief summary is that it uses KVM_GET_ONE_REG/KVM_SET_ONE_REG on the timer CNT register to save it on VM pause and write that value back on VM resume. That's probably good enough for this particular use case, but I think there is more. I can get into similar trouble if I suspend my laptop, or suspend QEMU. It also has the slightly bizarre effect of skewing time, and this will affect timekeeping in the guest in ways that are much more subtle than just shouty CPUs. Christoffer and Steve had some stuff regarding Live Physical Time, which should cover that, and other cases such as host system suspend, and QEMU being suspended. Thanks, M.
Re: [Qemu-devel] [RFC] arm/cpu: fix soft lockup panic after resuming from stop
Hi Richard, On 2019/3/12 22:59, Richard Henderson wrote: On 3/12/19 12:57 AM, Heyi Guo wrote: int kvm_arm_vcpu_init(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); struct kvm_vcpu_init init; +/* + * Only add change state handler for arch timer once, for KVM will help to + * synchronize virtual timer of all VCPUs. + */ +static bool arch_timer_change_state_handler_added; + + +if (!arch_timer_change_state_handler_added) { +qemu_add_vm_change_state_handler(arch_timer_change_state_handler, cs); +arch_timer_change_state_handler_added = true; +} Which means that this will not trigger for the second time that you pause the vm. You need to key this off of something else, like cpu id 0. I don't quite understand. Do you mean the handler will be deactivated after one trigger? Or something else? I suppose the state change handler will take effect for the whole VM life time, so I believed one handler for one VM is enough, in whichever vCPU initialization it is created. I also tested several times for one VM, and fortunately the rough code worked. Thanks, Heyi r~
Re: [Qemu-devel] [RFC] arm/cpu: fix soft lockup panic after resuming from stop
Hi all, I'm sorry this patch failed the docker-mingw@fedora build test. I'm going to move the code to target/arm/kvm.c. Please ignore this one. Thanks, Heyi On 2019/3/12 14:09, Heyi Guo wrote: When we stop a VM for more than 30 seconds and then resume it, by qemu monitor command "stop" and "cont", Linux on VM will complain of "soft lockup - CPU#x stuck for xxs!" as below: [ 2783.809517] watchdog: BUG: soft lockup - CPU#3 stuck for 2395s! [ 2783.809559] watchdog: BUG: soft lockup - CPU#2 stuck for 2395s! [ 2783.809561] watchdog: BUG: soft lockup - CPU#1 stuck for 2395s! [ 2783.809563] Modules linked in... This is because Guest Linux uses generic timer virtual counter as a software watchdog, and CNTVCT_EL0 does not stop when VM is stopped by qemu. This patch is to fix this issue by saving the value of CNTVCT_EL0 when stopping and restoring it when resuming. Cc: Peter Maydell Signed-off-by: Heyi Guo --- target/arm/cpu.c | 65 1 file changed, 65 insertions(+) diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 96f0ff0..7bbba3d 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -896,6 +896,60 @@ static void arm_cpu_finalizefn(Object *obj) #endif } +static int get_vcpu_timer_tick(CPUState *cs, uint64_t *tick_at_pause) +{ +int err; +struct kvm_one_reg reg; + +reg.id = KVM_REG_ARM_TIMER_CNT; +reg.addr = (uintptr_t) tick_at_pause; + +err = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ); +return err; +} + +static int set_vcpu_timer_tick(CPUState *cs, uint64_t tick_at_pause) +{ +int err; +struct kvm_one_reg reg; + +reg.id = KVM_REG_ARM_TIMER_CNT; +reg.addr = (uintptr_t) _at_pause; + +err = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ); +return err; +} + +static void arch_timer_change_state_handler(void *opaque, int running, +RunState state) +{ +static uint64_t hw_ticks_at_paused; +static RunState pre_state = RUN_STATE__MAX; +int err; +CPUState *cs = (CPUState *)opaque; + +switch (state) { +case RUN_STATE_PAUSED: +err = get_vcpu_timer_tick(cs, _ticks_at_paused); +if (err) { +error_report("Get vcpu timer tick failed: %d", err); +} +break; +case RUN_STATE_RUNNING: +if (pre_state == RUN_STATE_PAUSED) { +err = set_vcpu_timer_tick(cs, hw_ticks_at_paused); +if (err) { +error_report("Resume vcpu timer tick failed: %d", err); +} +} +break; +default: +break; +} + +pre_state = state; +} + static void arm_cpu_realizefn(DeviceState *dev, Error **errp) { CPUState *cs = CPU(dev); @@ -906,6 +960,12 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) Error *local_err = NULL; bool no_aa32 = false; +/* + * Only add change state handler for arch timer once, for KVM will help to + * synchronize virtual timer of all VCPUs. + */ +static bool arch_timer_change_state_handler_added; + /* If we needed to query the host kernel for the CPU features * then it's possible that might have failed in the initfn, but * this is the first point where we can report it. @@ -1181,6 +1241,11 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) init_cpreg_list(cpu); +if (!arch_timer_change_state_handler_added && kvm_enabled()) { +qemu_add_vm_change_state_handler(arch_timer_change_state_handler, cs); +arch_timer_change_state_handler_added = true; +} + #ifndef CONFIG_USER_ONLY if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) { cs->num_ases = 2;
[Qemu-devel] [RFC] arm/cpu: fix soft lockup panic after resuming from stop
When we stop a VM for more than 30 seconds and then resume it, by qemu monitor command "stop" and "cont", Linux on VM will complain of "soft lockup - CPU#x stuck for xxs!" as below: [ 2783.809517] watchdog: BUG: soft lockup - CPU#3 stuck for 2395s! [ 2783.809559] watchdog: BUG: soft lockup - CPU#2 stuck for 2395s! [ 2783.809561] watchdog: BUG: soft lockup - CPU#1 stuck for 2395s! [ 2783.809563] Modules linked in... This is because Guest Linux uses generic timer virtual counter as a software watchdog, and CNTVCT_EL0 does not stop when VM is stopped by qemu. This patch is to fix this issue by saving the value of CNTVCT_EL0 when stopping and restoring it when resuming. Cc: Peter Maydell Signed-off-by: Heyi Guo --- target/arm/kvm.c | 66 1 file changed, 66 insertions(+) diff --git a/target/arm/kvm.c b/target/arm/kvm.c index 79a79f0..73b9ecb 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -39,11 +39,77 @@ static bool cap_has_inject_serror_esr; static ARMHostCPUFeatures arm_host_cpu_features; +static int get_vcpu_timer_tick(CPUState *cs, uint64_t *tick_at_pause) +{ +int err; +struct kvm_one_reg reg; + +reg.id = KVM_REG_ARM_TIMER_CNT; +reg.addr = (uintptr_t) tick_at_pause; + +err = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ); +return err; +} + +static int set_vcpu_timer_tick(CPUState *cs, uint64_t tick_at_pause) +{ +int err; +struct kvm_one_reg reg; + +reg.id = KVM_REG_ARM_TIMER_CNT; +reg.addr = (uintptr_t) _at_pause; + +err = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ); +return err; +} + +static void arch_timer_change_state_handler(void *opaque, int running, +RunState state) +{ +static uint64_t hw_ticks_at_paused; +static RunState pre_state = RUN_STATE__MAX; +int err; +CPUState *cs = (CPUState *)opaque; + +switch (state) { +case RUN_STATE_PAUSED: +err = get_vcpu_timer_tick(cs, _ticks_at_paused); +if (err) { +error_report("Get vcpu timer tick failed: %d", err); +} +break; +case RUN_STATE_RUNNING: +if (pre_state == RUN_STATE_PAUSED) { +err = set_vcpu_timer_tick(cs, hw_ticks_at_paused); +if (err) { +error_report("Resume vcpu timer tick failed: %d", err); +} +} +break; +default: +break; +} + +pre_state = state; +} + int kvm_arm_vcpu_init(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); struct kvm_vcpu_init init; +/* + * Only add change state handler for arch timer once, for KVM will help to + * synchronize virtual timer of all VCPUs. + */ +static bool arch_timer_change_state_handler_added; + + +if (!arch_timer_change_state_handler_added) { +qemu_add_vm_change_state_handler(arch_timer_change_state_handler, cs); +arch_timer_change_state_handler_added = true; +} + init.target = cpu->kvm_target; memcpy(init.features, cpu->kvm_init_features, sizeof(init.features)); -- 1.8.3.1
[Qemu-devel] [RFC] arm/cpu: fix soft lockup panic after resuming from stop
When we stop a VM for more than 30 seconds and then resume it, by qemu monitor command "stop" and "cont", Linux on VM will complain of "soft lockup - CPU#x stuck for xxs!" as below: [ 2783.809517] watchdog: BUG: soft lockup - CPU#3 stuck for 2395s! [ 2783.809559] watchdog: BUG: soft lockup - CPU#2 stuck for 2395s! [ 2783.809561] watchdog: BUG: soft lockup - CPU#1 stuck for 2395s! [ 2783.809563] Modules linked in... This is because Guest Linux uses generic timer virtual counter as a software watchdog, and CNTVCT_EL0 does not stop when VM is stopped by qemu. This patch is to fix this issue by saving the value of CNTVCT_EL0 when stopping and restoring it when resuming. Cc: Peter Maydell Signed-off-by: Heyi Guo --- target/arm/cpu.c | 65 1 file changed, 65 insertions(+) diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 96f0ff0..7bbba3d 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -896,6 +896,60 @@ static void arm_cpu_finalizefn(Object *obj) #endif } +static int get_vcpu_timer_tick(CPUState *cs, uint64_t *tick_at_pause) +{ +int err; +struct kvm_one_reg reg; + +reg.id = KVM_REG_ARM_TIMER_CNT; +reg.addr = (uintptr_t) tick_at_pause; + +err = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ); +return err; +} + +static int set_vcpu_timer_tick(CPUState *cs, uint64_t tick_at_pause) +{ +int err; +struct kvm_one_reg reg; + +reg.id = KVM_REG_ARM_TIMER_CNT; +reg.addr = (uintptr_t) _at_pause; + +err = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ); +return err; +} + +static void arch_timer_change_state_handler(void *opaque, int running, +RunState state) +{ +static uint64_t hw_ticks_at_paused; +static RunState pre_state = RUN_STATE__MAX; +int err; +CPUState *cs = (CPUState *)opaque; + +switch (state) { +case RUN_STATE_PAUSED: +err = get_vcpu_timer_tick(cs, _ticks_at_paused); +if (err) { +error_report("Get vcpu timer tick failed: %d", err); +} +break; +case RUN_STATE_RUNNING: +if (pre_state == RUN_STATE_PAUSED) { +err = set_vcpu_timer_tick(cs, hw_ticks_at_paused); +if (err) { +error_report("Resume vcpu timer tick failed: %d", err); +} +} +break; +default: +break; +} + +pre_state = state; +} + static void arm_cpu_realizefn(DeviceState *dev, Error **errp) { CPUState *cs = CPU(dev); @@ -906,6 +960,12 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) Error *local_err = NULL; bool no_aa32 = false; +/* + * Only add change state handler for arch timer once, for KVM will help to + * synchronize virtual timer of all VCPUs. + */ +static bool arch_timer_change_state_handler_added; + /* If we needed to query the host kernel for the CPU features * then it's possible that might have failed in the initfn, but * this is the first point where we can report it. @@ -1181,6 +1241,11 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) init_cpreg_list(cpu); +if (!arch_timer_change_state_handler_added && kvm_enabled()) { +qemu_add_vm_change_state_handler(arch_timer_change_state_handler, cs); +arch_timer_change_state_handler_added = true; +} + #ifndef CONFIG_USER_ONLY if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) { cs->num_ases = 2; -- 1.8.3.1
Re: [Qemu-devel] [PATCH v4 1/2] hw/arm/acpi: simplify AML bit and/or statement
On 2019/3/7 0:34, Igor Mammedov wrote: On Wed, 6 Mar 2019 21:36:56 +0800 Heyi Guo wrote: The last argument of AML bit and/or statement is the target variable, so we don't need to use a NULL target and then an additional store operation; a single bit and/or statement is enough. s: a single bit and/or : using just aml_and() or aml_and() " With commit message fixed up: Reviewed-by: Igor Mammedov Thanks; have fixed it in v5. Heyi Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Suggested-by: Igor Mammedov Signed-off-by: Heyi Guo --- hw/arm/virt-acpi-build.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index d7e2e48..cebec4c 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -265,17 +265,17 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); -aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL), -aml_name("CTRL"))); +aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL), - aml_name("CDW1"))); +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL), - aml_name("CDW1"))); +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3"))); @@ -283,8 +283,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, ifctx); elsectx = aml_else(); -aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL), - aml_name("CDW1"))); +aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4), + aml_name("CDW1"))); aml_append(elsectx, aml_return(aml_arg(3))); aml_append(method, elsectx); aml_append(dev, method); .
[Qemu-devel] [PATCH v5 2/2] hw/arm/acpi: enable SHPC native hot plug
After the introduction of generic PCIe root port and PCIe-PCI bridge, we will also have SHPC controller on ARM, so just enable SHPC native hot plug. Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Reviewed-by: Michael S. Tsirkin Reviewed-by: Igor Mammedov Signed-off-by: Heyi Guo --- hw/arm/virt-acpi-build.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index cebec4c..b6fef28 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -265,7 +265,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); -aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + +/* + * Allow OS control for all 5 features: + * PCIeHotplug SHPCHotplug PME AER PCIeCapability. + */ +aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1F), aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1; -- 1.8.3.1
[Qemu-devel] [PATCH v5 1/2] hw/arm/acpi: simplify AML bit and/or statement
The last argument of AML bit and/or statement is the target variable, so we don't need to use a NULL target and then an additional store operation; using just aml_and() or aml_or() statement is enough. Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Suggested-by: Igor Mammedov Reviewed-by: Igor Mammedov Signed-off-by: Heyi Guo --- hw/arm/virt-acpi-build.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index d7e2e48..cebec4c 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -265,17 +265,17 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); -aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL), -aml_name("CTRL"))); +aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL), - aml_name("CDW1"))); +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL), - aml_name("CDW1"))); +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3"))); @@ -283,8 +283,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, ifctx); elsectx = aml_else(); -aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL), - aml_name("CDW1"))); +aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4), + aml_name("CDW1"))); aml_append(elsectx, aml_return(aml_arg(3))); aml_append(method, elsectx); aml_append(dev, method); -- 1.8.3.1
[Qemu-devel] [PATCH v5 0/2] arm/acpi: simplify aml code and enable SHPC
After the introduction of generic PCIe root port and PCIe-PCI bridge, we will also have SHPC controller on ARM, and we don't support ACPI hot plug, so just enable SHPC native hot plug. Igor also spotted the store operation outside of bit and/or is not necessary, so simply the code at first. v5: - Refine commit message of patch 1/2 v4: - Improve the code indention. Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Heyi Guo (2): hw/arm/acpi: simplify AML bit and/or statement hw/arm/acpi: enable SHPC native hot plug hw/arm/virt-acpi-build.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) -- 1.8.3.1
Re: [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement
Got it; thanks. Heyi On 2019/3/7 0:20, Igor Mammedov wrote: On Wed, 6 Mar 2019 21:09:11 +0800 Heyi Guo wrote: Sorry, I didn't know the indention policy of qemu code. So we need to indent the 2nd line just after the parentheses it belongs to? See "[PATCH v6 0/2] CODING_STYLE: trivial update" for clarification Will change that and send next version. Thanks, Heyi On 2019/3/6 18:33, Igor Mammedov wrote: On Wed, 6 Mar 2019 06:03:48 +0800 Heyi Guo wrote: The last argument of AML bit and/or statement is the target variable, so we don't need to use a NULL target and then an additional store operation; a single bit and/or statement is enough. Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Suggested-by: Igor Mammedov Signed-off-by: Heyi Guo --- hw/arm/virt-acpi-build.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 04b62c7..1c84e87 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -265,16 +265,16 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); -aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL), +aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL"))); pls fix indent in all such cases. I'd align it like this: aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL"))); or aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL"))); PS: When making multi-patch series use --cover-letter git option ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL), +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08), aml_name("CDW1"))); aml_append(ifctx, ifctx1); ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL), +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10), aml_name("CDW1"))); aml_append(ifctx, ifctx1); @@ -283,7 +283,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, ifctx); elsectx = aml_else(); -aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL), +aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4), aml_name("CDW1"))); aml_append(elsectx, aml_return(aml_arg(3))); aml_append(method, elsectx); . .
[Qemu-devel] [PATCH v4 1/2] hw/arm/acpi: simplify AML bit and/or statement
The last argument of AML bit and/or statement is the target variable, so we don't need to use a NULL target and then an additional store operation; a single bit and/or statement is enough. Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Suggested-by: Igor Mammedov Signed-off-by: Heyi Guo --- hw/arm/virt-acpi-build.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index d7e2e48..cebec4c 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -265,17 +265,17 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); -aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL), -aml_name("CTRL"))); +aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL), - aml_name("CDW1"))); +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL), - aml_name("CDW1"))); +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3"))); @@ -283,8 +283,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, ifctx); elsectx = aml_else(); -aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL), - aml_name("CDW1"))); +aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4), + aml_name("CDW1"))); aml_append(elsectx, aml_return(aml_arg(3))); aml_append(method, elsectx); aml_append(dev, method); -- 1.8.3.1
[Qemu-devel] [PATCH v4 2/2] hw/arm/acpi: enable SHPC native hot plug
After the introduction of generic PCIe root port and PCIe-PCI bridge, we will also have SHPC controller on ARM, so just enable SHPC native hot plug. Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Reviewed-by: Michael S. Tsirkin Signed-off-by: Heyi Guo --- hw/arm/virt-acpi-build.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index cebec4c..b6fef28 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -265,7 +265,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); -aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + +/* + * Allow OS control for all 5 features: + * PCIeHotplug SHPCHotplug PME AER PCIeCapability. + */ +aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1F), aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1; -- 1.8.3.1
[Qemu-devel] [PATCH v4 0/2] arm/acpi: simplify aml code and enable SHPC
After the introduction of generic PCIe root port and PCIe-PCI bridge, we will also have SHPC controller on ARM, and we don't support ACPI hot plug, so just enable SHPC native hot plug. Igor also spotted the store operation outside of bit and/or is not necessary, so simply the code at first. v4: - Improve the code indention. Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Heyi Guo (2): hw/arm/acpi: simplify AML bit and/or statement hw/arm/acpi: enable SHPC native hot plug hw/arm/virt-acpi-build.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) -- 1.8.3.1
Re: [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement
Sorry, I didn't know the indention policy of qemu code. So we need to indent the 2nd line just after the parentheses it belongs to? Will change that and send next version. Thanks, Heyi On 2019/3/6 18:33, Igor Mammedov wrote: On Wed, 6 Mar 2019 06:03:48 +0800 Heyi Guo wrote: The last argument of AML bit and/or statement is the target variable, so we don't need to use a NULL target and then an additional store operation; a single bit and/or statement is enough. Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Suggested-by: Igor Mammedov Signed-off-by: Heyi Guo --- hw/arm/virt-acpi-build.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 04b62c7..1c84e87 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -265,16 +265,16 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); -aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL), +aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL"))); pls fix indent in all such cases. I'd align it like this: aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL"))); or aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL"))); PS: When making multi-patch series use --cover-letter git option ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL), +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08), aml_name("CDW1"))); aml_append(ifctx, ifctx1); ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL), +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10), aml_name("CDW1"))); aml_append(ifctx, ifctx1); @@ -283,7 +283,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, ifctx); elsectx = aml_else(); -aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL), +aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4), aml_name("CDW1"))); aml_append(elsectx, aml_return(aml_arg(3))); aml_append(method, elsectx); .
[Qemu-devel] [PATCH v3 2/2] hw/arm/acpi: enable SHPC native hot plug
After the introduction of generic PCIe root port and PCIe-PCI bridge, we will also have SHPC controller on ARM, so just enalbe SHPC native hot plug. Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Reviewed-by: Michael S. Tsirkin Signed-off-by: Heyi Guo --- hw/arm/virt-acpi-build.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 1c84e87..e8e00fe 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -265,7 +265,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); -aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + +/* + * Allow OS control for all 5 features: + * PCIeHotplug SHPCHotplug PME AER PCIeCapability. + */ +aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1F), aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1; -- 1.8.3.1
[Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement
The last argument of AML bit and/or statement is the target variable, so we don't need to use a NULL target and then an additional store operation; a single bit and/or statement is enough. Cc: Shannon Zhao Cc: Peter Maydell Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Suggested-by: Igor Mammedov Signed-off-by: Heyi Guo --- hw/arm/virt-acpi-build.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 04b62c7..1c84e87 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -265,16 +265,16 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); -aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL), +aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL), +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08), aml_name("CDW1"))); aml_append(ifctx, ifctx1); ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"; -aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL), +aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10), aml_name("CDW1"))); aml_append(ifctx, ifctx1); @@ -283,7 +283,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, ifctx); elsectx = aml_else(); -aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL), +aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4), aml_name("CDW1"))); aml_append(elsectx, aml_return(aml_arg(3))); aml_append(method, elsectx); -- 1.8.3.1