ICH9 root PCI hotplug

[PATCH 0/4] ICH9 root PCI hotplug

Posted by Thierry Escande 1 year ago

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.

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(-)

-- 
2.42.0



Thierry Escande | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

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

Posted by Igor Mammedov 1 year ago

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.
"
)

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(-)
>

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

Posted by Michael S. Tsirkin 1 year ago

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?


> 
> 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(-)
> >

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

Posted by Thierry Escande 12 months ago

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?

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

> 
> 
>>
>> 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(-)
>>>
>

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

Posted by Igor Mammedov 11 months, 3 weeks ago

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(-)
> >>>  
> >  
>

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

Posted by Michael S. Tsirkin 12 months ago

On Wed, Nov 29, 2023 at 09:47:13AM +0000, Thierry Escande 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?
> 
> And for what it's worth, lifting this limitation would allow PCIe devices
> passthrough on Q35 VMs with Xen as such devices are hotplugged.


More motivation in the commit log would be a good idea here.

> > 
> > 
> > > 
> > > 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(-)
> > > > 
> >