[PATCH] q35: turn off power_controller_present when acpi hotplug is enabled

[PATCH] q35: turn off power_controller_present when acpi hotplug is enabled

[Gerd Hoffmann]

Disable power control for pcie slots in case acpi hotplug is enabled
(6.2+ only for compatibility reasons).  This makes sure we don't get
unpleasant surprises with pci devices not being functional due to slot
power being turned off.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/pc_q35.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index e1e100316d93..869ca4c130f0 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -247,9 +247,16 @@ static void pc_q35_init(MachineState *machine)
                                                  "x-keep-pci-slot-hpc",
                                                  NULL);
 
-    if (!keep_pci_slot_hpc && acpi_pcihp) {
-        object_register_sugar_prop(TYPE_PCIE_SLOT, "x-native-hotplug",
-                                   "false", true);
+    if (acpi_pcihp) {
+        if (keep_pci_slot_hpc) {
+            /* 6.2+ default: acpi-hotplug=on native-hotplug=on power-ctrl=off */
+            object_register_sugar_prop(TYPE_PCIE_SLOT, COMPAT_PROP_PCP,
+                                       "false", true);
+        } else {
+            /* 6.1 default: acpi-hotplug=on native-hotplug=off */
+            object_register_sugar_prop(TYPE_PCIE_SLOT, "x-native-hotplug",
+                                       "false", true);
+        }
     }
 
     /* irq lines */
-- 
2.33.1

Re: [PATCH] q35: turn off power_controller_present when acpi hotplug is enabled

[Igor Mammedov]

On Tue, 16 Nov 2021 10:04:33 +0100
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Disable power control for pcie slots in case acpi hotplug is enabled
> (6.2+ only for compatibility reasons).  This makes sure we don't get
> unpleasant surprises with pci devices not being functional due to slot
> power being turned off.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/i386/pc_q35.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index e1e100316d93..869ca4c130f0 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -247,9 +247,16 @@ static void pc_q35_init(MachineState *machine)
>                                                   "x-keep-pci-slot-hpc",
>                                                   NULL);
>  
> -    if (!keep_pci_slot_hpc && acpi_pcihp) {
> -        object_register_sugar_prop(TYPE_PCIE_SLOT, "x-native-hotplug",
> -                                   "false", true);
> +    if (acpi_pcihp) {
> +        if (keep_pci_slot_hpc) {
> +            /* 6.2+ default: acpi-hotplug=on native-hotplug=on power-ctrl=off */
> +            object_register_sugar_prop(TYPE_PCIE_SLOT, COMPAT_PROP_PCP,
> +                                       "false", true);

that will also turn off COMPAT_PROP_PCP on ports attached to PXBs,
where ACPI hotplug is not used and native one is active.
So question is if it's expected behavior?

> +        } else {
> +            /* 6.1 default: acpi-hotplug=on native-hotplug=off */
> +            object_register_sugar_prop(TYPE_PCIE_SLOT, "x-native-hotplug",
> +                                       "false", true);
> +        }
>      }
>  
>      /* irq lines */

Re: [PATCH] q35: turn off power_controller_present when acpi hotplug is enabled

[Gerd Hoffmann]

On Tue, Nov 16, 2021 at 08:26:41PM +0100, Igor Mammedov wrote:
> On Tue, 16 Nov 2021 10:04:33 +0100
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> > Disable power control for pcie slots in case acpi hotplug is enabled
> > (6.2+ only for compatibility reasons).  This makes sure we don't get
> > unpleasant surprises with pci devices not being functional due to slot
> > power being turned off.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  hw/i386/pc_q35.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index e1e100316d93..869ca4c130f0 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -247,9 +247,16 @@ static void pc_q35_init(MachineState *machine)
> >                                                   "x-keep-pci-slot-hpc",
> >                                                   NULL);
> >  
> > -    if (!keep_pci_slot_hpc && acpi_pcihp) {
> > -        object_register_sugar_prop(TYPE_PCIE_SLOT, "x-native-hotplug",
> > -                                   "false", true);
> > +    if (acpi_pcihp) {
> > +        if (keep_pci_slot_hpc) {
> > +            /* 6.2+ default: acpi-hotplug=on native-hotplug=on power-ctrl=off */
> > +            object_register_sugar_prop(TYPE_PCIE_SLOT, COMPAT_PROP_PCP,
> > +                                       "false", true);
> 
> that will also turn off COMPAT_PROP_PCP on ports attached to PXBs,
> where ACPI hotplug is not used and native one is active.

Oh, wasn't aware of that detail.

> So question is if it's expected behavior?

Nope.  When native hotplug is used slot power control should be enabled.

Not sure how to handle that best though ...

take care,
  Gerd