* [PULL 01/10] hw/ppc/e500.c: Handle qemu_find_file() failure
2020-04-07 4:35 [PULL 00/10] ppc-for-5.0 queue 20200407 David Gibson
@ 2020-04-07 4:35 ` David Gibson
2020-04-07 4:35 ` [PULL 02/10] vfio/spapr: Fix page size calculation David Gibson
` (9 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2020-04-07 4:35 UTC (permalink / raw)
To: peter.maydell
Cc: aik, qemu-devel, groug, qemu-ppc, clg,
Philippe Mathieu-Daudé, David Gibson
From: Peter Maydell <peter.maydell@linaro.org>
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 <peter.maydell@linaro.org>
Message-Id: <20200324121216.23899-1-peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
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,
--
2.25.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 02/10] vfio/spapr: Fix page size calculation
2020-04-07 4:35 [PULL 00/10] ppc-for-5.0 queue 20200407 David Gibson
2020-04-07 4:35 ` [PULL 01/10] hw/ppc/e500.c: Handle qemu_find_file() failure David Gibson
@ 2020-04-07 4:35 ` David Gibson
2020-04-07 4:35 ` [PULL 03/10] ppc/spapr: KVM FWNMI should not be enabled until guest requests it David Gibson
` (8 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2020-04-07 4:35 UTC (permalink / raw)
To: peter.maydell; +Cc: aik, qemu-devel, groug, qemu-ppc, clg, David Gibson
From: Alexey Kardashevskiy <aik@ozlabs.ru>
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 <aik@ozlabs.ru>
Message-Id: <20200324063912.25063-1-aik@ozlabs.ru>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
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",
--
2.25.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 03/10] ppc/spapr: KVM FWNMI should not be enabled until guest requests it
2020-04-07 4:35 [PULL 00/10] ppc-for-5.0 queue 20200407 David Gibson
2020-04-07 4:35 ` [PULL 01/10] hw/ppc/e500.c: Handle qemu_find_file() failure David Gibson
2020-04-07 4:35 ` [PULL 02/10] vfio/spapr: Fix page size calculation David Gibson
@ 2020-04-07 4:35 ` David Gibson
2020-04-07 4:36 ` [PULL 04/10] ppc/spapr: Improve FWNMI machine check delivery corner case comments David Gibson
` (7 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2020-04-07 4:35 UTC (permalink / raw)
To: peter.maydell
Cc: aik, qemu-devel, Nicholas Piggin, groug, qemu-ppc, clg,
David Gibson
From: Nicholas Piggin <npiggin@gmail.com>
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 <npiggin@gmail.com>
Message-Id: <20200325142906.221248-2-npiggin@gmail.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
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;
--
2.25.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 04/10] ppc/spapr: Improve FWNMI machine check delivery corner case comments
2020-04-07 4:35 [PULL 00/10] ppc-for-5.0 queue 20200407 David Gibson
` (2 preceding siblings ...)
2020-04-07 4:35 ` [PULL 03/10] ppc/spapr: KVM FWNMI should not be enabled until guest requests it David Gibson
@ 2020-04-07 4:36 ` David Gibson
2020-04-07 4:36 ` [PULL 05/10] ppc/spapr: Add FWNMI machine check delivery warnings David Gibson
` (6 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2020-04-07 4:36 UTC (permalink / raw)
To: peter.maydell
Cc: aik, qemu-devel, Nicholas Piggin, groug, qemu-ppc, clg,
David Gibson
From: Nicholas Piggin <npiggin@gmail.com>
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 <groug@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Message-Id: <20200325142906.221248-3-npiggin@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
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");
}
--
2.25.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 05/10] ppc/spapr: Add FWNMI machine check delivery warnings
2020-04-07 4:35 [PULL 00/10] ppc-for-5.0 queue 20200407 David Gibson
` (3 preceding siblings ...)
2020-04-07 4:36 ` [PULL 04/10] ppc/spapr: Improve FWNMI machine check delivery corner case comments David Gibson
@ 2020-04-07 4:36 ` David Gibson
2020-04-07 4:36 ` [PULL 06/10] ppc/spapr: Don't kill the guest if a recovered FWNMI machine check delivery fails David Gibson
` (5 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2020-04-07 4:36 UTC (permalink / raw)
To: peter.maydell
Cc: aik, qemu-devel, Nicholas Piggin, groug, qemu-ppc, clg,
David Gibson
From: Nicholas Piggin <npiggin@gmail.com>
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 <npiggin@gmail.com>
Message-Id: <20200325142906.221248-4-npiggin@gmail.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
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;
--
2.25.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 06/10] ppc/spapr: Don't kill the guest if a recovered FWNMI machine check delivery fails
2020-04-07 4:35 [PULL 00/10] ppc-for-5.0 queue 20200407 David Gibson
` (4 preceding siblings ...)
2020-04-07 4:36 ` [PULL 05/10] ppc/spapr: Add FWNMI machine check delivery warnings David Gibson
@ 2020-04-07 4:36 ` David Gibson
2020-04-07 4:36 ` [PULL 07/10] spapr: Fix failure path for attempting to hot unplug PCI bridges David Gibson
` (4 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2020-04-07 4:36 UTC (permalink / raw)
To: peter.maydell
Cc: aik, qemu-devel, Nicholas Piggin, groug, qemu-ppc, clg,
David Gibson
From: Nicholas Piggin <npiggin@gmail.com>
Try to be tolerant of FWNMI delivery errors if the machine check had been
recovered by the host.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Message-Id: <20200325142906.221248-5-npiggin@gmail.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
[dwg: Updated comment at Greg's suggestion]
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
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);
}
--
2.25.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 07/10] spapr: Fix failure path for attempting to hot unplug PCI bridges
2020-04-07 4:35 [PULL 00/10] ppc-for-5.0 queue 20200407 David Gibson
` (5 preceding siblings ...)
2020-04-07 4:36 ` [PULL 06/10] ppc/spapr: Don't kill the guest if a recovered FWNMI machine check delivery fails David Gibson
@ 2020-04-07 4:36 ` David Gibson
2020-04-07 4:36 ` [PULL 08/10] hw/ppc/ppc440_uc.c: Remove incorrect iothread locking from dcr_write_pcie() David Gibson
` (3 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2020-04-07 4:36 UTC (permalink / raw)
To: peter.maydell; +Cc: aik, qemu-devel, groug, qemu-ppc, clg, David Gibson
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 <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
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 */
--
2.25.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 08/10] hw/ppc/ppc440_uc.c: Remove incorrect iothread locking from dcr_write_pcie()
2020-04-07 4:35 [PULL 00/10] ppc-for-5.0 queue 20200407 David Gibson
` (6 preceding siblings ...)
2020-04-07 4:36 ` [PULL 07/10] spapr: Fix failure path for attempting to hot unplug PCI bridges David Gibson
@ 2020-04-07 4:36 ` David Gibson
2020-04-07 4:36 ` [PULL 09/10] pseries: Update SLOF firmware image David Gibson
` (2 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2020-04-07 4:36 UTC (permalink / raw)
To: peter.maydell; +Cc: aik, qemu-devel, groug, qemu-ppc, clg, David Gibson
From: Peter Maydell <peter.maydell@linaro.org>
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 <peter.maydell@linaro.org>
Message-Id: <20200330125228.24994-1-peter.maydell@linaro.org>
Tested-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
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);
--
2.25.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 09/10] pseries: Update SLOF firmware image
2020-04-07 4:35 [PULL 00/10] ppc-for-5.0 queue 20200407 David Gibson
` (7 preceding siblings ...)
2020-04-07 4:36 ` [PULL 08/10] hw/ppc/ppc440_uc.c: Remove incorrect iothread locking from dcr_write_pcie() David Gibson
@ 2020-04-07 4:36 ` David Gibson
2020-04-07 4:36 ` [PULL 10/10] ppc/pnv: Create BMC devices only when defaults are enabled David Gibson
2020-04-07 13:01 ` [PULL 00/10] ppc-for-5.0 queue 20200407 Peter Maydell
10 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2020-04-07 4:36 UTC (permalink / raw)
To: peter.maydell; +Cc: aik, qemu-devel, groug, qemu-ppc, clg, David Gibson
From: Alexey Kardashevskiy <aik@ozlabs.ru>
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 <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
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-<X{4=&|7Q(r*a
zu_mx@&zgpJs-CaWwlBSpGd*9TwJt48a&4<aoU&WsPb^kA4!Kt=H*zLVgfn|WT*A|!
z1=V-o=U<#fI#k~${Ol}TL;v8P5pd1^55Lor6JmN6Yj3d@Qp1GKHj{3MTIh$QTMY^H
zCNp>E(wn$=&yktXYoo{U-jhdnrJy<LAM$AX%)Icb#6%8OVP+K`F44kjT^XIrWB;MM
zX%%5@#>(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<J1et}jar_XNM=f~+Q5LL%U9q6@^xZh@Zg
z%;ADC)=u)XefQy@3pvx(fc61|o9HYa<3yY2+?@WKA1B7EFJt<I)(7z>>R{A(6J0SU
zT6r85KiB#zpX5xTr?i)`e4bU}@o~4Xg+cIbq8p|~SAF%>0Uwv#NmJR+IBtBi)(;<T
zqSvvQSTkKT3+v}OQzU@uLL76AHq_zglskV+6X9QI!!R{K*OHf^B|z^$#d86A)T#~L
zd^<5#{SKC;VOieIw2(*7#?5r+N>+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^co8Q<tnyd_17V
z)zEI*lS_o2TlKS`{8_q;2x{B2bW08?Qsd9l>W7H9U%5f?GU1h_DmTo3{6sDPi{+<P
z>e~gv3iH%wTYvT0b@Sujec^W>9w?ep_uSmK9^IzSm?PXt@Ci{<C{*$MjH_z*JmFs!
zj2XI1sJ0SOjV%$j@tDiKR7jj>C*uoY1&OE~UkGKQOTo!-);#)sTqs^{W^<-T6gBph
z@SfxfG1HpoxP(h4A5tTNIN{_Ui9<vY9k{9kig<E5#`jf<P8PpXJh6a(Y#fvz@{f(H
zo}hTq>|*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^1<DY$@0~jesQggX5<VrVc?8-BdyJzKn0tK7q65yc2+!uUbW|v
z&duk<lW5t_CnFV|7b|8E1$-ko3I)QcB{qVjqK=M;vx$Co?DLHyP*mGT;-L~!1Ca^Q
z2^Tv=^Ax-93MS1=?}1^)Za5~I;qMb-N%D@`>m8sR6sN&G3DGWkS*2Ec{d}lBhM1pW
zH9A(6!g>*d4I`ojUQLL_a0u`0^>3jM%auMZMKG4YDHrL|4-9l&uOc}~U9U$fONQd>
zKZVeU=;+-i<+<u~(SLMNdgtnTqv+1^^<I(kMBQ!UX|M&c)M-Sr6E_qQ^qC!G1K4;?
z58!11aqrv^B$6{7zahQ#X}S&~nRI&RzfKRsNbAFGNdJ7(V$TsCL+kxEG+{nAj<mK2
z+>F*sZos$AB)SfKymSc44&u)Ilb62UhK5TJ?ZDo?K_$D(gCg9Th7nzy9`!<~rIJ%z
zsM8wM;viukmGY9af}>PAkPJP=-uQs7^)hFih!?yjoEJMVu)~A{^r+n?={=I1AUtGF
zj<ngF-X;H@<SxN^^X>WjpN4n>6JE*0UI2xc9N@4>FS+!>{GVaQ%$Y2NVT-<3kR`B8
zYQ7+CBBTyN;_YEIy`$^W{T4wKX;B|_(-nxI34awO8<g>q1tvsk8CeKk>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>
z<zY6(p3moEjUP^+w&OCyCfIf`gmHYQIf?9d*d*0qlV6IgWY0df6W_!O|Kte!Z*@A^
zGcObG*Kx&wM|HgD$SK*mjGggJ)u_(0^O}jrb=+&f#~~V(9kBJ3Eauft=Cbo4j+~U8
z+H9xO2Qf?u&Qo$Oe82=jgP;byr<h<J;;f|*D5p`d^OTII<FqWe!jlCuvzcQb)S{y+
zf)<aLKAP+?yk~Sjgcu#2Oy`7jdH|w~j!mYs)1w%K_-WZ;cMco)82Au}S!ZOy?qsPl
z=4Bm)7$aAo!8Vb~EX!(xF~;pdF8L<QDA^ih&Q@nWdnaK{%4BIy9BoDy!7!uU26|AZ
zYmc18xv6JlJV!Pi^MM;dSI1;YuHI#S2z*HAbx)>kgJ3(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
zJZiwbI<B_^?dN4%S3%t1r8iwLZoormV|CMQ20X0eMFu>g<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%U8qOV9kd<unx^_4{UD>Y%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@q5q<Pl0AQ9!Cpi03HuD8rOKNTHL6SliL9
z5gpVQq_*W-jvG{lCBv!Vd4tRmol}m-M~BY!s3(d1;pxO9oq9AI(Cqt5ZfJVh{#j4&
z`QGp6^L>AR_a=#dSLVn6MS1>>P1|PIZdkb3F~4y^?ZSHdLk&NF#NIuf6dG(c&SJW`
z%R7mGhdFW(Oed7Q?o#4B8aY^H%Jf(;zhUlizca+npS#F5&pyv?n`<wdx3Fx1BU`fF
zzSg#;Ve7+9TOPDk*z~!Qp6yG|pOsFV82TrLhM<NctJstI5a7rna!8GF<RbrnMOG66
z>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@8lwT47g7<ea*$l)Hh&{(W<~
zN<!NV&*NT6-gxJ8tN0}1;10~nj$y$jESPz$GBN_q_1v`CBeeH_2$A8%#SCc^zF^x*
z7+c2?C)0%{&i#x@>z8I4fh*-)F?><a-CYo6stz*zlGh`H@n12__Qx4p)eeU5z7pl(
zrwv?z5~V}ppDbpW?ynHr*1&B->|$+VsH;56U%WLj*zLW`3KJT+5;6q#M(!4Is9oR&
z9Mc&ujI@yaEcb0V=ot(@+!Qr{zmY3qF;1|NyCFMv`HMun>H?-uL<S+&$mLNg-pH+(
zK3skfJzk8&%6Bll?}^CgSUyWDak!Z)T_sTl+#9(ECJ(Q?di6;+liVa<`a6b+Z;Hg=
z<Bi-!EGF#X78YUsEW`IVq4x|Nb9Iz&ZD4G(e$6HPzmEzqsfk-lK8NNe?q>ApY2x-P
zQU3B9iIJ)|v8)J|<=n*0V9~OE6ZhyHv<zj5Gm)smU{x5{@fhRnS{!A%Yoh$h(UNRh
zOMAzZHs=nqjoxp6-BM3E$Av%~eRDbve>I8P!;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}!<jOAsw2q<RE#)KYL?K8}`A%<G;)
z!Qo}6I>X<G?b^h{Q>;T&Rm?jtpvd6w$MEIiz%5WjB;!PL_|Eb4diyD_c-8<NqoM)+
z(jw*(hdR_EzDiQav#nx@wIUVyU4zcm0fVig#agD(ijJ<>+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;<C-h9oJhv%
zJR$B$S!e$VZN^*MPl|_Aq(Ub@j#NiZii@VgmV{`8Ip@V<I6&{#(>sc>vo3%?A?B$A
z=S3??nZ5myXil1`k&nbDOGF>WWRzU@1qkLyd9ZU7={-4;G1vXAmMuMA9|T9mQsRTy
zC@!)!<yTvvSvjeW&9{OnfwYfmN^Whs<n*3>;LM~CqE+TG@PjvjIsDL%R-#)A@+JgD
ztnHJ8Xtwrf*m=z}U0qL)gnqqt$zt_uCg}#!xqEuul7;m2wo0XGn+*@@!}NwRf+rPW
z!zqn*QXQM~^p0Q_57@|<QC%)lnLYG?i_%)5bu{~U+K055rq`tAeu_xi-0L>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
z<HoI;_q8Jlu1p%go{ZZ=5BMZ$1#b78l$=)_Mf&J^Se>ySoHC};4wgs%dhP+Qj6ph~
zAFc2UX9!<r1a}b58mrIq?RE8yP+E)Glz!8t%lCsVd(68j?>cW6(xgZAWJ@`$HGnjZ
zwe<td%t>{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%<N)8>%vfx%;P
z9#kHaMQdz4mqjb0y$yq#x{i#evt4_Sqv!r(vXGlkEi=P9z)r~zki0YBq$34ttyuDB
zqR8k>8Pae&%FJMK4R?U;xNNaHb#zdptHDKS*LYf?<}_OkexBk+empMcNJ@<cdLW3P
zxf9ilN^(u$I)UYXa~!AAhg?QACaTkTLHs3(=|Vc%qtW=2htlW9(>Y;{9t1CD=&({X
zGqWwlTdB;%=}a(dvibjpyLI*s&EBWO>2zqE{v<x@+CdQf8WBzs*YI4ohR48u5+4})
zrW{w8FU1$wz<pA-+~L&GL5;2%k5BH?aIX$e+jl}3!*%rQ=oso)enDtFPG2ZVd{Bqm
zG<;ZxJ5%@+Dl*2I=hpE26yA}3PV#7-n0SgkMO?#u8eXl#{TlAp;r$xks>6dCPW{FT
zl9{})Mg$;s3I{^fF`mhbY4nf|AJK3sGwdxC#y+MBI^3k;HXTlNYVSzl(4xxb<Rg|D
zl#<lQ1N|zVd0)ylJtIp^iY|1)AkxIE4y&?4*n1g>hvW|Yn$&t4g%9GV<(=#U^TGF;
zyok6VghD(-eHmlnA&9>wS83`?JMaU=CQO0*&&X5BAhe#5E7@C35IQ4Qk<hOA85yrE
zHGU@h(NAIZ3yOfhBwP0?PeXwImYW6295?`q1s(zAJ$T`_N)gQIQ;ZPqQ>Kw6@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
--
2.25.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 10/10] ppc/pnv: Create BMC devices only when defaults are enabled
2020-04-07 4:35 [PULL 00/10] ppc-for-5.0 queue 20200407 David Gibson
` (8 preceding siblings ...)
2020-04-07 4:36 ` [PULL 09/10] pseries: Update SLOF firmware image David Gibson
@ 2020-04-07 4:36 ` David Gibson
2020-06-19 18:02 ` Philippe Mathieu-Daudé
2020-04-07 13:01 ` [PULL 00/10] ppc-for-5.0 queue 20200407 Peter Maydell
10 siblings, 1 reply; 15+ messages in thread
From: David Gibson @ 2020-04-07 4:36 UTC (permalink / raw)
To: peter.maydell
Cc: aik, qemu-devel, groug, qemu-ppc, clg, Nathan Chancellor,
David Gibson
From: Cédric Le Goater <clg@kaod.org>
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 <natechancellor@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20200404153655.166834-1-clg@kaod.org>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
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
--
2.25.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PULL 10/10] ppc/pnv: Create BMC devices only when defaults are enabled
2020-04-07 4:36 ` [PULL 10/10] ppc/pnv: Create BMC devices only when defaults are enabled David Gibson
@ 2020-06-19 18:02 ` Philippe Mathieu-Daudé
2020-06-22 7:09 ` Cédric Le Goater
0 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-19 18:02 UTC (permalink / raw)
To: David Gibson, Cédric Le Goater
Cc: Peter Maydell, Thomas Huth, Alexey Kardashevskiy, QEMU Developers,
Greg Kurz, open list:sPAPR, Nathan Chancellor
Hi,
On Tue, Apr 7, 2020 at 6:42 AM David Gibson <david@gibson.dropbear.id.au> wrote:
>
> From: Cédric Le Goater <clg@kaod.org>
>
> 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 <natechancellor@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Message-Id: <20200404153655.166834-1-clg@kaod.org>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
Not sure if directly related to this patch, but on gitlab-ci we get:
TEST check-qtest-ppc64: tests/qtest/m48t59-test
TEST check-qtest-ppc64: tests/qtest/device-plug-test
TEST check-qtest-ppc64: tests/qtest/pnv-xscom-test
TEST check-qtest-ppc64: tests/qtest/migration-test
TEST check-qtest-ppc64: tests/qtest/rtas-test
TEST check-qtest-ppc64: tests/qtest/usb-hcd-uhci-test
TEST check-qtest-ppc64: tests/qtest/usb-hcd-xhci-test
TEST check-qtest-ppc64: tests/qtest/test-filter-mirror
TEST check-qtest-ppc64: tests/qtest/test-filter-redirector
TEST check-qtest-ppc64: tests/qtest/display-vga-test
TEST check-qtest-ppc64: tests/qtest/numa-test
TEST check-qtest-ppc64: tests/qtest/ivshmem-test
TEST check-qtest-ppc64: tests/qtest/cpu-plug-test
TEST check-qtest-ppc64: tests/qtest/cdrom-test
TEST check-qtest-ppc64: tests/qtest/device-introspect-test
qemu-system-ppc64: warning: machine has no BMC device. Use '-device
ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define
one
qemu-system-ppc64: warning: machine has no BMC device. Use '-device
ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define
one
qemu-system-ppc64: warning: machine has no BMC device. Use '-device
ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define
one
Since this is very confusing, can you adapt the test?
Thanks,
Phil.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PULL 10/10] ppc/pnv: Create BMC devices only when defaults are enabled
2020-06-19 18:02 ` Philippe Mathieu-Daudé
@ 2020-06-22 7:09 ` Cédric Le Goater
2020-06-22 8:27 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Cédric Le Goater @ 2020-06-22 7:09 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Gibson
Cc: Peter Maydell, Thomas Huth, Alexey Kardashevskiy, QEMU Developers,
Greg Kurz, open list:sPAPR, Nathan Chancellor
On 6/19/20 8:02 PM, Philippe Mathieu-Daudé wrote:
> Hi,
>
> On Tue, Apr 7, 2020 at 6:42 AM David Gibson <david@gibson.dropbear.id.au> wrote:
>>
>> From: Cédric Le Goater <clg@kaod.org>
>>
>> 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 <natechancellor@gmail.com>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Message-Id: <20200404153655.166834-1-clg@kaod.org>
>> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>
> Not sure if directly related to this patch, but on gitlab-ci we get:
>
> TEST check-qtest-ppc64: tests/qtest/m48t59-test
> TEST check-qtest-ppc64: tests/qtest/device-plug-test
> TEST check-qtest-ppc64: tests/qtest/pnv-xscom-test
> TEST check-qtest-ppc64: tests/qtest/migration-test
> TEST check-qtest-ppc64: tests/qtest/rtas-test
> TEST check-qtest-ppc64: tests/qtest/usb-hcd-uhci-test
> TEST check-qtest-ppc64: tests/qtest/usb-hcd-xhci-test
> TEST check-qtest-ppc64: tests/qtest/test-filter-mirror
> TEST check-qtest-ppc64: tests/qtest/test-filter-redirector
> TEST check-qtest-ppc64: tests/qtest/display-vga-test
> TEST check-qtest-ppc64: tests/qtest/numa-test
> TEST check-qtest-ppc64: tests/qtest/ivshmem-test
> TEST check-qtest-ppc64: tests/qtest/cpu-plug-test
> TEST check-qtest-ppc64: tests/qtest/cdrom-test
> TEST check-qtest-ppc64: tests/qtest/device-introspect-test
> qemu-system-ppc64: warning: machine has no BMC device. Use '-device
> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define
> one
> qemu-system-ppc64: warning: machine has no BMC device. Use '-device
> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define
> one
> qemu-system-ppc64: warning: machine has no BMC device. Use '-device
> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define
> one
I can not reproduce. Is gitlab-ci doing something special ?
C.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PULL 10/10] ppc/pnv: Create BMC devices only when defaults are enabled
2020-06-22 7:09 ` Cédric Le Goater
@ 2020-06-22 8:27 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-22 8:27 UTC (permalink / raw)
To: Cédric Le Goater, David Gibson
Cc: Peter Maydell, Thomas Huth, Alexey Kardashevskiy, QEMU Developers,
Greg Kurz, open list:sPAPR, Nathan Chancellor
On 6/22/20 9:09 AM, Cédric Le Goater wrote:
> On 6/19/20 8:02 PM, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> On Tue, Apr 7, 2020 at 6:42 AM David Gibson <david@gibson.dropbear.id.au> wrote:
>>>
>>> From: Cédric Le Goater <clg@kaod.org>
>>>
>>> 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 <natechancellor@gmail.com>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> Message-Id: <20200404153655.166834-1-clg@kaod.org>
>>> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>> ---
>>
>> Not sure if directly related to this patch, but on gitlab-ci we get:
>>
>> TEST check-qtest-ppc64: tests/qtest/m48t59-test
>> TEST check-qtest-ppc64: tests/qtest/device-plug-test
>> TEST check-qtest-ppc64: tests/qtest/pnv-xscom-test
>> TEST check-qtest-ppc64: tests/qtest/migration-test
>> TEST check-qtest-ppc64: tests/qtest/rtas-test
>> TEST check-qtest-ppc64: tests/qtest/usb-hcd-uhci-test
>> TEST check-qtest-ppc64: tests/qtest/usb-hcd-xhci-test
>> TEST check-qtest-ppc64: tests/qtest/test-filter-mirror
>> TEST check-qtest-ppc64: tests/qtest/test-filter-redirector
>> TEST check-qtest-ppc64: tests/qtest/display-vga-test
>> TEST check-qtest-ppc64: tests/qtest/numa-test
>> TEST check-qtest-ppc64: tests/qtest/ivshmem-test
>> TEST check-qtest-ppc64: tests/qtest/cpu-plug-test
>> TEST check-qtest-ppc64: tests/qtest/cdrom-test
>> TEST check-qtest-ppc64: tests/qtest/device-introspect-test
>> qemu-system-ppc64: warning: machine has no BMC device. Use '-device
>> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define
>> one
>> qemu-system-ppc64: warning: machine has no BMC device. Use '-device
>> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define
>> one
>> qemu-system-ppc64: warning: machine has no BMC device. Use '-device
>> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define
>> one
>
> I can not reproduce. Is gitlab-ci doing something special ?
(Greg already answered elsewhere, for for other readers):
The test is ran when using:
$ make check-qtest SPEED=slow
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PULL 00/10] ppc-for-5.0 queue 20200407
2020-04-07 4:35 [PULL 00/10] ppc-for-5.0 queue 20200407 David Gibson
` (9 preceding siblings ...)
2020-04-07 4:36 ` [PULL 10/10] ppc/pnv: Create BMC devices only when defaults are enabled David Gibson
@ 2020-04-07 13:01 ` Peter Maydell
10 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2020-04-07 13:01 UTC (permalink / raw)
To: David Gibson
Cc: Alexey Kardashevskiy, Cédric Le Goater, qemu-ppc,
QEMU Developers, Greg Kurz
On Tue, 7 Apr 2020 at 05:36, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> The following changes since commit 53ef8a92eb04ee19640f5aad3bff36cd4a36c250:
>
> Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200406' into staging (2020-04-06 12:36:45 +0100)
>
> are available in the Git repository at:
>
> git://github.com/dgibson/qemu.git tags/ppc-for-5.0-20200407
>
> for you to fetch changes up to 25f3170b06544e4de620336da5b2ea3b392d66bc:
>
> ppc/pnv: Create BMC devices only when defaults are enabled (2020-04-07 08:55:11 +1000)
>
> ----------------------------------------------------------------
> ppc patch queue 2020-04-07
>
> An assortment of fixes for qemu-5.0, including a number for the FWNMI
> feature which is new this release.
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread