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 mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by smtp.lore.kernel.org (Postfix) with ESMTP id B0494C43217 for ; Thu, 1 Dec 2022 18:11:43 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 1EAA149E34; Thu, 1 Dec 2022 13:11:43 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xQqMYD2+shBz; Thu, 1 Dec 2022 13:11:41 -0500 (EST) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id F0BE8408C1; Thu, 1 Dec 2022 13:11:41 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 7B2B2408C1 for ; Thu, 1 Dec 2022 13:11:40 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id sK7nGQr6-NJW for ; Thu, 1 Dec 2022 13:11:39 -0500 (EST) Received: from out-71.mta0.migadu.com (out-71.mta0.migadu.com [91.218.175.71]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 58C944089A for ; Thu, 1 Dec 2022 13:11:39 -0500 (EST) 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 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-Disposition: inline In-Reply-To: Cc: kvm@vger.kernel.org, Marc Zyngier , linux-kernel@vger.kernel.org, Catalin Marinas , kvmarm@lists.linux.dev, Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu 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 _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-240.mta0.migadu.com (out-240.mta0.migadu.com [91.218.175.240]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 493E7A20 for ; Thu, 1 Dec 2022 19:53:20 +0000 (UTC) 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> Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Message-ID: <20221201181133.DPe50bB4nzy4nixUT9LOIGE0bCoRqJzT5DybWbVZ5VM@z> 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