From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web11.548.1582677883074415540 for ; Tue, 25 Feb 2020 16:44:43 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=crKQfEzK; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1582677882; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nOM/B/SeGghvHQ4OLqRlohKdYX5OkO3HJpn11flXDT8=; b=crKQfEzKc0y/hv1OcGdq67Pc4dz9JCaFwVufAiOfSq7Y+IO/1EdDtXSba9cU0W9S1a3oNP DNH3PwzHxATyJoKP2OM2Fa9/UjmE9czOkGoK+pNc+DIp5/6z6r2kWbpQ+cBtoN4zxNOSZW ajb081CP1Pcq9jy6kt7zDVQoGX4oJQs= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-333-B2VSXswuPMiflhW6UlNcgQ-1; Tue, 25 Feb 2020 19:44:38 -0500 X-MC-Unique: B2VSXswuPMiflhW6UlNcgQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2FC9A8018A5; Wed, 26 Feb 2020 00:44:37 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-104.ams2.redhat.com [10.36.117.104]) by smtp.corp.redhat.com (Postfix) with ESMTP id CA2501BC6D; Wed, 26 Feb 2020 00:44:35 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 1/1] ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones To: devel@edk2.groups.io, ard.biesheuvel@linaro.org Cc: leif@nuviainc.com, pete@akeo.ie References: <20200225182834.19380-1-ard.biesheuvel@linaro.org> From: "Laszlo Ersek" Message-ID: <365f5f63-0259-6000-7444-8854e91f18e4@redhat.com> Date: Wed, 26 Feb 2020 01:44:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200225182834.19380-1-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 02/25/20 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 > --- > 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); > } > - > Acked-by: Laszlo Ersek