Re: [Qemu-devel] [PATCH v2 58/74] pc: acpi: piix4: move PCI0._PRT() into SSDT

[Igor Mammedov]

On Sun, 13 Dec 2015 17:19:23 +0200
Marcel Apfelbaum <address@hidden> wrote:

> On 12/10/2015 06:17 PM, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> > v2:
> >   - adapt build_prt() for using for PCI0._PRT(), reduces code
> > duplication, Suggested-by: Marcel Apfelbaum
> > <address@hidden>
> >
> > pc: acpi: piix4: adapt build_prt() for using for PCI0._PRT()
> >
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >   hw/i386/acpi-build.c  | 41 +++++++++++++++++++++++++++--------
> >   hw/i386/acpi-dsdt.dsl | 60
> > --------------------------------------------------- 2 files
> > changed, 32 insertions(+), 69 deletions(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index cf98037..1b065df 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -620,6 +620,17 @@ static void build_append_pci_bus_devices(Aml
> > *parent_scope, PCIBus *bus, qobject_decref(bsel);
> >   }
> >
> > +static Aml *build_prt_entry(const char *link_name)
> > +{
> > +    Aml *a_zero = aml_int(0);
> > +    Aml *pkg = aml_package(4);
> > +    aml_append(pkg, a_zero);
> > +    aml_append(pkg, a_zero);
> > +    aml_append(pkg, aml_name("%s", link_name));
> > +    aml_append(pkg, a_zero);
> > +    return pkg;
> > +}
> > +
> >   /*
> >    * initialize_route - Initialize the interrupt routing rule
> >    * through a specific LINK:
> > @@ -630,12 +641,8 @@ static Aml *initialize_route(Aml *route, const
> > char *link_name, Aml *lnk_idx, int idx)
> >   {
> >       Aml *if_ctx = aml_if(aml_equal(lnk_idx, aml_int(idx)));
> > -    Aml *pkg = aml_package(4);
> > +    Aml *pkg = build_prt_entry(link_name);
> >
> > -    aml_append(pkg, aml_int(0));
> > -    aml_append(pkg, aml_int(0));
> > -    aml_append(pkg, aml_name("%s", link_name));
> > -    aml_append(pkg, aml_int(0));
> >       aml_append(if_ctx, aml_store(pkg, route));
> >
> >       return if_ctx;
> > @@ -651,9 +658,9 @@ static Aml *initialize_route(Aml *route, const
> > char *link_name,
> >    * The hash function is  (slot + pin) & 3 -> "LNK[D|A|B|C]".
> >    *
> >    */
> > -static Aml *build_prt(void)
> > +static Aml *build_prt(bool is_pci0_prt)
> >   {
> > -    Aml *method, *while_ctx, *pin, *res;
> > +    Aml *method, *while_ctx, *pin, *res, *if_ctx, *if_ctx2,
> > *else_ctx2;
> >
> >       method = aml_method("_PRT", 0, AML_NOTSERIALIZED);
> >       res = aml_local(0);
> > @@ -678,7 +685,19 @@ static Aml *build_prt(void)
> >
> >           /* route[2] = "LNK[D|A|B|C]", selection based on pin % 3
> > */ aml_append(while_ctx, initialize_route(route, "LNKD", lnk_idx,
> > 0));
> > -        aml_append(while_ctx, initialize_route(route, "LNKA",
> > lnk_idx, 1));
> > +        if (is_pci0_prt) {
> > +            if_ctx = aml_if(aml_equal(lnk_idx, aml_int(1)));
> > +            /* device 1 is the power-management device, needs SCI
> > */
> > +            if_ctx2 = aml_if(aml_equal(pin, aml_int(4)));
> > +            aml_append(if_ctx2, aml_store(build_prt_entry("LNKS"),
> > route));
> > +            aml_append(if_ctx, if_ctx2);
> > +            else_ctx2 = aml_else();
> > +            aml_append(else_ctx2,
> > aml_store(build_prt_entry("LNKA"), route));
> > +            aml_append(if_ctx, else_ctx2);
> > +            aml_append(while_ctx, if_ctx);
> > +        } else {
> > +            aml_append(while_ctx, initialize_route(route, "LNKA",
> > lnk_idx, 1));
> > +        }
> >           aml_append(while_ctx, initialize_route(route, "LNKB",
> > lnk_idx, 2)); aml_append(while_ctx, initialize_route(route, "LNKC",
> > lnk_idx, 3));
> >
> > @@ -1348,6 +1367,10 @@ static void build_piix4_pci0_int(Aml *table)
> >       Aml *method;
> >       uint32_t irqs;
> >       Aml *sb_scope = aml_scope("_SB");
> > +    Aml *pci0_scope = aml_scope("PCI0");
> > +
> > +    aml_append(pci0_scope, build_prt(true));
> > +    aml_append(sb_scope, pci0_scope);
> >
> >       field = aml_field("PCI0.ISA.P40C", AML_BYTE_ACC, AML_NOLOCK,
> > AML_PRESERVE); aml_append(field, aml_named_field("PRQ0", 8));
> > @@ -1569,7 +1592,7 @@ build_ssdt(GArray *table_data, GArray *linker,
> >                   aml_append(dev, aml_name_decl("_PXM",
> > aml_int(numa_node))); }
> >
> > -            aml_append(dev, build_prt());
> > +            aml_append(dev, build_prt(false));
> >               crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent),
> >                               io_ranges, mem_ranges);
> >               aml_append(dev, aml_name_decl("_CRS", crs));
> > diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
> > index bc6bd45..5d741dd 100644
> > --- a/hw/i386/acpi-dsdt.dsl
> > +++ b/hw/i386/acpi-dsdt.dsl
> > @@ -78,64 +78,4 @@ DefinitionBlock (
> >           /* Hotplug notification method supplied by SSDT */
> >           External(\_SB.PCI0.PCNT, MethodObj)
> >       }
> > -
> > -
> > -/****************************************************************
> > - * PCI IRQs
> > - ****************************************************************/
> > -
> > -    Scope(\_SB) {
> > -        Scope(PCI0) {
> > -            Method (_PRT, 0) {
> > -                Store(Package(128) {}, Local0)
> > -                Store(Zero, Local1)
> > -                While(LLess(Local1, 128)) {
> > -                    // slot = pin >> 2
> > -                    Store(ShiftRight(Local1, 2), Local2)
> > -
> > -                    // lnk = (slot + pin) & 3
> > -                    Store(And(Add(Local1, Local2), 3), Local3)
> > -                    If (LEqual(Local3, 0)) {
> > -                        Store(Package(4) { Zero, Zero, LNKD,
> > Zero }, Local4)
> > -                    }
> > -                    If (LEqual(Local3, 1)) {
> > -                        // device 1 is the power-management
> > device, needs SCI
> > -                        If (LEqual(Local1, 4)) {
> > -                            Store(Package(4) { Zero, Zero, LNKS,
> > Zero }, Local4)
> > -                        } Else {
> > -                            Store(Package(4) { Zero, Zero, LNKA,
> > Zero }, Local4)
> > -                        }
> > -                    }
> > -                    If (LEqual(Local3, 2)) {
> > -                        Store(Package(4) { Zero, Zero, LNKB,
> > Zero }, Local4)
> > -                    }
> > -                    If (LEqual(Local3, 3)) {
> > -                        Store(Package(4) { Zero, Zero, LNKC,
> > Zero }, Local4)
> > -                    }
> > -
> > -                    // Complete the interrupt routing entry:
> > -                    //    Package(4) { 0x[slot]FFFF, [pin],
> > [link], 0) } -
> > -                    Store(Or(ShiftLeft(Local2, 16), 0xFFFF),
> > Index(Local4, 0))
> > -                    Store(And(Local1, 3),
> > Index(Local4, 1))
> > -                    Store(Local4,
> > Index(Local0, Local1)) -
> > -                    Increment(Local1)
> > -                }
> > -
> > -                Return(Local0)
> > -            }
> > -        }
> > -
> > -
> > -        External(PRQ0, FieldUnitObj)
> > -        External(PRQ1, FieldUnitObj)
> > -        External(PRQ2, FieldUnitObj)
> > -        External(PRQ3, FieldUnitObj)
> > -        External(LNKA, DeviceObj)
> > -        External(LNKB, DeviceObj)
> > -        External(LNKC, DeviceObj)
> > -        External(LNKD, DeviceObj)
> > -        External(LNKS, DeviceObj)
> > -    }
> >   }
> >
> 
> Assuming that there are no differences in the PRT:
> 
> Reviewed-by: Marcel Apfelbaum <address@hidden>
Yep, series still passes 'make check' so there aren't any
differences in DSDT nor SSDT

> 
> Thanks,
> Marcel