[PATCH v3 2/3] hw/i386/acpi-build: Return a pre-computed _PRT table

Ricardo Ribalda posted 3 patches 5 months, 2 weeks ago
There is a newer version of this series
[PATCH v3 2/3] hw/i386/acpi-build: Return a pre-computed _PRT table
Posted by Ricardo Ribalda 5 months, 2 weeks ago
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.

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.38d7b482@imammedo.users.ipa.redhat.com/T/#t

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 hw/i386/acpi-build.c | 120 ++++++++-----------------------------------
 1 file changed, 22 insertions(+), 98 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 53f804ac16..03216a6f29 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -725,120 +725,44 @@ 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;
-}
-
-/*
- * build_prt - Define interrupt rounting rules
+ * build_prt - Define interrupt routing rules
  *
  * Returns an array of 128 routes, one for each device,
  * based on device location.
  * The main goal is to equally distribute the interrupts
  * over the 4 existing ACPI links (works only for i440fx).
- * The hash function is  (slot + pin) & 3 -> "LNK[D|A|B|C]".
+ * The hash function is: (slot + pin) & 3 -> "LNK[D|A|B|C]".
  *
  */
 static Aml *build_prt(bool is_pci0_prt)
 {
-    Aml *method, *while_ctx, *pin, *res;
+    const int nroutes = 128;
+    Aml *rt_pkg, *method;
+    int pin;
 
     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(nroutes);
 
-    /* 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);
+    for (pin = 0; pin < nroutes; pin++) {
+        Aml *pkg = aml_package(4);
+        int slot = pin >> 2;
+
+        aml_append(pkg, aml_int((slot << 16) | 0xFFFF));
+        aml_append(pkg, aml_int(pin & 3));
+        /* device 1 is the power-management device, needs SCI */
+        if (is_pci0_prt && pin == 4) {
+            aml_append(pkg, aml_name("%s", "LNKS"));
         } else {
-            aml_append(while_ctx, initialize_route(route, "LNKA", lnk_idx, 1));
+            const char link_name[][5] = {"LNKD", "LNKA", "LNKB", "LNKC"};
+            int hash = (slot + pin) & 3;
+            aml_append(pkg, aml_name("%s", link_name[hash]));
         }
-        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(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.45.2.505.gda0bf45e8d-goog
Re: [PATCH v3 2/3] hw/i386/acpi-build: Return a pre-computed _PRT table
Posted by Igor Mammedov 4 months, 4 weeks ago
On Fri,  7 Jun 2024 14:17:25 +0000
Ricardo Ribalda <ribalda@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.
> 
> 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.38d7b482@imammedo.users.ipa.redhat.com/T/#t
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/acpi-build.c | 120 ++++++++-----------------------------------
>  1 file changed, 22 insertions(+), 98 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 53f804ac16..03216a6f29 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -725,120 +725,44 @@ 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;
> -}
> -
> -/*
> - * build_prt - Define interrupt rounting rules
> + * build_prt - Define interrupt routing rules
>   *
>   * Returns an array of 128 routes, one for each device,
>   * based on device location.
>   * The main goal is to equally distribute the interrupts
>   * over the 4 existing ACPI links (works only for i440fx).
> - * The hash function is  (slot + pin) & 3 -> "LNK[D|A|B|C]".
> + * The hash function is: (slot + pin) & 3 -> "LNK[D|A|B|C]".
>   *
>   */
>  static Aml *build_prt(bool is_pci0_prt)
>  {
> -    Aml *method, *while_ctx, *pin, *res;
> +    const int nroutes = 128;
> +    Aml *rt_pkg, *method;
> +    int pin;
>  
>      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(nroutes);
>  
> -    /* 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);
> +    for (pin = 0; pin < nroutes; pin++) {
> +        Aml *pkg = aml_package(4);
> +        int slot = pin >> 2;
> +
> +        aml_append(pkg, aml_int((slot << 16) | 0xFFFF));
> +        aml_append(pkg, aml_int(pin & 3));
> +        /* device 1 is the power-management device, needs SCI */
> +        if (is_pci0_prt && pin == 4) {
> +            aml_append(pkg, aml_name("%s", "LNKS"));
>          } else {
> -            aml_append(while_ctx, initialize_route(route, "LNKA", lnk_idx, 1));
> +            const char link_name[][5] = {"LNKD", "LNKA", "LNKB", "LNKC"};
> +            int hash = (slot + pin) & 3;
> +            aml_append(pkg, aml_name("%s", link_name[hash]));
>          }
> -        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(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;
>  }