[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 4/6] s390x/pci: Introduce unplug requests and
Re: [Qemu-devel] [PATCH v2 4/6] s390x/pci: Introduce unplug requests and split unplug handler
Thu, 31 Jan 2019 22:11:01 +0100
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1
On 31.01.19 21:40, Collin Walling wrote:
> On 1/30/19 10:57 AM, David Hildenbrand wrote:
>> PCI on s390x is really weird and how it was modeled in QEMU might not have
>> been the right choice. Anyhow, right now it is the case that:
>> - Hotplugging a PCI device will silently create a zPCI device
>> (if none is provided)
>> - Hotunplugging a zPCI device will unplug the PCI device (if any)
>> - Hotunplugging a PCI device will unplug also the zPCI device
>> As far as I can see, we can no longer change this behavior. But we
>> should fix it.
>> Both device types are handled via a single hotplug handler call. This
>> is problematic for various reasons:
>> 1. Unplugging via the zPCI device allows to unplug devices that are not
>> hot removable. (check performed in qdev_unplug()) - bad.
>> 2. Hotplug handler chains are not possible for the unplug case. In the
>> future, the machine might want to override hotplug handlers, to
>> process device specific stuff and to then branch off to the actual
>> hotplug handler. We need separate hotplug handler calls for both the
>> PCI and zPCI device to make this work reliably. All other PCI
>> implementations are already prepared to handle this correctly, only
>> s390x is missing.
>> Therefore, introduce the unplug_request handler and properly perform
>> unplug checks by redirecting to the separate unplug_request handlers.
>> When finally unplugging, perform two separate hotplug_handler_unplug()
>> calls, first for the PCI device, followed by the zPCI device. This now
>> nicely splits unplugging paths for both devices.
>> The redirect part is a little hairy, as the user is allowed to trigger
>> unplug either via the PCI or the zPCI device. So redirect always to the
>> PCI unplug request handler first and remember if that check has been
>> performed in the zPCI device. Redirect then to the zPCI device unplug
>> request handler to perform the magic. Remembering that we already
>> checked the PCI device breaks the redirect loop.
>> Signed-off-by: David Hildenbrand <address@hidden>
> Thanks for clearing up some of the areas I was having troubles following
> during the last post. I think this looks a lot better than what we had
> before. Patches 4, 5, and 6 make the code a lot easier to understand
> what is going on here.
> I assume you were able to squeeze in an easy test? At least making sure
> we can still hot unplug a device from a guest successfully?
I only tested under TCG, but I did quite some tests
- Plug/unplug a device
- Disable slot in guest, unplug device
- Plug bridges, try to unplug
- Create hierarchy of bridges (statically and dynamically)
- Plug devices to bridge hierarchies ...
> From the code perspective:
> Reviewed-by: Collin Walling <address@hidden>
David / dhildenb
- [Qemu-devel] [PATCH v2 0/6] s390x/pci: remaining hot/un)plug patches, David Hildenbrand, 2019/01/30
- [Qemu-devel] [PATCH v2 1/6] s390x/pci: Fix primary bus number for PCI bridges, David Hildenbrand, 2019/01/30
- [Qemu-devel] [PATCH v2 2/6] s390x/pci: Fix hotplugging of PCI bridges, David Hildenbrand, 2019/01/30
- [Qemu-devel] [PATCH v2 4/6] s390x/pci: Introduce unplug requests and split unplug handler, David Hildenbrand, 2019/01/30
- [Qemu-devel] [PATCH v2 3/6] s390x/pci: Warn when adding PCI devices without the 'zpci' feature, David Hildenbrand, 2019/01/30
- [Qemu-devel] [PATCH v2 5/6] s390x/pci: Drop release timer and replace it with a flag, David Hildenbrand, 2019/01/30
- [Qemu-devel] [PATCH v2 6/6] s390x/pci: Unplug remaining requested devices on pcihost reset, David Hildenbrand, 2019/01/30
- Re: [Qemu-devel] [PATCH v2 0/6] s390x/pci: remaining hot/un)plug patches, Cornelia Huck, 2019/01/30
- Re: [Qemu-devel] [qemu-s390x] [PATCH v2 0/6] s390x/pci: remaining hot/un)plug patches, Collin Walling, 2019/01/31