public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Andrei Warkentin" <awarkentin@vmware.com>
To: Jeremy Linton <jeremy.linton@arm.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Leif Lindholm <leif@nuviainc.com>, Pete Batard <pete@akeo.ie>,
	Ard Biesheuvel <ard.biesheuvel@arm.com>,
	Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Subject: Re: [PATCH v4 2/6] Platform/RaspberryPi: Monitor ACPI Table installs
Date: Mon, 31 Aug 2020 17:32:59 +0000	[thread overview]
Message-ID: <CY4PR05MB3413B3B6A4A9DCEB302EFB7CB9510@CY4PR05MB3413.namprd05.prod.outlook.com> (raw)
In-Reply-To: <20200831172549.24079-3-jeremy.linton@arm.com>

[-- Attachment #1: Type: text/plain, Size: 7218 bytes --]

This is really cool. Can be useful in a number of other places down the line (e.g. MAC address via device properties).

Reviewed-by: Andrei Warkentin <andrey.warkentin@gmail.com>
________________________________
From: Jeremy Linton <jeremy.linton@arm.com>
Sent: Monday, August 31, 2020 12:25 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Jeremy Linton <jeremy.linton@arm.com>; Leif Lindholm <leif@nuviainc.com>; Pete Batard <pete@akeo.ie>; Andrei Warkentin <awarkentin@vmware.com>; Ard Biesheuvel <ard.biesheuvel@arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Subject: [PATCH v4 2/6] Platform/RaspberryPi: Monitor ACPI Table installs

Hook the ACPI table install sequence and add some
basic conditional and AML NameOp update logic. If
a table has a non-zero PCD declared that pcd is
checked for a non-zero value before allowing the table
to be installed. We also add a table of NameOp to
PCD's which will be written into a DSDT/SSDT table
as part of its install process.

With this change we can declare something in
ASL like:

Name (VARN, 0x1234)

and then add a table entry like:

{"VARN", PcdToken(PcdVarn)}

and the value of PcdVarn will replace the
0x1234 declared in the ASL above.

Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Andrei Warkentin <awarkentin@vmware.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Reviewed-by: Pete Batard <@pbatard>
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 153 ++++++++++++++++++++-
 1 file changed, 152 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index af54136ade..9e5d9734ca 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -568,6 +568,156 @@ ApplyVariables (
 }





+typedef struct {

+  CHAR8 Name[4];

+  UINTN PcdToken;

+} AML_NAME_OP_REPLACE;

+

+typedef struct {

+  UINT64              OemTableId;

+  UINTN               PcdToken;

+  AML_NAME_OP_REPLACE *SdtNameOpReplace;

+} NAMESPACE_TABLES;

+

+#define SSDT_PATTERN_LEN 5

+#define AML_NAMEOP_8   0x0A

+#define AML_NAMEOP_16  0x0B

+#define AML_NAMEOP_32  0x0C

+#define AML_NAMEOP_STR 0x0D

+/*

+ * Scan the given namespace table (DSDT/SSDT) for AML NameOps

+ * listed in the NameOpReplace structure. If one is found then

+ * update the value in the table from the specified Pcd

+ *

+ * This allows us to have conditionals in AML controlled

+ * by options in the BDS or detected during firmware bootstrap.

+ * We could extend this concept for strings/etc but due to len

+ * variations its probably easier to encode the strings

+ * in the ASL and pick the correct one based off a variable.

+ */

+STATIC VOID

+UpdateSdtNameOps(

+  EFI_ACPI_DESCRIPTION_HEADER  *AcpiTable,

+  AML_NAME_OP_REPLACE          *NameOpReplace

+  )

+{

+  UINTN  Idx;

+  UINTN  Index;

+  CHAR8  Pattern[SSDT_PATTERN_LEN];

+  UINTN  PcdVal;

+  UINT8  *SdtPtr;

+  UINT32 SdtSize;

+

+  SdtSize = AcpiTable->Length;

+

+  if (SdtSize > 0) {

+    SdtPtr = (UINT8*)AcpiTable;

+

+    for (Idx = 0; NameOpReplace && NameOpReplace[Idx].PcdToken; Idx++) {

+      /*

+       * Do a single NameOp variable replacement these are of the

+       * form 08 XXXX SIZE VAL, where SIZE is 0A=byte, 0B=word, 0C=dword

+       * and XXXX is the name and VAL is the value

+      */

+      Pattern[0] = 0x08;

+      Pattern[1] = NameOpReplace[Idx].Name[0];

+      Pattern[2] = NameOpReplace[Idx].Name[1];

+      Pattern[3] = NameOpReplace[Idx].Name[2];

+      Pattern[4] = NameOpReplace[Idx].Name[3];

+

+      for (Index = 0; Index < (SdtSize - SSDT_PATTERN_LEN); Index++) {

+        if (CompareMem (SdtPtr + Index, Pattern, SSDT_PATTERN_LEN) == 0) {

+          PcdVal = LibPcdGet32 (NameOpReplace[Idx].PcdToken);

+          switch (SdtPtr[Index + SSDT_PATTERN_LEN]) {

+          case AML_NAMEOP_32:

+            SdtPtr[Index + SSDT_PATTERN_LEN + 4] = (PcdVal >> 24) & 0xFF;

+            SdtPtr[Index + SSDT_PATTERN_LEN + 3] = (PcdVal >> 16) & 0xFF;

+            // Fallthrough

+          case AML_NAMEOP_16:

+            SdtPtr[Index + SSDT_PATTERN_LEN + 2] = (PcdVal >> 8) & 0xFF;

+            // Fallthrough

+          case AML_NAMEOP_8:

+            SdtPtr[Index + SSDT_PATTERN_LEN + 1] = PcdVal & 0xFF;

+            break;

+          case 0:

+          case 1:

+            SdtPtr[Index + SSDT_PATTERN_LEN + 1] = !!PcdVal;

+            break;

+          case AML_NAMEOP_STR:

+            /*

+             * If the string val is added to the NameOpReplace, we can

+             * dynamically update things like _HID too as long as the

+             * string length matches.

+             */

+            break;

+          }

+          break;

+        }

+      }

+    }

+  }

+}

+

+

+STATIC BOOLEAN

+VerifyUpdateTable(

+  IN  EFI_ACPI_DESCRIPTION_HEADER *AcpiHeader,

+  IN  NAMESPACE_TABLES *SdtTable

+  )

+{

+  BOOLEAN ret;

+

+  ret = TRUE;

+  if (SdtTable->PcdToken && !LibPcdGet32 (SdtTable->PcdToken)) {

+    ret = FALSE;

+  }

+  if (ret && SdtTable->SdtNameOpReplace) {

+    UpdateSdtNameOps (AcpiHeader, SdtTable->SdtNameOpReplace);

+  }

+

+  return ret;

+}

+

+

+STATIC NAMESPACE_TABLES SdtTables[] = {

+  {

+    SIGNATURE_64 ('R', 'P', 'I', 0, 0, 0, 0, 0),

+    0,

+    NULL

+  },

+  { }

+};

+

+/*

+ * Monitor the ACPI tables being installed and when

+ * a DSDT/SSDT is detected validate that we want to

+ * install it, and if so update any "NameOp" defined

+ * variables contained in the table from PCD values

+ */

+STATIC BOOLEAN

+HandleDynamicNamespace (

+  IN  EFI_ACPI_DESCRIPTION_HEADER *AcpiHeader

+  )

+{

+  UINTN Tables;

+

+  switch (AcpiHeader->Signature) {

+  case SIGNATURE_32 ('D', 'S', 'D', 'T'):

+  case SIGNATURE_32 ('S', 'S', 'D', 'T'):

+    for (Tables = 0; SdtTables[Tables].OemTableId; Tables++) {

+      if (AcpiHeader->OemTableId == SdtTables[Tables].OemTableId) {

+        return VerifyUpdateTable (AcpiHeader, &SdtTables[Tables]);

+      }

+    }

+    DEBUG ((DEBUG_ERROR, "Found namespace table not in table list.\n"));

+

+    return FALSE;

+  }

+

+  return TRUE;

+}

+

+

 EFI_STATUS

 EFIAPI

 ConfigInitialize (

@@ -618,7 +768,8 @@ ConfigInitialize (


   if (PcdGet32 (PcdSystemTableMode) == SYSTEM_TABLE_MODE_ACPI ||

       PcdGet32 (PcdSystemTableMode) == SYSTEM_TABLE_MODE_BOTH) {

-     Status = LocateAndInstallAcpiFromFv (&mAcpiTableFile);

+     Status = LocateAndInstallAcpiFromFvConditional (&mAcpiTableFile,

+                                                     &HandleDynamicNamespace);

      ASSERT_EFI_ERROR (Status);

   }



--
2.13.7


[-- Attachment #2: Type: text/html, Size: 12641 bytes --]

  reply	other threads:[~2020-08-31 17:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-31 17:25 [PATCH v4 0/6] Platform/RasberryPi: Thermal zone Jeremy Linton
2020-08-31 17:25 ` [PATCH v4 1/6] Platform/RaspberryPi4: Add a basic thermal zone Jeremy Linton
2020-08-31 17:25 ` [PATCH v4 2/6] Platform/RaspberryPi: Monitor ACPI Table installs Jeremy Linton
2020-08-31 17:32   ` Andrei Warkentin [this message]
2020-08-31 17:25 ` [PATCH v4 3/6] Platform/RaspberryPi: Add entry for user fan control Jeremy Linton
2020-08-31 17:25 ` [PATCH v4 4/6] Platform/RaspberryPi4: Create ACPI fan object Jeremy Linton
2020-08-31 17:25 ` [PATCH v4 5/6] Platform/RaspberryPi4: Allow the user to set Temp Jeremy Linton
2020-08-31 18:57   ` Pete Batard
2020-08-31 19:05     ` Ard Biesheuvel
2020-08-31 17:25 ` [PATCH v4 6/6] Platform/RaspberryPi: Trivial whitespace cleanup Jeremy Linton
2020-09-01 11:39   ` Ard Biesheuvel
2020-09-01 11:46     ` Pete Batard
2020-09-01 12:23 ` [PATCH v4 0/6] Platform/RasberryPi: Thermal zone Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CY4PR05MB3413B3B6A4A9DCEB302EFB7CB9510@CY4PR05MB3413.namprd05.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox