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@xxxxxxx> > >>>> Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj- > Mahmoud@xxxxxxx> > >>>> --- > >>>> 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.