From 3b4f50bd7d16322a109a026a87a945dff660f38b Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 24 Mar 2020 12:12:16 +0000 Subject: [PATCH 01/10] hw/ppc/e500.c: Handle qemu_find_file() failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If qemu_find_file() doesn't find the BIOS it returns NULL; we were passing that unchecked through to load_elf(), which assumes a non-NULL pointer and may misbehave. In practice it fails with a weird message: $ qemu-system-ppc -M ppce500 -display none -kernel nonesuch Bad address qemu-system-ppc: could not load firmware '(null)' Handle the failure case better: $ qemu-system-ppc -M ppce500 -display none -kernel nonesuch qemu-system-ppc: could not find firmware/kernel file 'nonesuch' Spotted by Coverity (CID 1238954). Signed-off-by: Peter Maydell Message-Id: <20200324121216.23899-1-peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Gibson --- hw/ppc/e500.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 854cd3ac46..0d1f41197c 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -1047,6 +1047,10 @@ void ppce500_init(MachineState *machine) } filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, payload_name); + if (!filename) { + error_report("could not find firmware/kernel file '%s'", payload_name); + exit(1); + } payload_size = load_elf(filename, NULL, NULL, NULL, &bios_entry, &loadaddr, NULL, NULL, From 79178edd2a0b012c5cd27e0168beb83ef4b617ef Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Tue, 24 Mar 2020 17:39:12 +1100 Subject: [PATCH 02/10] vfio/spapr: Fix page size calculation Coverity detected an issue (CID 1421903) with potential call of clz64(0) which returns 64 which make it do "<<" with a negative number. This checks the mask and avoids undefined behaviour. In practice pgsizes and memory_region_iommu_get_min_page_size() always have some common page sizes and even if they did not, the resulting page size would be 0x8000.0000.0000.0000 (gcc 9.2) and ioctl(VFIO_IOMMU_SPAPR_TCE_CREATE) would fail anyway. Signed-off-by: Alexey Kardashevskiy Message-Id: <20200324063912.25063-1-aik@ozlabs.ru> Reviewed-by: Greg Kurz Signed-off-by: David Gibson --- hw/vfio/spapr.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c index 33692fc86f..2900bd1941 100644 --- a/hw/vfio/spapr.c +++ b/hw/vfio/spapr.c @@ -147,7 +147,7 @@ int vfio_spapr_create_window(VFIOContainer *container, { int ret = 0; IOMMUMemoryRegion *iommu_mr = IOMMU_MEMORY_REGION(section->mr); - uint64_t pagesize = memory_region_iommu_get_min_page_size(iommu_mr); + uint64_t pagesize = memory_region_iommu_get_min_page_size(iommu_mr), pgmask; unsigned entries, bits_total, bits_per_level, max_levels; struct vfio_iommu_spapr_tce_create create = { .argsz = sizeof(create) }; long rampagesize = qemu_minrampagesize(); @@ -159,8 +159,8 @@ int vfio_spapr_create_window(VFIOContainer *container, if (pagesize > rampagesize) { pagesize = rampagesize; } - pagesize = 1ULL << (63 - clz64(container->pgsizes & - (pagesize | (pagesize - 1)))); + pgmask = container->pgsizes & (pagesize | (pagesize - 1)); + pagesize = pgmask ? (1ULL << (63 - clz64(pgmask))) : 0; if (!pagesize) { error_report("Host doesn't support page size 0x%"PRIx64 ", the supported mask is 0x%lx", From ec010c00665ba1e78e6b3df104f923c4ea68504a Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Thu, 26 Mar 2020 00:29:03 +1000 Subject: [PATCH 03/10] ppc/spapr: KVM FWNMI should not be enabled until guest requests it The KVM FWNMI capability should be enabled with the "ibm,nmi-register" rtas call. Although MCEs from KVM will be delivered as architected interrupts to the guest before "ibm,nmi-register" is called, KVM has different behaviour depending on whether the guest has enabled FWNMI (it attempts to do more recovery on behalf of a non-FWNMI guest). Signed-off-by: Nicholas Piggin Message-Id: <20200325142906.221248-2-npiggin@gmail.com> Reviewed-by: Greg Kurz Signed-off-by: David Gibson --- hw/ppc/spapr_caps.c | 7 ++++--- hw/ppc/spapr_rtas.c | 7 +++++++ target/ppc/kvm.c | 7 +++++++ target/ppc/kvm_ppc.h | 6 ++++++ 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c index 679ae7959f..eb54f94227 100644 --- a/hw/ppc/spapr_caps.c +++ b/hw/ppc/spapr_caps.c @@ -517,9 +517,10 @@ static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val, } if (kvm_enabled()) { - if (kvmppc_set_fwnmi() < 0) { - error_setg(errp, "Firmware Assisted Non-Maskable Interrupts(FWNMI) " - "not supported by KVM"); + if (!kvmppc_get_fwnmi()) { + error_setg(errp, +"Firmware Assisted Non-Maskable Interrupts(FWNMI) not supported by KVM."); + error_append_hint(errp, "Try appending -machine cap-fwnmi=off\n"); } } } diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c index 9fb8c8632a..29abe66d01 100644 --- a/hw/ppc/spapr_rtas.c +++ b/hw/ppc/spapr_rtas.c @@ -437,6 +437,13 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu, return; } + if (kvm_enabled()) { + if (kvmppc_set_fwnmi() < 0) { + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED); + return; + } + } + spapr->fwnmi_system_reset_addr = sreset_addr; spapr->fwnmi_machine_check_addr = mce_addr; diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 597f72be1b..03d0667e8f 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -88,6 +88,7 @@ static int cap_ppc_safe_indirect_branch; static int cap_ppc_count_cache_flush_assist; static int cap_ppc_nested_kvm_hv; static int cap_large_decr; +static int cap_fwnmi; static uint32_t debug_inst_opcode; @@ -136,6 +137,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) kvmppc_get_cpu_characteristics(s); cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV); cap_large_decr = kvmppc_get_dec_bits(); + cap_fwnmi = kvm_vm_check_extension(s, KVM_CAP_PPC_FWNMI); /* * Note: setting it to false because there is not such capability * in KVM at this moment. @@ -2064,6 +2066,11 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy) } } +bool kvmppc_get_fwnmi(void) +{ + return cap_fwnmi; +} + int kvmppc_set_fwnmi(void) { PowerPCCPU *cpu = POWERPC_CPU(first_cpu); diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h index 332fa0aa1c..fcaf745516 100644 --- a/target/ppc/kvm_ppc.h +++ b/target/ppc/kvm_ppc.h @@ -27,6 +27,7 @@ void kvmppc_enable_h_page_init(void); void kvmppc_set_papr(PowerPCCPU *cpu); int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr); void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy); +bool kvmppc_get_fwnmi(void); int kvmppc_set_fwnmi(void); int kvmppc_smt_threads(void); void kvmppc_error_append_smt_possible_hint(Error *const *errp); @@ -163,6 +164,11 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy) { } +static inline bool kvmppc_get_fwnmi(void) +{ + return false; +} + static inline int kvmppc_set_fwnmi(void) { return -1; From 6c3dd24c054a0701169b17be57b611bac8d99b5d Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Thu, 26 Mar 2020 00:29:04 +1000 Subject: [PATCH 04/10] ppc/spapr: Improve FWNMI machine check delivery corner case comments Some of the conditions are not as clearly documented as they could be. Also the non-FWNMI case does not need a large comment. Reviewed-by: Greg Kurz Signed-off-by: Nicholas Piggin Message-Id: <20200325142906.221248-3-npiggin@gmail.com> Signed-off-by: David Gibson --- hw/ppc/spapr_events.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index a4a540f43d..a908c5d0e9 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -860,17 +860,13 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered) Error *local_err = NULL; if (spapr->fwnmi_machine_check_addr == -1) { - /* - * This implies that we have hit a machine check either when the - * guest has not registered FWNMI (i.e., "ibm,nmi-register" not - * called) or between system reset and "ibm,nmi-register". - * Fall back to the old machine check behavior in such cases. - */ + /* Non-FWNMI case, deliver it like an architected CPU interrupt. */ cs->exception_index = POWERPC_EXCP_MCHECK; ppc_cpu_do_interrupt(cs); return; } + /* Wait for FWNMI interlock. */ while (spapr->fwnmi_machine_check_interlock != -1) { /* * Check whether the same CPU got machine check error @@ -882,8 +878,13 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered) return; } qemu_cond_wait_iothread(&spapr->fwnmi_machine_check_interlock_cond); - /* Meanwhile if the system is reset, then just return */ if (spapr->fwnmi_machine_check_addr == -1) { + /* + * If the machine was reset while waiting for the interlock, + * abort the delivery. The machine check applies to a context + * that no longer exists, so it wouldn't make sense to deliver + * it now. + */ return; } } @@ -894,7 +895,9 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered) * We don't want to abort so we let the migration to continue. * In a rare case, the machine check handler will run on the target. * Though this is not preferable, it is better than aborting - * the migration or killing the VM. + * the migration or killing the VM. It is okay to call + * migrate_del_blocker on a blocker that was not added (which the + * nmi-interlock handler would do when it's called after this). */ warn_report("Received a fwnmi while migration was in progress"); } From b90b9ecb12ae37c089d1a837d8ff2c888d71902e Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Thu, 26 Mar 2020 00:29:05 +1000 Subject: [PATCH 05/10] ppc/spapr: Add FWNMI machine check delivery warnings Add some messages which explain problems and guest misbehaviour that may be difficult to diagnose in rare cases of machine checks. Signed-off-by: Nicholas Piggin Message-Id: <20200325142906.221248-4-npiggin@gmail.com> Reviewed-by: Greg Kurz Signed-off-by: David Gibson --- hw/ppc/spapr_events.c | 4 ++++ hw/ppc/spapr_rtas.c | 3 +++ 2 files changed, 7 insertions(+) diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index a908c5d0e9..c8964eb25d 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -833,6 +833,8 @@ static void spapr_mce_dispatch_elog(PowerPCCPU *cpu, bool recovered) /* get rtas addr from fdt */ rtas_addr = spapr_get_rtas_addr(); if (!rtas_addr) { + error_report( +"FWNMI: Unable to deliver machine check to guest: rtas_addr not found."); qemu_system_guest_panicked(NULL); g_free(ext_elog); return; @@ -874,6 +876,8 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered) * that CPU called "ibm,nmi-interlock") */ if (spapr->fwnmi_machine_check_interlock == cpu->vcpu_id) { + error_report( +"FWNMI: Unable to deliver machine check to guest: nested machine check."); qemu_system_guest_panicked(NULL); return; } diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c index 29abe66d01..bcac0d00e7 100644 --- a/hw/ppc/spapr_rtas.c +++ b/hw/ppc/spapr_rtas.c @@ -462,6 +462,9 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu, } if (spapr->fwnmi_machine_check_addr == -1) { + qemu_log_mask(LOG_GUEST_ERROR, +"FWNMI: ibm,nmi-interlock RTAS called with FWNMI not registered.\n"); + /* NMI register not called */ rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); return; From 4f7a11f93fcb970bdcd2faa12337f4d1269b45f4 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Thu, 26 Mar 2020 00:29:06 +1000 Subject: [PATCH 06/10] ppc/spapr: Don't kill the guest if a recovered FWNMI machine check delivery fails Try to be tolerant of FWNMI delivery errors if the machine check had been recovered by the host. Signed-off-by: Nicholas Piggin Message-Id: <20200325142906.221248-5-npiggin@gmail.com> Reviewed-by: Greg Kurz [dwg: Updated comment at Greg's suggestion] Signed-off-by: David Gibson --- hw/ppc/spapr_events.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index c8964eb25d..1069d0197b 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -833,13 +833,28 @@ static void spapr_mce_dispatch_elog(PowerPCCPU *cpu, bool recovered) /* get rtas addr from fdt */ rtas_addr = spapr_get_rtas_addr(); if (!rtas_addr) { - error_report( + if (!recovered) { + error_report( "FWNMI: Unable to deliver machine check to guest: rtas_addr not found."); - qemu_system_guest_panicked(NULL); + qemu_system_guest_panicked(NULL); + } else { + warn_report( +"FWNMI: Unable to deliver machine check to guest: rtas_addr not found. " +"Machine check recovered."); + } g_free(ext_elog); return; } + /* + * By taking the interlock, we assume that the MCE will be + * delivered to the guest. CAUTION: don't add anything that could + * prevent the MCE to be delivered after this line, otherwise the + * guest won't be able to release the interlock and ultimately + * hang/crash? + */ + spapr->fwnmi_machine_check_interlock = cpu->vcpu_id; + stq_be_phys(&address_space_memory, rtas_addr + RTAS_ERROR_LOG_OFFSET, env->gpr[3]); cpu_physical_memory_write(rtas_addr + RTAS_ERROR_LOG_OFFSET + @@ -876,9 +891,15 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered) * that CPU called "ibm,nmi-interlock") */ if (spapr->fwnmi_machine_check_interlock == cpu->vcpu_id) { - error_report( + if (!recovered) { + error_report( "FWNMI: Unable to deliver machine check to guest: nested machine check."); - qemu_system_guest_panicked(NULL); + qemu_system_guest_panicked(NULL); + } else { + warn_report( +"FWNMI: Unable to deliver machine check to guest: nested machine check. " +"Machine check recovered."); + } return; } qemu_cond_wait_iothread(&spapr->fwnmi_machine_check_interlock_cond); @@ -906,7 +927,6 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered) warn_report("Received a fwnmi while migration was in progress"); } - spapr->fwnmi_machine_check_interlock = cpu->vcpu_id; spapr_mce_dispatch_elog(cpu, recovered); } From 7aab5899764887f6b0512cb2e5c11bdc2a5d3644 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 26 Mar 2020 16:12:40 +1100 Subject: [PATCH 07/10] spapr: Fix failure path for attempting to hot unplug PCI bridges For various technical reasons we can't currently allow unplug a PCI to PCI bridge on the pseries machine. spapr_pci_unplug_request() correctly generates an error message if that's attempted. But.. if the given errp is not error_abort or error_fatal, it doesn't actually stop trying to unplug the bridge anyway. Fixes: 14e714900f6b "spapr: Allow hot plug/unplug of PCI bridges and devices under PCI bridges" Signed-off-by: David Gibson Reviewed-by: Greg Kurz --- hw/ppc/spapr_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 709a52780d..55ca9dee1e 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1663,6 +1663,7 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler, if (pc->is_bridge) { error_setg(errp, "PCI: Hot unplug of PCI bridges not supported"); + return; } /* ensure any other present functions are pending unplug */ From 2025fc6766ab25501e0041c564c44bb0f7389774 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 30 Mar 2020 13:52:28 +0100 Subject: [PATCH 08/10] hw/ppc/ppc440_uc.c: Remove incorrect iothread locking from dcr_write_pcie() In dcr_write_pcie() we take the iothread lock around a call to pcie_host_mmcfg_udpate(). This is an incorrect attempt to deal with the bug fixed in commit 235352ee6e73d7716, where we were not taking the iothread lock before calling device dcr read/write functions. (It's not sufficient locking, because although the other cases in the switch statement won't assert, there is no locking which prevents multiple guest CPUs from trying to access the PPC460EXPCIEState struct at the same time and corrupting data.) Unfortunately with commit 235352ee6e73d7716 we are now trying to recursively take the iothread lock, which will assert: $ qemu-system-ppc -M sam460ex --display none ** ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1830:qemu_mutex_lock_iothread_impl: assertion failed: (!qemu_mutex_iothread_locked()) Aborted (core dumped) Remove the locking within dcr_write_pcie(). Fixes: 235352ee6e73d7716 Signed-off-by: Peter Maydell Message-Id: <20200330125228.24994-1-peter.maydell@linaro.org> Tested-by: BALATON Zoltan Signed-off-by: David Gibson --- hw/ppc/ppc440_uc.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c index d5ea962249..b30e093cbb 100644 --- a/hw/ppc/ppc440_uc.c +++ b/hw/ppc/ppc440_uc.c @@ -13,7 +13,6 @@ #include "qemu/error-report.h" #include "qapi/error.h" #include "qemu/log.h" -#include "qemu/main-loop.h" #include "qemu/module.h" #include "cpu.h" #include "hw/irq.h" @@ -1183,9 +1182,7 @@ static void dcr_write_pcie(void *opaque, int dcrn, uint32_t val) case PEGPL_CFGMSK: s->cfg_mask = val; size = ~(val & 0xfffffffe) + 1; - qemu_mutex_lock_iothread(); pcie_host_mmcfg_update(PCIE_HOST_BRIDGE(s), val & 1, s->cfg_base, size); - qemu_mutex_unlock_iothread(); break; case PEGPL_MSGBAH: s->msg_base = ((uint64_t)val << 32) | (s->msg_base & 0xffffffff); From a872e4328bb3c2dda0699e04abf0f902950221f3 Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Fri, 27 Mar 2020 13:57:29 +1100 Subject: [PATCH 09/10] pseries: Update SLOF firmware image This is a single regression fix for for 5.0: Greg Kurz (1): slof: Only close stdout for virtio-serial devices Signed-off-by: Alexey Kardashevskiy Signed-off-by: David Gibson --- pc-bios/README | 2 +- pc-bios/slof.bin | Bin 965008 -> 965112 bytes roms/SLOF | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pc-bios/README b/pc-bios/README index f54c2743d0..a5a770f066 100644 --- a/pc-bios/README +++ b/pc-bios/README @@ -14,7 +14,7 @@ - SLOF (Slimline Open Firmware) is a free IEEE 1275 Open Firmware implementation for certain IBM POWER hardware. The sources are at https://github.com/aik/SLOF, and the image currently in qemu is - built from git tag qemu-slof-20200317. + built from git tag qemu-slof-20200327. - sgabios (the Serial Graphics Adapter option ROM) provides a means for legacy x86 software to communicate with an attached serial console as diff --git a/pc-bios/slof.bin b/pc-bios/slof.bin index 40499a1451f4f63f79f64f2457ae62b08d7d66eb..80bbf91a186c7eec76af3ae6af50c2c841a77579 100644 GIT binary patch delta 4653 zcmbVOeRNah8Ncs+)8u}%X&NAD%Gaf^*;Y$qK^m~ML5GyX#_Aag&I&1QN&^)lEu|fd z2IrutjIdgss^>st4mx+H;q!Ki5}jvuoT^=qJL`6K?uq=t7&z(Fxo%*y=S^-Ty?^YF z^_<)HKEL1dd%oT`_ula5OU6H660UHJZwj=Pdi?IY%NrLpH8yTs^o0NM9qyhvWTshB z=p9qm9u7`5T`H5_he|~#>2Z_0PHD%hZJCPljegIWuy=r8SYDx&yUX3m-3yl7Q?aCc zQI4cMuvU54zr8iEZL6|WG3H7tci%s9Q5qQK*q<~IfpwIu=Fi5VpOOmFqQ)t?Zu-9> zs|kU0QC_>MhO&wE5W5M#Nao9U1F;ii2?K7OAln6Y6MT(0mlK=X{RLTNCt>v?Grx5T z|EDqaTLt{Z<)l_M`S`UZ=ur1GUw98wgf8;;n@F?z?r->A0tu?#A-E(wn$=&yktXYoo{U-jhdnrJy(ld$(!Ipy4sXPJoMO|IsI6$4GZS>R%rv!(nx2Q4zS+;CTP*c#T@ArKIS`1 zIHl_qFLxMSyhk~k-Y?5E0Jkb>F?`%eA1u%YtB-T0d*9Uh#`kdCt{-tq^^=^b=ax1E z-}ci2t2W5WJ5bDVJ=YLx^V7|UU9V4!9>R{A(6J0SU zT6r85KiB#zpX5xTr?i)`e4bU}@o~4Xg+cIbq8p|~SAF%>0Uwv#NmJR+IBtBi)(;+yTUT-2%jlrrhuxBq9JiJ&N>{+J`t(qvwVYTe; zn%?PslC-h=U2i%Y8Rxm&KaSemu9MfMGJlx2g+7dfth~&bx{lU(4_mPF?k#iyiNS>} z^c5*JckFZsf-STl!&aYbp{H{lTKtE_4~{Iw-CkV0pLWiDYU96)v^co8QW7H9U%5f?GU1h_DmTo3{6sDPi{+

e~gv3iH%wTYvT0b@Sujec^W>9w?ep_uSmK9^IzSm?PXt@Ci{C*uoY1&OE~UkGKQOTo!-);#)sTqs^{W^<-T6gBph z@SfxfG1HpoxP(h4A5tTNIN{_Ui9|*gS;NWj32VBpoww>Z@gm4h+5X(pwjCF|4wD%=R_bz8@&a|Efr4#8aW?Azt z72uLl9n!UoW)Pzb4K#%iqfI(pVxUbB?Zgr|h@%(b)bUQSVhRz!eGpAStvM)m5ton> ze`+>}1vUN?af68N-oB*!x<2uS*=0+)Pox{s=e*tqzGGNQnsykX+0^hc5iG9!q}*;= zr1M^8;~etAds4hX^1m8sR6sN&G3DGWkS*2Ec{d}lBhM1pW zH9A(6!g>*d4I`ojUQLL_a0u`0^>3jM%auMZMKG4YDHrL|4-9l&uOc}~U9U$fONQd> zKZVeU=;+-i<+F*sZos$AB)SfKymSc44&u)Ilb62UhK5TJ?ZDo?K_$D(gCg9Th7nzy9`!<~rIJ%z zsM8wM;viukmGY9af}>PAkPJP=-uQs7^)hFih!?yjoEJMVu)~A{^r+n?={=I1AUtGF zjWjpN4n>6JE*0UI2xc9N@4>FS+!>{GVaQ%$Y2NVT-<3kR`B8 zYQ7+CBBTyN;_YEIy`$^W{T4wKX;B|_(-nxI34awO8q1tvsk8CeKk>Gn`IZA}fO zUAkTOrXQqwIe{3a9FU|Hr0$4}6|oZri)5)db;_G$eUkR(^zSZRN*#n{c8;N+>B{;= z3*USnxKD}>^-EbIBz3l%Q)e3-IkyjY^OuHc)PXk#+hD&dK44Fk)}A2<7@xRxY9vR> zkgJ3(0wH`eyi(P#NE~-anSu=XK zD>{DFfU|0`_~f@hrhghQGWdIR|C$tDlHON)txhyg76@S-@5Xnu?0lquGHnm)^q>I` z>G)Lx&iciA3j_lmh448%rqj~A2jZBni&d4;*0dPfOx|q3<2oLijI$S7(w~LRh~rRn zJZiwbIg<6Z;Ks$y~0CGkhU{_#0t zIuT3~xA9>e?>69Z9gi6BF&!TOpDH^FrZauUc$_w!zY1YhcCKVrCe8Z5`ex)CaF33= z4S0=?*T5KtOMVTq58u=y<(GT)YG!&=Wk9wPe$61be=T?6w@Z{QHQ;_--p8-M1p}|k z6$C$|a1mQrOwKa11>A4Q)uaW2Z0UpO8*&xz9fdJwhW5KJ$arO|?h83P|16FsMO;=d zM2}dXf%r?-9Dd~ku=fS43%&=|`KC2B%U8qOV9kdY%6W`Zx7VBls2?% zYxS45wl!|s(WbN(D#<&zFl?D;O;L^hb|o2ML!WhJ*`s$nru>fp^^J|CPwWWzTO0hP p9c|LR;C;}FAD+)x=Yi5=wW!zntXFt8b#?ef>xG%7*BrmE{4a5VBOw3) delta 4787 zcmbVPdvsIv9lyVOFUfthw@HDfDYOLiC@q5qAR_a=#dSLVn6MS1>>P1|PIZdkb3F~4y^?ZSHdLk&NF#NIuf6dG(c&SJW` z%R7mGhdFW(Oed7Q?o#4B8aY^H%Jf(;zhUlizca+npS#F5&pyv?n`EPI?P4#6FWe>gaKTqb!cmv@9QbvI*17xc}Z@jM(%W_h!c79J*=8|*j#|HMniR`B{ z)nx_j+2y2D<=yPs5;R!%6gy)9_3)i#m-3`vee2Kcc7X&`=X-1wSstih8NRP6^71`R zJAZXg-J6yBsv}1#Ze#d;C6SjZW+b^ktnx9oc7YvOY-1SYUMb(e@Q!}Q;P5dCM_nYK zzV$x)d=Uw%?yuQt6Hxm8$v!Qh%=$O`s4+W4{b{VniX$P_Pq=9Y5{96W`;dfHpTJ$F zYI_cM8HMvCDTH1VcM$I#W^Q{5ny&uM%(WDn&#g?1W@8lwT47g7z8I4fh*-)F?>|$+VsH;56U%WLj*zLW`3KJT+5;6q#M(!4Is9oR& z9Mc&ujI@yaEcb0V=ot(@+!Qr{zmY3qF;1|NyCFMv`HMun>H?-uLApY2x-P zQU3B9iIJ)|v8)J|<=n*0V9~OE6ZhyHvI8P!;PD{H8{xf3k=_}zuMVl#Lhc6bGMK< z{BbjPL`uyaJLSRK%oSwlYELtFEGI9T|I6aL&sX5K7Z>m1+HQDa!@p)m^TEEAv&!1U z<4w_g)z{8-=MZQ2mD(bh_Y8L{ajM&%;WlRzw;J2WRozRL?Xq3s@Z#~BpxQ1O{{A(r z>r?XK!_SP9l)C0E4>!!NZ)lP3sb0QH{oylXccuDFfw0Vwe`4oD3*P4+|ERLvU-8Y4 zlXk3Id)J;7OP7{G&S_=3Doz&+1m7X18A2t?7TQ$Tjlz#c%v${up-Lgk)Xp+t3yamn z%Z0=#9De*eVFejdo!<+kBB=rIRl!1P)XuBII}!X;T&Rm?jtpvd6w$MEIiz%5WjB;!PL_|Eb4diyD_c-8+m^u=Y3w{|&G74&fSuAy zsHT|Ty$J=aI!%{@8);Gwt!-jyvX+VFclEV9)Nq@)Xd+zVB*R4W@RwQ)Z8Gbfs_9v= zgCuF_-Hp9D)xd7?0a0`K;koM^Y<=P-15_n&7?Tf(x$r8Eg)@)lf;}NlhUIBU{n-KW zIU^j9B_sSWAexB_gu~*cB#=8HJtrrIr>(kFQ$SoRr`RkOG1{pUuOth;sc>vo3%?A?B$A z=S3??nZ5myXil1`k&nbDOGF>WWRzU@1qkLyd9ZU7={-4;G1vXAmMuMA9|T9mQsRTy zC@!)!;LM~CqE+TG@PjvjIsDL%R-#)A@+JgD ztnHJ8Xtwrf*m=z}U0qL)gnqqt$zt_uCg}#!xqEuul7;m2wo0XGn+*@@!}NwRf+rPW z!zqn*QXQM~^p0Q_57@|z9NZ8? z2RJL@QtE!8BYVKa(xzZ0EA2@yrS5Ib%84?yltHwDv$$7M2OT+hZ{Q@ewVBdNZrvv+ zXpQM%HiMlT8)TjSLGy!~llG8S7(pu(0xy*&gR}Mbfe(+7x`vnDAn7qS+^&UpqD+nv z>;}3ma2v+5H0(fb{Gj}l?xQ|RZo_VkPS@B2VWdeP3>l=uq#1lh)S98sh}+fn*Qh_7 z9yMl^Hj?yi8eh9Erd!0_xDm#bjc=#U-Chw#rIpe6mCmIP!lJYyJ;{yf<)y1^2Aee2 zySoHC};4wgs%dhP+Qj6ph~ zAFc2UX9!cW6(xgZAWJ@`$HGnjZ zwe{8ssiB06Pc#l(tV|q1R#tw3G5omk<_6a`EWnnd{oYX-9cG2yD$PhG&j*u z3?m1?byUtPkb_iHEEYG4Og;?0B%k{9v{<}4J_f-gA4i<}`^NKG0>qBW7Fc~$7Ltp| zh@CZ4^XUUG+K~RFsHI(JT2*USw8BihM#Doo+@s;)|AC*|drY>N2a%%vfx%;P z9#kHaMQdz4mqjb0y$yq#x{i#evt4_Sqv!r(vXGlkEi=P9z)r~zki0YBq$34ttyuDB zqR8k>8Pae&%FJMK4R?U;xNNaHb#zdptHDKS*LYf?<}_OkexBk+empMcNJ@Y;{9t1CD=&({X zGqWwlTdB;%=}a(dvibjpyLI*s&EBWO>2zqE{v6dCPW{FT zl9{})Mg$;s3I{^fF`mhbY4nf|AJK3sGwdxC#y+MBI^3k;HXTlNYVSzl(4xxbhvW|Yn$&t4g%9GV<(=#U^TGF; zyok6VghD(-eHmlnA&9>wS83`?JMaU=CQO0*&&X5BAhe#5E7@C35IQ4QkKw6@K=X2 g2|g`UW~;0BDPOWMGF7pttAC@sSjfM;^-$4&0O~#nhX4Qo diff --git a/roms/SLOF b/roms/SLOF index ab6984f5a6..8e012d6fdd 160000 --- a/roms/SLOF +++ b/roms/SLOF @@ -1 +1 @@ -Subproject commit ab6984f5a6d054e1f634dda855b32e5357111974 +Subproject commit 8e012d6fddb62be833d746cef3f03e6c8beecde0 From 25f3170b06544e4de620336da5b2ea3b392d66bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Sat, 4 Apr 2020 17:36:55 +0200 Subject: [PATCH 10/10] ppc/pnv: Create BMC devices only when defaults are enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit e2392d4395dd ("ppc/pnv: Create BMC devices at machine init") introduced default BMC devices which can be a problem when the same devices are defined on the command line with : -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10 QEMU fails with : qemu-system-ppc64: error creating device tree: node: FDT_ERR_EXISTS Use defaults_enabled() when creating the default BMC devices to let the user provide its own BMC devices using '-nodefaults'. If no BMC device are provided, output a warning but let QEMU run as this is a supported configuration. However, when multiple BMC devices are defined, stop QEMU with a clear error as the results are unexpected. Fixes: e2392d4395dd ("ppc/pnv: Create BMC devices at machine init") Reported-by: Nathan Chancellor Signed-off-by: Cédric Le Goater Message-Id: <20200404153655.166834-1-clg@kaod.org> Tested-by: Nathan Chancellor Signed-off-by: David Gibson --- hw/ppc/pnv.c | 32 ++++++++++++++++++++++++++----- hw/ppc/pnv_bmc.c | 45 ++++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/pnv.h | 2 ++ 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index b75ad06390..c9cb6fa357 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -571,10 +571,29 @@ static void pnv_powerdown_notify(Notifier *n, void *opaque) static void pnv_reset(MachineState *machine) { + PnvMachineState *pnv = PNV_MACHINE(machine); + IPMIBmc *bmc; void *fdt; qemu_devices_reset(); + /* + * The machine should provide by default an internal BMC simulator. + * If not, try to use the BMC device that was provided on the command + * line. + */ + bmc = pnv_bmc_find(&error_fatal); + if (!pnv->bmc) { + if (!bmc) { + warn_report("machine has no BMC device. Use '-device " + "ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' " + "to define one"); + } else { + pnv_bmc_set_pnor(bmc, pnv->pnor); + pnv->bmc = bmc; + } + } + fdt = pnv_dt_create(machine); /* Pack resulting tree */ @@ -833,9 +852,6 @@ static void pnv_init(MachineState *machine) } g_free(chip_typename); - /* Create the machine BMC simulator */ - pnv->bmc = pnv_bmc_create(pnv->pnor); - /* Instantiate ISA bus on chip 0 */ pnv->isa_bus = pnv_isa_create(pnv->chips[0], &error_fatal); @@ -845,8 +861,14 @@ static void pnv_init(MachineState *machine) /* Create an RTC ISA device too */ mc146818_rtc_init(pnv->isa_bus, 2000, NULL); - /* Create the IPMI BT device for communication with the BMC */ - pnv_ipmi_bt_init(pnv->isa_bus, pnv->bmc, 10); + /* + * Create the machine BMC simulator and the IPMI BT device for + * communication with the BMC + */ + if (defaults_enabled()) { + pnv->bmc = pnv_bmc_create(pnv->pnor); + pnv_ipmi_bt_init(pnv->isa_bus, pnv->bmc, 10); + } /* * OpenPOWER systems use a IPMI SEL Event message to notify the diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c index 8863354c1c..4e018b8b70 100644 --- a/hw/ppc/pnv_bmc.c +++ b/hw/ppc/pnv_bmc.c @@ -213,6 +213,18 @@ static const IPMINetfn hiomap_netfn = { .cmd_handlers = hiomap_cmds }; + +void pnv_bmc_set_pnor(IPMIBmc *bmc, PnvPnor *pnor) +{ + object_ref(OBJECT(pnor)); + object_property_add_const_link(OBJECT(bmc), "pnor", OBJECT(pnor), + &error_abort); + + /* Install the HIOMAP protocol handlers to access the PNOR */ + ipmi_sim_register_netfn(IPMI_BMC_SIMULATOR(bmc), IPMI_NETFN_OEM, + &hiomap_netfn); +} + /* * Instantiate the machine BMC. PowerNV uses the QEMU internal * simulator but it could also be external. @@ -232,3 +244,36 @@ IPMIBmc *pnv_bmc_create(PnvPnor *pnor) return IPMI_BMC(obj); } + +typedef struct ForeachArgs { + const char *name; + Object *obj; +} ForeachArgs; + +static int bmc_find(Object *child, void *opaque) +{ + ForeachArgs *args = opaque; + + if (object_dynamic_cast(child, args->name)) { + if (args->obj) { + return 1; + } + args->obj = child; + } + return 0; +} + +IPMIBmc *pnv_bmc_find(Error **errp) +{ + ForeachArgs args = { TYPE_IPMI_BMC_SIMULATOR, NULL }; + int ret; + + ret = object_child_foreach_recursive(object_get_root(), bmc_find, &args); + if (ret) { + error_setg(errp, "machine should have only one BMC device. " + "Use '-nodefaults'"); + return NULL; + } + + return args.obj ? IPMI_BMC(args.obj) : NULL; +} diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h index fb4d0c0234..d4b0b0e2ff 100644 --- a/include/hw/ppc/pnv.h +++ b/include/hw/ppc/pnv.h @@ -241,6 +241,8 @@ struct PnvMachineState { void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt); void pnv_bmc_powerdown(IPMIBmc *bmc); IPMIBmc *pnv_bmc_create(PnvPnor *pnor); +IPMIBmc *pnv_bmc_find(Error **errp); +void pnv_bmc_set_pnor(IPMIBmc *bmc, PnvPnor *pnor); /* * POWER8 MMIO base addresses