From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by mx.groups.io with SMTP id smtpd.web10.283.1582655513033028264 for ; Tue, 25 Feb 2020 10:31:53 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=urB8Y1cH; spf=pass (domain: linaro.org, ip: 209.85.221.67, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f67.google.com with SMTP id g3so15927100wrs.12 for ; Tue, 25 Feb 2020 10:31:52 -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=dJbPU2kXHBIeJI8hrxHmDwS8s+XME1Z2fQNQV6Yv7F0=; b=urB8Y1cHVaPw9/m8RkYyFVOJnJ1yEpGfASVlesKYk1uCUVykMJ5fF00z2G4cEo2mpp G6X3GqfMygMis/xe5O+6FlrI7xJGKaigaZc6/A/Ij4NCuNh369iUy9uhinMiuiZkpW1D vAQlSSS2FIH/m/CTpLgP2x5ora2lBEamHEL9voVOeyXONjNv7XLs7gB/Pty9dssTIhrp w6INIui1FzIKNwu+dSImv1W0EQs8XPky029BHK7fqa6EicNlgTVWjvA4UFMd1VDXbjyz 4agpyJO70W9cWewrzwBDJIsF9LeRjMkun8ztXM1bg3AQ+DUGaUeB+BsqJMXIMv5Szg+T r49w== 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=dJbPU2kXHBIeJI8hrxHmDwS8s+XME1Z2fQNQV6Yv7F0=; b=c/89GUrJSAj7OR+gI0NH3MiPxzY21KzJioZHYoUkKOzCukuZHHc43D52BRL7oqKqIM iyv5K3fUfqUXmQmCLIPzDy7F4RT7Dodxm793hCZwehc3rQDdDI1KOdXGEWTiQd2FioOP LnB6pJVO5UN2dQ2sR6tz6dtTkrg5VooYjzi0urU2Xs8r9vLQJk2SJoCKPVK+aJMDMD8O Wxzw/RJW2DFwoU24IJEdb5XJJV3zrkD2hMnUTXwyG9yDbrYqRqJXSZRZV2QLjcXEK0FE TKAQ3sfujYXlLVUonRHZY2GYe4H4JxMmS/7csWlyWztCH1bkot7fKY3yLb8dtmOG/Rzk FmUw== X-Gm-Message-State: APjAAAX6d08nRyBkseUfhtqnQUlWx/9hsklPKNWHN87RXkqq3p15ySB2 X6iUDeP1QsN+X+rZAvJe8PWYamwQQBNDXmpF12WCNeWR X-Google-Smtp-Source: APXvYqxmWP0cP2Vwx/B4yOGg/nJNZkuf/kJ+6b0ENqRG3N+48w91I96XpedTiTgNdXAn/Prhi8SeDtH3lVtrehBW+D8= X-Received: by 2002:a05:6000:110b:: with SMTP id z11mr487989wrw.252.1582655511090; Tue, 25 Feb 2020 10:31:51 -0800 (PST) MIME-Version: 1.0 References: <20200225182834.19380-1-ard.biesheuvel@linaro.org> In-Reply-To: <20200225182834.19380-1-ard.biesheuvel@linaro.org> From: "Ard Biesheuvel" Date: Tue, 25 Feb 2020 19:31:40 +0100 Message-ID: Subject: Re: [PATCH 1/1] ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones To: edk2-devel-groups-io Cc: Leif Lindholm , Laszlo Ersek , Pete Batard Content-Type: text/plain; charset="UTF-8" On Tue, 25 Feb 2020 at 19:28, Ard Biesheuvel wrote: > > Cache maintenance operations by set/way are only intended to be used > in the context of on/offlining a core, while it has been taken out of > the coherency domain. Any use intended to ensure that the contents of > the cache have made it to main memory is unreliable, since cacheline > migration and non-architected system caches may cause these contents > to linger elsewhere, without being visible in main memory once the > MMU and caches are disabled. > > In KVM on Linux, there are horrid hacks in place to ensure that such > set/way operations are trapped, and replaced with a single by-VA > clean/invalidate of the entire guest VA space once the MMU state > changes, which can be costly, and is unnecessary if we manage the > caches a bit more carefully, and perform maintenance by virtual > address only. > > So let's get rid of the call to ArmInvalidateDataCache () in the > PrePeiCore startup code, and instead, invalidate the UEFI memory > region by virtual address, which is the only memory region we will > be touching with the caches and MMU both disabled and enabled. > (This will lead to data corruption if data written with the MMU off > is shadowed by clean, stale cachelines that stick around when the > MMU is enabled again.) > > Signed-off-by: Ard Biesheuvel > --- Forgot to add a note that this is the *PrePi* version, not the PrePeiCore one that I sent before. @Pete: this might affect RPi3 and RPi4, and I am currently not able to test it. If it's not too much trouble, I'd appreciate a Tested-by. If not, I'll test it myself, but it may take me a while to get around to it. Thanks, > ArmPlatformPkg/PrePi/PeiMPCore.inf | 1 + > ArmPlatformPkg/PrePi/PeiUniCore.inf | 1 + > ArmPlatformPkg/PrePi/PrePi.c | 8 +++++--- > 3 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf > index 9c5da0d42a7b..053f9fd9e616 100644 > --- a/ArmPlatformPkg/PrePi/PeiMPCore.inf > +++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf > @@ -37,6 +37,7 @@ [Packages] > > [LibraryClasses] > BaseLib > + CacheMaintenanceLib > DebugLib > DebugAgentLib > ArmLib > diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf > index ee9b05b25337..78d218ae09ca 100644 > --- a/ArmPlatformPkg/PrePi/PeiUniCore.inf > +++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf > @@ -37,6 +37,7 @@ [Packages] > > [LibraryClasses] > BaseLib > + CacheMaintenanceLib > DebugLib > DebugAgentLib > ArmLib > diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c > index 2bb144958139..254fb331733e 100644 > --- a/ArmPlatformPkg/PrePi/PrePi.c > +++ b/ArmPlatformPkg/PrePi/PrePi.c > @@ -8,6 +8,7 @@ > > #include > > +#include > #include > #include > #include > @@ -178,8 +179,6 @@ CEntryPoint ( > > // Data Cache enabled on Primary core when MMU is enabled. > ArmDisableDataCache (); > - // Invalidate Data cache > - ArmInvalidateDataCache (); > // Invalidate instruction cache > ArmInvalidateInstructionCache (); > // Enable Instruction Caches on all cores. > @@ -200,6 +199,10 @@ CEntryPoint ( > > // If not primary Jump to Secondary Main > if (ArmPlatformIsPrimaryCore (MpId)) { > + > + InvalidateDataCacheRange ((VOID *)UefiMemoryBase, > + FixedPcdGet32(PcdSystemMemoryUefiRegionSize)); > + > // Goto primary Main. > PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp); > } else { > @@ -209,4 +212,3 @@ CEntryPoint ( > // DXE Core should always load and never return > ASSERT (FALSE); > } > - > -- > 2.20.1 >