From 1cb169b27a7e78176de2101ce7c0a577945c8dec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sat, 19 Sep 2020 15:24:35 +0200 Subject: [PATCH 01/12] hw/ssi/npcm7xx_fiu: Fix handling of unsigned integer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix integer handling issues handling issue reported by Coverity: hw/ssi/npcm7xx_fiu.c: 162 in npcm7xx_fiu_flash_read() >>> CID 1432730: Integer handling issues (NEGATIVE_RETURNS) >>> "npcm7xx_fiu_cs_index(fiu, f)" is passed to a parameter that cannot be negative. 162 npcm7xx_fiu_select(fiu, npcm7xx_fiu_cs_index(fiu, f)); hw/ssi/npcm7xx_fiu.c: 221 in npcm7xx_fiu_flash_write() 218 cs_id = npcm7xx_fiu_cs_index(fiu, f); 219 trace_npcm7xx_fiu_flash_write(DEVICE(fiu)->canonical_path, cs_id, addr, 220 size, v); >>> CID 1432729: Integer handling issues (NEGATIVE_RETURNS) >>> "cs_id" is passed to a parameter that cannot be negative. 221 npcm7xx_fiu_select(fiu, cs_id); Since the index of the flash can not be negative, return an unsigned type. Reported-by: Coverity (CID 1432729 & 1432730: NEGATIVE_RETURNS) Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Havard Skinnemoen Message-id: 20200919132435.310527-1-f4bug@amsat.org Signed-off-by: Peter Maydell --- hw/ssi/npcm7xx_fiu.c | 12 ++++++------ hw/ssi/trace-events | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/ssi/npcm7xx_fiu.c b/hw/ssi/npcm7xx_fiu.c index 104e8f2b96..5040132b07 100644 --- a/hw/ssi/npcm7xx_fiu.c +++ b/hw/ssi/npcm7xx_fiu.c @@ -103,7 +103,8 @@ enum NPCM7xxFIURegister { * Returns the index of flash in the fiu->flash array. This corresponds to the * chip select ID of the flash. */ -static int npcm7xx_fiu_cs_index(NPCM7xxFIUState *fiu, NPCM7xxFIUFlash *flash) +static unsigned npcm7xx_fiu_cs_index(NPCM7xxFIUState *fiu, + NPCM7xxFIUFlash *flash) { int index = flash - fiu->flash; @@ -113,20 +114,19 @@ static int npcm7xx_fiu_cs_index(NPCM7xxFIUState *fiu, NPCM7xxFIUFlash *flash) } /* Assert the chip select specified in the UMA Control/Status Register. */ -static void npcm7xx_fiu_select(NPCM7xxFIUState *s, int cs_id) +static void npcm7xx_fiu_select(NPCM7xxFIUState *s, unsigned cs_id) { trace_npcm7xx_fiu_select(DEVICE(s)->canonical_path, cs_id); if (cs_id < s->cs_count) { qemu_irq_lower(s->cs_lines[cs_id]); + s->active_cs = cs_id; } else { qemu_log_mask(LOG_GUEST_ERROR, "%s: UMA to CS%d; this module has only %d chip selects", DEVICE(s)->canonical_path, cs_id, s->cs_count); - cs_id = -1; + s->active_cs = -1; } - - s->active_cs = cs_id; } /* Deassert the currently active chip select. */ @@ -206,7 +206,7 @@ static void npcm7xx_fiu_flash_write(void *opaque, hwaddr addr, uint64_t v, NPCM7xxFIUFlash *f = opaque; NPCM7xxFIUState *fiu = f->fiu; uint32_t dwr_cfg; - int cs_id; + unsigned cs_id; int i; if (fiu->active_cs != -1) { diff --git a/hw/ssi/trace-events b/hw/ssi/trace-events index 2f83ef833f..612d3d6087 100644 --- a/hw/ssi/trace-events +++ b/hw/ssi/trace-events @@ -19,4 +19,4 @@ npcm7xx_fiu_deselect(const char *id, int cs) "%s deselect CS%d" npcm7xx_fiu_ctrl_read(const char *id, uint64_t addr, uint32_t data) "%s offset: 0x%04" PRIx64 " value: 0x%08" PRIx32 npcm7xx_fiu_ctrl_write(const char *id, uint64_t addr, uint32_t data) "%s offset: 0x%04" PRIx64 " value: 0x%08" PRIx32 npcm7xx_fiu_flash_read(const char *id, int cs, uint64_t addr, unsigned int size, uint64_t value) "%s[%d] offset: 0x%08" PRIx64 " size: %u value: 0x%" PRIx64 -npcm7xx_fiu_flash_write(const char *id, int cs, uint64_t addr, unsigned int size, uint64_t value) "%s[%d] offset: 0x%08" PRIx64 " size: %u value: 0x%" PRIx64 +npcm7xx_fiu_flash_write(const char *id, unsigned cs, uint64_t addr, unsigned int size, uint64_t value) "%s[%d] offset: 0x%08" PRIx64 " size: %u value: 0x%" PRIx64 From 1ef6a40608f8e20cb39762a9eeaa29d135310244 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 2 Oct 2020 10:09:35 +0200 Subject: [PATCH 02/12] hw/arm/fsl-imx25: Fix a typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Philippe Mathieu-Daudé Message-id: 20201002080935.1660005-1-f4bug@amsat.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- include/hw/arm/fsl-imx25.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/arm/fsl-imx25.h b/include/hw/arm/fsl-imx25.h index 971f35dd16..c1603b2ac2 100644 --- a/include/hw/arm/fsl-imx25.h +++ b/include/hw/arm/fsl-imx25.h @@ -179,7 +179,7 @@ struct FslIMX25State { * 0xBB00_0000 0xBB00_0FFF 4 Kbytes NAND flash main area buffer * 0xBB00_1000 0xBB00_11FF 512 B NAND flash spare area buffer * 0xBB00_1200 0xBB00_1DFF 3 Kbytes Reserved - * 0xBB00_1E00 0xBB00_1FFF 512 B NAND flash control regisers + * 0xBB00_1E00 0xBB00_1FFF 512 B NAND flash control registers * 0xBB01_2000 0xBFFF_FFFF 96 Mbytes (minus 8 Kbytes) Reserved * 0xC000_0000 0xFFFF_FFFF 1024 Mbytes Reserved */ From b8bf3472ccb4e5265dc6ec148a38f4b4dd5ac896 Mon Sep 17 00:00:00 2001 From: Graeme Gregory Date: Wed, 7 Oct 2020 11:07:31 +0100 Subject: [PATCH 03/12] hw/arm/sbsa-ref : Fix SMMUv3 Initialisation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SMMUv3 has an error in a previous patch where an i was transposed to a 1 meaning interrupts would not have been correctly assigned to the SMMUv3 instance. Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the machine state") Signed-off-by: Graeme Gregory Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Eric Auger Message-id: 20201007100732.4103790-2-graeme@nuviainc.com Signed-off-by: Peter Maydell --- hw/arm/sbsa-ref.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index 9c3a893bed..65e64883b5 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -525,7 +525,7 @@ static void create_smmu(const SBSAMachineState *sms, PCIBus *bus) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); for (i = 0; i < NUM_SMMU_IRQS; i++) { sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, - qdev_get_gpio_in(sms->gic, irq + 1)); + qdev_get_gpio_in(sms->gic, irq + i)); } } From 04788fd5c5577cbe5fb61107cdd9732479c793ca Mon Sep 17 00:00:00 2001 From: Graeme Gregory Date: Wed, 7 Oct 2020 11:07:32 +0100 Subject: [PATCH 04/12] hw/arm/sbsa-ref : allocate IRQs for SMMUv3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit did not allocate IRQs for the SMMUv3 in the irqmap effectively using irq 0->3 (shared with other devices). Assuming original intent was to allocate unique IRQs then add an allocation to the irqmap. Fixes: e9fdf453240 ("hw/arm: Add arm SBSA reference machine, devices part") Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Graeme Gregory Reviewed-by: Eric Auger Message-id: 20201007100732.4103790-3-graeme@nuviainc.com Signed-off-by: Peter Maydell --- hw/arm/sbsa-ref.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index 65e64883b5..01863510d0 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -133,6 +133,7 @@ static const int sbsa_ref_irqmap[] = { [SBSA_SECURE_UART_MM] = 9, [SBSA_AHCI] = 10, [SBSA_EHCI] = 11, + [SBSA_SMMU] = 12, /* ... to 15 */ }; static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx) From 3059344f01e1bf9625570ef2e8396fa011e9431d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 2 Oct 2020 20:10:32 +0200 Subject: [PATCH 05/12] hw/char/bcm2835_aux: Allow less than 32-bit accesses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "BCM2835 ARM Peripherals" datasheet [*] chapter 2 ("Auxiliaries: UART1 & SPI1, SPI2"), list the register sizes as 3/8/16/32 bits. We assume this means this peripheral allows 8-bit accesses. This was not an issue until commit 5d971f9e67 which reverted ("memory: accept mismatching sizes in memory_region_access_valid"). The model is implemented as 32-bit accesses (see commit 97398d900c, all registers are 32-bit) so replace MemoryRegionOps.valid as MemoryRegionOps.impl, and re-introduce MemoryRegionOps.valid with a 8/32-bit range. [*] https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf Fixes: 97398d900c ("bcm2835_aux: add emulation of BCM2835 AUX (aka UART1) block") Signed-off-by: Philippe Mathieu-Daudé Message-id: 20201002181032.1899463-1-f4bug@amsat.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/char/bcm2835_aux.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/char/bcm2835_aux.c b/hw/char/bcm2835_aux.c index ee3dd40e3c..dade2ab5fd 100644 --- a/hw/char/bcm2835_aux.c +++ b/hw/char/bcm2835_aux.c @@ -249,7 +249,9 @@ static const MemoryRegionOps bcm2835_aux_ops = { .read = bcm2835_aux_read, .write = bcm2835_aux_write, .endianness = DEVICE_NATIVE_ENDIAN, - .valid.min_access_size = 4, + .impl.min_access_size = 4, + .impl.max_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, }; From 94c7fefcb456b0b26f04b30e6df54a0c872e862d Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Thu, 1 Oct 2020 08:17:13 +0200 Subject: [PATCH 06/12] linux headers: sync to 5.9-rc7 Update against Linux 5.9-rc7. Cc: Paolo Bonzini Signed-off-by: Andrew Jones Message-id: 20201001061718.101915-2-drjones@redhat.com Signed-off-by: Peter Maydell --- linux-headers/linux/kvm.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h index 6683e2e1b0..43580c767c 100644 --- a/linux-headers/linux/kvm.h +++ b/linux-headers/linux/kvm.h @@ -790,9 +790,10 @@ struct kvm_ppc_resize_hpt { #define KVM_VM_PPC_HV 1 #define KVM_VM_PPC_PR 2 -/* on MIPS, 0 forces trap & emulate, 1 forces VZ ASE */ -#define KVM_VM_MIPS_TE 0 +/* on MIPS, 0 indicates auto, 1 forces VZ ASE, 2 forces trap & emulate */ +#define KVM_VM_MIPS_AUTO 0 #define KVM_VM_MIPS_VZ 1 +#define KVM_VM_MIPS_TE 2 #define KVM_S390_SIE_PAGE_OFFSET 1 @@ -1035,6 +1036,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_LAST_CPU 184 #define KVM_CAP_SMALLER_MAXPHYADDR 185 #define KVM_CAP_S390_DIAG318 186 +#define KVM_CAP_STEAL_TIME 187 #ifdef KVM_CAP_IRQ_ROUTING From 281a3c330e0d694ce4f364fa0f74738ac4afd6dc Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Thu, 1 Oct 2020 08:17:14 +0200 Subject: [PATCH 07/12] target/arm/kvm: Make uncalled stubs explicitly unreachable When we compile without KVM support !defined(CONFIG_KVM) we generate stubs for functions that the linker will still encounter. Sometimes these stubs can be executed safely and are placed in paths where they get executed with or without KVM. Other functions should never be called without KVM. Those functions should be guarded by kvm_enabled(), but should also be robust to refactoring mistakes. Putting a g_assert_not_reached() in the function should help. Additionally, the g_assert_not_reached() calls may actually help the linker remove some code. We remove the stubs for kvm_arm_get/put_virtual_time(), as they aren't necessary at all - the only caller is in kvm.c Reviewed-by: Eric Auger Signed-off-by: Andrew Jones Message-id: 20201001061718.101915-3-drjones@redhat.com Signed-off-by: Peter Maydell --- target/arm/kvm_arm.h | 51 +++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index bc178eeb84..f513702176 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -344,18 +344,10 @@ int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level); #else -static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) -{ - /* - * This should never actually be called in the "not KVM" case, - * but set up the fields to indicate an error anyway. - */ - cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE; - cpu->host_cpu_probe_failed = true; -} - -static inline void kvm_arm_add_vcpu_properties(Object *obj) {} - +/* + * It's safe to call these functions without KVM support. + * They should either do nothing or return "not supported". + */ static inline bool kvm_arm_aarch32_supported(void) { return false; @@ -371,23 +363,44 @@ static inline bool kvm_arm_sve_supported(void) return false; } +/* + * These functions should never actually be called without KVM support. + */ +static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) +{ + g_assert_not_reached(); +} + +static inline void kvm_arm_add_vcpu_properties(Object *obj) +{ + g_assert_not_reached(); +} + static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms) { - return -ENOENT; + g_assert_not_reached(); } static inline int kvm_arm_vgic_probe(void) { - return 0; + g_assert_not_reached(); } -static inline void kvm_arm_pmu_set_irq(CPUState *cs, int irq) {} -static inline void kvm_arm_pmu_init(CPUState *cs) {} +static inline void kvm_arm_pmu_set_irq(CPUState *cs, int irq) +{ + g_assert_not_reached(); +} -static inline void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map) {} +static inline void kvm_arm_pmu_init(CPUState *cs) +{ + g_assert_not_reached(); +} + +static inline void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map) +{ + g_assert_not_reached(); +} -static inline void kvm_arm_get_virtual_time(CPUState *cs) {} -static inline void kvm_arm_put_virtual_time(CPUState *cs) {} #endif static inline const char *gic_class_name(void) From fe11f058c5fda70360f810e7bddd4b6d69f76230 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Thu, 1 Oct 2020 08:17:15 +0200 Subject: [PATCH 08/12] hw/arm/virt: Move post cpu realize check into its own function We'll add more to this new function in coming patches so we also state the gic must be created and call it below create_gic(). No functional change intended. Reviewed-by: Eric Auger Reviewed-by: Peter Maydell Signed-off-by: Andrew Jones Message-id: 20201001061718.101915-4-drjones@redhat.com Signed-off-by: Peter Maydell --- hw/arm/virt.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 1231a197c8..524eafe22d 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1672,6 +1672,31 @@ static void finalize_gic_version(VirtMachineState *vms) } } +/* + * virt_cpu_post_init() must be called after the CPUs have + * been realized and the GIC has been created. + */ +static void virt_cpu_post_init(VirtMachineState *vms) +{ + bool aarch64; + + aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL); + + if (!kvm_enabled()) { + if (aarch64 && vms->highmem) { + int requested_pa_size = 64 - clz64(vms->highest_gpa); + int pamax = arm_pamax(ARM_CPU(first_cpu)); + + if (pamax < requested_pa_size) { + error_report("VCPU supports less PA bits (%d) than " + "requested by the memory map (%d)", + pamax, requested_pa_size); + exit(1); + } + } + } +} + static void machvirt_init(MachineState *machine) { VirtMachineState *vms = VIRT_MACHINE(machine); @@ -1886,22 +1911,6 @@ static void machvirt_init(MachineState *machine) fdt_add_timer_nodes(vms); fdt_add_cpu_nodes(vms); - if (!kvm_enabled()) { - ARMCPU *cpu = ARM_CPU(first_cpu); - bool aarch64 = object_property_get_bool(OBJECT(cpu), "aarch64", NULL); - - if (aarch64 && vms->highmem) { - int requested_pa_size, pamax = arm_pamax(cpu); - - requested_pa_size = 64 - clz64(vms->highest_gpa); - if (pamax < requested_pa_size) { - error_report("VCPU supports less PA bits (%d) than requested " - "by the memory map (%d)", pamax, requested_pa_size); - exit(1); - } - } - } - memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base, machine->ram); if (machine->device_memory) { @@ -1913,6 +1922,8 @@ static void machvirt_init(MachineState *machine) create_gic(vms); + virt_cpu_post_init(vms); + fdt_add_pmu_nodes(vms); create_uart(vms, VIRT_UART, sysmem, serial_hd(0)); From 946f1bb18c342fa548f9c0f52f64836ff49d99c8 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Thu, 1 Oct 2020 08:17:16 +0200 Subject: [PATCH 09/12] hw/arm/virt: Move kvm pmu setup to virt_cpu_post_init Move the KVM PMU setup part of fdt_add_pmu_nodes() to virt_cpu_post_init(), which is a more appropriate location. Now fdt_add_pmu_nodes() is also named more appropriately, because it no longer does anything but fdt node creation. No functional change intended. Reviewed-by: Peter Maydell Reviewed-by: Eric Auger Signed-off-by: Andrew Jones Message-id: 20201001061718.101915-5-drjones@redhat.com Signed-off-by: Peter Maydell --- hw/arm/virt.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 524eafe22d..92ab0fd094 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -521,21 +521,12 @@ static void fdt_add_gic_node(VirtMachineState *vms) static void fdt_add_pmu_nodes(const VirtMachineState *vms) { - CPUState *cpu; - ARMCPU *armcpu; + ARMCPU *armcpu = ARM_CPU(first_cpu); uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI; - CPU_FOREACH(cpu) { - armcpu = ARM_CPU(cpu); - if (!arm_feature(&armcpu->env, ARM_FEATURE_PMU)) { - return; - } - if (kvm_enabled()) { - if (kvm_irqchip_in_kernel()) { - kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ)); - } - kvm_arm_pmu_init(cpu); - } + if (!arm_feature(&armcpu->env, ARM_FEATURE_PMU)) { + assert(!object_property_get_bool(OBJECT(armcpu), "pmu", NULL)); + return; } if (vms->gic_version == VIRT_GIC_VERSION_2) { @@ -544,7 +535,6 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms) (1 << vms->smp_cpus) - 1); } - armcpu = ARM_CPU(qemu_get_cpu(0)); qemu_fdt_add_subnode(vms->fdt, "/pmu"); if (arm_feature(&armcpu->env, ARM_FEATURE_V8)) { const char compat[] = "arm,armv8-pmuv3"; @@ -1678,11 +1668,23 @@ static void finalize_gic_version(VirtMachineState *vms) */ static void virt_cpu_post_init(VirtMachineState *vms) { - bool aarch64; + bool aarch64, pmu; + CPUState *cpu; aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL); + pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL); - if (!kvm_enabled()) { + if (kvm_enabled()) { + CPU_FOREACH(cpu) { + if (pmu) { + assert(arm_feature(&ARM_CPU(cpu)->env, ARM_FEATURE_PMU)); + if (kvm_irqchip_in_kernel()) { + kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ)); + } + kvm_arm_pmu_init(cpu); + } + } + } else { if (aarch64 && vms->highmem) { int requested_pa_size = 64 - clz64(vms->highest_gpa); int pamax = arm_pamax(ARM_CPU(first_cpu)); From 05889d15d1c95163de917800cf0e1bf6faab1bc7 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Thu, 1 Oct 2020 08:17:17 +0200 Subject: [PATCH 10/12] tests/qtest: Restore aarch64 arm-cpu-features test arm-cpu-features got dropped from the AArch64 tests during the meson conversion shuffle. Signed-off-by: Andrew Jones Message-id: 20201001061718.101915-6-drjones@redhat.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- tests/qtest/meson.build | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index ad33ac311d..0f32ca0895 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -146,7 +146,8 @@ qtests_aarch64 = \ (cpu != 'arm' ? ['bios-tables-test'] : []) + \ (config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ? ['tpm-tis-device-test'] : []) + \ (config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ? ['tpm-tis-device-swtpm-test'] : []) + \ - ['numa-test', + ['arm-cpu-features', + 'numa-test', 'boot-serial-test', 'migration-test'] From 68970d1e0d07e3a266141bbd9038fd9890ca88f2 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Thu, 1 Oct 2020 08:17:18 +0200 Subject: [PATCH 11/12] hw/arm/virt: Implement kvm-steal-time We add the kvm-steal-time CPU property and implement it for machvirt. A tiny bit of refactoring was also done to allow pmu and pvtime to use the same vcpu device helper functions. Reviewed-by: Eric Auger Signed-off-by: Andrew Jones Message-id: 20201001061718.101915-7-drjones@redhat.com Signed-off-by: Peter Maydell --- docs/system/arm/cpu-features.rst | 11 ++++++ hw/arm/virt.c | 44 ++++++++++++++++++++-- include/hw/arm/virt.h | 5 +++ target/arm/cpu.c | 8 ++++ target/arm/cpu.h | 4 ++ target/arm/kvm.c | 16 ++++++++ target/arm/kvm64.c | 64 +++++++++++++++++++++++++++++--- target/arm/kvm_arm.h | 43 +++++++++++++++++++++ target/arm/monitor.c | 2 +- tests/qtest/arm-cpu-features.c | 25 +++++++++++-- 10 files changed, 209 insertions(+), 13 deletions(-) diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst index 2d5c06cd01..35196a6b75 100644 --- a/docs/system/arm/cpu-features.rst +++ b/docs/system/arm/cpu-features.rst @@ -200,6 +200,17 @@ the list of KVM VCPU features and their descriptions. adjustment, also restoring the legacy (pre-5.0) behavior. + kvm-steal-time Since v5.2, kvm-steal-time is enabled by + default when KVM is enabled, the feature is + supported, and the guest is 64-bit. + + When kvm-steal-time is enabled a 64-bit guest + can account for time its CPUs were not running + due to the host not scheduling the corresponding + VCPU threads. The accounting statistics may + influence the guest scheduler behavior and/or be + exposed to the guest userspace. + SVE CPU Properties ================== diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 92ab0fd094..e465a988d6 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -151,6 +151,7 @@ static const MemMapEntry base_memmap[] = { [VIRT_PCDIMM_ACPI] = { 0x09070000, MEMORY_HOTPLUG_IO_LEN }, [VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN }, [VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN}, + [VIRT_PVTIME] = { 0x090a0000, 0x00010000 }, [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 }, @@ -1666,15 +1667,40 @@ static void finalize_gic_version(VirtMachineState *vms) * virt_cpu_post_init() must be called after the CPUs have * been realized and the GIC has been created. */ -static void virt_cpu_post_init(VirtMachineState *vms) +static void virt_cpu_post_init(VirtMachineState *vms, int max_cpus, + MemoryRegion *sysmem) { - bool aarch64, pmu; + bool aarch64, pmu, steal_time; CPUState *cpu; aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL); pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL); + steal_time = object_property_get_bool(OBJECT(first_cpu), + "kvm-steal-time", NULL); if (kvm_enabled()) { + hwaddr pvtime_reg_base = vms->memmap[VIRT_PVTIME].base; + hwaddr pvtime_reg_size = vms->memmap[VIRT_PVTIME].size; + + if (steal_time) { + MemoryRegion *pvtime = g_new(MemoryRegion, 1); + hwaddr pvtime_size = max_cpus * PVTIME_SIZE_PER_CPU; + + /* The memory region size must be a multiple of host page size. */ + pvtime_size = REAL_HOST_PAGE_ALIGN(pvtime_size); + + if (pvtime_size > pvtime_reg_size) { + error_report("pvtime requires a %" HWADDR_PRId + " byte memory region for %d CPUs," + " but only %" HWADDR_PRId " has been reserved", + pvtime_size, max_cpus, pvtime_reg_size); + exit(1); + } + + memory_region_init_ram(pvtime, NULL, "pvtime", pvtime_size, NULL); + memory_region_add_subregion(sysmem, pvtime_reg_base, pvtime); + } + CPU_FOREACH(cpu) { if (pmu) { assert(arm_feature(&ARM_CPU(cpu)->env, ARM_FEATURE_PMU)); @@ -1683,6 +1709,10 @@ static void virt_cpu_post_init(VirtMachineState *vms) } kvm_arm_pmu_init(cpu); } + if (steal_time) { + kvm_arm_pvtime_init(cpu, pvtime_reg_base + + cpu->cpu_index * PVTIME_SIZE_PER_CPU); + } } } else { if (aarch64 && vms->highmem) { @@ -1853,6 +1883,11 @@ static void machvirt_init(MachineState *machine) object_property_set_bool(cpuobj, "kvm-no-adjvtime", true, NULL); } + if (vmc->no_kvm_steal_time && + object_property_find(cpuobj, "kvm-steal-time")) { + object_property_set_bool(cpuobj, "kvm-steal-time", false, NULL); + } + if (vmc->no_pmu && object_property_find(cpuobj, "pmu")) { object_property_set_bool(cpuobj, "pmu", false, NULL); } @@ -1924,7 +1959,7 @@ static void machvirt_init(MachineState *machine) create_gic(vms); - virt_cpu_post_init(vms); + virt_cpu_post_init(vms, possible_cpus->len, sysmem); fdt_add_pmu_nodes(vms); @@ -2566,8 +2601,11 @@ DEFINE_VIRT_MACHINE_AS_LATEST(5, 2) static void virt_machine_5_1_options(MachineClass *mc) { + VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc)); + virt_machine_5_2_options(mc); compat_props_add(mc->compat_props, hw_compat_5_1, hw_compat_5_1_len); + vmc->no_kvm_steal_time = true; } DEFINE_VIRT_MACHINE(5, 1) diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index 655b895d5e..aad6d69841 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -54,6 +54,9 @@ #define PPI(irq) ((irq) + 16) +/* See Linux kernel arch/arm64/include/asm/pvclock-abi.h */ +#define PVTIME_SIZE_PER_CPU 64 + enum { VIRT_FLASH, VIRT_MEM, @@ -81,6 +84,7 @@ enum { VIRT_PCDIMM_ACPI, VIRT_ACPI_GED, VIRT_NVDIMM_ACPI, + VIRT_PVTIME, VIRT_LOWMEMMAP_LAST, }; @@ -121,6 +125,7 @@ struct VirtMachineClass { bool no_highmem_ecam; bool no_ged; /* Machines < 4.2 has no support for ACPI GED device */ bool kvm_no_adjvtime; + bool no_kvm_steal_time; bool acpi_expose_flash; }; diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 858c5a4bcb..056319859f 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -1310,6 +1310,14 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp) return; } } + + if (kvm_enabled()) { + kvm_arm_steal_time_finalize(cpu, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); + return; + } + } } static void arm_cpu_realizefn(DeviceState *dev, Error **errp) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index e4549a8cc0..cfff1b5c8f 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -24,6 +24,7 @@ #include "hw/registerfields.h" #include "cpu-qom.h" #include "exec/cpu-defs.h" +#include "qapi/qapi-types-common.h" /* ARM processors have a weak memory model */ #define TCG_GUEST_DEFAULT_MO (0) @@ -863,6 +864,9 @@ struct ARMCPU { bool kvm_vtime_dirty; uint64_t kvm_vtime; + /* KVM steal time */ + OnOffAuto kvm_steal_time; + /* Uniprocessor system with MP extensions */ bool mp_is_up; diff --git a/target/arm/kvm.c b/target/arm/kvm.c index 0dcb9bfe13..ffe186de8d 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -192,6 +192,16 @@ static void kvm_no_adjvtime_set(Object *obj, bool value, Error **errp) ARM_CPU(obj)->kvm_adjvtime = !value; } +static bool kvm_steal_time_get(Object *obj, Error **errp) +{ + return ARM_CPU(obj)->kvm_steal_time != ON_OFF_AUTO_OFF; +} + +static void kvm_steal_time_set(Object *obj, bool value, Error **errp) +{ + ARM_CPU(obj)->kvm_steal_time = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; +} + /* KVM VCPU properties should be prefixed with "kvm-". */ void kvm_arm_add_vcpu_properties(Object *obj) { @@ -207,6 +217,12 @@ void kvm_arm_add_vcpu_properties(Object *obj) "the virtual counter. VM stopped time " "will be counted."); } + + cpu->kvm_steal_time = ON_OFF_AUTO_AUTO; + object_property_add_bool(obj, "kvm-steal-time", kvm_steal_time_get, + kvm_steal_time_set); + object_property_set_description(obj, "kvm-steal-time", + "Set off to disable KVM steal time."); } bool kvm_arm_pmu_supported(void) diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index fae07c3f04..f74bac2457 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -17,6 +17,7 @@ #include #include "qemu-common.h" +#include "qapi/error.h" #include "cpu.h" #include "qemu/timer.h" #include "qemu/error-report.h" @@ -397,19 +398,20 @@ static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr) return NULL; } -static bool kvm_arm_pmu_set_attr(CPUState *cs, struct kvm_device_attr *attr) +static bool kvm_arm_set_device_attr(CPUState *cs, struct kvm_device_attr *attr, + const char *name) { int err; err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr); if (err != 0) { - error_report("PMU: KVM_HAS_DEVICE_ATTR: %s", strerror(-err)); + error_report("%s: KVM_HAS_DEVICE_ATTR: %s", name, strerror(-err)); return false; } err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr); if (err != 0) { - error_report("PMU: KVM_SET_DEVICE_ATTR: %s", strerror(-err)); + error_report("%s: KVM_SET_DEVICE_ATTR: %s", name, strerror(-err)); return false; } @@ -426,7 +428,7 @@ void kvm_arm_pmu_init(CPUState *cs) if (!ARM_CPU(cs)->has_pmu) { return; } - if (!kvm_arm_pmu_set_attr(cs, &attr)) { + if (!kvm_arm_set_device_attr(cs, &attr, "PMU")) { error_report("failed to init PMU"); abort(); } @@ -443,12 +445,29 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq) if (!ARM_CPU(cs)->has_pmu) { return; } - if (!kvm_arm_pmu_set_attr(cs, &attr)) { + if (!kvm_arm_set_device_attr(cs, &attr, "PMU")) { error_report("failed to set irq for PMU"); abort(); } } +void kvm_arm_pvtime_init(CPUState *cs, uint64_t ipa) +{ + struct kvm_device_attr attr = { + .group = KVM_ARM_VCPU_PVTIME_CTRL, + .attr = KVM_ARM_VCPU_PVTIME_IPA, + .addr = (uint64_t)&ipa, + }; + + if (ARM_CPU(cs)->kvm_steal_time == ON_OFF_AUTO_OFF) { + return; + } + if (!kvm_arm_set_device_attr(cs, &attr, "PVTIME IPA")) { + error_report("failed to init PVTIME IPA"); + abort(); + } +} + static int read_sys_reg32(int fd, uint32_t *pret, uint64_t id) { uint64_t ret; @@ -655,6 +674,36 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) return true; } +void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp) +{ + bool has_steal_time = kvm_arm_steal_time_supported(); + + if (cpu->kvm_steal_time == ON_OFF_AUTO_AUTO) { + if (!has_steal_time || !arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { + cpu->kvm_steal_time = ON_OFF_AUTO_OFF; + } else { + cpu->kvm_steal_time = ON_OFF_AUTO_ON; + } + } else if (cpu->kvm_steal_time == ON_OFF_AUTO_ON) { + if (!has_steal_time) { + error_setg(errp, "'kvm-steal-time' cannot be enabled " + "on this host"); + return; + } else if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { + /* + * DEN0057A chapter 2 says "This specification only covers + * systems in which the Execution state of the hypervisor + * as well as EL1 of virtual machines is AArch64.". And, + * to ensure that, the smc/hvc calls are only specified as + * smc64/hvc64. + */ + error_setg(errp, "'kvm-steal-time' cannot be enabled " + "for AArch32 guests"); + return; + } + } +} + bool kvm_arm_aarch32_supported(void) { return kvm_check_extension(kvm_state, KVM_CAP_ARM_EL1_32BIT); @@ -665,6 +714,11 @@ bool kvm_arm_sve_supported(void) return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE); } +bool kvm_arm_steal_time_supported(void) +{ + return kvm_check_extension(kvm_state, KVM_CAP_STEAL_TIME); +} + QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1); void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map) diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index f513702176..eb81b7059e 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -267,6 +267,24 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu); */ void kvm_arm_add_vcpu_properties(Object *obj); +/** + * kvm_arm_steal_time_finalize: + * @cpu: ARMCPU for which to finalize kvm-steal-time + * @errp: Pointer to Error* for error propagation + * + * Validate the kvm-steal-time property selection and set its default + * based on KVM support and guest configuration. + */ +void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp); + +/** + * kvm_arm_steal_time_supported: + * + * Returns: true if KVM can enable steal time reporting + * and false otherwise. + */ +bool kvm_arm_steal_time_supported(void); + /** * kvm_arm_aarch32_supported: * @@ -340,6 +358,16 @@ int kvm_arm_vgic_probe(void); void kvm_arm_pmu_set_irq(CPUState *cs, int irq); void kvm_arm_pmu_init(CPUState *cs); + +/** + * kvm_arm_pvtime_init: + * @cs: CPUState + * @ipa: Per-vcpu guest physical base address of the pvtime structures + * + * Initializes PVTIME for the VCPU, setting the PVTIME IPA to @ipa. + */ +void kvm_arm_pvtime_init(CPUState *cs, uint64_t ipa); + int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level); #else @@ -363,6 +391,11 @@ static inline bool kvm_arm_sve_supported(void) return false; } +static inline bool kvm_arm_steal_time_supported(void) +{ + return false; +} + /* * These functions should never actually be called without KVM support. */ @@ -396,6 +429,16 @@ static inline void kvm_arm_pmu_init(CPUState *cs) g_assert_not_reached(); } +static inline void kvm_arm_pvtime_init(CPUState *cs, uint64_t ipa) +{ + g_assert_not_reached(); +} + +static inline void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp) +{ + g_assert_not_reached(); +} + static inline void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map) { g_assert_not_reached(); diff --git a/target/arm/monitor.c b/target/arm/monitor.c index 375f34bfaa..169d8a64b6 100644 --- a/target/arm/monitor.c +++ b/target/arm/monitor.c @@ -103,7 +103,7 @@ static const char *cpu_model_advertised_features[] = { "sve128", "sve256", "sve384", "sve512", "sve640", "sve768", "sve896", "sve1024", "sve1152", "sve1280", "sve1408", "sve1536", "sve1664", "sve1792", "sve1920", "sve2048", - "kvm-no-adjvtime", + "kvm-no-adjvtime", "kvm-steal-time", NULL }; diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c index 77b5e30a9c..d20094d5a7 100644 --- a/tests/qtest/arm-cpu-features.c +++ b/tests/qtest/arm-cpu-features.c @@ -452,6 +452,7 @@ static void test_query_cpu_model_expansion(const void *data) assert_set_feature(qts, "max", "pmu", true); assert_has_not_feature(qts, "max", "kvm-no-adjvtime"); + assert_has_not_feature(qts, "max", "kvm-steal-time"); if (g_str_equal(qtest_get_arch(), "aarch64")) { assert_has_feature_enabled(qts, "max", "aarch64"); @@ -493,6 +494,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data) assert_set_feature(qts, "host", "kvm-no-adjvtime", false); if (g_str_equal(qtest_get_arch(), "aarch64")) { + bool kvm_supports_steal_time; bool kvm_supports_sve; char max_name[8], name[8]; uint32_t max_vq, vq; @@ -500,6 +502,10 @@ static void test_query_cpu_model_expansion_kvm(const void *data) QDict *resp; char *error; + assert_error(qts, "cortex-a15", + "We cannot guarantee the CPU type 'cortex-a15' works " + "with KVM on this host", NULL); + assert_has_feature_enabled(qts, "host", "aarch64"); /* Enabling and disabling pmu should always work. */ @@ -507,16 +513,26 @@ static void test_query_cpu_model_expansion_kvm(const void *data) assert_set_feature(qts, "host", "pmu", false); assert_set_feature(qts, "host", "pmu", true); - assert_error(qts, "cortex-a15", - "We cannot guarantee the CPU type 'cortex-a15' works " - "with KVM on this host", NULL); - + /* + * Some features would be enabled by default, but they're disabled + * because this instance of KVM doesn't support them. Test that the + * features are present, and, when enabled, issue further tests. + */ + assert_has_feature(qts, "host", "kvm-steal-time"); assert_has_feature(qts, "host", "sve"); + resp = do_query_no_props(qts, "host"); + kvm_supports_steal_time = resp_get_feature(resp, "kvm-steal-time"); kvm_supports_sve = resp_get_feature(resp, "sve"); vls = resp_get_sve_vls(resp); qobject_unref(resp); + if (kvm_supports_steal_time) { + /* If we have steal-time then we should be able to toggle it. */ + assert_set_feature(qts, "host", "kvm-steal-time", false); + assert_set_feature(qts, "host", "kvm-steal-time", true); + } + if (kvm_supports_sve) { g_assert(vls != 0); max_vq = 64 - __builtin_clzll(vls); @@ -577,6 +593,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data) assert_has_not_feature(qts, "host", "aarch64"); assert_has_not_feature(qts, "host", "pmu"); assert_has_not_feature(qts, "host", "sve"); + assert_has_not_feature(qts, "host", "kvm-steal-time"); } qtest_quit(qts); From d1b6b7017572e8d82f26eb827a1dba0e8cf3cae6 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 1 Oct 2020 17:01:16 +0100 Subject: [PATCH 12/12] target/arm: Make '-cpu max' have a 48-bit PA QEMU supports a 48-bit physical address range, but we don't currently expose it in the '-cpu max' ID registers (you get the same range as Cortex-A57, which is 44 bits). Set the ID_AA64MMFR0.PARange field to indicate 48 bits. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20201001160116.18095-1-peter.maydell@linaro.org --- target/arm/cpu64.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index e00271b932..649213082f 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -653,6 +653,10 @@ static void aarch64_max_initfn(Object *obj) t = FIELD_DP64(t, ID_AA64PFR1, MTE, 2); cpu->isar.id_aa64pfr1 = t; + t = cpu->isar.id_aa64mmfr0; + t = FIELD_DP64(t, ID_AA64MMFR0, PARANGE, 5); /* PARange: 48 bits */ + cpu->isar.id_aa64mmfr0 = t; + t = cpu->isar.id_aa64mmfr1; t = FIELD_DP64(t, ID_AA64MMFR1, HPDS, 1); /* HPD */ t = FIELD_DP64(t, ID_AA64MMFR1, LO, 1);