From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8A18BC4321E for ; Thu, 1 Dec 2022 18:17:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230228AbiLASRe (ORCPT ); Thu, 1 Dec 2022 13:17:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43380 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230379AbiLASRT (ORCPT ); Thu, 1 Dec 2022 13:17:19 -0500 X-Greylist: delayed 338 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Thu, 01 Dec 2022 10:17:17 PST Received: from out-93.mta0.migadu.com (out-93.mta0.migadu.com [IPv6:2001:41d0:1004:224b::5d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E1F761A07E for ; Thu, 1 Dec 2022 10:17:17 -0800 (PST) Date: Thu, 1 Dec 2022 18:11:33 +0000 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Oliver Upton To: Ricardo Koller Cc: Marc Zyngier , James Morse , Alexandru Elisei , Suzuki K Poulose , Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/4] KVM: arm64: Don't serialize if the access flag isn't set Message-ID: References: <20221129191946.1735662-1-oliver.upton@linux.dev> <20221129191946.1735662-3-oliver.upton@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 29, 2022 at 09:15:21PM +0000, Oliver Upton wrote: > Hi Ricardo, > > Thanks for having a look. > > On Tue, Nov 29, 2022 at 12:52:12PM -0800, Ricardo Koller wrote: > > On Tue, Nov 29, 2022 at 07:19:44PM +0000, Oliver Upton wrote: > > [...] > > > > + ret = stage2_update_leaf_attrs(pgt, addr, 1, KVM_PTE_LEAF_ATTR_LO_S2_AF, 0, > > > + &pte, NULL, 0); > > > + if (!ret) > > > + dsb(ishst); > > > > At the moment, the only reason for stage2_update_leaf_attrs() to not > > update the PTE is if it's not valid: > > > > if (!kvm_pte_valid(pte)) > > return 0; > > > > I guess you could check that as well: > > > > + if (!ret || kvm_pte_valid(pte)) > > + dsb(ishst); > > Thanks for catching this. > > Instead of pivoting on the returned PTE value, how about we return > -EAGAIN from the early return in stage2_attr_walker()? It would better > match the pattern used elsewhere in the pgtable code. Bugh... Returning EAGAIN has some unfortunate consequences that I've missed until now... The stage2 attr walker is used to handle faults as well as range-based operations. In the former case, EAGAIN is sane as we retry execution but the latter is not. I stupidly got hung up on write protection not working as intended for some time. I think that callers into the page table walker should indicate whether or not the walk is to address a fault. If it is not, __kvm_pgtable_visit() and __kvm_pgtable_walk() should chug along instead of bailing for EAGAIN. Let me mess around with this and figure out what is least ugly. -- Thanks, Oliver