From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::144; helo=mail-it1-x144.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x144.google.com (mail-it1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6187A21BADAB2 for ; Tue, 29 Jan 2019 02:32:58 -0800 (PST) Received: by mail-it1-x144.google.com with SMTP id h193so3646167ita.5 for ; Tue, 29 Jan 2019 02:32:58 -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=uu8cr54eSDxhezBDYEP4g7ZozENpd9T7yXukYr4TX+0=; b=haRWZ4bL5SSMn6rEOgYcxUb2gUESpQCoE5jn/MabxIlNGVne+aX2W2iy5NL2xG9UM5 2UYk9j2GPayfqlFPv6PABwaH0StZTG/ee0f+VUo1MHLOF9Vens0nLucapPvwxAMAS4gn kdoNj6iwrRlgvfmTSyIJ+nL8MfvQ6SQ/HYJeQ= 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=uu8cr54eSDxhezBDYEP4g7ZozENpd9T7yXukYr4TX+0=; b=mBIBL8MiD/KOzYte2yrtssi1X2N1LCmwm/lTedEUG5KfDfCD3RMyW+CDxZi+GB1n8G 4VxI9X4r9jRJpCya3gHCcBSOLtCZx7Kz6zNlLhCUA1Hn5069foZteAFYen2o+opq6JHW kRg/8eNWOMJvtsE/2Nh/hOZJUzp0VAn6Fd9lYWAWMHioCh9sKRmFG0qof+clD9FVes0M w2eenjFG9ckmbopi5oKSwNKa9SkKm6gFEDk6kw5sGLtj4vMOF6PflCh/02rYSz8BTzIp eTl7OB1/4wvDb5Mr2tWcC0nxNpwzxMT1wzloYyiU9UP54735EVHWHiNRPnXng2Vl9J02 er4g== X-Gm-Message-State: AJcUukfdwbvLPG6pAGEn2Celzcem7eOhn9DNyDe/W+w2O7eaiFAmgtxV lOWEZXU5eEEYICNj11ALpSbMTyRqeGSd5Ov6g6zRGA== X-Google-Smtp-Source: ALg8bN4/FCMi9eu84lTuGY2y3v1rXC1FNn2S8VwHZHawQ0jUZv6OJh7eEYVEPJkXG8uTZFUF/GfrRpZoY4qdjrAy81c= X-Received: by 2002:a24:1d1:: with SMTP id 200mr12161003itk.146.1548757977348; Tue, 29 Jan 2019 02:32:57 -0800 (PST) MIME-Version: 1.0 References: <20190107071504.2431-1-ard.biesheuvel@linaro.org> <20190107071504.2431-3-ard.biesheuvel@linaro.org> <20190123154622.pibw6mi5gh6ywb26@bivouac.eciton.net> <20190123161257.nydm3s5c27oibn6e@bivouac.eciton.net> <20190123162026.i3id7zabzmo52sin@bivouac.eciton.net> <20190128180102.et36pybogdptwy25@bivouac.eciton.net> In-Reply-To: <20190128180102.et36pybogdptwy25@bivouac.eciton.net> From: Ard Biesheuvel Date: Tue, 29 Jan 2019 11:32:46 +0100 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" Subject: Re: [PATCH 2/5] ArmPkg/ArmMmuLib AARCH64: get rid of needless TLB invalidation X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Jan 2019 10:32:58 -0000 Content-Type: text/plain; charset="UTF-8" On Mon, 28 Jan 2019 at 19:01, Leif Lindholm wrote: > > On Mon, Jan 28, 2019 at 01:29:54PM +0100, Ard Biesheuvel wrote: > > > > > > > > @@ -296,7 +297,8 @@ GetBlockEntryListFromAddress ( > > > > > > > > > > > > > > > > // Fill the BlockEntry with the new TranslationTable > > > > > > > > ReplaceLiveEntry (BlockEntry, > > > > > > > > - ((UINTN)TranslationTable & TT_ADDRESS_MASK_DESCRIPTION_TABLE) | TableAttributes | TT_TYPE_TABLE_ENTRY); > > > > > > > > + (UINTN)TranslationTable | TableAttributes | TT_TYPE_TABLE_ENTRY, > > > > > > > > + RegionStart); > > > > > > > > > > > > > > > > > > > /me pages in the data ... > > > > > > > > > > > > > OK, this whole patch took a few times around the loop before I think I > > > > > > > caught on what was happening. > > > > > > > > > > > > > > I think I'm down to the only things confusing me being: > > > > > > > - The name "Address" to refer to something that is always the start > > > > > > > address of a 4KB-aligned translation region. > > > > > > > Is this because the function will be usable in other contexts in > > > > > > > later patches? > > > > > > > > > > > > I could change it to VirtualAddress if you prefer. > > > > > > It is only passed > > > > > > for TLB maintenance which is only needed at page granularity, and the > > > > > > low bits are shifted out anyway. > > > > > > > > > > Yeah, exactly. It would just be nice if the name reflected that. Not > > > > > sure VirtualAddress does. VirtualBase? PageBase? > > > > > > > > > > > > > Well, the argument passed in is called RegionStart, so let's just > > > > stick with that. > > > > > > Sure. With that: > > > Reviewed-by: Leif Lindholm > > > > > > > Thanks > > > > GIven the discussion in the other thread regarding shareability > > upgrades of barriers and TLB maintenance instructions when running > > under virt, mind if I fold in the hunk below? (and add a mention to > > the commit log) > > > > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S > > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S > > @@ -35,7 +35,7 @@ > > // flush translations for the target address from the TLBs > > lsr x2, x2, #12 > > tlbi vae\el, x2 > > - dsb sy > > + dsb nsh > > So, this one, definitely. MMU is off, shareability is a shady concept > at best, so it's arguably a fix. > > > // re-enable the MMU > > msr sctlr_el\el, x8 > > @@ -58,7 +58,7 @@ ASM_FUNC(ArmReplaceLiveTranslationEntry) > > // clean and invalidate first so that we don't clobber > > // adjacent entries that are dirty in the caches > > dc civac, x0 > > - dsb ish > > + dsb nsh > > This one I guess is safe because we know we're never going to get > pre-empted or migrated (because interrupts are disabled and we don't > do that sort of thing)? > Well, we can only get pre-empted under virt, in which case HCR_EL2.BSU will be set so that barriers are upgraded to inner-shareable. In bare metal cases, we only care about the local CPU since there are no other masters (nor DMA capable devices) that care about this cache maintenance having completed since the page tables are used by the CPU only. > If that's the rationale: > Reviewed-by: Leif Lindholm > Thanks Pushed as f34b38fae614..d5788777bcc7