From patchwork Sat Jun 24 08:23:52 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dongjiu Geng X-Patchwork-Id: 9807737 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0658660382 for ; Sat, 24 Jun 2017 08:25:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E651B28735 for ; Sat, 24 Jun 2017 08:25:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DA8942873C; Sat, 24 Jun 2017 08:25:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 2578C28735 for ; Sat, 24 Jun 2017 08:25:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=TBp71e0ujg7C/JVbed5RA+U2DB+DYwUfA/ksPvwxTPU=; b=V9zD7ihc5mOylB 0X3xggZyC/xoqQRpg9i3MohCj0CCsj/QHnEia5Un4dmmHg1v+REcBeD2CM5I/oYjEszAEMytW61/Z MyHGuWBosXxpBT08QtL9qzxWRRa+yFEMWKnUdn8Smif1lY1MVIEfKQ36FChB8C3am15E48eU6RShv HvKjNNRvN9cRbGd8i5KGHko49Iw8DAvkzIyw9p/5YstgIfhpvhClPS05QHlijynh0pDhiPHTodAnA tl4/r6xMij6NjIE/mdpCLV74ZXNYlgvCtC+6qe/nu4hYRZggx8CFSD7UFvgN05uTaheCd0tAFGHIB cP8mEzjwi9JnFxzbGpVw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dOgNe-0003ZX-AH; Sat, 24 Jun 2017 08:25:22 +0000 Received: from szxga01-in.huawei.com ([45.249.212.187]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dOgNa-0002MD-9r for linux-arm-kernel@lists.infradead.org; Sat, 24 Jun 2017 08:25:20 +0000 Received: from 172.30.72.53 (EHLO dggeml406-hub.china.huawei.com) ([172.30.72.53]) by dggrg01-dlp.huawei.com (MOS 4.4.6-GA FastPath queued) with ESMTP id AQW79709; Sat, 24 Jun 2017 16:24:04 +0800 (CST) Received: from [127.0.0.1] (10.142.68.147) by dggeml406-hub.china.huawei.com (10.3.17.50) with Microsoft SMTP Server id 14.3.301.0; Sat, 24 Jun 2017 16:23:54 +0800 Subject: Re: [PATCH v2] KVM: arm/arm64: Signal SIGBUS when stage2 discovers hwpoison memory To: James Morse References: <20170524163250.29281-1-james.morse@arm.com> <3dbafd74-57f2-e724-ace2-0f84abb57e59@huawei.com> <594A4205.70201@arm.com> <3a02e425-5782-dad4-efda-fdc5df73dcf7@huawei.com> <594A6A45.60909@arm.com> <594CE185.60902@arm.com> From: gengdongjiu Message-ID: <9fa76f8d-c360-9fcc-29f6-4bef6fbf85c2@huawei.com> Date: Sat, 24 Jun 2017 16:23:52 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <594CE185.60902@arm.com> X-Originating-IP: [10.142.68.147] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020201.594E21A5.0043, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 4cc7ae5d3ce5d21bacab9ff51b24ca20 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170624_012518_900235_C52C849F X-CRM114-Status: GOOD ( 21.61 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: wuquanming , Marc Zyngier , Achin Gupta , Punit Agrawal , Tyler Baicar , Huangshaoyu , linux-arm-kernel@lists.infradead.org, huhuajun , kvmarm@lists.cs.columbia.edu, Christoffer Dall Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Hi James, sorry for the late response. On 2017/6/23 17:38, James Morse wrote: > Hi gengdongjiu, > > I've spotted where we are disagreeing: there are two bits in the ESR relevant to > external aborts. I've been treating them the same. Ok. > > It looks like KVM is testing the wrong bit. I agree with you the KVM should not test this bit, because it is IMPLEMENTATION DEFINED classification. from this patch history, it is made by Marc. do know whether it has some special reason to make this patch. commit 4055710baca8cb067b059a635cf851eb86410a83 Author: Marc Zyngier Date: Tue Sep 6 14:02:15 2016 +0100 arm/arm64: KVM: Inject virtual abort when guest exits on external abort If we spot a data abort bearing the ESR_EL2.EA bit set, we know that this is an external abort, and that should be punished by the injection of an abort. Signed-off-by: Marc Zyngier Signed-off-by: Christoffer Dall > > > On 22/06/17 07:47, gengdongjiu wrote: >> On 2017/6/21 20:44, James Morse wrote: >>> I think we are looking at different parts of the code, here is what I see should >>> happen: >>> >>> For a Synchronous External Abort the ESR {D,I}FSC bits will be in the range that >>> indicates an external abort. For a data:external-abort kvm_handle_guest_abort() >>> will matches this with kvm_vcpu_dabt_isextabt() and makes no further attempt to >>> handle the fault. >> The kvm_vcpu_dabt_isextabt() will check the ESR EA bit[9] >> The EA bit [9] is "IMPLEMENTATION DEFINED", so whether match with the kvm_vcpu_dabt_isextabt() depend on the CPU design. >> In my platform the EA bit is zero for the RAS Synchronous External Abort, so will not matches the kvm_vcpu_dabt_isextabt(), so the code will continue handle the fault. >> >> >> static inline bool kvm_vcpu_dabt_isextabt(struct kvm_vcpu *vcpu) >> { >> return kvm_vcpu_get_hsr(vcpu) & HSR_DABT_EA; >> } >> >> EA, bit [9] >> External Abort. As described in the Architecture Reference Manual. Provides an IMPLEMENTATION >> DEFINED classification of the external abort. > > (and here you pointed it out, sorry I missed it). > > This bit 9 isn't the same as the {I,D}FSC bit 4, which is also always set for an > external abort. yes, they are two different bits > > >> DFSC, bits [5:0], when synchronous external Data Abort >> Data Fault Status Code. The possible values of this field are: >> 0b010000 Synchronous External Abort on memory access. >> 0b0101xx Synchronous External Abort on translation table walk or hardware update of translation >> table. DFSC[1:0] encode the level. >> Further values are described in the Architecture Reference Manual. The Parity Error codes are not >> used and reserved in the RAS extension. >> IFSC, bits [5:0], when synchronous external Instruction Abort >> Instruction Fault Status Code. The possible values of this field are: >> 0b010000 Synchronous External Abort on memory access. >> 0b0101xx Synchronous External Abort on translation table walk or hardware update of translation >> table. IFSC[1:0] encode the level. >> Further values are described in the Architecture Reference Manual. The Parity Error codes are not >> used and reserved in the RAS extension. > > > So the v8 ARM-ARM has two ways of indicating an external abort, KVM tests bit 9. > The v7 ARM-ARm has a '{D,I}FSR.ExT' bit, which I think is equivalent. Its > described as: >> An implementation can use the DFSR.ExT and IFSR.ExT bits to provide more >> information about external aborts. > > This is what the v8 version must mean with its >> External abort type. This bit can provide an IMPLEMENTATION DEFINED >> classification of External aborts. > > Which I read as IMP-DEF classification 'as external', as opposed to your reading > as an extra IMP-DEF classification for external aborts. OK. > > It looks like an implementation could use this to mean 'how external'. For RAS > an implementation could use it to separate external aborts handled first by > firmware, from those generated directly by the CPU. for the armv8.0 CPU which does not have RAS feature, also use the EA bit, so may not directly use it separate whether external aborts should be handled first by firmware > > So which bits should Linux test? Bit 9 has some extra IMP-DEF meaning, so Linux > should never look at it, which means KVM has a bug handling these. what do you want Linux to test? to test the type of error? check the {I,D}FSC should be OK. > I will send a patch, are you able to review and test it? sure, it is no problem. > > > Thanks! > > James > > . > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 344755d..60e0c1a 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -1432,6 +1432,11 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run) int ret, idx; is_iabt = kvm_vcpu_trap_is_iabt(vcpu); + if (unlikely(!is_iabt && kvm_vcpu_dabt_isextabt(vcpu))) { + kvm_inject_vabt(vcpu); + return 1; + } + fault_ipa = kvm_vcpu_get_fault_ipa(vcpu); trace_kvm_guest_fault(*vcpu_pc(vcpu), kvm_vcpu_get_hsr(vcpu)