Move aml_pci_edsm to pcihp since we want to reuse that for
ARM and acpi-index support.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
include/hw/acpi/pcihp.h | 2 ++
hw/acpi/pcihp.c | 53 +++++++++++++++++++++++++++++++++++++++++
hw/i386/acpi-build.c | 53 -----------------------------------------
3 files changed, 55 insertions(+), 53 deletions(-)
diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
index 4d820b4baf..bc31dbff39 100644
--- a/include/hw/acpi/pcihp.h
+++ b/include/hw/acpi/pcihp.h
@@ -82,6 +82,8 @@ bool build_append_notification_callback(Aml *parent_scope, const PCIBus *bus);
void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
+Aml *aml_pci_edsm(void);
+
/* Called on reset */
void acpi_pcihp_reset(AcpiPciHpState *s);
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index d800371ddc..57fe8938b1 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -937,6 +937,59 @@ void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
}
}
+Aml *aml_pci_edsm(void)
+{
+ Aml *method, *ifctx;
+ Aml *zero = aml_int(0);
+ Aml *func = aml_arg(2);
+ Aml *ret = aml_local(0);
+ Aml *aidx = aml_local(1);
+ Aml *params = aml_arg(4);
+
+ method = aml_method("EDSM", 5, AML_SERIALIZED);
+
+ /* get supported functions */
+ ifctx = aml_if(aml_equal(func, zero));
+ {
+ /* 1: have supported functions */
+ /* 7: support for function 7 */
+ const uint8_t caps = 1 | BIT(7);
+ build_append_pci_dsm_func0_common(ifctx, ret);
+ aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret, zero)));
+ aml_append(ifctx, aml_return(ret));
+ }
+ aml_append(method, ifctx);
+
+ /* handle specific functions requests */
+ /*
+ * PCI Firmware Specification 3.1
+ * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
+ * Operating Systems
+ */
+ ifctx = aml_if(aml_equal(func, aml_int(7)));
+ {
+ Aml *pkg = aml_package(2);
+ aml_append(pkg, zero);
+ /* optional, if not impl. should return null string */
+ aml_append(pkg, aml_string("%s", ""));
+ aml_append(ifctx, aml_store(pkg, ret));
+
+ /*
+ * IASL is fine when initializing Package with computational data,
+ * however it makes guest unhappy /it fails to process such AML/.
+ * So use runtime assignment to set acpi-index after initializer
+ * to make OSPM happy.
+ */
+ aml_append(ifctx,
+ aml_store(aml_derefof(aml_index(params, aml_int(0))), aidx));
+ aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
+ aml_append(ifctx, aml_return(ret));
+ }
+ aml_append(method, ifctx);
+
+ return method;
+}
+
const VMStateDescription vmstate_acpi_pcihp_pci_status = {
.name = "acpi_pcihp_pci_status",
.version_id = 1,
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index bcfba2ccb3..385e75d061 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -322,59 +322,6 @@ build_facs(GArray *table_data)
g_array_append_vals(table_data, reserved, 40); /* Reserved */
}
-static Aml *aml_pci_edsm(void)
-{
- Aml *method, *ifctx;
- Aml *zero = aml_int(0);
- Aml *func = aml_arg(2);
- Aml *ret = aml_local(0);
- Aml *aidx = aml_local(1);
- Aml *params = aml_arg(4);
-
- method = aml_method("EDSM", 5, AML_SERIALIZED);
-
- /* get supported functions */
- ifctx = aml_if(aml_equal(func, zero));
- {
- /* 1: have supported functions */
- /* 7: support for function 7 */
- const uint8_t caps = 1 | BIT(7);
- build_append_pci_dsm_func0_common(ifctx, ret);
- aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret, zero)));
- aml_append(ifctx, aml_return(ret));
- }
- aml_append(method, ifctx);
-
- /* handle specific functions requests */
- /*
- * PCI Firmware Specification 3.1
- * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
- * Operating Systems
- */
- ifctx = aml_if(aml_equal(func, aml_int(7)));
- {
- Aml *pkg = aml_package(2);
- aml_append(pkg, zero);
- /* optional, if not impl. should return null string */
- aml_append(pkg, aml_string("%s", ""));
- aml_append(ifctx, aml_store(pkg, ret));
-
- /*
- * IASL is fine when initializing Package with computational data,
- * however it makes guest unhappy /it fails to process such AML/.
- * So use runtime assignment to set acpi-index after initializer
- * to make OSPM happy.
- */
- aml_append(ifctx,
- aml_store(aml_derefof(aml_index(params, aml_int(0))), aidx));
- aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
- aml_append(ifctx, aml_return(ret));
- }
- aml_append(method, ifctx);
-
- return method;
-}
-
/*
* build_prt - Define interrupt routing rules
*
--
2.49.0Hi Eric,
On 5/14/25 14:01, Eric Auger wrote:
> Move aml_pci_edsm to pcihp since we want to reuse that for
> ARM and acpi-index support.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> include/hw/acpi/pcihp.h | 2 ++
> hw/acpi/pcihp.c | 53 +++++++++++++++++++++++++++++++++++++++++
> hw/i386/acpi-build.c | 53 -----------------------------------------
> 3 files changed, 55 insertions(+), 53 deletions(-)
>
> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
> index 4d820b4baf..bc31dbff39 100644
> --- a/include/hw/acpi/pcihp.h
> +++ b/include/hw/acpi/pcihp.h
> @@ -82,6 +82,8 @@ bool build_append_notification_callback(Aml *parent_scope, const PCIBus *bus);
>
> void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
>
> +Aml *aml_pci_edsm(void);
> +
> /* Called on reset */
> void acpi_pcihp_reset(AcpiPciHpState *s);
>
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index d800371ddc..57fe8938b1 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -937,6 +937,59 @@ void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
> }
> }
>
> +Aml *aml_pci_edsm(void)
> +{
> + Aml *method, *ifctx;
> + Aml *zero = aml_int(0);
> + Aml *func = aml_arg(2);
> + Aml *ret = aml_local(0);
> + Aml *aidx = aml_local(1);
> + Aml *params = aml_arg(4);
> +
> + method = aml_method("EDSM", 5, AML_SERIALIZED);
> +
> + /* get supported functions */
> + ifctx = aml_if(aml_equal(func, zero));
> + {
> + /* 1: have supported functions */
> + /* 7: support for function 7 */
> + const uint8_t caps = 1 | BIT(7);
> + build_append_pci_dsm_func0_common(ifctx, ret);
> + aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret, zero)));
> + aml_append(ifctx, aml_return(ret));
> + }
> + aml_append(method, ifctx);
> +
> + /* handle specific functions requests */
> + /*
> + * PCI Firmware Specification 3.1
> + * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
> + * Operating Systems
> + */
> + ifctx = aml_if(aml_equal(func, aml_int(7)));
> + {
> + Aml *pkg = aml_package(2);
> + aml_append(pkg, zero);
> + /* optional, if not impl. should return null string */
> + aml_append(pkg, aml_string("%s", ""));
> + aml_append(ifctx, aml_store(pkg, ret));
> +
> + /*
> + * IASL is fine when initializing Package with computational data,
> + * however it makes guest unhappy /it fails to process such AML/.
> + * So use runtime assignment to set acpi-index after initializer
> + * to make OSPM happy.
> + */
> + aml_append(ifctx,
> + aml_store(aml_derefof(aml_index(params, aml_int(0))), aidx));
> + aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
> + aml_append(ifctx, aml_return(ret));
> + }
> + aml_append(method, ifctx);
> +
> + return method;
> +}
> +
> const VMStateDescription vmstate_acpi_pcihp_pci_status = {
> .name = "acpi_pcihp_pci_status",
> .version_id = 1,
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index bcfba2ccb3..385e75d061 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -322,59 +322,6 @@ build_facs(GArray *table_data)
> g_array_append_vals(table_data, reserved, 40); /* Reserved */
> }
>
> -static Aml *aml_pci_edsm(void)
> -{
> - Aml *method, *ifctx;
> - Aml *zero = aml_int(0);
> - Aml *func = aml_arg(2);
> - Aml *ret = aml_local(0);
> - Aml *aidx = aml_local(1);
> - Aml *params = aml_arg(4);
> -
> - method = aml_method("EDSM", 5, AML_SERIALIZED);
> -
> - /* get supported functions */
> - ifctx = aml_if(aml_equal(func, zero));
> - {
> - /* 1: have supported functions */
> - /* 7: support for function 7 */
> - const uint8_t caps = 1 | BIT(7);
> - build_append_pci_dsm_func0_common(ifctx, ret);
> - aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret, zero)));
> - aml_append(ifctx, aml_return(ret));
> - }
> - aml_append(method, ifctx);
> -
> - /* handle specific functions requests */
> - /*
> - * PCI Firmware Specification 3.1
> - * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
> - * Operating Systems
> - */
> - ifctx = aml_if(aml_equal(func, aml_int(7)));
> - {
> - Aml *pkg = aml_package(2);
> - aml_append(pkg, zero);
> - /* optional, if not impl. should return null string */
> - aml_append(pkg, aml_string("%s", ""));
> - aml_append(ifctx, aml_store(pkg, ret));
> -
> - /*
> - * IASL is fine when initializing Package with computational data,
> - * however it makes guest unhappy /it fails to process such AML/.
> - * So use runtime assignment to set acpi-index after initializer
> - * to make OSPM happy.
> - */
> - aml_append(ifctx,
> - aml_store(aml_derefof(aml_index(params, aml_int(0))), aidx));
> - aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
> - aml_append(ifctx, aml_return(ret));
> - }
> - aml_append(method, ifctx);
> -
> - return method;
> -}
> -
> /*
> * build_prt - Define interrupt routing rules
> *
EDSM is not called from anywhere. _DSM method actually calls the PDSM, already defined
in pcihp.c and generated by aml_pci_pdsm(). build_acpi_pci_hotplug(), which calls
aml_pci_pdsm(), properly creates the PDSM method. Then in build_append_pcihp_slots()
the a _DSM is defined per slot and inside it there is a call to PDSM, so I understand
we should actually stick to the PDSM in pcihp.c instead of EDSM.
EDSM, being used in i386 code, feels a outdated PDSM, so maybe PDSM should be used there,
although a different story or a clean up for later. I'm not sure what "static endpoint"
means in the context of EDSM.
Hence this patch can be dropped in my understanding and:
aml_append(pci0_scope, aml_pci_edsm()) in 15/22 too, without any prejudice to the
hotplugging and unplugging in this series.
Cheers,
Gustavo
Hi Gustavo,
On 5/21/25 5:26 PM, Gustavo Romero wrote:
> Hi Eric,
>
> On 5/14/25 14:01, Eric Auger wrote:
>> Move aml_pci_edsm to pcihp since we want to reuse that for
>> ARM and acpi-index support.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>> include/hw/acpi/pcihp.h | 2 ++
>> hw/acpi/pcihp.c | 53 +++++++++++++++++++++++++++++++++++++++++
>> hw/i386/acpi-build.c | 53 -----------------------------------------
>> 3 files changed, 55 insertions(+), 53 deletions(-)
>>
>> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
>> index 4d820b4baf..bc31dbff39 100644
>> --- a/include/hw/acpi/pcihp.h
>> +++ b/include/hw/acpi/pcihp.h
>> @@ -82,6 +82,8 @@ bool build_append_notification_callback(Aml
>> *parent_scope, const PCIBus *bus);
>> void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
>> +Aml *aml_pci_edsm(void);
>> +
>> /* Called on reset */
>> void acpi_pcihp_reset(AcpiPciHpState *s);
>> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
>> index d800371ddc..57fe8938b1 100644
>> --- a/hw/acpi/pcihp.c
>> +++ b/hw/acpi/pcihp.c
>> @@ -937,6 +937,59 @@ void build_append_pci_bus_devices(Aml
>> *parent_scope, PCIBus *bus)
>> }
>> }
>> +Aml *aml_pci_edsm(void)
>> +{
>> + Aml *method, *ifctx;
>> + Aml *zero = aml_int(0);
>> + Aml *func = aml_arg(2);
>> + Aml *ret = aml_local(0);
>> + Aml *aidx = aml_local(1);
>> + Aml *params = aml_arg(4);
>> +
>> + method = aml_method("EDSM", 5, AML_SERIALIZED);
>> +
>> + /* get supported functions */
>> + ifctx = aml_if(aml_equal(func, zero));
>> + {
>> + /* 1: have supported functions */
>> + /* 7: support for function 7 */
>> + const uint8_t caps = 1 | BIT(7);
>> + build_append_pci_dsm_func0_common(ifctx, ret);
>> + aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret,
>> zero)));
>> + aml_append(ifctx, aml_return(ret));
>> + }
>> + aml_append(method, ifctx);
>> +
>> + /* handle specific functions requests */
>> + /*
>> + * PCI Firmware Specification 3.1
>> + * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
>> + * Operating Systems
>> + */
>> + ifctx = aml_if(aml_equal(func, aml_int(7)));
>> + {
>> + Aml *pkg = aml_package(2);
>> + aml_append(pkg, zero);
>> + /* optional, if not impl. should return null string */
>> + aml_append(pkg, aml_string("%s", ""));
>> + aml_append(ifctx, aml_store(pkg, ret));
>> +
>> + /*
>> + * IASL is fine when initializing Package with computational
>> data,
>> + * however it makes guest unhappy /it fails to process such
>> AML/.
>> + * So use runtime assignment to set acpi-index after initializer
>> + * to make OSPM happy.
>> + */
>> + aml_append(ifctx,
>> + aml_store(aml_derefof(aml_index(params, aml_int(0))),
>> aidx));
>> + aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
>> + aml_append(ifctx, aml_return(ret));
>> + }
>> + aml_append(method, ifctx);
>> +
>> + return method;
>> +}
>> +
>> const VMStateDescription vmstate_acpi_pcihp_pci_status = {
>> .name = "acpi_pcihp_pci_status",
>> .version_id = 1,
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index bcfba2ccb3..385e75d061 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -322,59 +322,6 @@ build_facs(GArray *table_data)
>> g_array_append_vals(table_data, reserved, 40); /* Reserved */
>> }
>> -static Aml *aml_pci_edsm(void)
>> -{
>> - Aml *method, *ifctx;
>> - Aml *zero = aml_int(0);
>> - Aml *func = aml_arg(2);
>> - Aml *ret = aml_local(0);
>> - Aml *aidx = aml_local(1);
>> - Aml *params = aml_arg(4);
>> -
>> - method = aml_method("EDSM", 5, AML_SERIALIZED);
>> -
>> - /* get supported functions */
>> - ifctx = aml_if(aml_equal(func, zero));
>> - {
>> - /* 1: have supported functions */
>> - /* 7: support for function 7 */
>> - const uint8_t caps = 1 | BIT(7);
>> - build_append_pci_dsm_func0_common(ifctx, ret);
>> - aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret,
>> zero)));
>> - aml_append(ifctx, aml_return(ret));
>> - }
>> - aml_append(method, ifctx);
>> -
>> - /* handle specific functions requests */
>> - /*
>> - * PCI Firmware Specification 3.1
>> - * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
>> - * Operating Systems
>> - */
>> - ifctx = aml_if(aml_equal(func, aml_int(7)));
>> - {
>> - Aml *pkg = aml_package(2);
>> - aml_append(pkg, zero);
>> - /* optional, if not impl. should return null string */
>> - aml_append(pkg, aml_string("%s", ""));
>> - aml_append(ifctx, aml_store(pkg, ret));
>> -
>> - /*
>> - * IASL is fine when initializing Package with computational
>> data,
>> - * however it makes guest unhappy /it fails to process such
>> AML/.
>> - * So use runtime assignment to set acpi-index after initializer
>> - * to make OSPM happy.
>> - */
>> - aml_append(ifctx,
>> - aml_store(aml_derefof(aml_index(params, aml_int(0))),
>> aidx));
>> - aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
>> - aml_append(ifctx, aml_return(ret));
>> - }
>> - aml_append(method, ifctx);
>> -
>> - return method;
>> -}
>> -
>> /*
>> * build_prt - Define interrupt routing rules
>> *
>
> EDSM is not called from anywhere. _DSM method actually calls the PDSM,
> already defined
> in pcihp.c and generated by aml_pci_pdsm(). build_acpi_pci_hotplug(),
> which calls
> aml_pci_pdsm(), properly creates the PDSM method. Then in
> build_append_pcihp_slots()
> the a _DSM is defined per slot and inside it there is a call to PDSM,
> so I understand
> we should actually stick to the PDSM in pcihp.c instead of EDSM.
I see build_append_pci_bus_devices -> aml_pci_static_endpoint_dsm
adds a _DSM method which eventually calls EDSM.
aml_pci_device_dsm builds another _DSM implementation which calls PDSM.
As we use build_append_pci_bus_devices and we are likely to have a _DSM
implementation that calls EDSM method, I think we shall have it in the
aml. What do I miss?
Thank you for the comprehensive review!
Cheers
Eric
>
> EDSM, being used in i386 code, feels a outdated PDSM, so maybe PDSM
> should be used there,
> although a different story or a clean up for later. I'm not sure what
> "static endpoint"
> means in the context of EDSM.
>
> Hence this patch can be dropped in my understanding and:
>
> aml_append(pci0_scope, aml_pci_edsm()) in 15/22 too, without any
> prejudice to the
> hotplugging and unplugging in this series.
>
>
> Cheers,
> Gustavo
>
Hi Eric,
On 5/21/25 12:56, Eric Auger wrote:
> Hi Gustavo,
>
> On 5/21/25 5:26 PM, Gustavo Romero wrote:
>> Hi Eric,
>>
>> On 5/14/25 14:01, Eric Auger wrote:
>>> Move aml_pci_edsm to pcihp since we want to reuse that for
>>> ARM and acpi-index support.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> ---
>>> include/hw/acpi/pcihp.h | 2 ++
>>> hw/acpi/pcihp.c | 53 +++++++++++++++++++++++++++++++++++++++++
>>> hw/i386/acpi-build.c | 53 -----------------------------------------
>>> 3 files changed, 55 insertions(+), 53 deletions(-)
>>>
>>> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
>>> index 4d820b4baf..bc31dbff39 100644
>>> --- a/include/hw/acpi/pcihp.h
>>> +++ b/include/hw/acpi/pcihp.h
>>> @@ -82,6 +82,8 @@ bool build_append_notification_callback(Aml
>>> *parent_scope, const PCIBus *bus);
>>> void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
>>> +Aml *aml_pci_edsm(void);
>>> +
>>> /* Called on reset */
>>> void acpi_pcihp_reset(AcpiPciHpState *s);
>>> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
>>> index d800371ddc..57fe8938b1 100644
>>> --- a/hw/acpi/pcihp.c
>>> +++ b/hw/acpi/pcihp.c
>>> @@ -937,6 +937,59 @@ void build_append_pci_bus_devices(Aml
>>> *parent_scope, PCIBus *bus)
>>> }
>>> }
>>> +Aml *aml_pci_edsm(void)
>>> +{
>>> + Aml *method, *ifctx;
>>> + Aml *zero = aml_int(0);
>>> + Aml *func = aml_arg(2);
>>> + Aml *ret = aml_local(0);
>>> + Aml *aidx = aml_local(1);
>>> + Aml *params = aml_arg(4);
>>> +
>>> + method = aml_method("EDSM", 5, AML_SERIALIZED);
>>> +
>>> + /* get supported functions */
>>> + ifctx = aml_if(aml_equal(func, zero));
>>> + {
>>> + /* 1: have supported functions */
>>> + /* 7: support for function 7 */
>>> + const uint8_t caps = 1 | BIT(7);
>>> + build_append_pci_dsm_func0_common(ifctx, ret);
>>> + aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret,
>>> zero)));
>>> + aml_append(ifctx, aml_return(ret));
>>> + }
>>> + aml_append(method, ifctx);
>>> +
>>> + /* handle specific functions requests */
>>> + /*
>>> + * PCI Firmware Specification 3.1
>>> + * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
>>> + * Operating Systems
>>> + */
>>> + ifctx = aml_if(aml_equal(func, aml_int(7)));
>>> + {
>>> + Aml *pkg = aml_package(2);
>>> + aml_append(pkg, zero);
>>> + /* optional, if not impl. should return null string */
>>> + aml_append(pkg, aml_string("%s", ""));
>>> + aml_append(ifctx, aml_store(pkg, ret));
>>> +
>>> + /*
>>> + * IASL is fine when initializing Package with computational
>>> data,
>>> + * however it makes guest unhappy /it fails to process such
>>> AML/.
>>> + * So use runtime assignment to set acpi-index after initializer
>>> + * to make OSPM happy.
>>> + */
>>> + aml_append(ifctx,
>>> + aml_store(aml_derefof(aml_index(params, aml_int(0))),
>>> aidx));
>>> + aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
>>> + aml_append(ifctx, aml_return(ret));
>>> + }
>>> + aml_append(method, ifctx);
>>> +
>>> + return method;
>>> +}
>>> +
>>> const VMStateDescription vmstate_acpi_pcihp_pci_status = {
>>> .name = "acpi_pcihp_pci_status",
>>> .version_id = 1,
>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>> index bcfba2ccb3..385e75d061 100644
>>> --- a/hw/i386/acpi-build.c
>>> +++ b/hw/i386/acpi-build.c
>>> @@ -322,59 +322,6 @@ build_facs(GArray *table_data)
>>> g_array_append_vals(table_data, reserved, 40); /* Reserved */
>>> }
>>> -static Aml *aml_pci_edsm(void)
>>> -{
>>> - Aml *method, *ifctx;
>>> - Aml *zero = aml_int(0);
>>> - Aml *func = aml_arg(2);
>>> - Aml *ret = aml_local(0);
>>> - Aml *aidx = aml_local(1);
>>> - Aml *params = aml_arg(4);
>>> -
>>> - method = aml_method("EDSM", 5, AML_SERIALIZED);
>>> -
>>> - /* get supported functions */
>>> - ifctx = aml_if(aml_equal(func, zero));
>>> - {
>>> - /* 1: have supported functions */
>>> - /* 7: support for function 7 */
>>> - const uint8_t caps = 1 | BIT(7);
>>> - build_append_pci_dsm_func0_common(ifctx, ret);
>>> - aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret,
>>> zero)));
>>> - aml_append(ifctx, aml_return(ret));
>>> - }
>>> - aml_append(method, ifctx);
>>> -
>>> - /* handle specific functions requests */
>>> - /*
>>> - * PCI Firmware Specification 3.1
>>> - * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
>>> - * Operating Systems
>>> - */
>>> - ifctx = aml_if(aml_equal(func, aml_int(7)));
>>> - {
>>> - Aml *pkg = aml_package(2);
>>> - aml_append(pkg, zero);
>>> - /* optional, if not impl. should return null string */
>>> - aml_append(pkg, aml_string("%s", ""));
>>> - aml_append(ifctx, aml_store(pkg, ret));
>>> -
>>> - /*
>>> - * IASL is fine when initializing Package with computational
>>> data,
>>> - * however it makes guest unhappy /it fails to process such
>>> AML/.
>>> - * So use runtime assignment to set acpi-index after initializer
>>> - * to make OSPM happy.
>>> - */
>>> - aml_append(ifctx,
>>> - aml_store(aml_derefof(aml_index(params, aml_int(0))),
>>> aidx));
>>> - aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
>>> - aml_append(ifctx, aml_return(ret));
>>> - }
>>> - aml_append(method, ifctx);
>>> -
>>> - return method;
>>> -}
>>> -
>>> /*
>>> * build_prt - Define interrupt routing rules
>>> *
>>
>> EDSM is not called from anywhere. _DSM method actually calls the PDSM,
>> already defined
>> in pcihp.c and generated by aml_pci_pdsm(). build_acpi_pci_hotplug(),
>> which calls
>> aml_pci_pdsm(), properly creates the PDSM method. Then in
>> build_append_pcihp_slots()
>> the a _DSM is defined per slot and inside it there is a call to PDSM,
>> so I understand
>> we should actually stick to the PDSM in pcihp.c instead of EDSM.
>
> I see build_append_pci_bus_devices -> aml_pci_static_endpoint_dsm
> adds a _DSM method which eventually calls EDSM.
Yes, but I meant in the final generated ACPI AML code.
> aml_pci_device_dsm builds another _DSM implementation which calls PDSM.
>
> As we use build_append_pci_bus_devices and we are likely to have a _DSM
> implementation that calls EDSM method, I think we shall have it in the
> aml. What do I miss?
Maybe some condition in build_append_pci_bus_devices() of:
if (pdev->acpi_index &&
!object_property_get_bool(OBJECT(pdev), "hotpluggable",
&error_abort)) { ... }
is not met?
Does your _DSM method in the machine's ACPI has a call to EDSM? I don't
see it. In my case it's the PDSM that is being called:
gromero@xps13:/tmp$ grep -n PDSM dsdt.dsl
1910: Method (PDSM, 5, Serialized)
2047: Return (PDSM (Arg0, Arg1, Arg2, Arg3, Local0))
gromero@xps13:/tmp$ grep -n EDSM dsdt.dsl
1959: Method (EDSM, 5, Serialized)
gromero@xps13:/tmp$
I also confirmed that PDSM is what is being used by reverting this patch and
dropping the aml_append(pci0_scope, aml_pci_edsm()); in 15/22.
But also, why shouldn't we use the PDSM defined already in pcihp.c? This is
indeed crafted for the acpi-pcihp-hotplug device as I understand it.
Cheers,
Gustavo
> Thank you for the comprehensive review!
>
> Cheers
>
> Eric
>>
>> EDSM, being used in i386 code, feels a outdated PDSM, so maybe PDSM
>> should be used there,
>> although a different story or a clean up for later. I'm not sure what
>> "static endpoint"
>> means in the context of EDSM.
>>
>> Hence this patch can be dropped in my understanding and:
>>
>> aml_append(pci0_scope, aml_pci_edsm()) in 15/22 too, without any
>> prejudice to the
>> hotplugging and unplugging in this series.
>>
>>
>> Cheers,
>> Gustavo
>>
>
Hi Eric,
On 5/21/25 13:24, Gustavo Romero wrote:
> Hi Eric,
>
> On 5/21/25 12:56, Eric Auger wrote:
>> Hi Gustavo,
>>
>> On 5/21/25 5:26 PM, Gustavo Romero wrote:
>>> Hi Eric,
>>>
>>> On 5/14/25 14:01, Eric Auger wrote:
>>>> Move aml_pci_edsm to pcihp since we want to reuse that for
>>>> ARM and acpi-index support.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>> ---
>>>> include/hw/acpi/pcihp.h | 2 ++
>>>> hw/acpi/pcihp.c | 53 +++++++++++++++++++++++++++++++++++++++++
>>>> hw/i386/acpi-build.c | 53 -----------------------------------------
>>>> 3 files changed, 55 insertions(+), 53 deletions(-)
>>>>
>>>> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
>>>> index 4d820b4baf..bc31dbff39 100644
>>>> --- a/include/hw/acpi/pcihp.h
>>>> +++ b/include/hw/acpi/pcihp.h
>>>> @@ -82,6 +82,8 @@ bool build_append_notification_callback(Aml
>>>> *parent_scope, const PCIBus *bus);
>>>> void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
>>>> +Aml *aml_pci_edsm(void);
>>>> +
>>>> /* Called on reset */
>>>> void acpi_pcihp_reset(AcpiPciHpState *s);
>>>> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
>>>> index d800371ddc..57fe8938b1 100644
>>>> --- a/hw/acpi/pcihp.c
>>>> +++ b/hw/acpi/pcihp.c
>>>> @@ -937,6 +937,59 @@ void build_append_pci_bus_devices(Aml
>>>> *parent_scope, PCIBus *bus)
>>>> }
>>>> }
>>>> +Aml *aml_pci_edsm(void)
>>>> +{
>>>> + Aml *method, *ifctx;
>>>> + Aml *zero = aml_int(0);
>>>> + Aml *func = aml_arg(2);
>>>> + Aml *ret = aml_local(0);
>>>> + Aml *aidx = aml_local(1);
>>>> + Aml *params = aml_arg(4);
>>>> +
>>>> + method = aml_method("EDSM", 5, AML_SERIALIZED);
>>>> +
>>>> + /* get supported functions */
>>>> + ifctx = aml_if(aml_equal(func, zero));
>>>> + {
>>>> + /* 1: have supported functions */
>>>> + /* 7: support for function 7 */
>>>> + const uint8_t caps = 1 | BIT(7);
>>>> + build_append_pci_dsm_func0_common(ifctx, ret);
>>>> + aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret,
>>>> zero)));
>>>> + aml_append(ifctx, aml_return(ret));
>>>> + }
>>>> + aml_append(method, ifctx);
>>>> +
>>>> + /* handle specific functions requests */
>>>> + /*
>>>> + * PCI Firmware Specification 3.1
>>>> + * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
>>>> + * Operating Systems
>>>> + */
>>>> + ifctx = aml_if(aml_equal(func, aml_int(7)));
>>>> + {
>>>> + Aml *pkg = aml_package(2);
>>>> + aml_append(pkg, zero);
>>>> + /* optional, if not impl. should return null string */
>>>> + aml_append(pkg, aml_string("%s", ""));
>>>> + aml_append(ifctx, aml_store(pkg, ret));
>>>> +
>>>> + /*
>>>> + * IASL is fine when initializing Package with computational
>>>> data,
>>>> + * however it makes guest unhappy /it fails to process such
>>>> AML/.
>>>> + * So use runtime assignment to set acpi-index after initializer
>>>> + * to make OSPM happy.
>>>> + */
>>>> + aml_append(ifctx,
>>>> + aml_store(aml_derefof(aml_index(params, aml_int(0))),
>>>> aidx));
>>>> + aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
>>>> + aml_append(ifctx, aml_return(ret));
>>>> + }
>>>> + aml_append(method, ifctx);
>>>> +
>>>> + return method;
>>>> +}
>>>> +
>>>> const VMStateDescription vmstate_acpi_pcihp_pci_status = {
>>>> .name = "acpi_pcihp_pci_status",
>>>> .version_id = 1,
>>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>>> index bcfba2ccb3..385e75d061 100644
>>>> --- a/hw/i386/acpi-build.c
>>>> +++ b/hw/i386/acpi-build.c
>>>> @@ -322,59 +322,6 @@ build_facs(GArray *table_data)
>>>> g_array_append_vals(table_data, reserved, 40); /* Reserved */
>>>> }
>>>> -static Aml *aml_pci_edsm(void)
>>>> -{
>>>> - Aml *method, *ifctx;
>>>> - Aml *zero = aml_int(0);
>>>> - Aml *func = aml_arg(2);
>>>> - Aml *ret = aml_local(0);
>>>> - Aml *aidx = aml_local(1);
>>>> - Aml *params = aml_arg(4);
>>>> -
>>>> - method = aml_method("EDSM", 5, AML_SERIALIZED);
>>>> -
>>>> - /* get supported functions */
>>>> - ifctx = aml_if(aml_equal(func, zero));
>>>> - {
>>>> - /* 1: have supported functions */
>>>> - /* 7: support for function 7 */
>>>> - const uint8_t caps = 1 | BIT(7);
>>>> - build_append_pci_dsm_func0_common(ifctx, ret);
>>>> - aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret,
>>>> zero)));
>>>> - aml_append(ifctx, aml_return(ret));
>>>> - }
>>>> - aml_append(method, ifctx);
>>>> -
>>>> - /* handle specific functions requests */
>>>> - /*
>>>> - * PCI Firmware Specification 3.1
>>>> - * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
>>>> - * Operating Systems
>>>> - */
>>>> - ifctx = aml_if(aml_equal(func, aml_int(7)));
>>>> - {
>>>> - Aml *pkg = aml_package(2);
>>>> - aml_append(pkg, zero);
>>>> - /* optional, if not impl. should return null string */
>>>> - aml_append(pkg, aml_string("%s", ""));
>>>> - aml_append(ifctx, aml_store(pkg, ret));
>>>> -
>>>> - /*
>>>> - * IASL is fine when initializing Package with computational
>>>> data,
>>>> - * however it makes guest unhappy /it fails to process such
>>>> AML/.
>>>> - * So use runtime assignment to set acpi-index after initializer
>>>> - * to make OSPM happy.
>>>> - */
>>>> - aml_append(ifctx,
>>>> - aml_store(aml_derefof(aml_index(params, aml_int(0))),
>>>> aidx));
>>>> - aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
>>>> - aml_append(ifctx, aml_return(ret));
>>>> - }
>>>> - aml_append(method, ifctx);
>>>> -
>>>> - return method;
>>>> -}
>>>> -
>>>> /*
>>>> * build_prt - Define interrupt routing rules
>>>> *
>>>
>>> EDSM is not called from anywhere. _DSM method actually calls the PDSM,
>>> already defined
>>> in pcihp.c and generated by aml_pci_pdsm(). build_acpi_pci_hotplug(),
>>> which calls
>>> aml_pci_pdsm(), properly creates the PDSM method. Then in
>>> build_append_pcihp_slots()
>>> the a _DSM is defined per slot and inside it there is a call to PDSM,
>>> so I understand
>>> we should actually stick to the PDSM in pcihp.c instead of EDSM.
>>
>> I see build_append_pci_bus_devices -> aml_pci_static_endpoint_dsm
>> adds a _DSM method which eventually calls EDSM.
>
> Yes, but I meant in the final generated ACPI AML code.
>
>
>> aml_pci_device_dsm builds another _DSM implementation which calls PDSM.
>>
>> As we use build_append_pci_bus_devices and we are likely to have a _DSM
>> implementation that calls EDSM method, I think we shall have it in the
>> aml. What do I miss?
>
> Maybe some condition in build_append_pci_bus_devices() of:
>
> if (pdev->acpi_index &&
> !object_property_get_bool(OBJECT(pdev), "hotpluggable",
> &error_abort)) { ... }
>
>
> is not met?
>
> Does your _DSM method in the machine's ACPI has a call to EDSM? I don't
> see it. In my case it's the PDSM that is being called:
>
> gromero@xps13:/tmp$ grep -n PDSM dsdt.dsl
> 1910: Method (PDSM, 5, Serialized)
> 2047: Return (PDSM (Arg0, Arg1, Arg2, Arg3, Local0))
> gromero@xps13:/tmp$ grep -n EDSM dsdt.dsl
> 1959: Method (EDSM, 5, Serialized)
> gromero@xps13:/tmp$
>
> I also confirmed that PDSM is what is being used by reverting this patch and
> dropping the aml_append(pci0_scope, aml_pci_edsm()); in 15/22.
>
> But also, why shouldn't we use the PDSM defined already in pcihp.c? This is
> indeed crafted for the acpi-pcihp-hotplug device as I understand it.
Sorry, I see now, the EDSM is actually only for static endpoint PCI devices,
i.e. non-hotpluggable, I see. So PDSM and EDSM serve different purposes, although
similar.
So, my point in the end is (and the one that confused me too) that EDSM can be
present even if it will never be used by any static device, working pretty much
like an ACPI deadcode. Could we avoid this? For instance, just emit EDSM if
a _DSM that relies on it is emitted, like:
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index e87846a1fa..4d6dd3dd96 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -905,8 +905,9 @@ void build_append_pcihp_slots(Aml *parent_scope, PCIBus *bus)
aml_append(parent_scope, notify_method);
}
-void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
+bool build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
{
+ bool has_dsm = false;
int devfn;
Aml *dev;
@@ -929,11 +930,14 @@ void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
!object_property_get_bool(OBJECT(pdev), "hotpluggable",
&error_abort)) {
aml_append(dev, aml_pci_static_endpoint_dsm(pdev));
+ has_dsm = true;
}
/* device descriptor has been composed, add it into parent context */
aml_append(parent_scope, dev);
}
+
+ return has_dsm;
}
Aml *aml_pci_edsm(void)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 05a754d368..e4788c03ed 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -844,15 +844,22 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
if (vms->acpi_pcihp) {
Aml *pci0_scope = aml_scope("\\_SB.PCI0");
- aml_append(pci0_scope, aml_pci_edsm());
build_acpi_pci_hotplug(dsdt, AML_SYSTEM_MEMORY,
memmap[VIRT_ACPI_PCIHP].base);
build_append_pcihp_resources(pci0_scope,
memmap[VIRT_ACPI_PCIHP].base,
memmap[VIRT_ACPI_PCIHP].size);
- /* Scan all PCI buses. Generate tables to support hotplug. */
- build_append_pci_bus_devices(pci0_scope, vms->bus);
+ /*
+ * Scan all PCI buses and devices. Generate tables to support static
+ * endpoint devices and hotplug.
+ */
+ /* Generate tables for static endpoints. */
+ if (build_append_pci_bus_devices(pci0_scope, vms->bus)) {
+ /* Has a _DSM method that requires a EDSM method. */
+ aml_append(pci0_scope, aml_pci_edsm());
+ }
+ /* Generate tables for PCI buses/slots/ */
if (object_property_find(OBJECT(vms->bus), ACPI_PCIHP_PROP_BSEL)) {
build_append_pcihp_slots(pci0_scope, vms->bus);
}
diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
index bda5ea24b5..25cc39f4ab 100644
--- a/include/hw/acpi/pcihp.h
+++ b/include/hw/acpi/pcihp.h
@@ -82,7 +82,7 @@ void build_append_pcihp_resources(Aml *table,
uint64_t io_addr, uint64_t io_len);
bool build_append_notification_callback(Aml *parent_scope, const PCIBus *bus);
-void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
+bool build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
Aml *aml_pci_edsm(void);
Another question that I have is: could you please share a QEMU command line option
that generates a static endpoint PCI device that matches the criteria for the emission
of the _DSM method accordingly to build_append_pci_bus_devices()?
Cheers,
Gustavo
On 5/21/25 10:19 PM, Gustavo Romero wrote:
> Hi Eric,
>
> On 5/21/25 13:24, Gustavo Romero wrote:
>> Hi Eric,
>>
>> On 5/21/25 12:56, Eric Auger wrote:
>>> Hi Gustavo,
>>>
>>> On 5/21/25 5:26 PM, Gustavo Romero wrote:
>>>> Hi Eric,
>>>>
>>>> On 5/14/25 14:01, Eric Auger wrote:
>>>>> Move aml_pci_edsm to pcihp since we want to reuse that for
>>>>> ARM and acpi-index support.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>> ---
>>>>> include/hw/acpi/pcihp.h | 2 ++
>>>>> hw/acpi/pcihp.c | 53
>>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>> hw/i386/acpi-build.c | 53
>>>>> -----------------------------------------
>>>>> 3 files changed, 55 insertions(+), 53 deletions(-)
>>>>>
>>>>> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
>>>>> index 4d820b4baf..bc31dbff39 100644
>>>>> --- a/include/hw/acpi/pcihp.h
>>>>> +++ b/include/hw/acpi/pcihp.h
>>>>> @@ -82,6 +82,8 @@ bool build_append_notification_callback(Aml
>>>>> *parent_scope, const PCIBus *bus);
>>>>> void build_append_pci_bus_devices(Aml *parent_scope, PCIBus
>>>>> *bus);
>>>>> +Aml *aml_pci_edsm(void);
>>>>> +
>>>>> /* Called on reset */
>>>>> void acpi_pcihp_reset(AcpiPciHpState *s);
>>>>> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
>>>>> index d800371ddc..57fe8938b1 100644
>>>>> --- a/hw/acpi/pcihp.c
>>>>> +++ b/hw/acpi/pcihp.c
>>>>> @@ -937,6 +937,59 @@ void build_append_pci_bus_devices(Aml
>>>>> *parent_scope, PCIBus *bus)
>>>>> }
>>>>> }
>>>>> +Aml *aml_pci_edsm(void)
>>>>> +{
>>>>> + Aml *method, *ifctx;
>>>>> + Aml *zero = aml_int(0);
>>>>> + Aml *func = aml_arg(2);
>>>>> + Aml *ret = aml_local(0);
>>>>> + Aml *aidx = aml_local(1);
>>>>> + Aml *params = aml_arg(4);
>>>>> +
>>>>> + method = aml_method("EDSM", 5, AML_SERIALIZED);
>>>>> +
>>>>> + /* get supported functions */
>>>>> + ifctx = aml_if(aml_equal(func, zero));
>>>>> + {
>>>>> + /* 1: have supported functions */
>>>>> + /* 7: support for function 7 */
>>>>> + const uint8_t caps = 1 | BIT(7);
>>>>> + build_append_pci_dsm_func0_common(ifctx, ret);
>>>>> + aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret,
>>>>> zero)));
>>>>> + aml_append(ifctx, aml_return(ret));
>>>>> + }
>>>>> + aml_append(method, ifctx);
>>>>> +
>>>>> + /* handle specific functions requests */
>>>>> + /*
>>>>> + * PCI Firmware Specification 3.1
>>>>> + * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
>>>>> + * Operating Systems
>>>>> + */
>>>>> + ifctx = aml_if(aml_equal(func, aml_int(7)));
>>>>> + {
>>>>> + Aml *pkg = aml_package(2);
>>>>> + aml_append(pkg, zero);
>>>>> + /* optional, if not impl. should return null string */
>>>>> + aml_append(pkg, aml_string("%s", ""));
>>>>> + aml_append(ifctx, aml_store(pkg, ret));
>>>>> +
>>>>> + /*
>>>>> + * IASL is fine when initializing Package with computational
>>>>> data,
>>>>> + * however it makes guest unhappy /it fails to process such
>>>>> AML/.
>>>>> + * So use runtime assignment to set acpi-index after
>>>>> initializer
>>>>> + * to make OSPM happy.
>>>>> + */
>>>>> + aml_append(ifctx,
>>>>> + aml_store(aml_derefof(aml_index(params, aml_int(0))),
>>>>> aidx));
>>>>> + aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
>>>>> + aml_append(ifctx, aml_return(ret));
>>>>> + }
>>>>> + aml_append(method, ifctx);
>>>>> +
>>>>> + return method;
>>>>> +}
>>>>> +
>>>>> const VMStateDescription vmstate_acpi_pcihp_pci_status = {
>>>>> .name = "acpi_pcihp_pci_status",
>>>>> .version_id = 1,
>>>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>>>> index bcfba2ccb3..385e75d061 100644
>>>>> --- a/hw/i386/acpi-build.c
>>>>> +++ b/hw/i386/acpi-build.c
>>>>> @@ -322,59 +322,6 @@ build_facs(GArray *table_data)
>>>>> g_array_append_vals(table_data, reserved, 40); /* Reserved */
>>>>> }
>>>>> -static Aml *aml_pci_edsm(void)
>>>>> -{
>>>>> - Aml *method, *ifctx;
>>>>> - Aml *zero = aml_int(0);
>>>>> - Aml *func = aml_arg(2);
>>>>> - Aml *ret = aml_local(0);
>>>>> - Aml *aidx = aml_local(1);
>>>>> - Aml *params = aml_arg(4);
>>>>> -
>>>>> - method = aml_method("EDSM", 5, AML_SERIALIZED);
>>>>> -
>>>>> - /* get supported functions */
>>>>> - ifctx = aml_if(aml_equal(func, zero));
>>>>> - {
>>>>> - /* 1: have supported functions */
>>>>> - /* 7: support for function 7 */
>>>>> - const uint8_t caps = 1 | BIT(7);
>>>>> - build_append_pci_dsm_func0_common(ifctx, ret);
>>>>> - aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret,
>>>>> zero)));
>>>>> - aml_append(ifctx, aml_return(ret));
>>>>> - }
>>>>> - aml_append(method, ifctx);
>>>>> -
>>>>> - /* handle specific functions requests */
>>>>> - /*
>>>>> - * PCI Firmware Specification 3.1
>>>>> - * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
>>>>> - * Operating Systems
>>>>> - */
>>>>> - ifctx = aml_if(aml_equal(func, aml_int(7)));
>>>>> - {
>>>>> - Aml *pkg = aml_package(2);
>>>>> - aml_append(pkg, zero);
>>>>> - /* optional, if not impl. should return null string */
>>>>> - aml_append(pkg, aml_string("%s", ""));
>>>>> - aml_append(ifctx, aml_store(pkg, ret));
>>>>> -
>>>>> - /*
>>>>> - * IASL is fine when initializing Package with computational
>>>>> data,
>>>>> - * however it makes guest unhappy /it fails to process such
>>>>> AML/.
>>>>> - * So use runtime assignment to set acpi-index after
>>>>> initializer
>>>>> - * to make OSPM happy.
>>>>> - */
>>>>> - aml_append(ifctx,
>>>>> - aml_store(aml_derefof(aml_index(params, aml_int(0))),
>>>>> aidx));
>>>>> - aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
>>>>> - aml_append(ifctx, aml_return(ret));
>>>>> - }
>>>>> - aml_append(method, ifctx);
>>>>> -
>>>>> - return method;
>>>>> -}
>>>>> -
>>>>> /*
>>>>> * build_prt - Define interrupt routing rules
>>>>> *
>>>>
>>>> EDSM is not called from anywhere. _DSM method actually calls the PDSM,
>>>> already defined
>>>> in pcihp.c and generated by aml_pci_pdsm(). build_acpi_pci_hotplug(),
>>>> which calls
>>>> aml_pci_pdsm(), properly creates the PDSM method. Then in
>>>> build_append_pcihp_slots()
>>>> the a _DSM is defined per slot and inside it there is a call to PDSM,
>>>> so I understand
>>>> we should actually stick to the PDSM in pcihp.c instead of EDSM.
>>>
>>> I see build_append_pci_bus_devices -> aml_pci_static_endpoint_dsm
>>> adds a _DSM method which eventually calls EDSM.
>>
>> Yes, but I meant in the final generated ACPI AML code.
>>
>>
>>> aml_pci_device_dsm builds another _DSM implementation which calls PDSM.
>>>
>>> As we use build_append_pci_bus_devices and we are likely to have a _DSM
>>> implementation that calls EDSM method, I think we shall have it in the
>>> aml. What do I miss?
>>
>> Maybe some condition in build_append_pci_bus_devices() of:
>>
>> if (pdev->acpi_index &&
>> !object_property_get_bool(OBJECT(pdev), "hotpluggable",
>> &error_abort)) { ... }
>>
>>
>> is not met?
>>
>> Does your _DSM method in the machine's ACPI has a call to EDSM? I don't
>> see it. In my case it's the PDSM that is being called:
>>
>> gromero@xps13:/tmp$ grep -n PDSM dsdt.dsl
>> 1910: Method (PDSM, 5, Serialized)
>> 2047: Return (PDSM (Arg0, Arg1, Arg2, Arg3, Local0))
>> gromero@xps13:/tmp$ grep -n EDSM dsdt.dsl
>> 1959: Method (EDSM, 5, Serialized)
>> gromero@xps13:/tmp$
>>
>> I also confirmed that PDSM is what is being used by reverting this
>> patch and
>> dropping the aml_append(pci0_scope, aml_pci_edsm()); in 15/22.
>>
>> But also, why shouldn't we use the PDSM defined already in pcihp.c?
>> This is
>> indeed crafted for the acpi-pcihp-hotplug device as I understand it.
>
> Sorry, I see now, the EDSM is actually only for static endpoint PCI
> devices,
> i.e. non-hotpluggable, I see. So PDSM and EDSM serve different
> purposes, although
> similar.
>
> So, my point in the end is (and the one that confused me too) that
> EDSM can be
> present even if it will never be used by any static device, working
> pretty much
> like an ACPI deadcode. Could we avoid this? For instance, just emit
> EDSM if
> a _DSM that relies on it is emitted, like:
I see your point and it's true that for some PCIe configs the code can
be there while not used. However it looks it is done the same way on x86
and I would tend to be not more popish than the pope ;-)
Maybe this can be a patch sent afterwards that would also address the
x86 case then.
>
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index e87846a1fa..4d6dd3dd96 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -905,8 +905,9 @@ void build_append_pcihp_slots(Aml *parent_scope,
> PCIBus *bus)
> aml_append(parent_scope, notify_method);
> }
>
> -void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
> +bool build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
> {
> + bool has_dsm = false;
> int devfn;
> Aml *dev;
>
> @@ -929,11 +930,14 @@ void build_append_pci_bus_devices(Aml
> *parent_scope, PCIBus *bus)
> !object_property_get_bool(OBJECT(pdev), "hotpluggable",
> &error_abort)) {
> aml_append(dev, aml_pci_static_endpoint_dsm(pdev));
> + has_dsm = true;
> }
>
> /* device descriptor has been composed, add it into parent
> context */
> aml_append(parent_scope, dev);
> }
> +
> + return has_dsm;
> }
>
> Aml *aml_pci_edsm(void)
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 05a754d368..e4788c03ed 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -844,15 +844,22 @@ build_dsdt(GArray *table_data, BIOSLinker
> *linker, VirtMachineState *vms)
> if (vms->acpi_pcihp) {
> Aml *pci0_scope = aml_scope("\\_SB.PCI0");
>
> - aml_append(pci0_scope, aml_pci_edsm());
> build_acpi_pci_hotplug(dsdt, AML_SYSTEM_MEMORY,
> memmap[VIRT_ACPI_PCIHP].base);
> build_append_pcihp_resources(pci0_scope,
> memmap[VIRT_ACPI_PCIHP].base,
> memmap[VIRT_ACPI_PCIHP].size);
>
> - /* Scan all PCI buses. Generate tables to support hotplug. */
> - build_append_pci_bus_devices(pci0_scope, vms->bus);
> + /*
> + * Scan all PCI buses and devices. Generate tables to support
> static
> + * endpoint devices and hotplug.
> + */
> + /* Generate tables for static endpoints. */
> + if (build_append_pci_bus_devices(pci0_scope, vms->bus)) {
> + /* Has a _DSM method that requires a EDSM method. */
> + aml_append(pci0_scope, aml_pci_edsm());
> + }
> + /* Generate tables for PCI buses/slots/ */
> if (object_property_find(OBJECT(vms->bus),
> ACPI_PCIHP_PROP_BSEL)) {
> build_append_pcihp_slots(pci0_scope, vms->bus);
> }
> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
> index bda5ea24b5..25cc39f4ab 100644
> --- a/include/hw/acpi/pcihp.h
> +++ b/include/hw/acpi/pcihp.h
> @@ -82,7 +82,7 @@ void build_append_pcihp_resources(Aml *table,
> uint64_t io_addr, uint64_t io_len);
> bool build_append_notification_callback(Aml *parent_scope, const
> PCIBus *bus);
>
> -void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
> +bool build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
>
> Aml *aml_pci_edsm(void);
>
>
> Another question that I have is: could you please share a QEMU command
> line option
> that generates a static endpoint PCI device that matches the criteria
> for the emission
> of the _DSM method accordingly to build_append_pci_bus_devices()?
You can instantiate a virtio-net-pci device on pci.0 and with acpi-index
set.
-device
virtio-net-pci,bus=pcie.0,netdev=nic0,mac=6a:f5:10:b1:3d:d2,acpi-index=12
In that case EDSM is being called:
Device (S08)
{
Name (_ADR, 0x00010000) // _ADR: Address
Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
{
Local0 = Package (0x01)
{
0x0C
}
Return (EDSM (Arg0, Arg1, Arg2, Arg3, Local0))
}
}
Putting it on a root port will let the EDSM unused.
Cheers
Eric
>
>
> Cheers,
> Gustavo
>
Hi Eric,
On 5/27/25 04:18, Eric Auger wrote:
>
>
> On 5/21/25 10:19 PM, Gustavo Romero wrote:
>> Hi Eric,
>>
>> On 5/21/25 13:24, Gustavo Romero wrote:
>>> Hi Eric,
>>>
>>> On 5/21/25 12:56, Eric Auger wrote:
>>>> Hi Gustavo,
>>>>
>>>> On 5/21/25 5:26 PM, Gustavo Romero wrote:
>>>>> Hi Eric,
>>>>>
>>>>> On 5/14/25 14:01, Eric Auger wrote:
>>>>>> Move aml_pci_edsm to pcihp since we want to reuse that for
>>>>>> ARM and acpi-index support.
>>>>>>
>>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>> ---
>>>>>> include/hw/acpi/pcihp.h | 2 ++
>>>>>> hw/acpi/pcihp.c | 53
>>>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>>> hw/i386/acpi-build.c | 53
>>>>>> -----------------------------------------
>>>>>> 3 files changed, 55 insertions(+), 53 deletions(-)
>>>>>>
>>>>>> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
>>>>>> index 4d820b4baf..bc31dbff39 100644
>>>>>> --- a/include/hw/acpi/pcihp.h
>>>>>> +++ b/include/hw/acpi/pcihp.h
>>>>>> @@ -82,6 +82,8 @@ bool build_append_notification_callback(Aml
>>>>>> *parent_scope, const PCIBus *bus);
>>>>>> void build_append_pci_bus_devices(Aml *parent_scope, PCIBus
>>>>>> *bus);
>>>>>> +Aml *aml_pci_edsm(void);
>>>>>> +
>>>>>> /* Called on reset */
>>>>>> void acpi_pcihp_reset(AcpiPciHpState *s);
>>>>>> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
>>>>>> index d800371ddc..57fe8938b1 100644
>>>>>> --- a/hw/acpi/pcihp.c
>>>>>> +++ b/hw/acpi/pcihp.c
>>>>>> @@ -937,6 +937,59 @@ void build_append_pci_bus_devices(Aml
>>>>>> *parent_scope, PCIBus *bus)
>>>>>> }
>>>>>> }
>>>>>> +Aml *aml_pci_edsm(void)
>>>>>> +{
>>>>>> + Aml *method, *ifctx;
>>>>>> + Aml *zero = aml_int(0);
>>>>>> + Aml *func = aml_arg(2);
>>>>>> + Aml *ret = aml_local(0);
>>>>>> + Aml *aidx = aml_local(1);
>>>>>> + Aml *params = aml_arg(4);
>>>>>> +
>>>>>> + method = aml_method("EDSM", 5, AML_SERIALIZED);
>>>>>> +
>>>>>> + /* get supported functions */
>>>>>> + ifctx = aml_if(aml_equal(func, zero));
>>>>>> + {
>>>>>> + /* 1: have supported functions */
>>>>>> + /* 7: support for function 7 */
>>>>>> + const uint8_t caps = 1 | BIT(7);
>>>>>> + build_append_pci_dsm_func0_common(ifctx, ret);
>>>>>> + aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret,
>>>>>> zero)));
>>>>>> + aml_append(ifctx, aml_return(ret));
>>>>>> + }
>>>>>> + aml_append(method, ifctx);
>>>>>> +
>>>>>> + /* handle specific functions requests */
>>>>>> + /*
>>>>>> + * PCI Firmware Specification 3.1
>>>>>> + * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
>>>>>> + * Operating Systems
>>>>>> + */
>>>>>> + ifctx = aml_if(aml_equal(func, aml_int(7)));
>>>>>> + {
>>>>>> + Aml *pkg = aml_package(2);
>>>>>> + aml_append(pkg, zero);
>>>>>> + /* optional, if not impl. should return null string */
>>>>>> + aml_append(pkg, aml_string("%s", ""));
>>>>>> + aml_append(ifctx, aml_store(pkg, ret));
>>>>>> +
>>>>>> + /*
>>>>>> + * IASL is fine when initializing Package with computational
>>>>>> data,
>>>>>> + * however it makes guest unhappy /it fails to process such
>>>>>> AML/.
>>>>>> + * So use runtime assignment to set acpi-index after
>>>>>> initializer
>>>>>> + * to make OSPM happy.
>>>>>> + */
>>>>>> + aml_append(ifctx,
>>>>>> + aml_store(aml_derefof(aml_index(params, aml_int(0))),
>>>>>> aidx));
>>>>>> + aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
>>>>>> + aml_append(ifctx, aml_return(ret));
>>>>>> + }
>>>>>> + aml_append(method, ifctx);
>>>>>> +
>>>>>> + return method;
>>>>>> +}
>>>>>> +
>>>>>> const VMStateDescription vmstate_acpi_pcihp_pci_status = {
>>>>>> .name = "acpi_pcihp_pci_status",
>>>>>> .version_id = 1,
>>>>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>>>>> index bcfba2ccb3..385e75d061 100644
>>>>>> --- a/hw/i386/acpi-build.c
>>>>>> +++ b/hw/i386/acpi-build.c
>>>>>> @@ -322,59 +322,6 @@ build_facs(GArray *table_data)
>>>>>> g_array_append_vals(table_data, reserved, 40); /* Reserved */
>>>>>> }
>>>>>> -static Aml *aml_pci_edsm(void)
>>>>>> -{
>>>>>> - Aml *method, *ifctx;
>>>>>> - Aml *zero = aml_int(0);
>>>>>> - Aml *func = aml_arg(2);
>>>>>> - Aml *ret = aml_local(0);
>>>>>> - Aml *aidx = aml_local(1);
>>>>>> - Aml *params = aml_arg(4);
>>>>>> -
>>>>>> - method = aml_method("EDSM", 5, AML_SERIALIZED);
>>>>>> -
>>>>>> - /* get supported functions */
>>>>>> - ifctx = aml_if(aml_equal(func, zero));
>>>>>> - {
>>>>>> - /* 1: have supported functions */
>>>>>> - /* 7: support for function 7 */
>>>>>> - const uint8_t caps = 1 | BIT(7);
>>>>>> - build_append_pci_dsm_func0_common(ifctx, ret);
>>>>>> - aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret,
>>>>>> zero)));
>>>>>> - aml_append(ifctx, aml_return(ret));
>>>>>> - }
>>>>>> - aml_append(method, ifctx);
>>>>>> -
>>>>>> - /* handle specific functions requests */
>>>>>> - /*
>>>>>> - * PCI Firmware Specification 3.1
>>>>>> - * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
>>>>>> - * Operating Systems
>>>>>> - */
>>>>>> - ifctx = aml_if(aml_equal(func, aml_int(7)));
>>>>>> - {
>>>>>> - Aml *pkg = aml_package(2);
>>>>>> - aml_append(pkg, zero);
>>>>>> - /* optional, if not impl. should return null string */
>>>>>> - aml_append(pkg, aml_string("%s", ""));
>>>>>> - aml_append(ifctx, aml_store(pkg, ret));
>>>>>> -
>>>>>> - /*
>>>>>> - * IASL is fine when initializing Package with computational
>>>>>> data,
>>>>>> - * however it makes guest unhappy /it fails to process such
>>>>>> AML/.
>>>>>> - * So use runtime assignment to set acpi-index after
>>>>>> initializer
>>>>>> - * to make OSPM happy.
>>>>>> - */
>>>>>> - aml_append(ifctx,
>>>>>> - aml_store(aml_derefof(aml_index(params, aml_int(0))),
>>>>>> aidx));
>>>>>> - aml_append(ifctx, aml_store(aidx, aml_index(ret, zero)));
>>>>>> - aml_append(ifctx, aml_return(ret));
>>>>>> - }
>>>>>> - aml_append(method, ifctx);
>>>>>> -
>>>>>> - return method;
>>>>>> -}
>>>>>> -
>>>>>> /*
>>>>>> * build_prt - Define interrupt routing rules
>>>>>> *
>>>>>
>>>>> EDSM is not called from anywhere. _DSM method actually calls the PDSM,
>>>>> already defined
>>>>> in pcihp.c and generated by aml_pci_pdsm(). build_acpi_pci_hotplug(),
>>>>> which calls
>>>>> aml_pci_pdsm(), properly creates the PDSM method. Then in
>>>>> build_append_pcihp_slots()
>>>>> the a _DSM is defined per slot and inside it there is a call to PDSM,
>>>>> so I understand
>>>>> we should actually stick to the PDSM in pcihp.c instead of EDSM.
>>>>
>>>> I see build_append_pci_bus_devices -> aml_pci_static_endpoint_dsm
>>>> adds a _DSM method which eventually calls EDSM.
>>>
>>> Yes, but I meant in the final generated ACPI AML code.
>>>
>>>
>>>> aml_pci_device_dsm builds another _DSM implementation which calls PDSM.
>>>>
>>>> As we use build_append_pci_bus_devices and we are likely to have a _DSM
>>>> implementation that calls EDSM method, I think we shall have it in the
>>>> aml. What do I miss?
>>>
>>> Maybe some condition in build_append_pci_bus_devices() of:
>>>
>>> if (pdev->acpi_index &&
>>> !object_property_get_bool(OBJECT(pdev), "hotpluggable",
>>> &error_abort)) { ... }
>>>
>>>
>>> is not met?
>>>
>>> Does your _DSM method in the machine's ACPI has a call to EDSM? I don't
>>> see it. In my case it's the PDSM that is being called:
>>>
>>> gromero@xps13:/tmp$ grep -n PDSM dsdt.dsl
>>> 1910: Method (PDSM, 5, Serialized)
>>> 2047: Return (PDSM (Arg0, Arg1, Arg2, Arg3, Local0))
>>> gromero@xps13:/tmp$ grep -n EDSM dsdt.dsl
>>> 1959: Method (EDSM, 5, Serialized)
>>> gromero@xps13:/tmp$
>>>
>>> I also confirmed that PDSM is what is being used by reverting this
>>> patch and
>>> dropping the aml_append(pci0_scope, aml_pci_edsm()); in 15/22.
>>>
>>> But also, why shouldn't we use the PDSM defined already in pcihp.c?
>>> This is
>>> indeed crafted for the acpi-pcihp-hotplug device as I understand it.
>>
>> Sorry, I see now, the EDSM is actually only for static endpoint PCI
>> devices,
>> i.e. non-hotpluggable, I see. So PDSM and EDSM serve different
>> purposes, although
>> similar.
>>
>> So, my point in the end is (and the one that confused me too) that
>> EDSM can be
>> present even if it will never be used by any static device, working
>> pretty much
>> like an ACPI deadcode. Could we avoid this? For instance, just emit
>> EDSM if
>> a _DSM that relies on it is emitted, like:
>
> I see your point and it's true that for some PCIe configs the code can
> be there while not used. However it looks it is done the same way on x86
> and I would tend to be not more popish than the pope ;-)
>
> Maybe this can be a patch sent afterwards that would also address the
> x86 case then.
Got it. Well, I think it's just a matter of making build_append_pci_bus_devices()
tell if any static endpoint is added and just call aml_pci_edsm() to emit EDSM if
there is any, like in my diff:
+ /* Generate tables for static endpoints. */
+ if (build_append_pci_bus_devices(pci0_scope, vms->bus)) {
+ /* Has a _DSM method that requires a EDSM method. */
+ aml_append(pci0_scope, aml_pci_edsm());
+ }
But I'm not really the maintainer or the official reviewer for this part of
the code, and you certainly have more karma than I do in this area, so it's
up to you, of course. I just think ACPI code is tricky enough to review,
and having dead code makes it a tad more daunting :)
>>
>> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
>> index e87846a1fa..4d6dd3dd96 100644
>> --- a/hw/acpi/pcihp.c
>> +++ b/hw/acpi/pcihp.c
>> @@ -905,8 +905,9 @@ void build_append_pcihp_slots(Aml *parent_scope,
>> PCIBus *bus)
>> aml_append(parent_scope, notify_method);
>> }
>>
>> -void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
>> +bool build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
>> {
>> + bool has_dsm = false;
>> int devfn;
>> Aml *dev;
>>
>> @@ -929,11 +930,14 @@ void build_append_pci_bus_devices(Aml
>> *parent_scope, PCIBus *bus)
>> !object_property_get_bool(OBJECT(pdev), "hotpluggable",
>> &error_abort)) {
>> aml_append(dev, aml_pci_static_endpoint_dsm(pdev));
>> + has_dsm = true;
>> }
>>
>> /* device descriptor has been composed, add it into parent
>> context */
>> aml_append(parent_scope, dev);
>> }
>> +
>> + return has_dsm;
>> }
>>
>> Aml *aml_pci_edsm(void)
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 05a754d368..e4788c03ed 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -844,15 +844,22 @@ build_dsdt(GArray *table_data, BIOSLinker
>> *linker, VirtMachineState *vms)
>> if (vms->acpi_pcihp) {
>> Aml *pci0_scope = aml_scope("\\_SB.PCI0");
>>
>> - aml_append(pci0_scope, aml_pci_edsm());
>> build_acpi_pci_hotplug(dsdt, AML_SYSTEM_MEMORY,
>> memmap[VIRT_ACPI_PCIHP].base);
>> build_append_pcihp_resources(pci0_scope,
>> memmap[VIRT_ACPI_PCIHP].base,
>> memmap[VIRT_ACPI_PCIHP].size);
>>
>> - /* Scan all PCI buses. Generate tables to support hotplug. */
>> - build_append_pci_bus_devices(pci0_scope, vms->bus);
>> + /*
>> + * Scan all PCI buses and devices. Generate tables to support
>> static
>> + * endpoint devices and hotplug.
>> + */
>> + /* Generate tables for static endpoints. */
>> + if (build_append_pci_bus_devices(pci0_scope, vms->bus)) {
>> + /* Has a _DSM method that requires a EDSM method. */
>> + aml_append(pci0_scope, aml_pci_edsm());
>> + }
>> + /* Generate tables for PCI buses/slots/ */
>> if (object_property_find(OBJECT(vms->bus),
>> ACPI_PCIHP_PROP_BSEL)) {
>> build_append_pcihp_slots(pci0_scope, vms->bus);
>> }
>> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
>> index bda5ea24b5..25cc39f4ab 100644
>> --- a/include/hw/acpi/pcihp.h
>> +++ b/include/hw/acpi/pcihp.h
>> @@ -82,7 +82,7 @@ void build_append_pcihp_resources(Aml *table,
>> uint64_t io_addr, uint64_t io_len);
>> bool build_append_notification_callback(Aml *parent_scope, const
>> PCIBus *bus);
>>
>> -void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
>> +bool build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
>>
>> Aml *aml_pci_edsm(void);
>>
>>
>> Another question that I have is: could you please share a QEMU command
>> line option
>> that generates a static endpoint PCI device that matches the criteria
>> for the emission
>> of the _DSM method accordingly to build_append_pci_bus_devices()?
> You can instantiate a virtio-net-pci device on pci.0 and with acpi-index
> set.
>
> -device
> virtio-net-pci,bus=pcie.0,netdev=nic0,mac=6a:f5:10:b1:3d:d2,acpi-index=12
>
> In that case EDSM is being called:
>
> Device (S08)
> {
> Name (_ADR, 0x00010000) // _ADR: Address
> Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
> {
> Local0 = Package (0x01)
> {
> 0x0C
> }
> Return (EDSM (Arg0, Arg1, Arg2, Arg3, Local0))
> }
> }
>
> Putting it on a root port will let the EDSM unused.
Thanks for the example. Would it be correct to say so that EDSM is part of
the coldplug and so could be left for another series, so out of scope for
hotpluging?
Cheers,
Gustavo
© 2016 - 2025 Red Hat, Inc.