Re: [PATCH 0/4] ICH9 root PCI hotplug

[Igor Mammedov]

On Wed, 29 Nov 2023 09:47:13 +0000
Thierry Escande <thierry.escande@vates.tech> wrote:

> On 24/11/2023 15:54, Michael S. Tsirkin wrote:
> > On Fri, Nov 24, 2023 at 03:01:35PM +0100, Igor Mammedov wrote:  
> >> On Wed, 15 Nov 2023 17:18:53 +0000
> >> Thierry Escande <thierry.escande@vates.tech> wrote:
> >>  
> >>> Hi,
> >>>
> >>> This series fixes acpi_hotplug_bridge accessor names, adds new accessors
> >>> for acpi-root-pci-hotplug property, and enables root PCI hotplug by
> >>> default for Q35 machine.  
> >>
> >> hotplug on Q35 hostbridge is not implemented intentionally
> >> to keep machine close to the real world.  
> >   
> >> PCIe spec 3.1a, 1.3.2.3. Root Complex Integrated Endpoint Rules
> >> "
> >> A Root Complex Integrated Endpoint may not be hot-plugged independent of 
> >> the Root
> >> Complex as a whole.
> >> "
> >> )  
> > 
> > To be more precise close to native hotplug.
> > But we used ACPI for several years now and it seems to be fine.
> > Maybe it's time we lifted the limitation?  

Pardon me for being conservative, but adding ACPI PCI hotplug to
PCIe hostbridge complicates its maintenance not to mention that
on guest side it likely to cause issues as well (since it's
architecturally not possible).

I've grudgingly accepted ACPI hotplug on Q35 root-ports/bridges
(since it possible for them to support hotplug in principle
(aka guest OSes account somewhat for that)), and even that wasn't
smooth ride despite of it working on PC machine for ages.
I'd expect more issues to pop up in case of PCIe hostbridge.

This why I dislike the idea very much.

For those who are desperate there is PC machine which supports
hotplug to root bus. For Q35 one can use root-ports for that.

Anyways, it's up to you.

> And for what it's worth, lifting this limitation would allow PCIe 
> devices passthrough on Q35 VMs with Xen as such devices are hotplugged.

maybe ask Xen folks to fix their impl. and make it work with root-ports
like the rest of the world or at least provide a very good justification
why they aren't able to that.

   
> >>
> >> PS:
> >> but patch 1/4 is good cleanup, pls include Reviewed-by's and resend it
> >> as a separate patch after 8.2 has been released (so it wouldn't get lost 
> >> in the traffic).
> >>  
> >>>
> >>> Thierry Escande (4):
> >>>    ich9: Remove unused hotplug field from ICH9LPCPMRegs struct
> >>>    ich9: Renamed use_acpi_hotplug_bridge accessors
> >>>    ich9: Add accessors for acpi-root-pci-hotplug
> >>>    ich9: Enable root PCI hotplug by default
> >>>
> >>>   hw/acpi/ich9.c         | 23 +++++++++++++++++++++--
> >>>   include/hw/acpi/ich9.h |  1 -
> >>>   2 files changed, 21 insertions(+), 3 deletions(-)
> >>>  
> >  
>