From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by mx.groups.io with SMTP id smtpd.web09.10196.1583323828505319424 for ; Wed, 04 Mar 2020 04:10:29 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=rVQ0F8J+; spf=pass (domain: nuviainc.com, ip: 209.85.221.65, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f65.google.com with SMTP id j7so2071661wrp.13 for ; Wed, 04 Mar 2020 04:10:28 -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=x1stD2/+KmHmYLp96N/HMNY3EpYEMRC2IuUrftA6ktk=; b=rVQ0F8J+aMKcEMyT337MD293LMZ+CByz4sDEMP7WdvTwwQL3jf30RlcfBzoQxhx4FC P1vMVZuDno28lURetzEUu++QdH3Iiz5SFysZre41Tm0RSbnRI0LmlQSwfpdMm26oFWhS viSb184hlwmtiaGhLmsFmhhmVm4+4wsJGQGUKD7P3f0eYnAsISbYb69VMag4eIj0V2sj Qdj+3qBmJOhXDqhCJdZozwwCTJVttQAxXKtt2ldQg8AlKP6Bqsx/WCq5agNKqh5HqNXh XSeNGhffCYQjwIXovsocnqGrVQCM+etVdthbfBqk2ysXVYBTEKIM3j8UrqA8gVBCJvlD IEtg== 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=x1stD2/+KmHmYLp96N/HMNY3EpYEMRC2IuUrftA6ktk=; b=Xxd9RaThHW89oUoYMpqAP6I+dScQa99D0xpKW+9956F2xXojvaAjcG3sgcRoh9YGWf u1N9b88ZONoLi/m/4Nvs0ZNb9e/pldjUyzjmDpk11h6qxYSaqfd7w16g287VukMhWqsm YzK6tB7hufRZ3fD5Ef1QLi0BvgtVG3fASZ4tTbOsP9IHghbc7WaihkSOLDSQvzG57x9A myuX70VZ5RramsmDqaTTaX0Oj0WTUUVXaL6/ubliaajn8K/xQI4RbX6pyCTpt6Ee55Qt XIBVUM62ygoLdIk0S3QG/gSnvrZsN6UUCC9e8zvc/JXuIheBPI0zX9UXoxuUHblp8aZt dbZQ== X-Gm-Message-State: ANhLgQ3+xayj7P2Et3WBl3DrNsGXdkMh7wst64MeCyBspaDyBQd7/0BT XacQpGP0EHl7iAOG+CWu2WeriQ== X-Google-Smtp-Source: ADFU+vvGlPELeiBLU5KgjQc9WVcKDIdeEuqaZrOUxO05v08GNJML1sDRJcxBHtLaH1VllOCLZx7Z0Q== X-Received: by 2002:a5d:628e:: with SMTP id k14mr3686859wru.425.1583323826879; Wed, 04 Mar 2020 04:10:26 -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 i7sm23625353wro.87.2020.03.04.04.10.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Mar 2020 04:10:26 -0800 (PST) Date: Wed, 4 Mar 2020 12:10:24 +0000 From: "Leif Lindholm" To: Ard Biesheuvel Cc: edk2-devel-groups-io , Laszlo Ersek , Sami Mujawar Subject: Re: [edk2-devel] [PATCH 3/6] ArmPkg/ArmMmuLib ARM: cache-invalidate initial page table entries Message-ID: <20200304121024.GS23627@bivouac.eciton.net> References: <20200226100353.31962-1-ard.biesheuvel@linaro.org> <20200226100353.31962-4-ard.biesheuvel@linaro.org> <20200302122522.GF23627@bivouac.eciton.net> <20200302131011.GI23627@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Mar 02, 2020 at 14:15:58 +0100, Ard Biesheuvel wrote: > > > > > @@ -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++); > > > > > + > > > > > > > > Since the sequence is somewhat conterintuitive, could we add a comment > > > > to the extent that // Force subsequent acces to fetch from main memory? > > > > > > The barrier is there to ensure that the write made it to meain memory, > > > so we could actually relax this to a DMB. > > > > If there's no risk there could be a stale entry for that line (i.e., > > D-cache has not been enabled since reset). Otherwise, I *think* there > > could be a potential race condition in v7. > > I don't follow. Getting rid of stale, clean cachelines is the whole > point. But to ensure that a speculative fetch doesn't fetch the same > stale value again, we need to order the invalidate wrt the store. What I'm getting at is that *some* v7 platforms don't use TF-A, and some of those could have run with caches enabled before ever going into EDK2. Since v7 architecture says: --- When a cache is disabled for a type of access, for a particular translation regime: - it is IMPLEMENTATION DEFINED whether a cache hit occurs if a location that is held in the cache is accessed by that type of access. - any location that is not held in the cache is not brought into the cache as a result of a memory access of that type. --- Which I *think* means that if there *was* a stale entry in the D-cache for that location, we need to ensure that entry is gone before we do the write, or we risk it going missing. Race condition was an inaccurate way to describe this, but I had forgotten about the second bullet point. > > > > Obnoxious question: do we need another DSB here? Or are we reasonably > > > > guaranteed that one will appear in the instruction stream between here > > > > and anything else that would touch the same line? > > > > > > The MMU enable will issue a DSB to ensure that all the cache > > > invalidations have completed. > > > > And that happens on our return path from here? > > If so, fine. > > It happens later. But remember that all of this code runs with the MMU > and caches off. We are just ensuring that our page table changes are > not shadowed in the PTW's view by clean but stale cachelines when we > turn on the MMU a bit later. Second bullet point again. Ignore me. / Leif