| Message ID | 20230518105202.451739-1-jose.marinho@arm.com |
|---|---|
| Headers | show |
| Series | Update ACPI documentation for Arm systems | expand |
On 2023-05-19 08:10, Hanjun Guo wrote: > On 2023/5/18 21:40, Robin Murphy wrote: >> On 2023-05-18 13:07, Hanjun Guo wrote: >>> Hi Jose, >>> >>> On 2023/5/18 18:52, Jose Marinho wrote: >>>> The BBR specification requires (or conditionally requires) a set of >>>> ACPI >>>> tables for a proper working system. >>>> This commit updates: >>>> - the list of ACPI tables to reflect the contents of >>>> BBR version 2.0 (see >>>> https://developer.arm.com/documentation/den0044/g). >>>> - the list of ACPI tables in acpi_object_usage. This last update >>>> ensures >>>> that both files remain coherent. >>> >>> Thanks for the update, some comments inline. >>> >>>> >>>> Signed-off-by: Jose Marinho <jose.marinho@arm.com> >>>> Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com> >>>> --- >>>> Documentation/arm64/acpi_object_usage.rst | 81 >>>> +++++++++++++++++++++-- >>>> Documentation/arm64/arm-acpi.rst | 71 +++++++++++++++++--- >>>> 2 files changed, 139 insertions(+), 13 deletions(-) >>>> >>>> diff --git a/Documentation/arm64/acpi_object_usage.rst >>>> b/Documentation/arm64/acpi_object_usage.rst >>>> index 484ef9676653..1da22200fdf8 100644 >>>> --- a/Documentation/arm64/acpi_object_usage.rst >>>> +++ b/Documentation/arm64/acpi_object_usage.rst >>>> @@ -17,16 +17,37 @@ For ACPI on arm64, tables also fall into the >>>> following categories: >>>> - Recommended: BERT, EINJ, ERST, HEST, PCCT, SSDT >>>> - - Optional: BGRT, CPEP, CSRT, DBG2, DRTM, ECDT, FACS, FPDT, >>>> IBFT, >>>> - IORT, MCHI, MPST, MSCT, NFIT, PMTT, RASF, SBST, SLIT, >>>> SPMI, SRAT, >>>> - STAO, TCPA, TPM2, UEFI, XENV >>>> + - Optional: AGDI, BGRT, CEDT, CPEP, CSRT, DBG2, DRTM, ECDT, >>>> FACS, FPDT, >>>> + HMAT, IBFT, IORT, MCHI, MPAM, MPST, MSCT, NFIT, PMTT, >>>> PPTT, RASF, SBST, >>>> + SDEI, SLIT, SPMI, SRAT, STAO, TCPA, TPM2, UEFI, XENV >>>> - - Not supported: BOOT, DBGP, DMAR, ETDT, HPET, IVRS, LPIT, >>>> MSDM, OEMx, >>>> - PSDT, RSDT, SLIC, WAET, WDAT, WDRT, WPBT >>>> + - Not supported: AEST, APMT, BOOT, DBGP, DMAR, ETDT, HPET, >>>> IVRS, LPIT, >>> >>> AEST is ARM Error Source Table, and it can be used for ARM platforms, so >>> I thinsk AEST is not belong to "Not supportted", "Optional" instead. >> >> Can you point to the code in Linux which does anything with AEST, >> optionally or otherwise? ;) >>> and APMT is the same. >>> >>>> + MSDM, OEMx, PDTT, PSDT, RAS2, RSDT, SLIC, WAET, WDAT, >>>> WDRT, WPBT >>> >>> PDTT and RAS2 are now used for ARM too, please move it to Optional :) >> >> Ditto; as stated in arm-acpi.rst this is Linux documentation covering >> the interaction between Linux and ACPI. It is not some kind of generic > > Hmm, let me see... > > OK, I checked the arm-acpi.rst, it is saying: > > "Detailed expectations for ACPI tables and object are listed in the file > Documentation/arm64/acpi_object_usage.rst." > > So if I remember correctly, it is the guidance of ACPI tables and > methods usage on arm64, to align with the BBR. "The purpose of this document is to describe the interaction between ACPI and Linux only, on an ARMv8 system -- that is, what Linux expects of ACPI and what ACPI can expect of Linux." I don't see how it could get much clearer than that. Yes, phrasing like "ACPI on arm64" is used elsewhere, but remember that in context "arm64" means "AArch64 Linux". >> ACPI-on-Arm guidance whitepaper. If and when Linux actually supports >> these tables in the sense of meaningfully consuming them, that is when >> we can document such support. > > If this is the case, we don't need categories of "Required", > "Recommmened" and etc. Certainly the distinction between required and optional is significant and useful, since Linux may fail to boot at all if a required table is missing. I'd agree I can't really make sense of the "recommended" category though - it's not like firmware could make up RAS support if the hardware doesn't have it, and whether SSDTs are appropriate or not seems to depend on the fundamental design of the system, rather than being something an OS should dictate :/ However that's something we can think about separately, since it's orthogonal to this content update. Thanks, Robin.
Hi Hanjun, Robin, > On 2023-05-19 08:10, Hanjun Guo wrote: > > On 2023/5/18 21:40, Robin Murphy wrote: > >> On 2023-05-18 13:07, Hanjun Guo wrote: > >>> Hi Jose, > >>> > >>> On 2023/5/18 18:52, Jose Marinho wrote: > >>>> The BBR specification requires (or conditionally requires) a set of > >>>> ACPI tables for a proper working system. > >>>> This commit updates: > >>>> - the list of ACPI tables to reflect the contents of BBR version > >>>> 2.0 (see https://developer.arm.com/documentation/den0044/g). > >>>> - the list of ACPI tables in acpi_object_usage. This last update > >>>> ensures that both files remain coherent. > >>> > >>> Thanks for the update, some comments inline. > >>> > >>>> > >>>> Signed-off-by: Jose Marinho <jose.marinho@arm.com> > >>>> Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj- > Mahmoud@arm.com> > >>>> --- > >>>> Documentation/arm64/acpi_object_usage.rst | 81 > >>>> +++++++++++++++++++++-- > >>>> Documentation/arm64/arm-acpi.rst | 71 > >>>> +++++++++++++++++--- > >>>> 2 files changed, 139 insertions(+), 13 deletions(-) > >>>> > >>>> diff --git a/Documentation/arm64/acpi_object_usage.rst > >>>> b/Documentation/arm64/acpi_object_usage.rst > >>>> index 484ef9676653..1da22200fdf8 100644 > >>>> --- a/Documentation/arm64/acpi_object_usage.rst > >>>> +++ b/Documentation/arm64/acpi_object_usage.rst > >>>> @@ -17,16 +17,37 @@ For ACPI on arm64, tables also fall into the > >>>> following categories: > >>>> - Recommended: BERT, EINJ, ERST, HEST, PCCT, SSDT > >>>> - - Optional: BGRT, CPEP, CSRT, DBG2, DRTM, ECDT, FACS, > >>>> FPDT, IBFT, > >>>> - IORT, MCHI, MPST, MSCT, NFIT, PMTT, RASF, SBST, SLIT, > >>>> SPMI, SRAT, > >>>> - STAO, TCPA, TPM2, UEFI, XENV > >>>> + - Optional: AGDI, BGRT, CEDT, CPEP, CSRT, DBG2, DRTM, > >>>> +ECDT, > >>>> FACS, FPDT, > >>>> + HMAT, IBFT, IORT, MCHI, MPAM, MPST, MSCT, NFIT, PMTT, > >>>> PPTT, RASF, SBST, > >>>> + SDEI, SLIT, SPMI, SRAT, STAO, TCPA, TPM2, UEFI, XENV > >>>> - - Not supported: BOOT, DBGP, DMAR, ETDT, HPET, IVRS, LPIT, > >>>> MSDM, OEMx, > >>>> - PSDT, RSDT, SLIC, WAET, WDAT, WDRT, WPBT > >>>> + - Not supported: AEST, APMT, BOOT, DBGP, DMAR, ETDT, HPET, > >>>> IVRS, LPIT, > >>> > >>> AEST is ARM Error Source Table, and it can be used for ARM > >>> platforms, so I thinsk AEST is not belong to "Not supportted", "Optional" > instead. > >> > >> Can you point to the code in Linux which does anything with AEST, > >> optionally or otherwise? ;) > >>> and APMT is the same. > >>> > >>>> + MSDM, OEMx, PDTT, PSDT, RAS2, RSDT, SLIC, WAET, WDAT, > >>>> WDRT, WPBT > >>> > >>> PDTT and RAS2 are now used for ARM too, please move it to Optional > >>> :) The 6.3 kernel does not yet have code consuming either table. Perhaps the categories {"Required", "Recommended", "Optional", "Not supported"} listed in Documentation/arm64/acpi_object_usage.rst should be defined. My opinion (which may be unaligned with the original intent behind the categories) is, If a table is consumed by kernel code, then it is supported, i.e. in {"Required", "Recommended", "Optional"}. Otherwise, the table is "Not supported". > >> > >> Ditto; as stated in arm-acpi.rst this is Linux documentation covering > >> the interaction between Linux and ACPI. It is not some kind of > >> generic > > > > Hmm, let me see... > > > > OK, I checked the arm-acpi.rst, it is saying: > > > > "Detailed expectations for ACPI tables and object are listed in the > > file Documentation/arm64/acpi_object_usage.rst." > > > > So if I remember correctly, it is the guidance of ACPI tables and > > methods usage on arm64, to align with the BBR. > > "The purpose of this document is to describe the interaction between ACPI > and Linux only, on an ARMv8 system -- that is, what Linux expects of ACPI > and what ACPI can expect of Linux." > > I don't see how it could get much clearer than that. Yes, phrasing like "ACPI > on arm64" is used elsewhere, but remember that in context "arm64" > means "AArch64 Linux". > > >> ACPI-on-Arm guidance whitepaper. If and when Linux actually supports > >> these tables in the sense of meaningfully consuming them, that is > >> when we can document such support. > > > > If this is the case, we don't need categories of "Required", > > "Recommmened" and etc. > > Certainly the distinction between required and optional is significant and > useful, since Linux may fail to boot at all if a required table is missing. I'd > agree I can't really make sense of the "recommended" > category though - it's not like firmware could make up RAS support if the > hardware doesn't have it, and whether SSDTs are appropriate or not seems > to depend on the fundamental design of the system, rather than being > something an OS should dictate :/ > I agree with Robin that it isn’t clear what "Recommended" versus "Optional" signifies. Maybe the distinction should be better discussed, and we can make those clarifications in a separate change? > However that's something we can think about separately, since it's > orthogonal to this content update. > > Thanks, > Robin.
On 2023/5/22 18:55, Jose Marinho wrote: > Hi Hanjun, Robin, > >> On 2023-05-19 08:10, Hanjun Guo wrote: >>> On 2023/5/18 21:40, Robin Murphy wrote: >>>> On 2023-05-18 13:07, Hanjun Guo wrote: >>>>> Hi Jose, >>>>> >>>>> On 2023/5/18 18:52, Jose Marinho wrote: >>>>>> The BBR specification requires (or conditionally requires) a set of >>>>>> ACPI tables for a proper working system. >>>>>> This commit updates: >>>>>> - the list of ACPI tables to reflect the contents of BBR version >>>>>> 2.0 (seehttps://developer.arm.com/documentation/den0044/g). >>>>>> - the list of ACPI tables in acpi_object_usage. This last update >>>>>> ensures that both files remain coherent. >>>>> Thanks for the update, some comments inline. >>>>> >>>>>> Signed-off-by: Jose Marinho<jose.marinho@arm.com> >>>>>> Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj- >> Mahmoud@arm.com> >>>>>> --- >>>>>> Documentation/arm64/acpi_object_usage.rst | 81 >>>>>> +++++++++++++++++++++-- >>>>>> Documentation/arm64/arm-acpi.rst | 71 >>>>>> +++++++++++++++++--- >>>>>> 2 files changed, 139 insertions(+), 13 deletions(-) >>>>>> >>>>>> diff --git a/Documentation/arm64/acpi_object_usage.rst >>>>>> b/Documentation/arm64/acpi_object_usage.rst >>>>>> index 484ef9676653..1da22200fdf8 100644 >>>>>> --- a/Documentation/arm64/acpi_object_usage.rst >>>>>> +++ b/Documentation/arm64/acpi_object_usage.rst >>>>>> @@ -17,16 +17,37 @@ For ACPI on arm64, tables also fall into the >>>>>> following categories: >>>>>> - Recommended: BERT, EINJ, ERST, HEST, PCCT, SSDT >>>>>> - - Optional: BGRT, CPEP, CSRT, DBG2, DRTM, ECDT, FACS, >>>>>> FPDT, IBFT, >>>>>> - IORT, MCHI, MPST, MSCT, NFIT, PMTT, RASF, SBST, SLIT, >>>>>> SPMI, SRAT, >>>>>> - STAO, TCPA, TPM2, UEFI, XENV >>>>>> + - Optional: AGDI, BGRT, CEDT, CPEP, CSRT, DBG2, DRTM, >>>>>> +ECDT, >>>>>> FACS, FPDT, >>>>>> + HMAT, IBFT, IORT, MCHI, MPAM, MPST, MSCT, NFIT, PMTT, >>>>>> PPTT, RASF, SBST, >>>>>> + SDEI, SLIT, SPMI, SRAT, STAO, TCPA, TPM2, UEFI, XENV >>>>>> - - Not supported: BOOT, DBGP, DMAR, ETDT, HPET, IVRS, LPIT, >>>>>> MSDM, OEMx, >>>>>> - PSDT, RSDT, SLIC, WAET, WDAT, WDRT, WPBT >>>>>> + - Not supported: AEST, APMT, BOOT, DBGP, DMAR, ETDT, HPET, >>>>>> IVRS, LPIT, >>>>> AEST is ARM Error Source Table, and it can be used for ARM >>>>> platforms, so I thinsk AEST is not belong to "Not supportted", "Optional" >> instead. >>>> Can you point to the code in Linux which does anything with AEST, >>>> optionally or otherwise?;) >>>>> and APMT is the same. >>>>> >>>>>> + MSDM, OEMx, PDTT, PSDT, RAS2, RSDT, SLIC, WAET, WDAT, >>>>>> WDRT, WPBT >>>>> PDTT and RAS2 are now used for ARM too, please move it to Optional >>>>> :) > The 6.3 kernel does not yet have code consuming either table. > > Perhaps the categories {"Required", "Recommended", "Optional", "Not supported"} > listed in Documentation/arm64/acpi_object_usage.rst should be defined. > > My opinion (which may be unaligned with the original intent behind the categories) is, If a table > is consumed by kernel code, then it is supported, i.e. in {"Required", "Recommended", "Optional"}. > Otherwise, the table is "Not supported". > >>>> Ditto; as stated in arm-acpi.rst this is Linux documentation covering >>>> the interaction between Linux and ACPI. It is not some kind of >>>> generic >>> Hmm, let me see... >>> >>> OK, I checked the arm-acpi.rst, it is saying: >>> >>> "Detailed expectations for ACPI tables and object are listed in the >>> file Documentation/arm64/acpi_object_usage.rst." >>> >>> So if I remember correctly, it is the guidance of ACPI tables and >>> methods usage on arm64, to align with the BBR. >> "The purpose of this document is to describe the interaction between ACPI >> and Linux only, on an ARMv8 system -- that is, what Linux expects of ACPI >> and what ACPI can expect of Linux." >> >> I don't see how it could get much clearer than that. Yes, phrasing like "ACPI >> on arm64" is used elsewhere, but remember that in context "arm64" >> means "AArch64 Linux". >> >>>> ACPI-on-Arm guidance whitepaper. If and when Linux actually supports >>>> these tables in the sense of meaningfully consuming them, that is >>>> when we can document such support. >>> If this is the case, we don't need categories of "Required", >>> "Recommmened" and etc. >> Certainly the distinction between required and optional is significant and >> useful, since Linux may fail to boot at all if a required table is missing. I'd >> agree I can't really make sense of the "recommended" >> category though - it's not like firmware could make up RAS support if the >> hardware doesn't have it, and whether SSDTs are appropriate or not seems >> to depend on the fundamental design of the system, rather than being >> something an OS should dictate :/ >> > I agree with Robin that it isn’t clear what "Recommended" versus "Optional" signifies. > Maybe the distinction should be better discussed, and we can make those clarifications in a separate change? Looks good to me. Thanks Hanjun