qemu-devel
[Top][All Lists]
Advanced

[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


From: David Hildenbrand
Subject: Re: [Qemu-devel] [PATCH v2 4/6] s390x/pci: Introduce unplug requests and split unplug handler
Date: Thu, 31 Jan 2019 22:11:01 +0100
User-agent: 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 ...

Thanks!

> 
>  From the code perspective:
> 
> Reviewed-by: Collin Walling <address@hidden>
> 


-- 

Thanks,

David / dhildenb



reply via email to

[Prev in Thread] Current Thread [Next in Thread]