On Fri, 3 May 2024 11:09:47 +0200
Ricardo Ribalda <riba...@chromium.org> wrote:
> Friendly ping
>
> On Wed, 17 Apr 2024 at 15:56, Ricardo Ribalda <riba...@chromium.org> wrote:
> >
> > When qemu runs without kvm acceleration the ACPI executions take a great
> > amount of time. If they take more than the default time (30sec), the
> > ACPI calls fail and the system might not behave correctly.
> >
> > Now the _PRT table is computed on the fly. We can drastically reduce the
> > execution of the _PRT method if we return a pre-computed table.
It's heavily depends on used hw or resources if it's running inside VM,
one can always find a slow enough environment where above stated limit
will not be sufficient regardless of what QEMU does.
Correct approach would be to fix guest OS timeout issue so it won't
timeout if there is a progress.
As for the patch, question is what DSDT size difference is between
static _PRT version and the current dynamic one?
(if it shrinks _PRT, I'm all for it)
Also see tests/qtest/bios-tables-test.c
In the top of process to follow when one touches ACPI tables
to avoid breaking tests.
> > Without this patch:
> > [ 51.343484] ACPI Error: Aborting method \_SB.PCI0._PRT due to previous
> > error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> > [ 51.527032] ACPI Error: Method execution failed \_SB.PCI0._PRT due to
> > previous error (AE_AML_LOOP_TIMEOUT) (20230628/uteval-68)
> > [ 51.530049] virtio-pci 0000:00:02.0: can't derive routing for PCI INT A
> > [ 51.530797] virtio-pci 0000:00:02.0: PCI INT A: no GSI
> > [ 81.922901] ACPI Error: Aborting method \_SB.PCI0._PRT due to previous
> > error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> > [ 82.103534] ACPI Error: Method execution failed \_SB.PCI0._PRT due to
> > previous error (AE_AML_LOOP_TIMEOUT) (20230628/uteval-68)
> > [ 82.106088] virtio-pci 0000:00:04.0: can't derive routing for PCI INT A
> > [ 82.106761] virtio-pci 0000:00:04.0: PCI INT A: no GSI
> > [ 112.192568] ACPI Error: Aborting method \_SB.PCI0._PRT due to previous
> > error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> > [ 112.486687] ACPI Error: Method execution failed \_SB.PCI0._PRT due to
> > previous error (AE_AML_LOOP_TIMEOUT) (20230628/uteval-68)
> > [ 112.489554] virtio-pci 0000:00:05.0: can't derive routing for PCI INT A
> > [ 112.490027] virtio-pci 0000:00:05.0: PCI INT A: no GSI
> > [ 142.559448] ACPI Error: Aborting method \_SB.PCI0._PRT due to previous
> > error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> > [ 142.718596] ACPI Error: Method execution failed \_SB.PCI0._PRT due to
> > previous error (AE_AML_LOOP_TIMEOUT) (20230628/uteval-68)
> > [ 142.722889] virtio-pci 0000:00:06.0: can't derive routing for PCI INT A
> > [ 142.724578] virtio-pci 0000:00:06.0: PCI INT A: no GSI
> >
> > With this patch:
> > [ 22.938076] ACPI: \_SB_.LNKB: Enabled at IRQ 10
> > [ 24.214002] ACPI: \_SB_.LNKD: Enabled at IRQ 11
> > [ 25.465170] ACPI: \_SB_.LNKA: Enabled at IRQ 10
> > [ 27.944920] ACPI: \_SB_.LNKC: Enabled at IRQ 11
> >
> > ACPI disassembly:
> > Scope (PCI0)
> > {
> > Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table
> > {
> > Return (Package (0x80)
> > {
> > Package (0x04)
> > {
> > 0xFFFF,
> > Zero,
> > LNKD,
> > Zero
> > },
> >
> > Package (0x04)
> > {
> > 0xFFFF,
> > One,
> > LNKA,
> > Zero
> > },
> >
> > Package (0x04)
> > {
> > 0xFFFF,
> > 0x02,
> > LNKB,
> > Zero
> > },
> >
> > Package (0x04)
> > {
> > 0xFFFF,
> > 0x03,
> > LNKC,
> > Zero
> > },
> >
> > Package (0x04)
> > {
> > 0x0001FFFF,
> > Zero,
> > LNKS,
> > Zero
> > },
> > Context:
> > https://lore.kernel.org/virtualization/20240417145544.38d7b...@imammedo.users.ipa.redhat.com/T/#t
> >
> > Signed-off-by: Ricardo Ribalda <riba...@chromium.org>
> > ---
> > hw/i386/acpi-build.c | 118 ++++++++-----------------------------------
> > 1 file changed, 21 insertions(+), 97 deletions(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 53f804ac16..4c14d39173 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -725,40 +725,7 @@ static Aml *aml_pci_pdsm(void)
> > return method;
> > }
> >
> > -/**
> > - * build_prt_entry:
> > - * @link_name: link name for PCI route entry
> > - *
> > - * build AML package containing a PCI route entry for @link_name
> > - */
> > -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:
> > - * if (lnk_idx == idx)
> > - * route using link 'link_name'
> > - */
> > -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 = build_prt_entry(link_name);
> > -
> > - aml_append(if_ctx, aml_store(pkg, route));
> > -
> > - return if_ctx;
> > -}
> > -
> > +#define N_ROUTES 128
> > /*
> > * build_prt - Define interrupt rounting rules
> > *
> > @@ -771,74 +738,31 @@ static Aml *initialize_route(Aml *route, const char
> > *link_name,
> > */
> > static Aml *build_prt(bool is_pci0_prt)
> > {
> > - Aml *method, *while_ctx, *pin, *res;
> > + Aml *rt_pkg, *method;
> > + const char link_name[][2] = {"D", "A", "B", "C"};
> > + int i;
> >
> > method = aml_method("_PRT", 0, AML_NOTSERIALIZED);
> > - res = aml_local(0);
> > - pin = aml_local(1);
> > - aml_append(method, aml_store(aml_package(128), res));
> > - aml_append(method, aml_store(aml_int(0), pin));
> > + rt_pkg = aml_varpackage(N_ROUTES);
> >
> > - /* while (pin < 128) */
> > - while_ctx = aml_while(aml_lless(pin, aml_int(128)));
> > - {
> > - Aml *slot = aml_local(2);
> > - Aml *lnk_idx = aml_local(3);
> > - Aml *route = aml_local(4);
> > -
> > - /* slot = pin >> 2 */
> > - aml_append(while_ctx,
> > - aml_store(aml_shiftright(pin, aml_int(2), NULL), slot));
> > - /* lnk_idx = (slot + pin) & 3 */
> > - aml_append(while_ctx,
> > - aml_store(aml_and(aml_add(pin, slot, NULL), aml_int(3), NULL),
> > - lnk_idx));
> > -
> > - /* route[2] = "LNK[D|A|B|C]", selection based on pin % 3 */
> > - aml_append(while_ctx, initialize_route(route, "LNKD", lnk_idx, 0));
> > - if (is_pci0_prt) {
> > - Aml *if_device_1, *if_pin_4, *else_pin_4;
> > -
> > - /* device 1 is the power-management device, needs SCI */
> > - if_device_1 = aml_if(aml_equal(lnk_idx, aml_int(1)));
> > - {
> > - if_pin_4 = aml_if(aml_equal(pin, aml_int(4)));
> > - {
> > - aml_append(if_pin_4,
> > - aml_store(build_prt_entry("LNKS"), route));
> > - }
> > - aml_append(if_device_1, if_pin_4);
> > - else_pin_4 = aml_else();
> > - {
> > - aml_append(else_pin_4,
> > - aml_store(build_prt_entry("LNKA"), route));
> > - }
> > - aml_append(if_device_1, else_pin_4);
> > - }
> > - aml_append(while_ctx, if_device_1);
> > - } else {
> > - aml_append(while_ctx, initialize_route(route, "LNKA", lnk_idx,
> > 1));
> > + for (i = 0; i < N_ROUTES; i++) {
> > + Aml *pkg = aml_package(4);
> > + const char *name;
> > +
> > + name = link_name[((i >> 2) + i) & 3];
> > +
> > + if (is_pci0_prt && i == 4) {
> > + name = "S";
> > }
> > - aml_append(while_ctx, initialize_route(route, "LNKB", lnk_idx, 2));
> > - aml_append(while_ctx, initialize_route(route, "LNKC", lnk_idx, 3));
> > -
> > - /* route[0] = 0x[slot]FFFF */
> > - aml_append(while_ctx,
> > - aml_store(aml_or(aml_shiftleft(slot, aml_int(16)),
> > aml_int(0xFFFF),
> > - NULL),
> > - aml_index(route, aml_int(0))));
> > - /* route[1] = pin & 3 */
> > - aml_append(while_ctx,
> > - aml_store(aml_and(pin, aml_int(3), NULL),
> > - aml_index(route, aml_int(1))));
> > - /* res[pin] = route */
> > - aml_append(while_ctx, aml_store(route, aml_index(res, pin)));
> > - /* pin++ */
> > - aml_append(while_ctx, aml_increment(pin));
> > +
> > + aml_append(pkg, aml_int((i << 14) | 0xFFFF));
> > + aml_append(pkg, aml_int(i & 3));
> > + aml_append(pkg, aml_name("LNK%s", name));
> > + aml_append(pkg, aml_int(0));
> > + aml_append(rt_pkg, pkg);
> > }
> > - aml_append(method, while_ctx);
> > - /* return res*/
> > - aml_append(method, aml_return(res));
> > +
> > + aml_append(method, aml_return(rt_pkg));
> >
> > return method;
> > }
> > --
> > 2.44.0.683.g7961c838ac-goog
> >
>
>
