From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ot1-f65.google.com (mail-ot1-f65.google.com [209.85.210.65]) by mx.groups.io with SMTP id smtpd.web12.12965.1583588066468538922 for ; Sat, 07 Mar 2020 05:34:26 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=ICD8zMSR; spf=pass (domain: linaro.org, ip: 209.85.210.65, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-ot1-f65.google.com with SMTP id i14so5271343otp.5 for ; Sat, 07 Mar 2020 05:34:26 -0800 (PST) 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=lisR4zhiJ8nfpvjfACWzbNpKEH9GLaJWNhkmj96MM3A=; b=ICD8zMSRe28uKTUZB7yB0lVyxtaaZR8hUZwX7fxPFNmxOLtdcViPN+zMGCfe5wtoGV JbEXWpR9+ukxpRcGwpAo9yH6RzhSXjwUibRZZLs+mYGavDVIJerDrRCgKiSSeTe2YL+X bk+lHDYUCW17hg4yTUWLu7pRezPgSpOxLopPCoAL6VMdeyhioUpGzxkGG3UHwuapCxue eLbdRPKA0OkdJmOJW1+5Wav3z+TbVSUhsYAhgchYmTeVzx8MvADPKL9Q1fH5R4TLlcJI rDl+nOnb0/TTy0o7eZHe7NHvgtx2viDIC4aMK/Ek9XLq4Rn0TUoD2ToUXpIoqm+J9zoT lo2g== 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=lisR4zhiJ8nfpvjfACWzbNpKEH9GLaJWNhkmj96MM3A=; b=f21njXC5DeF0is9HdWjGfANhzFqM1mdaZ4AjppI21YfE+RQcFDJojHTQzlBwiAyRRh Mu+JqF1YIAxhGSTT0OUhlGo2/21GViOGqzAl19ohtwxlH9Pmwr3qSG0JhqGwFXQ8fntF 2o9Lc+RyO2AZMu08IdIrdBoAZ2HERDsbahGTq8Q97FwuUKwNgKq6K3Yg2F7fJJn99RND V1r7CX+wIOc3V9YWJSpeq9dPFvDoQP51Hw5uscqxSKwPrqubs5U7KKEyNmqc7+MNhI0b Lpd3d5Cx4+3eigGJizruYla/wnuTjInlx2q0NSqlNIbmZIEgrEhcHUoaLN1/YFXgjPP5 KY8Q== X-Gm-Message-State: ANhLgQ3T+PpZBOF+NFLxhAH630CYBOlpn6QplhOAgHCkCvYDonWzP9N/ S5Ho3DMy2FPyzmpi6RPxZBYJ065DuJXOXg== X-Google-Smtp-Source: ADFU+vt8eVIty465QJTCDlI9aBBhrKAPSxyBBHxDymc00t03blQHEXqbHxEpqSif8gTo0i2FYvDqjQ== X-Received: by 2002:a05:6830:1e09:: with SMTP id s9mr6073477otr.149.1583588065339; Sat, 07 Mar 2020 05:34:25 -0800 (PST) Return-Path: Received: from cam-smtp0.cambridge.arm.com ([2a01:cb1d:112:6f00:816e:ff0d:fb69:f613]) by smtp.gmail.com with ESMTPSA id l15sm7775815otf.12.2020.03.07.05.34.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 07 Mar 2020 05:34:24 -0800 (PST) From: "Ard Biesheuvel" To: devel@edk2.groups.io Cc: leif@nuviainc.com, Ard Biesheuvel Subject: [PATCH 2/2] ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table entry Date: Sat, 7 Mar 2020 14:34:15 +0100 Message-Id: <20200307133415.18857-3-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200307133415.18857-1-ard.biesheuvel@linaro.org> References: <20200307133415.18857-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 | 21 ++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c index 6f6ef5b05fbc..7b2c36a7a538 100644 --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c @@ -223,8 +223,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)) { @@ -300,7 +304,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); + } else { + ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE); + } } } return EFI_SUCCESS; -- 2.17.1