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

[Igor Mammedov]

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);