On Tue, Nov 05, 2024 at 01:23:49AM -0500, Xiaoyao Li wrote:
> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>
> When level trigger isn't supported on x86 platform,
> forcibly report edge trigger in acpi tables.
This commit message is pretty sparse. I was thinking of suggesting to squash
this with patch 40 but it occurred to me that perhaps these are split to accept
TDX specifics from general functionality. Is that the case here? Is that true
with other patches in the series? If so what other situations would require
this in the generic code beyond TDX?
Ira
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
> Acked-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> ---
> hw/i386/acpi-build.c | 99 ++++++++++++++++++++++++++++---------------
> hw/i386/acpi-common.c | 45 +++++++++++++++-----
> 2 files changed, 101 insertions(+), 43 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 4967aa745902..d0a5bfc69e9a 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -888,7 +888,8 @@ static void build_dbg_aml(Aml *table)
> aml_append(table, scope);
> }
>
> -static Aml *build_link_dev(const char *name, uint8_t uid, Aml *reg)
> +static Aml *build_link_dev(const char *name, uint8_t uid, Aml *reg,
> + bool level_trigger_unsupported)
> {
> Aml *dev;
> Aml *crs;
> @@ -900,7 +901,10 @@ static Aml *build_link_dev(const char *name, uint8_t uid, Aml *reg)
> aml_append(dev, aml_name_decl("_UID", aml_int(uid)));
>
> crs = aml_resource_template();
> - aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> + aml_append(crs, aml_interrupt(AML_CONSUMER,
> + level_trigger_unsupported ?
> + AML_EDGE : AML_LEVEL,
> + AML_ACTIVE_HIGH,
> AML_SHARED, irqs, ARRAY_SIZE(irqs)));
> aml_append(dev, aml_name_decl("_PRS", crs));
>
> @@ -924,7 +928,8 @@ static Aml *build_link_dev(const char *name, uint8_t uid, Aml *reg)
> return dev;
> }
>
> -static Aml *build_gsi_link_dev(const char *name, uint8_t uid, uint8_t gsi)
> +static Aml *build_gsi_link_dev(const char *name, uint8_t uid,
> + uint8_t gsi, bool level_trigger_unsupported)
> {
> Aml *dev;
> Aml *crs;
> @@ -937,7 +942,10 @@ static Aml *build_gsi_link_dev(const char *name, uint8_t uid, uint8_t gsi)
>
> crs = aml_resource_template();
> irqs = gsi;
> - aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> + aml_append(crs, aml_interrupt(AML_CONSUMER,
> + level_trigger_unsupported ?
> + AML_EDGE : AML_LEVEL,
> + AML_ACTIVE_HIGH,
> AML_SHARED, &irqs, 1));
> aml_append(dev, aml_name_decl("_PRS", crs));
>
> @@ -956,7 +964,7 @@ static Aml *build_gsi_link_dev(const char *name, uint8_t uid, uint8_t gsi)
> }
>
> /* _CRS method - get current settings */
> -static Aml *build_iqcr_method(bool is_piix4)
> +static Aml *build_iqcr_method(bool is_piix4, bool level_trigger_unsupported)
> {
> Aml *if_ctx;
> uint32_t irqs;
> @@ -964,7 +972,9 @@ static Aml *build_iqcr_method(bool is_piix4)
> Aml *crs = aml_resource_template();
>
> irqs = 0;
> - aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL,
> + aml_append(crs, aml_interrupt(AML_CONSUMER,
> + level_trigger_unsupported ?
> + AML_EDGE : AML_LEVEL,
> AML_ACTIVE_HIGH, AML_SHARED, &irqs, 1));
> aml_append(method, aml_name_decl("PRR0", crs));
>
> @@ -998,7 +1008,7 @@ static Aml *build_irq_status_method(void)
> return method;
> }
>
> -static void build_piix4_pci0_int(Aml *table)
> +static void build_piix4_pci0_int(Aml *table, bool level_trigger_unsupported)
> {
> Aml *dev;
> Aml *crs;
> @@ -1011,12 +1021,16 @@ static void build_piix4_pci0_int(Aml *table)
> aml_append(sb_scope, pci0_scope);
>
> aml_append(sb_scope, build_irq_status_method());
> - aml_append(sb_scope, build_iqcr_method(true));
> + aml_append(sb_scope, build_iqcr_method(true, level_trigger_unsupported));
>
> - aml_append(sb_scope, build_link_dev("LNKA", 0, aml_name("PRQ0")));
> - aml_append(sb_scope, build_link_dev("LNKB", 1, aml_name("PRQ1")));
> - aml_append(sb_scope, build_link_dev("LNKC", 2, aml_name("PRQ2")));
> - aml_append(sb_scope, build_link_dev("LNKD", 3, aml_name("PRQ3")));
> + aml_append(sb_scope, build_link_dev("LNKA", 0, aml_name("PRQ0"),
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_link_dev("LNKB", 1, aml_name("PRQ1"),
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_link_dev("LNKC", 2, aml_name("PRQ2"),
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_link_dev("LNKD", 3, aml_name("PRQ3"),
> + level_trigger_unsupported));
>
> dev = aml_device("LNKS");
> {
> @@ -1025,7 +1039,9 @@ static void build_piix4_pci0_int(Aml *table)
>
> crs = aml_resource_template();
> irqs = 9;
> - aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL,
> + aml_append(crs, aml_interrupt(AML_CONSUMER,
> + level_trigger_unsupported ?
> + AML_EDGE : AML_LEVEL,
> AML_ACTIVE_HIGH, AML_SHARED,
> &irqs, 1));
> aml_append(dev, aml_name_decl("_PRS", crs));
> @@ -1111,7 +1127,7 @@ static Aml *build_q35_routing_table(const char *str)
> return pkg;
> }
>
> -static void build_q35_pci0_int(Aml *table)
> +static void build_q35_pci0_int(Aml *table, bool level_trigger_unsupported)
> {
> Aml *method;
> Aml *sb_scope = aml_scope("_SB");
> @@ -1150,25 +1166,41 @@ static void build_q35_pci0_int(Aml *table)
> aml_append(sb_scope, pci0_scope);
>
> aml_append(sb_scope, build_irq_status_method());
> - aml_append(sb_scope, build_iqcr_method(false));
> + aml_append(sb_scope, build_iqcr_method(false, level_trigger_unsupported));
>
> - aml_append(sb_scope, build_link_dev("LNKA", 0, aml_name("PRQA")));
> - aml_append(sb_scope, build_link_dev("LNKB", 1, aml_name("PRQB")));
> - aml_append(sb_scope, build_link_dev("LNKC", 2, aml_name("PRQC")));
> - aml_append(sb_scope, build_link_dev("LNKD", 3, aml_name("PRQD")));
> - aml_append(sb_scope, build_link_dev("LNKE", 4, aml_name("PRQE")));
> - aml_append(sb_scope, build_link_dev("LNKF", 5, aml_name("PRQF")));
> - aml_append(sb_scope, build_link_dev("LNKG", 6, aml_name("PRQG")));
> - aml_append(sb_scope, build_link_dev("LNKH", 7, aml_name("PRQH")));
> + aml_append(sb_scope, build_link_dev("LNKA", 0, aml_name("PRQA"),
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_link_dev("LNKB", 1, aml_name("PRQB"),
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_link_dev("LNKC", 2, aml_name("PRQC"),
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_link_dev("LNKD", 3, aml_name("PRQD"),
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_link_dev("LNKE", 4, aml_name("PRQE"),
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_link_dev("LNKF", 5, aml_name("PRQF"),
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_link_dev("LNKG", 6, aml_name("PRQG"),
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_link_dev("LNKH", 7, aml_name("PRQH"),
> + level_trigger_unsupported));
>
> - aml_append(sb_scope, build_gsi_link_dev("GSIA", 0x10, 0x10));
> - aml_append(sb_scope, build_gsi_link_dev("GSIB", 0x11, 0x11));
> - aml_append(sb_scope, build_gsi_link_dev("GSIC", 0x12, 0x12));
> - aml_append(sb_scope, build_gsi_link_dev("GSID", 0x13, 0x13));
> - aml_append(sb_scope, build_gsi_link_dev("GSIE", 0x14, 0x14));
> - aml_append(sb_scope, build_gsi_link_dev("GSIF", 0x15, 0x15));
> - aml_append(sb_scope, build_gsi_link_dev("GSIG", 0x16, 0x16));
> - aml_append(sb_scope, build_gsi_link_dev("GSIH", 0x17, 0x17));
> + aml_append(sb_scope, build_gsi_link_dev("GSIA", 0x10, 0x10,
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_gsi_link_dev("GSIB", 0x11, 0x11,
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_gsi_link_dev("GSIC", 0x12, 0x12,
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_gsi_link_dev("GSID", 0x13, 0x13,
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_gsi_link_dev("GSIE", 0x14, 0x14,
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_gsi_link_dev("GSIF", 0x15, 0x15,
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_gsi_link_dev("GSIG", 0x16, 0x16,
> + level_trigger_unsupported));
> + aml_append(sb_scope, build_gsi_link_dev("GSIH", 0x17, 0x17,
> + level_trigger_unsupported));
>
> aml_append(table, sb_scope);
> }
> @@ -1350,6 +1382,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> PCMachineState *pcms = PC_MACHINE(machine);
> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> X86MachineState *x86ms = X86_MACHINE(machine);
> + bool level_trigger_unsupported = x86ms->eoi_intercept_unsupported;
> AcpiMcfgInfo mcfg;
> bool mcfg_valid = !!acpi_get_mcfg(&mcfg);
> uint32_t nr_mem = machine->ram_slots;
> @@ -1382,7 +1415,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> if (pm->pcihp_bridge_en || pm->pcihp_root_en) {
> build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base);
> }
> - build_piix4_pci0_int(dsdt);
> + build_piix4_pci0_int(dsdt, level_trigger_unsupported);
> } else if (q35) {
> sb_scope = aml_scope("_SB");
> dev = aml_device("PCI0");
> @@ -1426,7 +1459,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> if (pm->pcihp_bridge_en) {
> build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base);
> }
> - build_q35_pci0_int(dsdt);
> + build_q35_pci0_int(dsdt, level_trigger_unsupported);
> }
>
> if (misc->has_hpet) {
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> index 0cc2919bb851..ad38a6b31162 100644
> --- a/hw/i386/acpi-common.c
> +++ b/hw/i386/acpi-common.c
> @@ -103,6 +103,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
> AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = oem_id,
> .oem_table_id = oem_table_id };
> + bool level_trigger_unsupported = x86ms->eoi_intercept_unsupported;
>
> acpi_table_begin(&table, table_data);
> /* Local APIC Address */
> @@ -124,18 +125,42 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> IO_APIC_SECONDARY_ADDRESS, IO_APIC_SECONDARY_IRQBASE);
> }
>
> - if (x86mc->apic_xrupt_override) {
> - build_xrupt_override(table_data, 0, 2,
> - 0 /* Flags: Conforms to the specifications of the bus */);
> - }
> + if (level_trigger_unsupported) {
> + /* Force edge trigger */
> + if (x86mc->apic_xrupt_override) {
> + build_xrupt_override(table_data, 0, 2,
> + /* Flags: active high, edge triggered */
> + 1 | (1 << 2));
> + }
> +
> + for (i = x86mc->apic_xrupt_override ? 1 : 0; i < 16; i++) {
> + build_xrupt_override(table_data, i, i,
> + /* Flags: active high, edge triggered */
> + 1 | (1 << 2));
> + }
> +
> + if (x86ms->ioapic2) {
> + for (i = 0; i < 16; i++) {
> + build_xrupt_override(table_data, IO_APIC_SECONDARY_IRQBASE + i,
> + IO_APIC_SECONDARY_IRQBASE + i,
> + /* Flags: active high, edge triggered */
> + 1 | (1 << 2));
> + }
> + }
> + } else {
> + if (x86mc->apic_xrupt_override) {
> + build_xrupt_override(table_data, 0, 2,
> + 0 /* Flags: Conforms to the specifications of the bus */);
> + }
>
> - for (i = 1; i < 16; i++) {
> - if (!(x86ms->pci_irq_mask & (1 << i))) {
> - /* No need for a INT source override structure. */
> - continue;
> + for (i = 1; i < 16; i++) {
> + if (!(x86ms->pci_irq_mask & (1 << i))) {
> + /* No need for a INT source override structure. */
> + continue;
> + }
> + build_xrupt_override(table_data, i, i,
> + 0xd /* Flags: Active high, Level Triggered */);
> }
> - build_xrupt_override(table_data, i, i,
> - 0xd /* Flags: Active high, Level Triggered */);
> }
>
> if (x2apic_mode) {
> --
> 2.34.1
>
