From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f177.google.com (mail-pf1-f177.google.com [209.85.210.177]) by mx.groups.io with SMTP id smtpd.web10.1396.1688055635317406557 for ; Thu, 29 Jun 2023 09:20:35 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@taylorbeebe.com header.s=google header.b=GtUJ4wT3; spf=pass (domain: taylorbeebe.com, ip: 209.85.210.177, mailfrom: t@taylorbeebe.com) Received: by mail-pf1-f177.google.com with SMTP id d2e1a72fcca58-666fb8b1bc8so825919b3a.1 for ; Thu, 29 Jun 2023 09:20:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=taylorbeebe.com; s=google; t=1688055634; x=1690647634; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=WTryFyIfAUdbZQf0Pxu5btHS5RlxSqJLuf66BzgFz3c=; b=GtUJ4wT3TuM8s7W3HE6loMbM9eanWcAWozeh1282FJcmP753eUjNHnPux9D3GvWFcd Cjz6ZxPpGa0QLmqqQ6kXJ0Z4PR0wGRZMfh394ThRV+OpqVKCE2VzB7kmw9i4hbvvvM1B 9RqBGn0AAxvu5U8AdEBi/95FsekL6djHgYvYCdRn0YgvfLKLJyw1UqKM1U29AJ9MnbOD C0G7F4fINeJaEnx3Ci1rtKVW4pwKTW6AoaLHiC85Zo/fQvL3a2verXdfHaWPHO3+TJJ1 XVbFvMvcNsW24S0T+wbZtDYJeyctGQ5bp/JYY9wC0LgUfRaqxSq1fB1nIgDuZKLb168i V4Vw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688055634; x=1690647634; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=WTryFyIfAUdbZQf0Pxu5btHS5RlxSqJLuf66BzgFz3c=; b=aY3XRsHQuIlAIf/uou1UbIWamiX7oj1tmtuFyc5+qTq7V9FmYXdQXgmPfbwiOSPkpR SNChL6ArLKbyTF7V8y19XgxEk7e6XghvRTVy6LecdevIDDp5i3oyBECFXSxPHgvPA6Lq ZrrMjIREncOZ3W8OgGszYtTDuSF4gOe0PeLNa0s+jKhUkrESuLh5+eJZ59Ga1r9ahhRa 9Xtn8N3EExVIgkR0/fLlOV3PkuIBU7RFuh+bOyTBwb82qrumYzfQCSnqHc/VJZsirDw6 Kp+mFosnKG9Cc59zzHMvruxkNVb7lQg3sGvAoP7z3EOJcZuQ9PGZ5b/IThyHnUqe6dlk dB7A== X-Gm-Message-State: ABy/qLYG+4z9N5GIPo9vgzp5ai6NwHS9vTFYamo+/JOMF72Q5DO2ruuP YWKTfkUZmAYQfI4pQdVixZ9DT38XDYKVtMTc3hDEyQ== X-Google-Smtp-Source: APBJJlG8MXBsTm+g+HWrR6SmnzjeYefysr5KKJZIYMYw8dzHhg3I+raDuD62NCMPpJLx9G0jCU6ImA== X-Received: by 2002:a05:6a00:148b:b0:668:9fb6:b311 with SMTP id v11-20020a056a00148b00b006689fb6b311mr435347pfu.32.1688055634578; Thu, 29 Jun 2023 09:20:34 -0700 (PDT) Return-Path: Received: from localhost.localdomain ([50.46.230.135]) by smtp.gmail.com with ESMTPSA id b5-20020aa78705000000b0064f7c56d8b7sm6993578pfo.219.2023.06.29.09.20.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Jun 2023 09:20:34 -0700 (PDT) From: "Taylor Beebe" To: devel@edk2.groups.io Cc: Taylor Beebe , Leif Lindholm , Ard Biesheuvel , Taylor Beebe Subject: [PATCH 2/4] ArmPkg: Update GetMemoryRegion() to Handle No mapping Date: Thu, 29 Jun 2023 09:17:56 -0700 Message-ID: <20032a1dfef2d6b83e40821532038f186420d318.1687989723.git.t@taylorbeebe.com> X-Mailer: git-send-email 2.41.0.windows.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Taylor Beebe =0D This patch updates the GetMemoryRegion() function to handle the case=0D where there is no mapping for the requested address.=0D =0D The original logic for the ARM would hit an ASSERT after=0D GetMemoryRegionPage() returned EFI_SUCCESS but did not update The=0D RegionLength parameter.=0D =0D The original logic for the AARCH64 would never initialize the=0D RegionLength parameter to zero and return EFI_SUCCESS after=0D traversing an unknown number of pages.=0D =0D To fix this, the logic for both architecture has updated to=0D return EFI_NO_MAPPING if the BaseAddress being checked is unmapped.=0D =0D Cc: Leif Lindholm =0D Cc: Ard Biesheuvel =0D =0D Signed-off-by: Taylor Beebe =0D ---=0D ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 30 +++++++------=0D ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 65 +++++++++++++++++++----------=0D 2 files changed, 60 insertions(+), 35 deletions(-)=0D =0D diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c b/ArmPkg/Drivers/CpuDxe/AA= rch64/Mmu.c=0D index 1d02e41e18d8..0d3bc2809682 100644=0D --- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c=0D +++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c=0D @@ -380,10 +380,10 @@ GetMemoryRegionRec (=0D RegionAttributes=0D );=0D =0D - // In case of 'Success', it means the end of the block region has been= found into the upper=0D - // level translation table=0D - if (!EFI_ERROR (Status)) {=0D - return EFI_SUCCESS;=0D + // EFI_SUCCESS: The end of the end of the region was found.=0D + // EFI_NO_MAPPING: The translation entry associated with BaseAddress = is invalid.=0D + if (Status !=3D EFI_NOT_FOUND) {=0D + return Status;=0D }=0D =0D // Now we processed the table move to the next entry=0D @@ -395,12 +395,13 @@ GetMemoryRegionRec (=0D *RegionLength =3D 0;=0D *RegionAttributes =3D *BlockEntry & TT_ATTRIBUTES_MASK;=0D } else {=0D - // We have an 'Invalid' entry=0D - return EFI_UNSUPPORTED;=0D + return EFI_NO_MAPPING;=0D }=0D =0D while (BlockEntry <=3D LastBlockEntry) {=0D - if ((*BlockEntry & TT_ATTRIBUTES_MASK) =3D=3D *RegionAttributes) {=0D + if (((*BlockEntry & TT_TYPE_MASK) =3D=3D BlockEntryType) &&=0D + ((*BlockEntry & TT_ATTRIBUTES_MASK) =3D=3D *RegionAttributes))=0D + {=0D *RegionLength =3D *RegionLength + TT_BLOCK_ENTRY_SIZE_AT_LEVEL (Tabl= eLevel);=0D } else {=0D // In case we have found the end of the region we return success=0D @@ -412,7 +413,7 @@ GetMemoryRegionRec (=0D =0D // If we have reached the end of the TranslationTable and we have not fo= und the end of the region then=0D // we return EFI_NOT_FOUND.=0D - // The caller will continue to look for the memory region at its level=0D + // The caller will continue to look for the memory region at its level.= =0D return EFI_NOT_FOUND;=0D }=0D =0D @@ -433,6 +434,11 @@ GetMemoryRegion (=0D =0D TranslationTable =3D ArmGetTTBR0BaseAddress ();=0D =0D + // Initialize the output parameters. These paramaters are only valid if = the=0D + // result is EFI_SUCCESS.=0D + *RegionLength =3D 0;=0D + *RegionAttributes =3D 0;=0D +=0D T0SZ =3D ArmGetTCR () & TCR_T0SZ_MASK;=0D // Get the Table info from T0SZ=0D GetRootTranslationTableInfo (T0SZ, &TableLevel, &EntryCount);=0D @@ -447,10 +453,10 @@ GetMemoryRegion (=0D );=0D =0D // If the region continues up to the end of the root table then GetMemor= yRegionRec()=0D - // will return EFI_NOT_FOUND=0D - if (Status =3D=3D EFI_NOT_FOUND) {=0D + // will return EFI_NOT_FOUND. Check if the region length was updated.=0D + if ((Status =3D=3D EFI_NOT_FOUND) && (*RegionLength > 0)) {=0D return EFI_SUCCESS;=0D - } else {=0D - return Status;=0D }=0D +=0D + return Status;=0D }=0D diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mm= u.c=0D index afd6aab60204..268c0bf3f9ae 100644=0D --- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c=0D +++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c=0D @@ -427,17 +427,20 @@ EfiAttributeToArmAttribute (=0D EFI_STATUS=0D GetMemoryRegionPage (=0D IN UINT32 *PageTable,=0D - IN OUT UINTN *BaseAddress,=0D - OUT UINTN *RegionLength,=0D - OUT UINTN *RegionAttributes=0D + IN UINTN *BaseAddress,=0D + IN UINTN *RegionAttributes,=0D + OUT UINTN *RegionLength=0D )=0D {=0D - UINT32 PageAttributes;=0D - UINT32 TableIndex;=0D - UINT32 PageDescriptor;=0D + UINT32 PageAttributes;=0D + UINT32 TableIndex;=0D + UINT32 PageDescriptor;=0D + EFI_STATUS Status;=0D =0D // Convert the section attributes into page attributes=0D PageAttributes =3D ConvertSectionAttributesToPageAttributes (*RegionAttr= ibutes);=0D + Status =3D EFI_NOT_FOUND;=0D + *RegionLength =3D 0;=0D =0D // Calculate index into first level translation table for start of modif= ication=0D TableIndex =3D ((*BaseAddress) & TT_DESCRIPTOR_PAGE_INDEX_MASK) >> TT_D= ESCRIPTOR_PAGE_BASE_SHIFT;=0D @@ -449,23 +452,24 @@ GetMemoryRegionPage (=0D PageDescriptor =3D PageTable[TableIndex];=0D =0D if ((PageDescriptor & TT_DESCRIPTOR_PAGE_TYPE_MASK) =3D=3D TT_DESCRIPT= OR_PAGE_TYPE_FAULT) {=0D - // Case: End of the boundary of the region=0D - return EFI_SUCCESS;=0D + Status =3D (*RegionLength > 0) ? EFI_SUCCESS : EFI_NO_MAPPING;=0D + break;=0D } else if ((PageDescriptor & TT_DESCRIPTOR_PAGE_TYPE_PAGE) =3D=3D TT_D= ESCRIPTOR_PAGE_TYPE_PAGE) {=0D - if ((PageDescriptor & TT_DESCRIPTOR_PAGE_ATTRIBUTE_MASK) =3D=3D Page= Attributes) {=0D - *RegionLength =3D *RegionLength + TT_DESCRIPTOR_PAGE_SIZE;=0D - } else {=0D - // Case: End of the boundary of the region=0D - return EFI_SUCCESS;=0D + if ((PageDescriptor & TT_DESCRIPTOR_PAGE_ATTRIBUTE_MASK) !=3D PageAt= tributes) {=0D + Status =3D EFI_SUCCESS;=0D + break;=0D }=0D +=0D + *RegionLength +=3D TT_DESCRIPTOR_PAGE_SIZE;=0D } else {=0D - // We do not support Large Page yet. We return EFI_SUCCESS that mean= s end of the region.=0D + // Large pages are unsupported.=0D + Status =3D EFI_UNSUPPORTED;=0D ASSERT (0);=0D - return EFI_SUCCESS;=0D + break;=0D }=0D }=0D =0D - return EFI_NOT_FOUND;=0D + return Status;=0D }=0D =0D EFI_STATUS=0D @@ -482,6 +486,7 @@ GetMemoryRegion (=0D UINT32 SectionDescriptor;=0D ARM_FIRST_LEVEL_DESCRIPTOR *FirstLevelTable;=0D UINT32 *PageTable;=0D + UINTN Length;=0D =0D // Initialize the arguments=0D *RegionLength =3D 0;=0D @@ -491,7 +496,11 @@ GetMemoryRegion (=0D =0D // Calculate index into first level translation table for start of modif= ication=0D TableIndex =3D TT_DESCRIPTOR_SECTION_BASE_ADDRESS (*BaseAddress) >> TT_D= ESCRIPTOR_SECTION_BASE_SHIFT;=0D - ASSERT (TableIndex < TRANSLATION_TABLE_SECTION_COUNT);=0D +=0D + if (TableIndex >=3D TRANSLATION_TABLE_SECTION_COUNT) {=0D + ASSERT (TableIndex < TRANSLATION_TABLE_SECTION_COUNT);=0D + return EFI_INVALID_PARAMETER;=0D + }=0D =0D // Get the section at the given index=0D SectionDescriptor =3D FirstLevelTable[TableIndex];=0D @@ -524,6 +533,8 @@ GetMemoryRegion (=0D TT_DESCRIPTOR_CONVERT_TO_SECTION_AP (PageAttribute= s);=0D }=0D =0D + Status =3D EFI_NOT_FOUND;=0D +=0D for ( ; TableIndex < TRANSLATION_TABLE_SECTION_COUNT; TableIndex++) {=0D // Get the section at the given index=0D SectionDescriptor =3D FirstLevelTable[TableIndex];=0D @@ -532,15 +543,18 @@ GetMemoryRegion (=0D if (TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE (SectionDescriptor)) {=0D // Extract the page table location from the descriptor=0D PageTable =3D (UINT32 *)(SectionDescriptor & TT_DESCRIPTOR_SECTION_P= AGETABLE_ADDRESS_MASK);=0D + Length =3D 0;=0D =0D // Scan the page table to find the end of the region.=0D - Status =3D GetMemoryRegionPage (PageTable, BaseAddress, RegionLength= , RegionAttributes);=0D - ASSERT (*RegionLength > 0);=0D + Status =3D GetMemoryRegionPage (PageTable, BaseAddress, Regi= onAttributes, &Length);=0D + *RegionLength +=3D Length;=0D =0D - // If we have found the end of the region (Status =3D=3D EFI_SUCCESS= ) then we exit the for-loop=0D - if (Status =3D=3D EFI_SUCCESS) {=0D - break;=0D + // Status =3D=3D EFI_NOT_FOUND implies we have not reached the end o= f the region.=0D + if ((Status =3D=3D EFI_NOT_FOUND) && (Length > 0)) {=0D + continue;=0D }=0D +=0D + break;=0D } else if (((SectionDescriptor & TT_DESCRIPTOR_SECTION_TYPE_MASK) =3D= =3D TT_DESCRIPTOR_SECTION_TYPE_SECTION) ||=0D ((SectionDescriptor & TT_DESCRIPTOR_SECTION_TYPE_MASK) =3D= =3D TT_DESCRIPTOR_SECTION_TYPE_SUPERSECTION))=0D {=0D @@ -556,5 +570,10 @@ GetMemoryRegion (=0D }=0D }=0D =0D - return EFI_SUCCESS;=0D + // Check if the region length was updated.=0D + if (*RegionLength > 0) {=0D + Status =3D EFI_SUCCESS;=0D + }=0D +=0D + return Status;=0D }=0D -- =0D 2.41.0.windows.1=0D =0D