Change in coreboot[master]: soc/intel/common/block/acpi: [WIP]Add provision to inject PCI PIRQ AC...
Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: [WIP]Add provision to inject PCI PIRQ ACPI mapping ......................................................................
soc/intel/common/block/acpi: [WIP]Add provision to inject PCI PIRQ ACPI mapping
This implementation adds provision to inject PCI IRQ ACPI mapping into DSDT acpi table.
Change-Id: I60ae7cbcfe53c4cb21e88997b06ec8af4b59b844 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/common/block/acpi/acpi.c 1 file changed, 40 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/34658/1
diff --git a/src/soc/intel/common/block/acpi/acpi.c b/src/soc/intel/common/block/acpi/acpi.c index 3a34c79..b7b5d57 100644 --- a/src/soc/intel/common/block/acpi/acpi.c +++ b/src/soc/intel/common/block/acpi/acpi.c @@ -25,6 +25,7 @@ #include <cpu/x86/msr.h> #include <cpu/x86/smm.h> #include <intelblocks/acpi.h> +#include <intelblocks/irq.h> #include <intelblocks/msr.h> #include <intelblocks/pmclib.h> #include <intelblocks/uart.h> @@ -228,7 +229,37 @@ { }
-void southbridge_inject_dsdt(struct device *device) +#if CONFIG(SOC_INTEL_COMMON_ITSS_INTERRUPT_OVERRIDE) + +static void generate_pci_pirq_entries(void) +{ + uint8_t num_entries; + struct dev_irq *irqlist; + + irqlist = (struct dev_irq *)soc_get_irq_config(&num_entries); + if (!num_entries) + return; + acpigen_write_scope("\_SB.PCI0"); + acpigen_write_name("PICP"); + acpigen_write_package(num_entries); + + for (int i = 0; i < num_entries; i++) { + acpigen_write_package(4); + acpigen_write_dword((irqlist->slot << 16) | 0xffff); + acpigen_write_byte(irqlist->int_pin - 1); + acpigen_write_zero(); + acpigen_write_byte(irqlist->int_line); + acpigen_pop_len(); + irqlist++; + } + + acpigen_pop_len(); + acpigen_pop_len(); + +} +#endif + +static void generate_gnvs_entry(void) { struct global_nvs_t *gnvs;
@@ -249,6 +280,14 @@ acpigen_write_name_dword("NVSA", (uintptr_t) gnvs); acpigen_pop_len(); } + +} +void southbridge_inject_dsdt(struct device *device) +{ + generate_gnvs_entry(); +#if CONFIG(SOC_INTEL_COMMON_ITSS_INTERRUPT_OVERRIDE) + generate_pci_pirq_entries(); +#endif }
static int calculate_power(int tdp, int p1_ratio, int ratio)
Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34658
to look at the new patch set (#3).
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping
This implementation adds provision to inject PCI IRQ ACPI mapping into DSDT acpi table. This creates a PIRX acpi package with PCI IRQ routing information dynamically.
Change-Id: I60ae7cbcfe53c4cb21e88997b06ec8af4b59b844 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/common/block/acpi/acpi.c 1 file changed, 39 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/34658/3
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34658/3/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/34658/3/src/soc/intel/common/block/... PS3, Line 242: acpigen_write_scope("\"); nit: blank line after return;
https://review.coreboot.org/c/coreboot/+/34658/3/src/soc/intel/common/block/... PS3, Line 284: void southbridge_inject_dsdt(struct device *device) nit: blank line above
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34658/3/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/34658/3/src/soc/intel/common/block/... PS3, Line 232: #if CONFIG(SOC_INTEL_COMMON_ITSS_INTERRUPT_OVERRIDE) why does it use the preprocessor?
Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34658
to look at the new patch set (#4).
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping
This implementation adds provision to inject PCI IRQ ACPI mapping into DSDT acpi table. This creates a PIRX acpi package with PCI IRQ routing information dynamically.
Change-Id: I60ae7cbcfe53c4cb21e88997b06ec8af4b59b844 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/common/block/acpi/acpi.c 1 file changed, 41 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/34658/4
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34658/3/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/34658/3/src/soc/intel/common/block/... PS3, Line 232: #if CONFIG(SOC_INTEL_COMMON_ITSS_INTERRUPT_OVERRIDE)
why does it use the preprocessor?
To selectively make generate_pci_pirq_entries available based on the config. The config is tied up with creating the irq configuration data in common irq.c file. and generate_pci_pirq_entries would use the same data to generate ACPI package out of it.
If the SOC selects the SOC_INTEL_COMMON_ITSS_INTERRUPT_OVERRIDE, only then the irq common driver would generate the irq configuration package based on UIRQ configurattion rules.
https://review.coreboot.org/c/coreboot/+/34658/3/src/soc/intel/common/block/... PS3, Line 242: acpigen_write_scope("\");
nit: blank line after return;
Done
https://review.coreboot.org/c/coreboot/+/34658/3/src/soc/intel/common/block/... PS3, Line 284: void southbridge_inject_dsdt(struct device *device)
nit: blank line above
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34658/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34658/4//COMMIT_MSG@12 PS4, Line 12: How and where is the PIRX package expected to be used?
Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34658
to look at the new patch set (#5).
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping
This implementation adds provision to inject PCI IRQ ACPI mapping into DSDT acpi table. This dynamically creates a PIRX acpi package with PCI IRQ routing for IOAPIC interrupt mode.
Change-Id: I60ae7cbcfe53c4cb21e88997b06ec8af4b59b844 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/common/block/acpi/acpi.c 1 file changed, 41 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/34658/5
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34658/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34658/5//COMMIT_MSG@10 PS5, Line 10: reates a PIRX acpi package with PCI : IRQ routing for IOAPIC interrupt mode. I don't see mention of this package anywhere in upstream Linux. Could you kindly point me to its usage?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34658/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34658/5//COMMIT_MSG@10 PS5, Line 10: reates a PIRX acpi package with PCI : IRQ routing for IOAPIC interrupt mode.
I don't see mention of this package anywhere in upstream Linux. […]
Nevermind, I see it's in the next patch, from _PTR.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... PS5, Line 242: : acpigen_write_scope("\"); : acpigen_write_name("PIRX"); : acpigen_write_package(num_entries); Seems like this could be better-scoped; maybe the same scope that _PRT is in?
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... PS5, Line 242: : acpigen_write_scope("\"); : acpigen_write_name("PIRX"); : acpigen_write_package(num_entries);
Seems like this could be better-scoped; maybe the same scope that _PRT is in?
I tried to scope it SB.PCI0, where the current PICN abd PICM packages are present, and had provided external reference for the same.(patch set 1)
However the kernel could not locate the PIRX package and hanged. Unless I set the scope to '\' same as GNVS
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... PS5, Line 242: : acpigen_write_scope("\"); : acpigen_write_name("PIRX"); : acpigen_write_package(num_entries);
I tried to scope it SB. […]
I think when you have the "External" declaration in the ASL, you need to also include the full scope, i.e., External(_SB.PCI0.PIRX) instead. Could you try that?
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... PS5, Line 242: : acpigen_write_scope("\"); : acpigen_write_name("PIRX"); : acpigen_write_package(num_entries);
I think when you have the "External" declaration in the ASL, you need to also include the full scope […]
I has tried that too, still could see the PIRX reference missing in kernel. I dumped the dsdt table and could see PIRX present but _PRT could not return the PIRX package.
But I can quickly check again and update.
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... PS5, Line 242: : acpigen_write_scope("\"); : acpigen_write_name("PIRX"); : acpigen_write_package(num_entries);
I has tried that too, still could see the PIRX reference missing in kernel. […]
Tim , I tried setting the scope to _SB.PCI0 for PIRX and changed the External reference to External(_SB.PCI0.PIRX) and could still see the kernel hang. I had it earlier too, even configuring the return from PRT to _SB.PCI0.PIRX. I then had to follow the same implementation as GNVS.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 6: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/34658/5/src/soc/intel/common/block/... PS5, Line 242: : acpigen_write_scope("\"); : acpigen_write_name("PIRX"); : acpigen_write_package(num_entries);
Tim , I tried setting the scope to _SB. […]
Hm, that's weird. I guess let's just go with it for now.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34658/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34658/4//COMMIT_MSG@12 PS4, Line 12:
PIRX package contains irq routing information and is referred by kernel driver when IOAPIC interrup […]
Done
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 6: Code-Review+1
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34658 )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34658/6/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/34658/6/src/soc/intel/common/block/... PS6, Line 249: slot You likely have multiple identical entries with the same int_pin, slot combo, which you should avoid to generate twice.
https://review.coreboot.org/c/coreboot/+/34658/6/src/soc/intel/common/block/... PS6, Line 289: #if CONFIG(SOC_INTEL_COMMON_ITSS_INTERRUPT_OVERRIDE) Don't use preprocessor.
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/34658?usp=email )
Change subject: soc/intel/common/block/acpi: Add provision to inject PCI PIRQ ACPI mapping ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Aamir Bohra (Code Review) -
Arthur Heymans (Code Review)
-
Lean Sheng Tan (Code Review)
-
Martin L Roth (Code Review)
-
Patrick Rudolph (Code Review)
-
Tim Wawrzynczak (Code Review)