Hi Marcel, thanks for your reply.
On 14-05-28 05:10 AM, Marcel Apfelbaum wrote:
> This is correct since now you are using a Root Complex Port.
> If you don't want to use a config file use:
>
> <qemu-bin> [...] -device ioh3420,bus=pcie.0,id=ich9-pcie-port-1,slot=3
You're right I could have done it on the command line directly.
> As I said before, QEMU it should not crash.
Well without the fix QEMU assert in memory_region_del_eventfd(). I can provide
the bt if
needed?
> However, the reason that it does not work
> is because the current implementation uses "suprise"removal and the OS does
> not have
> the chance to unload the driver and power down the device.
Yes exactly
> Your timer solves this by letting the OS to acknowledge "PRESENT DETECT"
> change and
> unload the device, this however is not the optimal approach since you have a
> race condition.
I must agree with you that if the disk is removed and plugged back within the
timer period then it
will race.
>
> I have an "in progress" implementation that works for Linux only and should
> solve your issue.
This patch works fine for me. Thanks!
I was looking for a mechanism to initiate the object_unparent() based on some
feedback from the OS
inside the pcie_cap_slot_write_config() but could not find anything useful with
the current
notification scheme.
The 'attention button' definitely provide a nice handshake to QEMU for clean-up.
>
> Subject: [Qemu-devel] [PATCH] pcie: hotplug/unplug for linux
>
> Signed-off-by: Marcel Apfelbaum <marce...@redhat.com>
> ---
> hw/pci/pcie.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 02cde6f..8f06713 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -224,7 +224,7 @@ static void pcie_cap_slot_hotplug_common(PCIDevice
> *hotplug_dev,
> *exp_cap = hotplug_dev->config + hotplug_dev->exp.exp_cap;
> uint16_t sltsta = pci_get_word(*exp_cap + PCI_EXP_SLTSTA);
>
> - PCIE_DEV_PRINTF(PCI_DEVICE(dev), "hotplug state: %d\n", state);
> + PCIE_DEV_PRINTF(PCI_DEVICE(dev), "hotplug state: 0x%x\n", sltsta);
> if (sltsta & PCI_EXP_SLTSTA_EIS) {
> /* the slot is electromechanically locked.
> * This error is propagated up to qdev and then to HMP/QMP.
> @@ -258,7 +258,7 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler
> *hotplug_dev, DeviceState *dev,
>
> pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
> PCI_EXP_SLTSTA_PDS);
> - pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), PCI_EXP_HP_EV_PDC);
> + pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
> }
>
> void pcie_cap_slot_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState
> *dev,
> @@ -268,10 +268,7 @@ void pcie_cap_slot_hot_unplug_cb(HotplugHandler
> *hotplug_dev, DeviceState *dev,
>
> pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap,
> errp);
>
> - object_unparent(OBJECT(dev));
> - pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
> - PCI_EXP_SLTSTA_PDS);
> - pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), PCI_EXP_HP_EV_PDC);
> + pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
> }
>
> /* pci express slot for pci express root/downstream port
> @@ -292,6 +289,7 @@ void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot)
> PCI_EXP_SLTCAP_HPC |
> PCI_EXP_SLTCAP_PIP |
> PCI_EXP_SLTCAP_AIP |
> + PCI_EXP_SLTCAP_PCP |
> PCI_EXP_SLTCAP_ABP);
>
> pci_word_test_and_clear_mask(dev->config + pos + PCI_EXP_SLTCTL,
> @@ -302,6 +300,7 @@ void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot)
> PCI_EXP_SLTCTL_AIC_OFF);
> pci_word_test_and_set_mask(dev->wmask + pos + PCI_EXP_SLTCTL,
> PCI_EXP_SLTCTL_PIC |
> + PCI_EXP_SLTCTL_PCC |
> PCI_EXP_SLTCTL_AIC |
> PCI_EXP_SLTCTL_HPIE |
> PCI_EXP_SLTCTL_CCIE |
> @@ -376,6 +375,16 @@ void pcie_cap_slot_write_config(PCIDevice *dev,
> sltsta);
> }
>
> + if ((val & PCI_EXP_SLTCTL_PCC) &&
> + ((val & PCI_EXP_SLTCTL_PIC_OFF) == PCI_EXP_SLTCTL_PIC_OFF)) {
> + PCIDevice *slot_dev =
> pci_bridge_get_sec_bus(PCI_BRIDGE(dev))->devices[0];
> + if (slot_dev) {
> + object_unparent(OBJECT(slot_dev));
> + pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
> + PCI_EXP_SLTSTA_PDS);
> + }
> + }
> +
> hotplug_event_notify(dev);
>
> /*
>
thanks,
Etienne
