Re: [PATCH V2] Add a new PIIX option to control PCI hot unplugging of devices on non-root buses

[Ani Sinha]

> On Apr 29, 2020, at 1:39 PM, Michael S. Tsirkin <address@hidden> wrote:
> 
> On Wed, Apr 29, 2020 at 07:43:04AM +0000, Ani Sinha wrote:
>> 
>> 
>>> On Apr 29, 2020, at 1:08 PM, Michael S. Tsirkin <address@hidden> wrote:
>>> 
>>> On Wed, Apr 29, 2020 at 07:02:56AM +0000, Ani Sinha wrote:
>>>> 
>>>> 
>>>>> On Apr 29, 2020, at 12:27 PM, Michael S. Tsirkin <address@hidden> wrote:
>>>>> 
>>>>> On Wed, Apr 29, 2020 at 06:54:52AM +0000, Ani Sinha wrote:
>>>>>> 
>>>>>> 
>>>>>>> On Apr 29, 2020, at 12:22 PM, Michael S. Tsirkin <address@hidden> wrote:
>>>>>>> 
>>>>>>> On Wed, Apr 29, 2020 at 06:11:20AM +0000, Ani Sinha wrote:
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> On Apr 29, 2020, at 10:58 AM, Michael S. Tsirkin <address@hidden> 
>>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>> o if there's a need to disable
>>>>>>>>> just one of these, commit log needs to do a better job documenting the
>>>>>>>>> usecase.
>>>>>>>> 
>>>>>>>> The use case is simple. With this feature admins will be able to do 
>>>>>>>> what they were forced to do from Windows driver level but now at the 
>>>>>>>> bus level. Hence, 
>>>>>>>> (a) They need not have access to the guest VM to change or update 
>>>>>>>> windows driver registry settings. They can enable the same setting 
>>>>>>>> from admin management console without any access to VM.
>>>>>>>> (b) It is more robust. No need to mess with driver settings. Incorrect 
>>>>>>>> settings can brick guest OS. Also no guest specific knowhow required.
>>>>>>>> (c) It is more scalable since a single cluster wide setting can be 
>>>>>>>> used for all VM power ons and the management plane can take care of 
>>>>>>>> the rest automatically. No need to access individual VMs to enforce 
>>>>>>>> this.
>>>>>>>> (d) I am told that the driver level solution does not persist across a 
>>>>>>>> reboot. 
>>>>>>>> 
>>>>>>>> Ani
>>>>>>> 
>>>>>>> Looks like disabling both plug and unplug would also address these 
>>>>>>> needs.
>>>>>> 
>>>>>> No the driver level solution does not prevent hot plugging of devices 
>>>>>> but blocks just hot unplugging. The solution I am proposing tries to do 
>>>>>> the same but from the bus/hypervisor level.
>>>>> 
>>>>> Why does the driver level solution need to prevent just hot unplugging?
>>>> 
>>>> Because it not fair to prevent end users from hot plugging new devices 
>>>> when it is the (accidental?) hot unplugging of existing devices which 
>>>> causes issues.
>>> 
>>> Accidental? So maybe what you need is actually something else then -
>>> avoid *removing* the device when it's powered down.
>> 
>> You don’t get it. It is not hypervisor admins who are unplugging it. It is 
>> the end users. Even RedHat customers want this feature. See following 
>> resources: 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com_archives_libvir-2Dlist_2020-2DFebruary_msg00110.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=IIUxIyRwG4RGy57y2nvMNYcDkqW-NHozZ2R38VYcg5U&m=Mf70_yU9LUbRFZOy4rYM5N43B_MDbO7SxEMSSJKVaJY&s=KgR1-KlzL-bGr51uY1vupOIgTpTjNAecbuOUIpcuMUs&e=
>>  
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugzilla.redhat.com_show-5Fbug.cgi-3Fid-3D1802592&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=IIUxIyRwG4RGy57y2nvMNYcDkqW-NHozZ2R38VYcg5U&m=Mf70_yU9LUbRFZOy4rYM5N43B_MDbO7SxEMSSJKVaJY&s=KVis9gzVeA7nnGauZpXWm_sEnl_UpsIzSlggwb60Fg8&e=
>>  
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugzilla.redhat.com_show-5Fbug.cgi-3Fid-3D1790899&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=IIUxIyRwG4RGy57y2nvMNYcDkqW-NHozZ2R38VYcg5U&m=Mf70_yU9LUbRFZOy4rYM5N43B_MDbO7SxEMSSJKVaJY&s=Y0K8tiyqpmKeXU245pnhTTAr2e3YSuxxw4BkiDxGwB8&e=
>>  
> 
> That doesn't seem to require that hotplug keeps working.

Like I said, that is because PCIE limits this at this moment. We can do better 
than this solution on i440fx. We can be narrower in limiting just hot unplug 
leaving hot plugging as is. Why should we always go with the least common 
denominator for all the features? Why do we have to be limited when we can do 
better?


> 
>> My approach is much more fine grained than just disable everything approach 
>> that we have for q35. For i440fx we can do better than that.
>> 
>> 
>>> 
>>>>> 
>>>>> 
>>>>>> 
>>>>>>> -- 
>>>>>>> MST