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 X-Spam-Level: X-Spam-Status: No, score=-14.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C1F0BC433E6 for ; Mon, 18 Jan 2021 13:45:47 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 778922225C for ; Mon, 18 Jan 2021 13:45:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 778922225C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:References:In-Reply-To:Subject:To:From: Date:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=ODIxi/rmILnyTeaGRMruc5CJCp7dn3lIACyCTF5u+x0=; b=dSnzOZfBQwQ2X1UI6Clz6REmv YQoUn6N4keA0V71McUhFv+gqD3zHCOozPu7Ev85btUZW9fewqsmxunHhFQ09AXiRb17FNRnGLBg+T fJNGekhBq1g4hrXBxKxPPC7cz7m4g6EG5722VL3/LH8Q6afJ/fHGytSFIMFM3Nc+w/ybgSXd4wyjo m1hOE7viG5l5S2bEEzT47QN++q/S5cpzKFR6IY+w9t1GK+8WQ4WcJA0GUx4zzjoYLgQaaHTVBZBgQ v43mJ14GvjrS3ms/sfbVKQvak7yj52vNRAKlsUKY46Nt9eGxVUrjXkGw1uPE2mlVvzLGfKTqd+arH A+KYG5tPw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l1UpI-0003lM-FA; Mon, 18 Jan 2021 13:44:12 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1l1UpE-0003kh-Pe for linux-arm-kernel@lists.infradead.org; Mon, 18 Jan 2021 13:44:10 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B68C420E65; Mon, 18 Jan 2021 13:44:07 +0000 (UTC) Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94) (envelope-from ) id 1l1UpB-008VJT-Ih; Mon, 18 Jan 2021 13:44:05 +0000 MIME-Version: 1.0 Date: Mon, 18 Jan 2021 13:44:05 +0000 From: Marc Zyngier To: Jianyong Wu Subject: Re: [PATCH] arm64/kvm: correct the error report in kvm_handle_guest_abort In-Reply-To: References: <20210115093028.6504-1-jianyong.wu@arm.com> <6f5a2ce458e33f879635f45140293517@kernel.org> <31dd6e2f8b3980337c8093d2ab626636@kernel.org> User-Agent: Roundcube Webmail/1.4.10 Message-ID: X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: Jianyong.Wu@arm.com, Justin.He@arm.com, nd@arm.com, will@kernel.org, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210118_084408_969521_3795C8D6 X-CRM114-Status: GOOD ( 36.34 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: will@kernel.org, nd , Justin He , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2021-01-18 13:38, Jianyong Wu wrote: >> -----Original Message----- >> From: Marc Zyngier >> Sent: Monday, January 18, 2021 9:26 PM >> To: Jianyong Wu >> Cc: Justin He ; nd ; will@kernel.org; >> kvmarm@lists.cs.columbia.edu; linux-arm-kernel@lists.infradead.org >> Subject: Re: [PATCH] arm64/kvm: correct the error report in >> kvm_handle_guest_abort >> >> On 2021-01-18 13:04, Jianyong Wu wrote: >> > Hi Marc, >> > >> >> -----Original Message----- >> >> From: kvmarm-bounces@lists.cs.columbia.edu > >> bounces@lists.cs.columbia.edu> On Behalf Of Jianyong Wu >> >> Sent: Saturday, January 16, 2021 4:47 PM >> >> To: Marc Zyngier >> >> Cc: Justin He ; nd ; will@kernel.org; >> >> kvmarm@lists.cs.columbia.edu; linux-arm-kernel@lists.infradead.org >> >> Subject: RE: [PATCH] arm64/kvm: correct the error report in >> >> kvm_handle_guest_abort >> >> >> >> Hi Marc, >> >> >> >> > -----Original Message----- >> >> > From: Marc Zyngier >> >> > Sent: Friday, January 15, 2021 7:21 PM >> >> > To: Jianyong Wu >> >> > Cc: James Morse ; will@kernel.org; Suzuki >> >> Poulose >> >> > ; linux-arm-kernel@lists.infradead.org; >> >> > kvmarm@lists.cs.columbia.edu; Steve Capper >> ; >> >> > Justin He ; nd >> >> > Subject: Re: [PATCH] arm64/kvm: correct the error report in >> >> > kvm_handle_guest_abort >> >> > >> >> > On 2021-01-15 09:30, Jianyong Wu wrote: >> >> > > Currently, error report when cache maintenance at read-only >> >> > > memory range, like rom, is not clear enough and even not correct. >> >> > > As the specific error is definitely known by kvm, it is obliged >> >> > > to give it out. >> >> > > >> >> > > Fox example, in a qemu/kvm VM, if the guest do dc at the pflash >> >> > > range from >> >> > > 0 to 128M, error is reported by kvm as "Data abort outside >> >> > > memslots with no valid syndrome info" which is not quite correct. >> >> > > >> >> > > Signed-off-by: Jianyong Wu >> >> > > --- >> >> > > arch/arm64/kvm/mmu.c | 12 +++++++++--- >> >> > > 1 file changed, 9 insertions(+), 3 deletions(-) >> >> > > >> >> > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index >> >> > > 7d2257cc5438..de66b7e38a5b 100644 >> >> > > --- a/arch/arm64/kvm/mmu.c >> >> > > +++ b/arch/arm64/kvm/mmu.c >> >> > > @@ -1022,9 +1022,15 @@ int kvm_handle_guest_abort(struct >> kvm_vcpu >> >> > > *vcpu) >> >> > > * So let's assume that the guest is just being >> >> > > * cautious, and skip the instruction. >> >> > > */ >> >> > > - if (kvm_is_error_hva(hva) && kvm_vcpu_dabt_is_cm(vcpu)) >> >> > { >> >> > > - kvm_incr_pc(vcpu); >> >> > > - ret = 1; >> >> > > + if (kvm_vcpu_dabt_is_cm(vcpu)) { >> >> > > + if (kvm_is_error_hva(hva)) { >> >> > > + kvm_incr_pc(vcpu); >> >> > > + ret = 1; >> >> > > + goto out_unlock; >> >> > > + } >> >> > > + >> >> > > + kvm_err("Do cache maintenance in the read- >> only >> >> > memory range\n"); >> >> > >> >> > We don't scream on the console for guests bugs. >> >> Ok >> >> >> >> > >> >> > > + ret = -EFAULT; >> >> > > goto out_unlock; >> >> > > } >> >> > >> >> > And what is userspace going to do with that? To be honest, I'd >> >> > rather not report it in any case: >> >> > >> >> > - either it isn't mapped, and there is no cache to clean/invalidate >> >> > - or it is mapped read-only: >> >> > - if it is a "DC IVAC", the guest should get the fault as per >> >> > the ARM ARM. But I don't think we can identify the particular CMO >> >> > at this stage, so actually performing an invalidation is the least >> >> > bad thing to do. >> >> > >> >> > How about this (untested)? >> > >> > I have tested for this. It works that DC ops can pass on memory range >> > for rom. But there is performance issue. It takes too long a time that >> > do DC on rom range compared with on normal memory range. Here is >> some >> > data: >> > Ops memory type size >> > time >> > dc civac rom memory 128M >> > 6700ms; >> > dc civac writable normal memory 128M >> > 300ms; >> > >> > It's a single thread test and may be worse on multi thread. I'm not >> > sure we can bear it. WDYT? >> >> The problem is that the guest is invalidating one cache-line at a >> time, but we >> invalidate 128M at a time in your example. >> >> I would say that I really don't care how slow it is. We cannot know >> which >> address the guest is trying to invalidate (as your commit message >> shows, >> there is no syndrome information available). >> >> So it seems our only choices are: >> - don't do any invalidation, which is likely to break the guest >> - invalidate everything, always >> >> Given that, I'd rather have a slow guest. Also, it very much looks >> like no >> existing SW does this, so I cannot say I care much. > > OK, get it. Actually, there could be a way to make this a bit faster. Once we have cleaned+invalidated the memslot, we could unmap it, speeding up the following cache invalidations (nothing will be mapped). Could you please share your test case? Thanks, M. -- Jazz is not dead. It just smells funny... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel