From 3bf0844f3be77b24cc8f56fc8df9ff199f8324cb Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 21 May 2021 18:07:35 +0200 Subject: spapr: Don't hijack current_machine->boot_order QEMU 6.0 moved all the -boot variables to the machine. Especially, the removal of the boot_order static changed the handling of '-boot once' from: if (boot_once) { qemu_boot_set(boot_once, &error_fatal); qemu_register_reset(restore_boot_order, g_strdup(boot_order)); } to if (current_machine->boot_once) { qemu_boot_set(current_machine->boot_once, &error_fatal); qemu_register_reset(restore_boot_order, g_strdup(current_machine->boot_order)); } This means that we now register as subsequent boot order a copy of current_machine->boot_once that was just set with the previous call to qemu_boot_set(), i.e. we never transition away from the once boot order. It is certainly fragile^Wwrong for the spapr code to hijack a field of the base machine type object like that. The boot order rework simply turned this software boundary violation into an actual bug. Have the spapr code to handle that with its own field in SpaprMachineState. Also kfree() the initial boot device string when "once" was used. Fixes: 4b7acd2ac821 ("vl: clean up -boot variables") Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1960119 Cc: pbonzini@redhat.com Signed-off-by: Greg Kurz Message-Id: <20210521160735.1901914-1-groug@kaod.org> Signed-off-by: David Gibson --- hw/ppc/spapr.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index c23bcc4..4dd90b7 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1005,7 +1005,7 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt, bool reset) _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen")); if (reset) { - const char *boot_device = machine->boot_order; + const char *boot_device = spapr->boot_device; char *stdout_path = spapr_vio_stdout_path(spapr->vio_bus); size_t cb = 0; char *bootlist = get_boot_devices_list(&cb); @@ -2376,8 +2376,10 @@ static SaveVMHandlers savevm_htab_handlers = { static void spapr_boot_set(void *opaque, const char *boot_device, Error **errp) { - MachineState *machine = MACHINE(opaque); - machine->boot_order = g_strdup(boot_device); + SpaprMachineState *spapr = SPAPR_MACHINE(opaque); + + g_free(spapr->boot_device); + spapr->boot_device = g_strdup(boot_device); } static void spapr_create_lmb_dr_connectors(SpaprMachineState *spapr) -- cgit v1.1 From ac9ef668321ebb6eb871a0c4dd380fa7d7891b4e Mon Sep 17 00:00:00 2001 From: Mahesh Salgaonkar Date: Fri, 21 May 2021 13:35:51 +0530 Subject: spapr: Fix EEH capability issue on KVM guest for PCI passthru With upstream kernel, especially after commit 98ba956f6a389 ("powerpc/pseries/eeh: Rework device EEH PE determination") we see that KVM guest isn't able to enable EEH option for PCI pass-through devices anymore. [root@atest-guest ~]# dmesg | grep EEH [ 0.032337] EEH: pSeries platform initialized [ 0.298207] EEH: No capable adapters found: recovery disabled. [root@atest-guest ~]# So far the linux kernel was assuming pe_config_addr equal to device's config_addr and using it to enable EEH on the PE through ibm,set-eeh-option RTAS call. Which wasn't the correct way as per PAPR. The linux kernel commit 98ba956f6a389 fixed this flow. With that fixed, linux now uses PE config address returned by ibm,get-config-addr-info2 RTAS call to enable EEH option per-PE basis instead of per-device basis. However this has uncovered a bug in qemu where ibm,set-eeh-option is treating PE config address as per-device config address. Hence in qemu guest with recent kernel the ibm,set-eeh-option RTAS call fails with -3 return value indicating that there is no PCI device exist for the specified PE config address. The rtas_ibm_set_eeh_option call uses pci_find_device() to get the PC device that matches specific bus and devfn extracted from PE config address passed as argument. Thus it tries to map the PE config address to a single specific PCI device 'bus->devices[devfn]' which always results into checking device on slot 0 'bus->devices[0]'. This succeeds when there is a pass-through device (vfio-pci) present on slot 0. But in cases where there is no pass-through device present in slot 0, but present in non-zero slots, ibm,set-eeh-option call fails to enable the EEH capability. hw/ppc/spapr_pci_vfio.c: spapr_phb_vfio_eeh_set_option() case RTAS_EEH_ENABLE: { PCIHostState *phb; PCIDevice *pdev; /* * The EEH functionality is enabled on basis of PCI device, * instead of PE. We need check the validity of the PCI * device address. */ phb = PCI_HOST_BRIDGE(sphb); pdev = pci_find_device(phb->bus, (addr >> 16) & 0xFF, (addr >> 8) & 0xFF); if (!pdev || !object_dynamic_cast(OBJECT(pdev), "vfio-pci")) { return RTAS_OUT_PARAM_ERROR; } hw/pci/pci.c:pci_find_device() PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn) { bus = pci_find_bus_nr(bus, bus_num); if (!bus) return NULL; return bus->devices[devfn]; } This patch fixes ibm,set-eeh-option to check for presence of any PCI device (vfio-pci) under specified bus and enable the EEH if found. The current code already makes sure that all the devices on that bus are from same iommu group (within same PE) and fail very early if it does not. After this fix guest is able to find EEH capable devices and enable EEH recovery on it. [root@atest-guest ~]# dmesg | grep EEH [ 0.048139] EEH: pSeries platform initialized [ 0.405115] EEH: Capable adapter found: recovery enabled. [root@atest-guest ~]# Reviewed-by: Daniel Henrique Barboza Signed-off-by: Mahesh Salgaonkar Message-Id: <162158429107.145117.5843504911924013125.stgit@jupiter> Signed-off-by: David Gibson --- hw/ppc/spapr_pci_vfio.c | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c index 7817cf7..f3b37df 100644 --- a/hw/ppc/spapr_pci_vfio.c +++ b/hw/ppc/spapr_pci_vfio.c @@ -46,6 +46,16 @@ void spapr_phb_vfio_reset(DeviceState *qdev) spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev)); } +static void spapr_eeh_pci_find_device(PCIBus *bus, PCIDevice *pdev, + void *opaque) +{ + bool *found = opaque; + + if (object_dynamic_cast(OBJECT(pdev), "vfio-pci")) { + *found = true; + } +} + int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb, unsigned int addr, int option) { @@ -58,17 +68,33 @@ int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb, break; case RTAS_EEH_ENABLE: { PCIHostState *phb; - PCIDevice *pdev; + bool found = false; /* - * The EEH functionality is enabled on basis of PCI device, - * instead of PE. We need check the validity of the PCI - * device address. + * The EEH functionality is enabled per sphb level instead of + * per PCI device. We have already identified this specific sphb + * based on buid passed as argument to ibm,set-eeh-option rtas + * call. Now we just need to check the validity of the PCI + * pass-through devices (vfio-pci) under this sphb bus. + * We have already validated that all the devices under this sphb + * are from same iommu group (within same PE) before comming here. + * + * Prior to linux commit 98ba956f6a389 ("powerpc/pseries/eeh: + * Rework device EEH PE determination") kernel would call + * eeh-set-option for each device in the PE using the device's + * config_address as the argument rather than the PE address. + * Hence if we check validity of supplied config_addr whether + * it matches to this PHB will cause issues with older kernel + * versions v5.9 and older. If we return an error from + * eeh-set-option when the argument isn't a valid PE address + * then older kernels (v5.9 and older) will interpret that as + * EEH not being supported. */ phb = PCI_HOST_BRIDGE(sphb); - pdev = pci_find_device(phb->bus, - (addr >> 16) & 0xFF, (addr >> 8) & 0xFF); - if (!pdev || !object_dynamic_cast(OBJECT(pdev), "vfio-pci")) { + pci_for_each_device(phb->bus, (addr >> 16) & 0xFF, + spapr_eeh_pci_find_device, &found); + + if (!found) { return RTAS_OUT_PARAM_ERROR; } -- cgit v1.1 From f93c8f148c0f6c2e20c29c54276862ee79a53d02 Mon Sep 17 00:00:00 2001 From: Shivaprasad G Bhat Date: Tue, 18 May 2021 08:03:17 -0400 Subject: spapr: nvdimm: Forward declare and move the definitions The subsequent patches add definitions which tend to get the compilation to cyclic dependency. So, prepare with forward declarations, move the definitions and clean up. Signed-off-by: Shivaprasad G Bhat Message-Id: <162133925415.610.11584121797866216417.stgit@4f1e6f2bd33e> Signed-off-by: David Gibson --- hw/ppc/spapr_nvdimm.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'hw') diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c index 252204e..3f57a8b 100644 --- a/hw/ppc/spapr_nvdimm.c +++ b/hw/ppc/spapr_nvdimm.c @@ -35,6 +35,18 @@ /* SCM device is unable to persist memory contents */ #define PAPR_PMEM_UNARMED PPC_BIT(0) +/* + * The nvdimm size should be aligned to SCM block size. + * The SCM block size should be aligned to SPAPR_MEMORY_BLOCK_SIZE + * in order to have SCM regions not to overlap with dimm memory regions. + * The SCM devices can have variable block sizes. For now, fixing the + * block size to the minimum value. + */ +#define SPAPR_MINIMUM_SCM_BLOCK_SIZE SPAPR_MEMORY_BLOCK_SIZE + +/* Have an explicit check for alignment */ +QEMU_BUILD_BUG_ON(SPAPR_MINIMUM_SCM_BLOCK_SIZE % SPAPR_MEMORY_BLOCK_SIZE); + bool spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm, uint64_t size, Error **errp) { -- cgit v1.1 From 9f9f82dacebbb816c62d730658f14a615c3ea003 Mon Sep 17 00:00:00 2001 From: Shivaprasad G Bhat Date: Wed, 26 May 2021 11:27:15 -0400 Subject: spapr: nvdimm: Fix the persistent-memory root node name in device tree The FDT code is adding the pmem root node by name "persistent-memory" which should have been "ibm,persistent-memory". The linux fetches the device tree nodes by type and it has been working correctly as the type is correct. If someone searches by its intended name it would fail, so fix that. Reported-by: Aneesh Kumar K.V Signed-off-by: Shivaprasad G Bhat Message-Id: <162204278956.219.9061511386011411578.stgit@cc493db1e665> Signed-off-by: David Gibson --- hw/ppc/spapr_nvdimm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c index 3f57a8b..91de105 100644 --- a/hw/ppc/spapr_nvdimm.c +++ b/hw/ppc/spapr_nvdimm.c @@ -175,11 +175,11 @@ int spapr_pmem_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr, void spapr_dt_persistent_memory(SpaprMachineState *spapr, void *fdt) { - int offset = fdt_subnode_offset(fdt, 0, "persistent-memory"); + int offset = fdt_subnode_offset(fdt, 0, "ibm,persistent-memory"); GSList *iter, *nvdimms = nvdimm_get_device_list(); if (offset < 0) { - offset = fdt_add_subnode(fdt, 0, "persistent-memory"); + offset = fdt_add_subnode(fdt, 0, "ibm,persistent-memory"); _FDT(offset); _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 0x1))); _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 0x0))); -- cgit v1.1 From 7be3bf6c8429969f97728bb712d9a99997835607 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Wed, 26 May 2021 19:16:24 +1000 Subject: spapr: Remove stale comment about power-saving LPCR bits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 47a9b551547 ("spapr: Clean up handling of LPCR power-saving exit bits") moved this logic but did not remove the comment from the previous location. Signed-off-by: Nicholas Piggin Message-Id: <20210526091626.3388262-2-npiggin@gmail.com> Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz Signed-off-by: David Gibson --- hw/ppc/spapr_rtas.c | 1 - 1 file changed, 1 deletion(-) (limited to 'hw') diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c index 03355b4..63d9695 100644 --- a/hw/ppc/spapr_rtas.c +++ b/hw/ppc/spapr_rtas.c @@ -164,7 +164,6 @@ static void rtas_start_cpu(PowerPCCPU *callcpu, SpaprMachineState *spapr, env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME); hreg_compute_hflags(env); - /* Enable Power-saving mode Exit Cause exceptions for the new CPU */ lpcr = env->spr[SPR_LPCR]; if (!pcc->interrupts_big_endian(callcpu)) { lpcr |= LPCR_ILE; -- cgit v1.1 From ac559ecbea2649819e7b3fdd09f4e0243e0128db Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Wed, 26 May 2021 19:16:25 +1000 Subject: spapr: Set LPCR to current AIL mode when starting a new CPU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TCG does not keep track of AIL mode in a central place, it's based on the current LPCR[AIL] bits. Synchronize the new CPU's LPCR to the current LPCR in rtas_start_cpu(), similarly to the way the ILE bit is synchronized. Open-code the ILE setting as well now that the caller's LPCR is available directly, there is no need for the indirection. Without this, under both TCG and KVM, adding a POWER8/9/10 class CPU with a new core ID after a modern Linux has booted results in the new CPU's LPCR missing the LPCR[AIL]=0b11 setting that the other CPUs have. This can cause crashes and unexpected behaviour. Signed-off-by: Nicholas Piggin Message-Id: <20210526091626.3388262-3-npiggin@gmail.com> Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz Signed-off-by: David Gibson --- hw/ppc/spapr_rtas.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c index 63d9695..b476382 100644 --- a/hw/ppc/spapr_rtas.c +++ b/hw/ppc/spapr_rtas.c @@ -132,8 +132,8 @@ static void rtas_start_cpu(PowerPCCPU *callcpu, SpaprMachineState *spapr, target_ulong id, start, r3; PowerPCCPU *newcpu; CPUPPCState *env; - PowerPCCPUClass *pcc; target_ulong lpcr; + target_ulong caller_lpcr; if (nargs != 3 || nret != 1) { rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); @@ -152,7 +152,6 @@ static void rtas_start_cpu(PowerPCCPU *callcpu, SpaprMachineState *spapr, } env = &newcpu->env; - pcc = POWERPC_CPU_GET_CLASS(newcpu); if (!CPU(newcpu)->halted) { rtas_st(rets, 0, RTAS_OUT_HW_ERROR); @@ -164,10 +163,15 @@ static void rtas_start_cpu(PowerPCCPU *callcpu, SpaprMachineState *spapr, env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME); hreg_compute_hflags(env); + caller_lpcr = callcpu->env.spr[SPR_LPCR]; lpcr = env->spr[SPR_LPCR]; - if (!pcc->interrupts_big_endian(callcpu)) { - lpcr |= LPCR_ILE; - } + + /* Set ILE the same way */ + lpcr = (lpcr & ~LPCR_ILE) | (caller_lpcr & LPCR_ILE); + + /* Set AIL the same way */ + lpcr = (lpcr & ~LPCR_AIL) | (caller_lpcr & LPCR_AIL); + if (env->mmu_model == POWERPC_MMU_3_00) { /* * New cpus are expected to start in the same radix/hash mode -- cgit v1.1 From b873ed83311d96644b544b10f6869a430660585a Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Fri, 28 May 2021 17:16:19 -0300 Subject: ppc/pef.c: initialize cgs->ready in kvmppc_svm_init() QEMU is failing to launch a CGS pSeries guest in a host that has PEF support: qemu-system-ppc64: ../softmmu/vl.c:2585: qemu_machine_creation_done: Assertion `machine->cgs->ready' failed. Aborted This is happening because we're not setting the cgs->ready flag that is asserted in qemu_machine_creation_done() during machine start. cgs->ready is set in s390_pv_kvm_init() and sev_kvm_init(). Let's set it in kvmppc_svm_init() as well. Reported-by: Ram Pai Signed-off-by: Daniel Henrique Barboza Message-Id: <20210528201619.52363-1-danielhb413@gmail.com> Acked-by: Ram Pai Signed-off-by: David Gibson --- hw/ppc/pef.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/ppc/pef.c b/hw/ppc/pef.c index 573be3e..cc44d5e 100644 --- a/hw/ppc/pef.c +++ b/hw/ppc/pef.c @@ -41,7 +41,7 @@ struct PefGuest { ConfidentialGuestSupport parent_obj; }; -static int kvmppc_svm_init(Error **errp) +static int kvmppc_svm_init(ConfidentialGuestSupport *cgs, Error **errp) { #ifdef CONFIG_KVM static Error *pef_mig_blocker; @@ -65,6 +65,8 @@ static int kvmppc_svm_init(Error **errp) /* NB: This can fail if --only-migratable is used */ migrate_add_blocker(pef_mig_blocker, &error_fatal); + cgs->ready = true; + return 0; #else g_assert_not_reached(); @@ -102,7 +104,7 @@ int pef_kvm_init(ConfidentialGuestSupport *cgs, Error **errp) return -1; } - return kvmppc_svm_init(errp); + return kvmppc_svm_init(cgs, errp); } int pef_kvm_reset(ConfidentialGuestSupport *cgs, Error **errp) -- cgit v1.1 From 78d6c4c33d872c6790f8115b2bf5b0a00d710c22 Mon Sep 17 00:00:00 2001 From: "Bruno Larsen (billionai)" Date: Mon, 31 May 2021 11:56:26 -0300 Subject: hw/core/cpu: removed cpu_dump_statistics function No more architectures set the pointer to dump_statistics, so there's no point in keeping it, or the related cpu_dump_statistics function. Suggested-by: Richard Henderson Signed-off-by: Bruno Larsen (billionai) Message-Id: <20210526202104.127910-6-bruno.larsen@eldorado.org.br> Reviewed-by: Richard Henderson Reviewed-by: Luis Pires Signed-off-by: David Gibson Message-Id: <20210531145629.21300-2-bruno.larsen@eldorado.org.br> Acked-by: Eduardo Habkost Signed-off-by: David Gibson --- hw/core/cpu-common.c | 9 --------- 1 file changed, 9 deletions(-) (limited to 'hw') diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c index 9530e26..e2f5a64 100644 --- a/hw/core/cpu-common.c +++ b/hw/core/cpu-common.c @@ -109,15 +109,6 @@ void cpu_dump_state(CPUState *cpu, FILE *f, int flags) } } -void cpu_dump_statistics(CPUState *cpu, int flags) -{ - CPUClass *cc = CPU_GET_CLASS(cpu); - - if (cc->dump_statistics) { - cc->dump_statistics(cpu, flags); - } -} - void cpu_reset(CPUState *cpu) { device_cold_reset(DEVICE(cpu)); -- cgit v1.1