Hi Ard, Please see my response inline.
Regards, Sami Mujawar -----Original Message----- From: Ard Biesheuvel <[email protected]> Sent: 22 January 2019 09:22 AM To: Sami Mujawar <[email protected]> Cc: [email protected]; Arvind Chauhan <[email protected]>; Daniil Egranov <[email protected]>; Thomas Abraham <[email protected]>; Leif Lindholm <[email protected]>; Kinney, Michael D <[email protected]>; Alexei Fedorov <[email protected]>; Matteo Carlini <[email protected]>; Stephanie Hughes-Fitt <[email protected]>; nd <[email protected]> Subject: Re: [PATCH edk2-platforms v1 5/6] Platform/ARM: Add OEM CPU generator for FVP On Fri, 21 Dec 2018 at 19:14, Sami Mujawar <[email protected]> wrote: > > Hi Ard, > > Please see my response inline. > > Regards, > > Sami Mujawar > > -----Original Message----- > From: Ard Biesheuvel <[email protected]> > Sent: 21 December 2018 05:08 PM > To: Sami Mujawar <[email protected]> > Cc: [email protected]; Arvind Chauhan <[email protected]>; > Daniil Egranov <[email protected]>; Thomas Abraham > <[email protected]>; Leif Lindholm <[email protected]>; > Kinney, Michael D <[email protected]>; Alexei Fedorov > <[email protected]>; Matteo Carlini <[email protected]>; > Stephanie Hughes-Fitt <[email protected]>; nd <[email protected]> > Subject: Re: [PATCH edk2-platforms v1 5/6] Platform/ARM: Add OEM CPU > generator for FVP > > Hi Sami, > > On Fri, 21 Dec 2018 at 18:01, Sami Mujawar <[email protected]> wrote: > > > > Add support for dynamic generation of ACPI CPU device information. > > This generator uses the compiled data from a template asl file and > > patches it at runtime to generate the CPU information based on the > > number of CPUs and their ACPI UID. This patched data is then > > installed as a SSDT table. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Sami Mujawar <[email protected]> > > --- > > > > Platform/ARM/VExpressPkg/ConfigurationManager/AcpiOemCpuGeneratorLib/AcpiOemCpuASLLib.inf > > | 27 ++ > > > > Platform/ARM/VExpressPkg/ConfigurationManager/AcpiOemCpuGeneratorLib/AcpiOemCpuGeneratorLib.inf > > | 42 ++ > > > > Platform/ARM/VExpressPkg/ConfigurationManager/AcpiOemCpuGeneratorLib/OemCpuGenerator.c > > | 403 ++++++++++++++++++++ > > > > Platform/ARM/VExpressPkg/ConfigurationManager/AcpiOemCpuGeneratorLib/OemCpuGenerator.h > > | 23 ++ > > > > Platform/ARM/VExpressPkg/ConfigurationManager/AcpiOemCpuGeneratorLib/SsdtCpuTemplate.asl > > | 25 ++ > > 5 files changed, 520 insertions(+) > > > ... > > diff --git > > a/Platform/ARM/VExpressPkg/ConfigurationManager/AcpiOemCpuGeneratorL > > ib > > /OemCpuGenerator.c > > b/Platform/ARM/VExpressPkg/ConfigurationManager/AcpiOemCpuGeneratorL > > ib > > /OemCpuGenerator.c > > new file mode 100644 > > index > > 0000000000000000000000000000000000000000..d544244bfbd566c128b57d80b0 > > c8 > > c2bdc0cca374 > > --- /dev/null > > +++ b/Platform/ARM/VExpressPkg/ConfigurationManager/AcpiOemCpuGenera > > +++ to > > +++ rLib/OemCpuGenerator.c > > @@ -0,0 +1,403 @@ > > +/** @file > > + OEM CPU Table Generator > > + > > + Copyright (c) 2018, ARM Limited. All rights reserved. > > + This program and the accompanying materials are licensed and > > + made available under the terms and conditions of the BSD License > > + which accompanies this distribution. The full text of the license > > + may be found at http://opensource.org/licenses/bsd-license.php > > + > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" > > +BASIS, > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > > IMPLIED. > > +**/ > > + > > +#include <Library/AcpiLib.h> > > +#include <Library/BaseLib.h> > > +#include <Library/BaseMemoryLib.h> > > +#include <Library/DebugLib.h> > > +#include <Library/MemoryAllocationLib.h> #include > > +<Library/UefiBootServicesTableLib.h> > > +#include <Protocol/AcpiTable.h> > > + > > +// Module specific include files. > > +#include <AcpiTableGenerator.h> > > +#include <ConfigurationManagerObject.h> #include > > +<ConfigurationManagerHelper.h> #include <Library/TableHelperLib.h> > > +#include <Protocol/ConfigurationManagerProtocol.h> > > +#include <Protocol/DynamicTableFactoryProtocol.h> > > + > > +#include <OemCpuGenerator.h> > > + > > +// AML Code Include files generated by iASL Compiler #include > > +<SsdtCpuTemplate.hex> > > + > > +// AML Code offsets file generated by iASL Compiler #include > > +<SsdtCpuTemplate.offset.h> > > + > > Apologies if I should have spotted this before, but this is a no-go. > We are relying on intermediate output of some version of the IASL compiler > here, which [AFAIK] is not formally specified or documented. > We cannot base an elaborate framework like DynamicTables on this. > > I guess this only affects DSDT/SSDT generation, right? > [SAMI] Yes. The last 2 patches in this series add this feature. We probably > need more discussion on this topic. > Until then can we proceed with review of the remaining patches, or should I > submit a new patch series that drops the last 2 patches? > Could you look into whether EFI's ACPI SDT protocol has what you need to manipulate AML at runtime? [SAMI] Thank you for pointing me to the EFI_ACPI_SDT_PROTOCOL. I will experiment with this and let you know. I am going to resubmit the patch series as I have done some changes based on internal feedback. I will drop this patch [5/6] and the next when I resubmit. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
