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.web09.6214.1582713438812656318 for ; Wed, 26 Feb 2020 02:37:19 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=HJnnfUyF; 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 p9so2406564wmc.2 for ; Wed, 26 Feb 2020 02:37:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=OP9ZL4XDv8MBHxzFNx6FtIdYdKLhgvDGoJ42WWzmNGc=; b=HJnnfUyFSHTCNGXXd7PcnZFqsNdlrVUWi/UOBulv5bAggoLG8+E61y3FmD8aQJ9vKl 9ZI3bQGv3n3H6pXdxjZmV3G0XEo4LCbZqRYS2Jzya6R9Q3GDajSYAvn/cpz028b7Q3ax R8sHwf/M5dQw6mGxSf5M4kI8GwbCaUDJPswOF8Y8DO42tkkLKR6DQzrwztcG64a0FYDq Hd0C/vmX7G4mgMiNj1VGQGaZ8lc4iyTICvBuNirEKEDq3pIBbEAjTSUki12XuGdrZQOV is5YNgIxO22IsVF0wt/SkcszWuxOObetTZtHR9g4zDfkLpoT4syVaF3itHGYHMmUPl+O 0mqw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OP9ZL4XDv8MBHxzFNx6FtIdYdKLhgvDGoJ42WWzmNGc=; b=hDqEVor53aHT4Q6+WqKIVzPMBg6WSBlV6bkrraAJgAZbjdilSgkTto+XFYGLxilrwg cB3Uz1AEOo12yLUgFLWez9xQCJyn16uRt3cIDr6ZcxAZXMvytebQwaLgrEHQotRRF2UT F3Pz393JspHbUxix7gmM+t/axUpHhjeB6GPbf72tt5GdEsrRabjPCo5L6xuGWUR0uaTe 1F2tTLey2XUrMp3ljDrhIQJzd7W761kjs6/wyW2VU3TxMAiFjbdQ1tBLhpIaOQHliQ6B PaEAjScgrFGkNKMz/gHSHLqbMCKWlizAVKYxEWu4zObNAT9fnofILay04JF+GBuyWddY DTGg== X-Gm-Message-State: APjAAAUj0i5zPpuqZSHx9tm2gGhkQMpux4NO4pPlCiPOCOIRSoovuypt 8lIz7WIor6byOD0oHjP1PjO6v2z4VedHAEnTIjeE7sCry1W6Lg== X-Google-Smtp-Source: APXvYqzKJf9sVcC4s3b+u+iLoPlPMDa6CAc8UgEq1NmN0uhssN5njsXQE5PSn1ppuEwUpF5km9iMMWnlA0lqVbOgTXo= X-Received: by 2002:a7b:cb93:: with SMTP id m19mr1276217wmi.133.1582713436872; Wed, 26 Feb 2020 02:37:16 -0800 (PST) MIME-Version: 1.0 References: <20200226100353.31962-1-ard.biesheuvel@linaro.org> <20200226100353.31962-4-ard.biesheuvel@linaro.org> In-Reply-To: <20200226100353.31962-4-ard.biesheuvel@linaro.org> From: "Ard Biesheuvel" Date: Wed, 26 Feb 2020 11:37:05 +0100 Message-ID: Subject: Re: [PATCH 3/6] ArmPkg/ArmMmuLib ARM: cache-invalidate initial page table entries To: edk2-devel-groups-io Cc: Leif Lindholm , Laszlo Ersek , Sami Mujawar Content-Type: text/plain; charset="UTF-8" On Wed, 26 Feb 2020 at 11:04, Ard Biesheuvel wrote: > > In the ARM version of ArmMmuLib, we are currently relying on set/way > invalidation to ensure that the caches are in a consistent state with > respect to main memory once we turn the MMU on. Even if set/way > operations were the appropriate method to achieve this, doing an > invalidate-all first and then populating the page table entries creates > a window where page table entries could be loaded speculatively into > the caches before we modify them, and shadow the new values that we > write there. > > So let's get rid of the blanket clean/invalidate operations, and > instead, update ArmUpdateTranslationTableEntry () to invalidate each > page table entry *after* it is written if the MMU is still disabled > at this point. > Uhm, apologies. This paragraph was copy-pasted from the AARCH64 version (along with the preceding one), but it doesn't apply here. Instead, it should read, """ So let's get rid of the blanket clean/invalidate operations, and instead, invalidate each section entry right after it is updated, and invalidate sets of level 2 entries in blocks, using the generic invalidation routine from CacheMaintenanceLib. """ > On ARMv7, cache maintenance may be required also when the MMU is > enabled, in case the page table walker is not cache coherent. However, > the code being updated here is guaranteed to run only when the MMU is > still off, and so we can disregard the case when the MMU and caches > are on. > > Since the MMU and D-cache are already off when we reach this point, we > can drop the MMU and D-cache disables as well. Maintenance of the I-cache > is unnecessary, since we are not modifying any code, and the installed > mapping is guaranteed to be 1:1. This means we can also leave it enabled > while the page table population code is running. > > Signed-off-by: Ard Biesheuvel > --- > ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 25 +++++++++----------- > 1 file changed, 11 insertions(+), 14 deletions(-) > > diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > index aca7a37facac..c5906b4310cc 100644 > --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > @@ -183,6 +183,8 @@ PopulateLevel2PageTable ( > PhysicalBase += TT_DESCRIPTOR_PAGE_SIZE; > } > > + InvalidateDataCacheRange ((UINT32 *)TranslationTable + FirstPageOffset, > + RemainLength / TT_DESCRIPTOR_PAGE_SIZE * sizeof (*PageEntry)); > } > > STATIC > @@ -257,7 +259,11 @@ FillTranslationTable ( > RemainLength >= TT_DESCRIPTOR_SECTION_SIZE) { > // Case: Physical address aligned on the Section Size (1MB) && the length > // is greater than the Section Size > - *SectionEntry++ = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(PhysicalBase) | Attributes; > + *SectionEntry = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(PhysicalBase) | Attributes; > + > + ArmDataSynchronizationBarrier (); > + ArmInvalidateDataCacheEntryByMVA ((UINTN)SectionEntry++); > + > PhysicalBase += TT_DESCRIPTOR_SECTION_SIZE; > RemainLength -= TT_DESCRIPTOR_SECTION_SIZE; > } else { > @@ -267,9 +273,12 @@ FillTranslationTable ( > // Case: Physical address aligned on the Section Size (1MB) && the length > // does not fill a section > // Case: Physical address NOT aligned on the Section Size (1MB) > - PopulateLevel2PageTable (SectionEntry++, PhysicalBase, PageMapLength, > + PopulateLevel2PageTable (SectionEntry, PhysicalBase, PageMapLength, > MemoryRegion->Attributes); > > + ArmDataSynchronizationBarrier (); > + ArmInvalidateDataCacheEntryByMVA ((UINTN)SectionEntry++); > + > // If it is the last entry > if (RemainLength < TT_DESCRIPTOR_SECTION_SIZE) { > break; > @@ -349,18 +358,6 @@ ArmConfigureMmu ( > } > } > > - ArmCleanInvalidateDataCache (); > - ArmInvalidateInstructionCache (); > - > - ArmDisableDataCache (); > - ArmDisableInstructionCache(); > - // TLBs are also invalidated when calling ArmDisableMmu() > - ArmDisableMmu (); > - > - // Make sure nothing sneaked into the cache > - ArmCleanInvalidateDataCache (); > - ArmInvalidateInstructionCache (); > - > ArmSetTTBR0 ((VOID *)(UINTN)(((UINTN)TranslationTable & ~TRANSLATION_TABLE_SECTION_ALIGNMENT_MASK) | (TTBRAttributes & 0x7F))); > > // > -- > 2.17.1 >