From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by mx.groups.io with SMTP id smtpd.web09.830.1583425391376205541 for ; Thu, 05 Mar 2020 08:23:11 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=TUvFcxdQ; spf=pass (domain: nuviainc.com, ip: 209.85.128.65, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f65.google.com with SMTP id i9so7019153wml.4 for ; Thu, 05 Mar 2020 08:23:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=anHL68kxquLnpFufnLq3kNE8fBV01/0nrE3E1IN0p4s=; b=TUvFcxdQbLUQhG9r6L1cGUJKA1YNROO4+tUvAMboKtiqtlQrOO9KD6LHj6eaifbZaX T4LfuSTXj3m5dekXu3tD3Iv+LPwfny9nysyNzW5atK+YZETmxhRMaFYkEKysT0a1qPmk wpIKcUg5q8m5rxWYEmoLnplkdNFG34kM+dYlHy7ZOqRdEz+DIEFDoPL262tmY0XxHZ25 XCfwRVnU9wcJpNuydZKh9HA56L1rbHN9cO+bURiVWS3t/bp69T10gZrEeyBgrsVwKtQu kSlHRZpC71nQ7GbxJciRo4id19OrDMpYT53pyxgMYd3wb2sCbXsypxillBR0zhL40S38 rxZA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=anHL68kxquLnpFufnLq3kNE8fBV01/0nrE3E1IN0p4s=; b=fPKm8UdMqpXsmGeWaBW2aGirlQxREIs6jemWKVhnWd8/pfZDCUjzYmdNs8fUHIkWD4 mIVIUtknPQWXPWbmEerAaEaNEy6rcug7zdiSBHaf1lUwADTFC3NKLQ6GTzE17WI+FNvb N5QEFOHnhRKjqpPb9ODcQpQIN4Q65GPNbc7BWbTweear1SxPUpazeEH3Ng+RFjJKRKZR VsmoSqTo2yBDV6F2+2y9v25hqwY01kjGvolrYlRn3SbbxbJm1e0KbcnBkkSQQotJwZQ/ /Nqnk3blD1Pkyfs0uFP6O2IzwmRDgW36Eb23/Snu86ZVPMXTCsP16F/Lu7xw1ynYceGj DGfg== X-Gm-Message-State: ANhLgQ1JiF9nLalb2syLVgCokLydnlWGaOQPY2jKqBjeOuezGIk+aQtD IJ5zsDMNBZqBVSNVJdot7ukHpA== X-Google-Smtp-Source: ADFU+vuI5ZDVwXeiX+sKLPBQFg8xXbdtwJpoSpCTrpedm0upcb/Cxdd+HdKiovhf3YIqIZut24IvwQ== X-Received: by 2002:a7b:c857:: with SMTP id c23mr10562027wml.68.1583425389803; Thu, 05 Mar 2020 08:23:09 -0800 (PST) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id z2sm40591918wrq.95.2020.03.05.08.23.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Mar 2020 08:23:09 -0800 (PST) Date: Thu, 5 Mar 2020 16:23:07 +0000 From: "Leif Lindholm" To: Ard Biesheuvel Cc: devel@edk2.groups.io Subject: Re: [PATCH v2 1/9] ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones Message-ID: <20200305162307.GZ23627@bivouac.eciton.net> References: <20200304181246.23513-1-ard.biesheuvel@linaro.org> <20200304181246.23513-2-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20200304181246.23513-2-ard.biesheuvel@linaro.org> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Mar 04, 2020 at 19:12:38 +0100, 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 > Acked-by: Laszlo Ersek > Tested-By: Pete Batard > --- > 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)); Space before that (. With that: Reviewed-by: Leif Lindholm > + > // Goto primary Main. > PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp); > } else { > @@ -209,4 +212,3 @@ CEntryPoint ( > // DXE Core should always load and never return > ASSERT (FALSE); > } > - > -- > 2.17.1 >