From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by mx.groups.io with SMTP id smtpd.web12.14183.1585150190655340848 for ; Wed, 25 Mar 2020 08:29:51 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=KjI9VBKJ; spf=pass (domain: linaro.org, ip: 209.85.128.66, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wm1-f66.google.com with SMTP id g62so3205073wme.1 for ; Wed, 25 Mar 2020 08:29:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=F44m6FHSxU7Lx9/E0R+H9oRnjcG0sEIavuRCfpzjX+4=; b=KjI9VBKJVI/3wyHMnkKmNzZJHWNHH2Pq7XTnflFhc1Qod7r6TlT75vMWWTHABY1jkT NMBL7XGdT+GedbALsGGg4K1x2tDlraQFbaYoLKRjghim3wgyiJnyG4BQLgxd9qBpZBKK +49IhNjC/6gOiYW9shphHUF4w6qNlX+7JDyyHmXJ4CDyfAk9H7O1QdML+S4rDF+9o2jE 0t122ttlJ8LUFJgCpr+1bed1AThsf5v0lGioEzv4LNY2p+fZiRyg8jzLuiDuBY4U+ckU bn2GjTiGLOKUejFyQIFCmZa4L0IzKlBwYYcGEIYBi8JWopIM640nQbl3BBlfV3HXJkjh xMWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=F44m6FHSxU7Lx9/E0R+H9oRnjcG0sEIavuRCfpzjX+4=; b=Kiuy6U1fdJ5wGUstcO/0Iww/t8YMFOWDMC2xFIRRrlc2+U6yVizyNzGtDtfEBKDgFr LuFv+q8kLVWN+Nr6vzQ5OD4bQkRBQzKSmdZQN6342bNfyXR6iF1rJPfulQNTX2LVR6Tv gMm9CIGXH7ZB2sekinwXtQ3UcUpDSaRKdvWA2ALQZHGiCtIAJadyrgarRGUP1c0szRDb BTR+RsnfMTp0mPVzQ9KY0qmJPC/af74hpqVj3IJ861L/1e9+KBW/rzTng50fQPQQRUdz b0ALcqIRo4lYb5/ZVsm1RMAZuy+7sb4zNA/fDGDrUweNo0lWXCK8uAAcHiyixM78xcO+ ut/A== X-Gm-Message-State: ANhLgQ0uBkxDdUrrp+7zOGvV4tQl5OAsF5hfCsfOanoO2wRF8kv4a29v Hz9JHBJpGGAqKzTY6QRbMSoyoU6N+0JLa9CS X-Google-Smtp-Source: ADFU+vtMCEarBbJOrOdHV3KMb1eqaHGl0BBrvPnBkHA/lVq3AAffUnCIszFiVJnm7OCkEUQK13TYLw== X-Received: by 2002:a7b:c7d4:: with SMTP id z20mr3904944wmk.48.1585150188872; Wed, 25 Mar 2020 08:29:48 -0700 (PDT) Return-Path: Received: from e123331-lin.home (amontpellier-657-1-18-247.w109-210.abo.wanadoo.fr. [109.210.65.247]) by smtp.gmail.com with ESMTPSA id r15sm29249296wra.19.2020.03.25.08.29.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Mar 2020 08:29:48 -0700 (PDT) From: "Ard Biesheuvel" To: devel@edk2.groups.io Cc: Ard Biesheuvel , Laszlo Ersek , Leif Lindholm , Ashish Singhal Subject: [PATCH v3 3/3] ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table entry Date: Wed, 25 Mar 2020 16:29:40 +0100 Message-Id: <20200325152940.1492-4-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200325152940.1492-1-ard.biesheuvel@linaro.org> References: <20200325152940.1492-1-ard.biesheuvel@linaro.org> Currently, depending on the size of the region being (re)mapped, the page table manipulation code may replace a table entry with a block entry, even if the existing table entry uses different mapping attributes to describe different parts of the region it covers. This is undesirable, and instead, we should avoid doing so unless we are disregarding the original attributes anyway. And if we make such a replacement, we should free all the page tables that have become orphaned in the process. So let's implement this, by taking the table entry path through the code for block sized regions if a table entry already exists, and the clear mask is set (which means we are preserving attributes from the existing mapping). And when we do replace a table entry with a block entry, free all the pages that are no longer referenced. Signed-off-by: Ard Biesheuvel --- ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 25 ++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c index 0680ba36d907..3b10ef58f0a2 100644 --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c @@ -229,8 +229,12 @@ UpdateRegionMappingRecursive ( // than a block, and recurse to create the block or page entries at // the next level. No block mappings are allowed at all at level 0, // so in that case, we have to recurse unconditionally. + // If we are changing a table entry and the AttributeClearMask is non-zero, + // we cannot replace it with a block entry without potentially losing + // attribute information, so keep the table entry in that case. // - if (Level == 0 || ((RegionStart | BlockEnd) & BlockMask) != 0) { + if (Level == 0 || ((RegionStart | BlockEnd) & BlockMask) != 0 || + (IsTableEntry (*Entry, Level) && AttributeClearMask != 0)) { ASSERT (Level < 3); if (!IsTableEntry (*Entry, Level)) { @@ -251,6 +255,8 @@ UpdateRegionMappingRecursive ( InvalidateDataCacheRange (TranslationTable, EFI_PAGE_SIZE); } + ZeroMem (TranslationTable, EFI_PAGE_SIZE); + if (IsBlockEntry (*Entry, Level)) { // // We are splitting an existing block entry, so we have to populate @@ -268,8 +274,6 @@ UpdateRegionMappingRecursive ( FreePages (TranslationTable, 1); return Status; } - } else { - ZeroMem (TranslationTable, EFI_PAGE_SIZE); } } else { TranslationTable = (VOID *)(UINTN)(*Entry & TT_ADDRESS_MASK_BLOCK_ENTRY); @@ -306,7 +310,20 @@ UpdateRegionMappingRecursive ( EntryValue |= (Level == 3) ? TT_TYPE_BLOCK_ENTRY_LEVEL3 : TT_TYPE_BLOCK_ENTRY; - ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE); + if (IsTableEntry (*Entry, Level)) { + // + // We are replacing a table entry with a block entry. This is only + // possible if we are keeping none of the original attributes. + // We can free the table entry's page table, and all the ones below + // it, since we are dropping the only possible reference to it. + // + ASSERT (AttributeClearMask == 0); + TranslationTable = (VOID *)(UINTN)(*Entry & TT_ADDRESS_MASK_BLOCK_ENTRY); + ReplaceTableEntry (Entry, EntryValue, RegionStart, TRUE); + FreePageTablesRecursive (TranslationTable, Level + 1); + } else { + ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE); + } } } return EFI_SUCCESS; -- 2.17.1