diff mbox series

[RFC,v2,1/2] hw/pci-host/gpex: Allow to generate preserve boot config DSM #5

Message ID 20211005085313.493858-2-eric.auger@redhat.com (mailing list archive)
State New, archived
Headers show
Series hw/arm/virt-acpi-build: Add IORT RMR regions to handle MSI nested binding | expand

Commit Message

Eric Auger Oct. 5, 2021, 8:53 a.m. UTC 
Add a 'preserve_config' field in struct GPEXConfig and
if set generate the DSM #5 for preserving PCI boot configurations.
The DSM presence is needed to expose RMRs.

At the moment the DSM generation is not yet enabled.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 include/hw/pci-host/gpex.h |  1 +
 hw/pci-host/gpex-acpi.c    | 12 ++++++++++++
 2 files changed, 13 insertions(+)

Comments

Igor Mammedov Oct. 11, 2021, 3:15 p.m. UTC | #1 
On Tue,  5 Oct 2021 10:53:12 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> Add a 'preserve_config' field in struct GPEXConfig and
> if set generate the DSM #5 for preserving PCI boot configurations.
> The DSM presence is needed to expose RMRs.

here should be pointers to spec and location within it
where it says preserving PCI boot configuration is necessary
or in absence of that a bit more detailed explanation
why it's necessary.

> 
> At the moment the DSM generation is not yet enabled.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  include/hw/pci-host/gpex.h |  1 +
>  hw/pci-host/gpex-acpi.c    | 12 ++++++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> index fcf8b63820..3f8f8ec38d 100644
> --- a/include/hw/pci-host/gpex.h
> +++ b/include/hw/pci-host/gpex.h
> @@ -64,6 +64,7 @@ struct GPEXConfig {
>      MemMapEntry pio;
>      int         irq;
>      PCIBus      *bus;
> +    bool        preserve_config;
s/^^^/preserve_fw_config/

>  };
>  
>  int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index e7e162a00a..7dab259379 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -164,6 +164,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>                  aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node)));
>              }
>  


> +            if (cfg->preserve_config) {
> +                method = aml_method("_DSM", 5, AML_SERIALIZED);
> +                aml_append(method, aml_return(aml_int(0)));
> +                aml_append(dev, method);
> +            }
> +
>              acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>  
>              /*
> @@ -191,6 +197,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>      aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
>      aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>  
> +    if (cfg->preserve_config) {
> +        method = aml_method("_DSM", 5, AML_SERIALIZED);
> +        aml_append(method, aml_return(aml_int(0)));
> +        aml_append(dev, method);
> +    }
> +


these ones seem to wrong ,
it adds duplicate _DSM methods with wrong ARGs number.

virt board already has _DSM defined, see

  acpi_dsdt_add_pci_osc()
    E5C937D0-3553-4D7A-9117-EA4D19C3434D

you need to modify that one (and possibly move out DSM into a separate function),
also preserving config might regress what commit 40c3472a29c9a was fixing.


>      acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>  
>      method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
Zhijian Li (Fujitsu)" via Dec. 29, 2021, 7:13 a.m. UTC | #2 
Hi Eric,


在 2021/10/5 16:53, Eric Auger 写道:
> Add a 'preserve_config' field in struct GPEXConfig and
> if set generate the DSM #5 for preserving PCI boot configurations.
> The DSM presence is needed to expose RMRs.
>
> At the moment the DSM generation is not yet enabled.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   include/hw/pci-host/gpex.h |  1 +
>   hw/pci-host/gpex-acpi.c    | 12 ++++++++++++
>   2 files changed, 13 insertions(+)
>
> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> index fcf8b63820..3f8f8ec38d 100644
> --- a/include/hw/pci-host/gpex.h
> +++ b/include/hw/pci-host/gpex.h
> @@ -64,6 +64,7 @@ struct GPEXConfig {
>       MemMapEntry pio;
>       int         irq;
>       PCIBus      *bus;
> +    bool        preserve_config;
>   };
>   
>   int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index e7e162a00a..7dab259379 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -164,6 +164,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>                   aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node)));
>               }
>   
> +            if (cfg->preserve_config) {
> +                method = aml_method("_DSM", 5, AML_SERIALIZED);

I notice there is a ACPI BIOS Error when booting virtual machine which 
seems be caused by this patch as I add this patchset in my branch to 
test the function of vsmmu.
It seems that it requires only 4 parameter for method _DSM, but use 5 
parameters here.
The error log is as following:

[    2.355459] ACPI BIOS Error (bug): Failure creating named object 
[\_SB.PCI0._DSM], AE_ALREADY_EXISTS (20210930/dswload2-327)
[    2.355467] ACPI Error: AE_ALREADY_EXISTS, During name lookup/catalog 
(20210930/psobject-221)
[    2.355470] ACPI: Skipping parse of AML opcode: OpcodeName 
unavailable (0x0014)
[    2.355657] ACPI: 1 ACPI AML tables successfully acquired and loaded
[    2.356321] ACPI: Interpreter enabled
[    2.356323] ACPI: Using GIC for interrupt routing
[    2.356333] ACPI: MCFG table detected, 1 entries
[    2.361359] ARMH0011:00: ttyAMA0 at MMIO 0x9000000 (irq = 16, 
base_baud = 0) is a SBSA
[    2.619805] printk: console [ttyAMA0] enabled
[    2.622114] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
[    2.622788] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM 
ClockPM Segments MSI HPX-Type3]
[    2.623776] acpi PNP0A08:00: _OSC: platform does not support [LTR]
[    2.624600] acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME 
AER PCIeCapability]
[    2.625721] acpi PNP0A08:00: ECAM area [mem 
0x4010000000-0x401fffffff] reserved by PNP0C02:00
[    2.626645] acpi PNP0A08:00: ECAM at [mem 0x4010000000-0x401fffffff] 
for [bus 00-ff]
[    2.627450] ACPI: Remapped I/O 0x000000003eff0000 to [io 
0x0000-0xffff window]
[    2.628229] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments - 
ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)
[    2.629576] PCI host bridge to bus 0000:00
[    2.630008] pci_bus 0000:00: root bus resource [mem 
0x10000000-0x3efeffff window]
[    2.630747] pci_bus 0000:00: root bus resource [io  0x0000-0xffff window]
[    2.631405] pci_bus 0000:00: root bus resource [mem 
0x8000000000-0xffffffffff window]
[    2.632177] pci_bus 0000:00: root bus resource [bus 00-ff]
[    2.632731] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments - 
ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)


> +                aml_append(method, aml_return(aml_int(0)));
> +                aml_append(dev, method);
> +            }
> +
>               acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>   
>               /*
> @@ -191,6 +197,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>       aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
>       aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>   
> +    if (cfg->preserve_config) {
> +        method = aml_method("_DSM", 5, AML_SERIALIZED);
> +        aml_append(method, aml_return(aml_int(0)));
> +        aml_append(dev, method);
> +    }
> +
>       acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>   
>       method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
Eric Auger Jan. 6, 2022, 11 a.m. UTC | #3 
Hi CHenxiangn

On 12/29/21 8:13 AM, chenxiang (M) via wrote:
> Hi Eric,
>
>
> 在 2021/10/5 16:53, Eric Auger 写道:
>> Add a 'preserve_config' field in struct GPEXConfig and
>> if set generate the DSM #5 for preserving PCI boot configurations.
>> The DSM presence is needed to expose RMRs.
>>
>> At the moment the DSM generation is not yet enabled.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>   include/hw/pci-host/gpex.h |  1 +
>>   hw/pci-host/gpex-acpi.c    | 12 ++++++++++++
>>   2 files changed, 13 insertions(+)
>>
>> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
>> index fcf8b63820..3f8f8ec38d 100644
>> --- a/include/hw/pci-host/gpex.h
>> +++ b/include/hw/pci-host/gpex.h
>> @@ -64,6 +64,7 @@ struct GPEXConfig {
>>       MemMapEntry pio;
>>       int         irq;
>>       PCIBus      *bus;
>> +    bool        preserve_config;
>>   };
>>     int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
>> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
>> index e7e162a00a..7dab259379 100644
>> --- a/hw/pci-host/gpex-acpi.c
>> +++ b/hw/pci-host/gpex-acpi.c
>> @@ -164,6 +164,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct
>> GPEXConfig *cfg)
>>                   aml_append(dev, aml_name_decl("_PXM",
>> aml_int(numa_node)));
>>               }
>>   +            if (cfg->preserve_config) {
>> +                method = aml_method("_DSM", 5, AML_SERIALIZED);
>
> I notice there is a ACPI BIOS Error when booting virtual machine which
> seems be caused by this patch as I add this patchset in my branch to
> test the function of vsmmu.
> It seems that it requires only 4 parameter for method _DSM, but use 5
> parameters here.
> The error log is as following:

Thank you for the heads up. Yes the problem was reported by Igor too in
https://www.mail-archive.com/qemu-devel@nongnu.org/msg842972.html.

At the moment the RMRR ACPI situation has not progressed on spec side or
kernel if I have not missed anything but sure I will take this into
account in my next respin.

Thanks!

Eric
>
> [    2.355459] ACPI BIOS Error (bug): Failure creating named object
> [\_SB.PCI0._DSM], AE_ALREADY_EXISTS (20210930/dswload2-327)
> [    2.355467] ACPI Error: AE_ALREADY_EXISTS, During name
> lookup/catalog (20210930/psobject-221)
> [    2.355470] ACPI: Skipping parse of AML opcode: OpcodeName
> unavailable (0x0014)
> [    2.355657] ACPI: 1 ACPI AML tables successfully acquired and loaded
> [    2.356321] ACPI: Interpreter enabled
> [    2.356323] ACPI: Using GIC for interrupt routing
> [    2.356333] ACPI: MCFG table detected, 1 entries
> [    2.361359] ARMH0011:00: ttyAMA0 at MMIO 0x9000000 (irq = 16,
> base_baud = 0) is a SBSA
> [    2.619805] printk: console [ttyAMA0] enabled
> [    2.622114] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
> [    2.622788] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM
> ClockPM Segments MSI HPX-Type3]
> [    2.623776] acpi PNP0A08:00: _OSC: platform does not support [LTR]
> [    2.624600] acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME
> AER PCIeCapability]
> [    2.625721] acpi PNP0A08:00: ECAM area [mem
> 0x4010000000-0x401fffffff] reserved by PNP0C02:00
> [    2.626645] acpi PNP0A08:00: ECAM at [mem
> 0x4010000000-0x401fffffff] for [bus 00-ff]
> [    2.627450] ACPI: Remapped I/O 0x000000003eff0000 to [io
> 0x0000-0xffff window]
> [    2.628229] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments
> - ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)
> [    2.629576] PCI host bridge to bus 0000:00
> [    2.630008] pci_bus 0000:00: root bus resource [mem
> 0x10000000-0x3efeffff window]
> [    2.630747] pci_bus 0000:00: root bus resource [io  0x0000-0xffff
> window]
> [    2.631405] pci_bus 0000:00: root bus resource [mem
> 0x8000000000-0xffffffffff window]
> [    2.632177] pci_bus 0000:00: root bus resource [bus 00-ff]
> [    2.632731] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments
> - ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)
>
>
>> +                aml_append(method, aml_return(aml_int(0)));
>> +                aml_append(dev, method);
>> +            }
>> +
>>               acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>>                 /*
>> @@ -191,6 +197,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct
>> GPEXConfig *cfg)
>>       aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0
>> Device")));
>>       aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>>   +    if (cfg->preserve_config) {
>> +        method = aml_method("_DSM", 5, AML_SERIALIZED);
>> +        aml_append(method, aml_return(aml_int(0)));
>> +        aml_append(dev, method);
>> +    }
>> +
>>       acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>>         method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
>
>
Zhijian Li (Fujitsu)" via Jan. 6, 2022, 11:16 a.m. UTC | #4 
在 2022/1/6 19:00, Eric Auger 写道:
> Hi CHenxiangn
>
> On 12/29/21 8:13 AM, chenxiang (M) via wrote:
>> Hi Eric,
>>
>>
>> 在 2021/10/5 16:53, Eric Auger 写道:
>>> Add a 'preserve_config' field in struct GPEXConfig and
>>> if set generate the DSM #5 for preserving PCI boot configurations.
>>> The DSM presence is needed to expose RMRs.
>>>
>>> At the moment the DSM generation is not yet enabled.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> ---
>>>    include/hw/pci-host/gpex.h |  1 +
>>>    hw/pci-host/gpex-acpi.c    | 12 ++++++++++++
>>>    2 files changed, 13 insertions(+)
>>>
>>> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
>>> index fcf8b63820..3f8f8ec38d 100644
>>> --- a/include/hw/pci-host/gpex.h
>>> +++ b/include/hw/pci-host/gpex.h
>>> @@ -64,6 +64,7 @@ struct GPEXConfig {
>>>        MemMapEntry pio;
>>>        int         irq;
>>>        PCIBus      *bus;
>>> +    bool        preserve_config;
>>>    };
>>>      int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
>>> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
>>> index e7e162a00a..7dab259379 100644
>>> --- a/hw/pci-host/gpex-acpi.c
>>> +++ b/hw/pci-host/gpex-acpi.c
>>> @@ -164,6 +164,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct
>>> GPEXConfig *cfg)
>>>                    aml_append(dev, aml_name_decl("_PXM",
>>> aml_int(numa_node)));
>>>                }
>>>    +            if (cfg->preserve_config) {
>>> +                method = aml_method("_DSM", 5, AML_SERIALIZED);
>> I notice there is a ACPI BIOS Error when booting virtual machine which
>> seems be caused by this patch as I add this patchset in my branch to
>> test the function of vsmmu.
>> It seems that it requires only 4 parameter for method _DSM, but use 5
>> parameters here.
>> The error log is as following:
> Thank you for the heads up. Yes the problem was reported by Igor too in
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg842972.html.
>
> At the moment the RMRR ACPI situation has not progressed on spec side or
> kernel if I have not missed anything but sure I will take this into
> account in my next respin.

Ok, thanks.

>
> Thanks!
>
> Eric
>> [    2.355459] ACPI BIOS Error (bug): Failure creating named object
>> [\_SB.PCI0._DSM], AE_ALREADY_EXISTS (20210930/dswload2-327)
>> [    2.355467] ACPI Error: AE_ALREADY_EXISTS, During name
>> lookup/catalog (20210930/psobject-221)
>> [    2.355470] ACPI: Skipping parse of AML opcode: OpcodeName
>> unavailable (0x0014)
>> [    2.355657] ACPI: 1 ACPI AML tables successfully acquired and loaded
>> [    2.356321] ACPI: Interpreter enabled
>> [    2.356323] ACPI: Using GIC for interrupt routing
>> [    2.356333] ACPI: MCFG table detected, 1 entries
>> [    2.361359] ARMH0011:00: ttyAMA0 at MMIO 0x9000000 (irq = 16,
>> base_baud = 0) is a SBSA
>> [    2.619805] printk: console [ttyAMA0] enabled
>> [    2.622114] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
>> [    2.622788] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM
>> ClockPM Segments MSI HPX-Type3]
>> [    2.623776] acpi PNP0A08:00: _OSC: platform does not support [LTR]
>> [    2.624600] acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME
>> AER PCIeCapability]
>> [    2.625721] acpi PNP0A08:00: ECAM area [mem
>> 0x4010000000-0x401fffffff] reserved by PNP0C02:00
>> [    2.626645] acpi PNP0A08:00: ECAM at [mem
>> 0x4010000000-0x401fffffff] for [bus 00-ff]
>> [    2.627450] ACPI: Remapped I/O 0x000000003eff0000 to [io
>> 0x0000-0xffff window]
>> [    2.628229] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments
>> - ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)
>> [    2.629576] PCI host bridge to bus 0000:00
>> [    2.630008] pci_bus 0000:00: root bus resource [mem
>> 0x10000000-0x3efeffff window]
>> [    2.630747] pci_bus 0000:00: root bus resource [io  0x0000-0xffff
>> window]
>> [    2.631405] pci_bus 0000:00: root bus resource [mem
>> 0x8000000000-0xffffffffff window]
>> [    2.632177] pci_bus 0000:00: root bus resource [bus 00-ff]
>> [    2.632731] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments
>> - ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)
>>
>>
>>> +                aml_append(method, aml_return(aml_int(0)));
>>> +                aml_append(dev, method);
>>> +            }
>>> +
>>>                acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>>>                  /*
>>> @@ -191,6 +197,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct
>>> GPEXConfig *cfg)
>>>        aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0
>>> Device")));
>>>        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>>>    +    if (cfg->preserve_config) {
>>> +        method = aml_method("_DSM", 5, AML_SERIALIZED);
>>> +        aml_append(method, aml_return(aml_int(0)));
>>> +        aml_append(dev, method);
>>> +    }
>>> +
>>>        acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>>>          method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
>>
> .
>
diff mbox series

Patch

diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
index fcf8b63820..3f8f8ec38d 100644
--- a/include/hw/pci-host/gpex.h
+++ b/include/hw/pci-host/gpex.h
@@ -64,6 +64,7 @@  struct GPEXConfig {
     MemMapEntry pio;
     int         irq;
     PCIBus      *bus;
+    bool        preserve_config;
 };
 
 int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
index e7e162a00a..7dab259379 100644
--- a/hw/pci-host/gpex-acpi.c
+++ b/hw/pci-host/gpex-acpi.c
@@ -164,6 +164,12 @@  void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
                 aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node)));
             }
 
+            if (cfg->preserve_config) {
+                method = aml_method("_DSM", 5, AML_SERIALIZED);
+                aml_append(method, aml_return(aml_int(0)));
+                aml_append(dev, method);
+            }
+
             acpi_dsdt_add_pci_route_table(dev, cfg->irq);
 
             /*
@@ -191,6 +197,12 @@  void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
     aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
     aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
 
+    if (cfg->preserve_config) {
+        method = aml_method("_DSM", 5, AML_SERIALIZED);
+        aml_append(method, aml_return(aml_int(0)));
+        aml_append(dev, method);
+    }
+
     acpi_dsdt_add_pci_route_table(dev, cfg->irq);
 
     method = aml_method("_CBA", 0, AML_NOTSERIALIZED);