Yes, thank you for the quick reply. That should be fine. Our implementation is currently using the Override method to take care of the ID generation so internally we are able to make progress without an official answer. Eventually we would like to use upstream fixes of course. The Override method works for us because it means the ProcHierarchyInfo structure is our single source of truth that both table generators refer to. I just didn't want to submit a patch that doesn't correctly support the auto-generated ID method since I assume that wouldn't be accepted.
ID generation and management is indeed an important but sometimes tricky problem. In my opinion the most important thing is to make sure that there is a single source of truth that all other code consults, vs. trying to make sure different pieces of code independently generate the same ID. From: Sami Mujawar <sami.muja...@arm.com> Sent: Friday, August 16, 2024 7:05 AM To: Jeshua Smith <jesh...@nvidia.com>; r...@edk2.groups.io; devel@edk2.groups.io Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>; Leif Lindholm <quic_llind...@quicinc.com>; Pierre Gondois <pierre.gond...@arm.com>; Yeo Reum Yun <yeoreum....@arm.com>; Varshit Pandya <varshit.pan...@arm.com>; nd <n...@arm.com> Subject: Re: DynamicTablesPkg: Keeping AcpiProcessorId in PPTT and _UID in SSDT in sync? External email: Use caution opening links or attachments Hi Jeshua, Thank you for your email and for bringing up this concern for discussion on the list. The ACPI ID is used at multiple places, and we need to investigate this in detail and come up with a solution. Pierre has worked on most of the SSDT CPU generator, but he is on leave until the end of next week. We had internally discussed few ideas some time back to solve a similar issue. I think we need a generic solution for ID generation and management. Is it ok if we get back with our findings once Pierre is back, please? Regards, Sami Mujawar From: Jeshua Smith <jesh...@nvidia.com<mailto:jesh...@nvidia.com>> Date: Wednesday 14 August 2024 at 17:46 To: "r...@edk2.groups.io<mailto:r...@edk2.groups.io>" <r...@edk2.groups.io<mailto:r...@edk2.groups.io>> Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org<mailto:ardb+tianoc...@kernel.org>>, Leif Lindholm <quic_llind...@quicinc.com<mailto:quic_llind...@quicinc.com>>, Pierre Gondois <pierre.gond...@arm.com<mailto:pierre.gond...@arm.com>>, Sami Mujawar <sami.muja...@arm.com<mailto:sami.muja...@arm.com>> Subject: DynamicTablesPkg: Keeping AcpiProcessorId in PPTT and _UID in SSDT in sync? Currently the SSDT and PPTT generators in DynamicTablesPkg use the ProcHierarchyInfo structure to generate their ACPI tables. The current code for PPTT will: 1. Set the AcpiProcessorId field in PPTT to 0 if the AcpiProcessorIdValid flag isn't set, OR 2. Throw an error if AcpiIdObjectToken is CM_NULL_TOKEN, OR 3. Set the AcpiProcessorId field in PPTT to the AcpiProcessorUid of the object pointed to by AcpiIdObjectToken In practice (on ARM), this means that all PPTT nodes that reference an actual processor get their ID from their GIC and all PPTT nodes that are processor containers have their ID forced to 0. If you try to set an ID for a processor container by setting the IdValid flag for the container it results in an error. Similarly, the SSDT node checker expects that all nodes that have the IdValid flag set also have their Leaf flag set and are leaf nodes; it will throw an error if any non-leaf node has the IdValid flag set. However, the PPTT spec says (5. ACPI Software Programming Model - ACPI Specification 6.5 documentation (uefi.org)<https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#processor-hierarchy-node-structure>): ACPI Processor ID 4 12 If the processor structure represents an actual processor, this field must match the value of ACPI processor ID field in the processor's entry in the MADT. If the processor structure represents a group of associated processors, the structure might match a processor container in the name space. In that case this entry will match the value of the _UID method of the associated processor container. Where there is a match it must be represented. The flags field, described in Processor Structure Flags, includes a bit to describe whether the ACPI processor ID is valid. and the flag ACPI Processor ID valid 1 1 For non-leaf entries in the processor topology, the ACPI Processor ID entry can relate to a Processor container in the namespace. The processor container will have a matching ID value returned through the _UID method. As not every processor hierarchy node structure in PPTT may have a matching processor container, this flag indicates whether the ACPI processor ID points to valid entry. Where a valid entry is possible the ACPI Processor ID and _UID method are mandatory. For leaf entries in PPTT that represent processors listed in MADT, the ACPI Processor ID must always be provided and this flag must be set to 1. I read this to mean that the processor containers in PPTT are REQUIRED to have a valid ID that matches the _UID of the corresponding container in SSDT. To properly implement APMT support for a chip I'm working on I need to ensure that the PPTT has a valid (non-zero) ID for some processor containers in order to match the requirements of the APMT table: ACPI for CoreSight Performance Monitoring Unit Architecture (arm.com)<https://developer.arm.com/documentation/den0117/latest> Processor affinity 4 48 Processor affinity for this PMU. This field must match the ACPI Processor ID of the PPTT Type 0 structure that represents the processor or processor container that this PMU is associated with. As a result, I'm working on a patch to enable the SSDT and PPTT generators to generate a valid ID for processor container nodes, but I need some guidance. It isn't too hard to update the existing code to: 1. Allow non-leaf nodes to have the ID valid flag set 2. Generate the ID if the ID valid flag is set AND the ProcHierarchyInfo structure specifies it explicitly via the OverrideNameUidEnabled flag and its corresponding OverrideUid field. However, I'm not sure how to handle the case where ProcHierarchyInfo says a valid ID is needed but one is not provided via Override. In this case SSDT has some logic to auto-assign _UIDs, but the spec requires that PPTT set the AcpiProcessorId to that same value as those auto-assigned SSDT _UIDs. How should we guarantee that these auto-assigned values are in sync? Any guidance on how to handle this? Thanks, Jeshua Smith NVIDIA -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120360): https://edk2.groups.io/g/devel/message/120360 Mute This Topic: https://groups.io/mt/107931323/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
