From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by mx.groups.io with SMTP id smtpd.web11.52843.1598879824178490956 for ; Mon, 31 Aug 2020 06:17:04 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=BMlTtQjS; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.128.65, mailfrom: pete@akeo.ie) Received: by mail-wm1-f65.google.com with SMTP id z9so5296308wmk.1 for ; Mon, 31 Aug 2020 06:17:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=Svy0Mw2/9XWIvEcl9ou5ZQ4oRLTpJNSE91Mrua3GHRQ=; b=BMlTtQjS8PsOgIIynN+aKjcgfAmQLeAc+CQV0P8iS+u+EfWtMzHpocxIvAssTiKVvr xoemjTwsuRfjubtCJKJKLmhmXmgAmkaQbvqv1hyRp31IfxKkqLBFCB1VTlE0z+k9V7/O LVDwiIsFm2EXLQQ16EgTkX/J+WF0x3cHLjj1lzEXtMXy71vsqA55Ci06n8+xwYzxlwI/ lubQsZRLqHLAN0Tnet9H28Uuuy6JKAe5heTBRKKvBEyxHGn1SO0qxQhmwok8kUXiyXpn c9a0wEVucDVyMY/Z7R6Re5MugjaL6E5Uw6firSTjVZsqj/DqKvuCCUCX5AP7VzZH4Uz5 rDiQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Svy0Mw2/9XWIvEcl9ou5ZQ4oRLTpJNSE91Mrua3GHRQ=; b=ry8zo3jYHsBVMTn9AtTkLWP+8uSnNVg+p1r92OKElhp+DEfWrdV2EvYXfDZomllF9N 6HtKnuB7MU9z5xbbveTYAu83LMeW1eA67b7gU8baWPBpu7pQ5OZKHR2Z6OzBYAXeGTql g77Z7Ijtw18EyjmL0ozaeuZreJki5neRYuYZjnIP94ft/xlE/NtHRVZ3eDlSkfVarv8I 5DPkTInVsnjsGmK14LPAtX6Qq+1iKlxzONcXJwbOAVFDiUgdKk2oz7mAHM/fkNcCkcSW mO0X9mmByEzhC/Ky5eaKmUnbqz9SNi6PLLnF8BGcEJz4BHt6qIaCsBKAUUcLbAlFG2JD 88ug== X-Gm-Message-State: AOAM5320WRZ62ruDHZV+pSZ7shOXo6dY+DvlmRg47aQkjB+OAaxsBcE9 aFxG82lG6clVcOfjmj1xlyV5uw== X-Google-Smtp-Source: ABdhPJwzEXVVaE+iq+G8d3fz33pm+iVcK4qbXyqVhFj6CnpVkiOZZSBV1Zru2jShuFCJuLL6t5kDJA== X-Received: by 2002:a7b:cc11:: with SMTP id f17mr1410881wmh.21.1598879822641; Mon, 31 Aug 2020 06:17:02 -0700 (PDT) Return-Path: Received: from [10.0.0.130] ([84.203.85.10]) by smtp.googlemail.com with ESMTPSA id c4sm12136476wrt.41.2020.08.31.06.17.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 31 Aug 2020 06:17:01 -0700 (PDT) Subject: Re: [PATCH v3 2/5] Platform/RaspberryPi: Monitor ACPI Table installs To: Jeremy Linton , devel@edk2.groups.io Cc: Leif Lindholm , Andrei Warkentin , Ard Biesheuvel , Samer El-Haj-Mahmoud References: <20200828220215.101919-1-jeremy.linton@arm.com> <20200828220215.101919-3-jeremy.linton@arm.com> From: "Pete Batard" Message-ID: <3848a8b1-b8e0-89ef-38a9-3ffdbc4023b8@akeo.ie> Date: Mon, 31 Aug 2020 14:17:00 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20200828220215.101919-3-jeremy.linton@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit On 2020.08.28 23:02, Jeremy Linton wrote: > 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 > Cc: Pete Batard > Cc: Andrei Warkentin > Cc: Ard Biesheuvel > Cc: Samer El-Haj-Mahmoud > Signed-off-by: Jeremy Linton > --- > 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); > > } > > > Reviewed-by: Pete Batard