Quoting Thomas Huth (2019-02-28 12:40:52) > On 26/02/2019 05.52, David Gibson wrote: > > From: Michael Roth <mdr...@linux.vnet.ibm.com> > > > > Extend the existing EPOW event format we use for PCI > > devices to emit PHB plug/unplug events. > > > > Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com> > > Reviewed-by: David Gibson <da...@gibson.dropbear.id.au> > > Signed-off-by: Greg Kurz <gr...@kaod.org> > > Message-Id: > > <155059671405.1466090.535964535260503283.st...@bahia.lab.toulouse-stg.fr.ibm.com> > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > > --- > > hw/ppc/spapr_events.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c > > index b9c7ecb9e9..ab9a1f0063 100644 > > --- a/hw/ppc/spapr_events.c > > +++ b/hw/ppc/spapr_events.c > > @@ -526,6 +526,9 @@ static void spapr_hotplug_req_event(uint8_t hp_id, > > uint8_t hp_action, > > case SPAPR_DR_CONNECTOR_TYPE_CPU: > > hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_CPU; > > break; > > + case SPAPR_DR_CONNECTOR_TYPE_PHB: > > + hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_PHB; > > + break; > > default: > > /* we shouldn't be signaling hotplug events for resources > > * that don't support them > > I think this patch (or something else in this PULL request) broke CPU > hot-plugging with older machine types: > > $ QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 ./tests/cpu-plug-test > -m=slow > /ppc64/cpu-plug/pseries-3.1/device-add/2x3x1&maxcpus=6: OK > /ppc64/cpu-plug/pseries-2.12-sxxm/device-add/2x3x1&maxcpus=6: OK > /ppc64/cpu-plug/pseries-3.0/device-add/2x3x1&maxcpus=6: OK > /ppc64/cpu-plug/pseries-2.10/device-add/2x3x1&maxcpus=6: OK > /ppc64/cpu-plug/pseries-2.11/device-add/2x3x1&maxcpus=6: OK > /ppc64/cpu-plug/pseries-2.12/device-add/2x3x1&maxcpus=6: OK > /ppc64/cpu-plug/pseries-2.9/device-add/2x3x1&maxcpus=6: OK > /ppc64/cpu-plug/pseries-2.7/device-add/2x3x1&maxcpus=6: ** > ERROR:/home/thuth/devel/qemu/hw/ppc/spapr_events.c:313:rtas_event_log_to_source: > assertion failed: (source->enabled) > Broken pipe > /home/thuth/devel/qemu/tests/libqtest.c:143: kill_qemu() detected QEMU death > from signal 6 (Aborted) (core dumped) > Aborted (core dumped) > > Could you please have a look?
Bisected to: commit b8165118f52ce5ee88565d3cec83d30374efdc96 Author: David Hildenbrand <da...@redhat.com> Date: Mon Feb 18 10:21:58 2019 +0100 spapr: support memory unplug for qtest Fake availability of OV5_HP_EVT, so we can test memory unplug in qtest. Which makes sense since OV5_HP_EVT assumes that spapr->spapr->use_hotplug_event_source == true, which isn't the default for 2.7 and below. If I revert that I think I hit the bug it was meant to fix: mdroth@sif:~/w/qemu-build3$ make V=1 check-qtest-ppc64 ... PASS 1 device-plug-test /ppc64/device-plug/pci-unplug-request PASS 2 device-plug-test /ppc64/device-plug/spapr-cpu-unplug-request ** ERROR:/home/mdroth/w/qemu3.git/tests/device-plug-test.c:28:device_del_finish: assertion failed: (qdict_haskey(resp, "return")) ERROR - Bail out! ERROR:/home/mdroth/w/qemu3.git/tests/device-plug-test.c:28:device_del_finish: assertion failed: (qdict_haskey(resp, "return")) Aborted (core dumped) /home/mdroth/w/qemu3.git/tests/Makefile.include:875: recipe for target 'check-qtest-ppc64' failed make: *** [check-qtest-ppc64] Error 1 mdroth@sif:~/w/qemu-build3$ Which is probably due to this check in spapr_machine_device_unplug_request(): if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { if (spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) { spapr_memory_unplug_request(hotplug_dev, dev, errp); } else { /* NOTE: this means there is a window after guest reset, prior to * CAS negotiation, where unplug requests will fail due to the * capability not being detected yet. This is a bit different than * the case with PCI unplug, where the events will be queued and * eventually handled by the guest after boot */ error_setg(errp, "Memory hot unplug not supported for this guest"); } This spapr-cpu-unplug-request test is failing because spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT) relies on the CAS-negotiated OV5 bit, which wouldn't have happened with qtest. If we want to make these tests run in this scenario we probably need a different approach than the original patch. > > Thomas >