[v5] acpi: i386 tweaks

[PATCH v5 13/15] acpi: drop build_piix4_pm()

Posted by Gerd Hoffmann 5 years, 6 months ago

The _SB.PCI0.PX13.P13C opregion (holds isa device enable bits)
is not used any more, remove it from DSDT.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/acpi-build.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 765409a90eb6..c1e63cce5e8e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1441,21 +1441,6 @@ static void build_q35_isa_bridge(Aml *table)
     aml_append(table, scope);
 }
 
-static void build_piix4_pm(Aml *table)
-{
-    Aml *dev;
-    Aml *scope;
-
-    scope =  aml_scope("_SB.PCI0");
-    dev = aml_device("PX13");
-    aml_append(dev, aml_name_decl("_ADR", aml_int(0x00010003)));
-
-    aml_append(dev, aml_operation_region("P13C", AML_PCI_CONFIG,
-                                         aml_int(0x00), 0xff));
-    aml_append(scope, dev);
-    aml_append(table, scope);
-}
-
 static void build_piix4_isa_bridge(Aml *table)
 {
     Aml *dev;
@@ -1607,7 +1592,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         aml_append(dsdt, sb_scope);
 
         build_hpet_aml(dsdt);
-        build_piix4_pm(dsdt);
         build_piix4_isa_bridge(dsdt);
         build_isa_devices_aml(dsdt);
         build_piix4_pci_hotplug(dsdt);
-- 
2.18.4

Re: [PATCH v5 13/15] acpi: drop build_piix4_pm()

Posted by Igor Mammedov 5 years, 6 months ago

On Thu,  7 May 2020 15:16:38 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> The _SB.PCI0.PX13.P13C opregion (holds isa device enable bits)
> is not used any more, remove it from DSDT.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/i386/acpi-build.c | 16 ----------------
>  1 file changed, 16 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 765409a90eb6..c1e63cce5e8e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1441,21 +1441,6 @@ static void build_q35_isa_bridge(Aml *table)
>      aml_append(table, scope);
>  }
>  
> -static void build_piix4_pm(Aml *table)
> -{
> -    Aml *dev;
> -    Aml *scope;
> -
> -    scope =  aml_scope("_SB.PCI0");
> -    dev = aml_device("PX13");
> -    aml_append(dev, aml_name_decl("_ADR", aml_int(0x00010003)));
I agree about removing P13C but I'm not sure if it's safe to remove
whole isa bridge

> -
> -    aml_append(dev, aml_operation_region("P13C", AML_PCI_CONFIG,
> -                                         aml_int(0x00), 0xff));
> -    aml_append(scope, dev);
> -    aml_append(table, scope);
> -}
> -
>  static void build_piix4_isa_bridge(Aml *table)
>  {
>      Aml *dev;
> @@ -1607,7 +1592,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          aml_append(dsdt, sb_scope);
>  
>          build_hpet_aml(dsdt);
> -        build_piix4_pm(dsdt);
>          build_piix4_isa_bridge(dsdt);
>          build_isa_devices_aml(dsdt);
>          build_piix4_pci_hotplug(dsdt);

Re: [PATCH v5 13/15] acpi: drop build_piix4_pm()

Posted by Gerd Hoffmann 5 years, 6 months ago

On Mon, May 11, 2020 at 09:37:32PM +0200, Igor Mammedov wrote:
> On Thu,  7 May 2020 15:16:38 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> > The _SB.PCI0.PX13.P13C opregion (holds isa device enable bits)
> > is not used any more, remove it from DSDT.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  hw/i386/acpi-build.c | 16 ----------------
> >  1 file changed, 16 deletions(-)
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 765409a90eb6..c1e63cce5e8e 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1441,21 +1441,6 @@ static void build_q35_isa_bridge(Aml *table)
> >      aml_append(table, scope);
> >  }
> >  
> > -static void build_piix4_pm(Aml *table)
> > -{
> > -    Aml *dev;
> > -    Aml *scope;
> > -
> > -    scope =  aml_scope("_SB.PCI0");
> > -    dev = aml_device("PX13");
> > -    aml_append(dev, aml_name_decl("_ADR", aml_int(0x00010003)));
> I agree about removing P13C but I'm not sure if it's safe to remove
> whole isa bridge

The isa bridge is _SB0.PCI0.ISA (piix4 function 0).
_SB.PCI0.PX13 is the acpi pm device (piix4 function 3).

So this does _not_ remove the isa bridge.  And given that PX13 has
nothing declared beside the opregion we are removing and I have not
found any other references to the PX13 device I assumed it is ok to drop
that one too.  Didn't notice any odd side effects in testing.

take care,
  Gerd

Re: [PATCH v5 13/15] acpi: drop build_piix4_pm()

Posted by Igor Mammedov 5 years, 6 months ago

On Tue, 12 May 2020 13:16:05 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Mon, May 11, 2020 at 09:37:32PM +0200, Igor Mammedov wrote:
> > On Thu,  7 May 2020 15:16:38 +0200
> > Gerd Hoffmann <kraxel@redhat.com> wrote:
> >   
> > > The _SB.PCI0.PX13.P13C opregion (holds isa device enable bits)
> > > is not used any more, remove it from DSDT.
> > > 
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > ---
> > >  hw/i386/acpi-build.c | 16 ----------------
> > >  1 file changed, 16 deletions(-)
> > > 
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index 765409a90eb6..c1e63cce5e8e 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -1441,21 +1441,6 @@ static void build_q35_isa_bridge(Aml *table)
> > >      aml_append(table, scope);
> > >  }
> > >  
> > > -static void build_piix4_pm(Aml *table)
> > > -{
> > > -    Aml *dev;
> > > -    Aml *scope;
> > > -
> > > -    scope =  aml_scope("_SB.PCI0");
> > > -    dev = aml_device("PX13");
> > > -    aml_append(dev, aml_name_decl("_ADR", aml_int(0x00010003)));  
> > I agree about removing P13C but I'm not sure if it's safe to remove
> > whole isa bridge  
> 
> The isa bridge is _SB0.PCI0.ISA (piix4 function 0).
> _SB.PCI0.PX13 is the acpi pm device (piix4 function 3).
> 
> So this does _not_ remove the isa bridge.  And given that PX13 has
> nothing declared beside the opregion we are removing and I have not
> found any other references to the PX13 device I assumed it is ok to drop
> that one too.  Didn't notice any odd side effects in testing.
I don't see HID or some other way to tell guest that it should load a driver
for it so probably we can remove it.
If it breaks some legacy OS we can always add it back.


> 
> take care,
>   Gerd
>