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=-11.3 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 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 3AC86C433DF for ; Mon, 27 Jul 2020 10:31:09 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id B76D82075A for ; Mon, 27 Jul 2020 10:31:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="zOnWcLJT" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B76D82075A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 5E5714B9F8; Mon, 27 Jul 2020 06:31:08 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@kernel.org 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 mUQY-2FEb3Gr; Mon, 27 Jul 2020 06:31:07 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 2E14D4B9C8; Mon, 27 Jul 2020 06:31:07 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id F2A9B4B9BF for ; Mon, 27 Jul 2020 06:31:05 -0400 (EDT) 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 3CMtyOCgW78V for ; Mon, 27 Jul 2020 06:31:05 -0400 (EDT) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id CC8324B993 for ; Mon, 27 Jul 2020 06:31:04 -0400 (EDT) Received: from willie-the-truck (236.31.169.217.in-addr.arpa [217.169.31.236]) (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 A27192075A; Mon, 27 Jul 2020 10:31:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595845863; bh=kCduUPmhmWHrdYHdz4LJj1ElV3O2YJPq+2s+xYu2b2I=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=zOnWcLJTCikTxHLjNWz/REwKd5Q5A2WZ/YUYyV9k5mTkg9z8We49yfNclrRdybrUx YK1o2hRF8zVdHaSnePD7V6fxYVxATEX3gNXWVcWgFniuer8L7QhYV2UnjWs5FszJT/ aBxpLeVVvla2b7JwpqsUah+FI/SmYB4PGYEfm17E= Date: Mon, 27 Jul 2020 11:30:59 +0100 From: Will Deacon To: Marc Zyngier Subject: Re: [PATCH 1/7] KVM: arm64: Update comment when skipping guest MMIO access instruction Message-ID: <20200727103059.GC20194@willie-the-truck> References: <20200724143506.17772-1-will@kernel.org> <20200724143506.17772-2-will@kernel.org> <87v9iawn2r.wl-maz@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87v9iawn2r.wl-maz@kernel.org> User-Agent: Mutt/1.10.1 (2018-07-13) Cc: kernel-team@android.com, 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 Sun, Jul 26, 2020 at 12:08:28PM +0100, Marc Zyngier wrote: > On Fri, 24 Jul 2020 15:35:00 +0100, > Will Deacon wrote: > > > > If a 32-bit guest accesses MMIO using a 16-bit Thumb-2 instruction that > > is reported to the hypervisor without a valid syndrom (for example, > > because of the addressing mode), then we may hand off the fault to > > userspace. When resuming the guest, we unconditionally advance the PC > > by 4 bytes, since ESR_EL2.IL is always 1 for data aborts generated without > > a valid syndrome. This is a bit rubbish, but it's also difficult to see > > how we can fix it without potentially introducing regressions in userspace > > MMIO fault handling. > > Not quite, see below. > > > > > Update the comment when skipping a guest MMIO access instruction so that > > this corner case is at least written down. > > > > Cc: Marc Zyngier > > Cc: Quentin Perret > > Signed-off-by: Will Deacon > > --- > > arch/arm64/kvm/mmio.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/arch/arm64/kvm/mmio.c b/arch/arm64/kvm/mmio.c > > index 4e0366759726..b54ea5aa6c06 100644 > > --- a/arch/arm64/kvm/mmio.c > > +++ b/arch/arm64/kvm/mmio.c > > @@ -113,6 +113,13 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run) > > /* > > * The MMIO instruction is emulated and should not be re-executed > > * in the guest. > > + * > > + * Note: If user space handled the emulation because the abort > > + * symdrome information was not valid (ISV set in the ESR), then > > nits: syndrome, ISV *clear*. Duh, thanks. > > + * this will assume that the faulting instruction was 32-bit. > > + * If the faulting instruction was a 16-bit Thumb instruction, > > + * then userspace would need to rewind the PC by 2 bytes prior to > > + * resuming the vCPU (yuck!). > > */ > > kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu)); > > > > That's not how I read it. On ESR_EL2.ISV being clear, and in the > absence of KVM_CAP_ARM_NISV_TO_USER being set, we return a -ENOSYS from > io_mem_abort(), exiting back to userspace *without* advertising a MMIO > access. The VMM is free to do whatever it can to handle it (i.e. not > much), but crucially we don't go via kvm_handle_mmio_return() on > resuming the vcpu (unless the VMM sets run->exit_reason to > KVM_EXIT_MMIO, but that's clearly its own decision). > > Instead, the expectation is that userspace willing to handle an exit > resulting in ESR_EL2.ISV being clear would instead request a > KVM_EXIT_ARM_NISV exit type (by enabling KVM_CAP_ARM_NISV_TO_USER), > getting extra information in the process such as as the fault > IPA). KVM_EXIT_ARM_NISV clearly states in the documentation: > > "Note that KVM does not skip the faulting instruction as it does for > KVM_EXIT_MMIO, but userspace has to emulate any change to the > processing state if it decides to decode and emulate the instruction." Thanks, I think you're right. I _thought_ we always reported EXIT_MMIO for write faults on read-only memslots (as per the documented behaviour), but actually that goes down the io_mem_abort() path too and so the skipping only ever occurs when the syndrome is valid. I'll drop this patch. Will _______________________________________________ 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 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=-11.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 03916C433E1 for ; Mon, 27 Jul 2020 10:32:20 +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 C5A742075A for ; Mon, 27 Jul 2020 10:32:19 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="2tcniapu"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="zOnWcLJT" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C5A742075A 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-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=AR0x/UskgGqS+BgoLkxQfp0lHXvZ5O9YGdkN3N0T4No=; b=2tcniapuCoJJXaCfAmMUdHhEP MoH1pkTOmvaP/zB2BmqtBRp/CiiG3p8g7Rv+xgY3qOzcHWxPG9eC0+ZeSWCVhw+7h7x8TZjsr2hr8 rQUE558O+N+IWygK2lQFp1G9aT68mBqxsUekbTwc2FxF0xzBT17tWssV0GZADzds3VYFnBE8KAhpj zCXSIyzy3Qhlpzn/WkOIz17L9zx1/O/5WgRDAaB8M6CC2Qqo5WCWU8NngMs3OzpYzkzgTFPdUA7pz haoeVXJ0IYBKXzn/+UHQarp2iOb1jAidUqjj0ese6X3yMBor8L3rnCFOToyiBV11EoMfq+q0RtBDe hsO+ai7Kg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k00PT-0002Sv-Ii; Mon, 27 Jul 2020 10:31:07 +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 1k00PQ-0002RY-Ji for linux-arm-kernel@lists.infradead.org; Mon, 27 Jul 2020 10:31:05 +0000 Received: from willie-the-truck (236.31.169.217.in-addr.arpa [217.169.31.236]) (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 A27192075A; Mon, 27 Jul 2020 10:31:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595845863; bh=kCduUPmhmWHrdYHdz4LJj1ElV3O2YJPq+2s+xYu2b2I=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=zOnWcLJTCikTxHLjNWz/REwKd5Q5A2WZ/YUYyV9k5mTkg9z8We49yfNclrRdybrUx YK1o2hRF8zVdHaSnePD7V6fxYVxATEX3gNXWVcWgFniuer8L7QhYV2UnjWs5FszJT/ aBxpLeVVvla2b7JwpqsUah+FI/SmYB4PGYEfm17E= Date: Mon, 27 Jul 2020 11:30:59 +0100 From: Will Deacon To: Marc Zyngier Subject: Re: [PATCH 1/7] KVM: arm64: Update comment when skipping guest MMIO access instruction Message-ID: <20200727103059.GC20194@willie-the-truck> References: <20200724143506.17772-1-will@kernel.org> <20200724143506.17772-2-will@kernel.org> <87v9iawn2r.wl-maz@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87v9iawn2r.wl-maz@kernel.org> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200727_063104_787383_269B4628 X-CRM114-Status: GOOD ( 30.02 ) 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: Suzuki Poulose , Quentin Perret , James Morse , kernel-team@android.com, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sun, Jul 26, 2020 at 12:08:28PM +0100, Marc Zyngier wrote: > On Fri, 24 Jul 2020 15:35:00 +0100, > Will Deacon wrote: > > > > If a 32-bit guest accesses MMIO using a 16-bit Thumb-2 instruction that > > is reported to the hypervisor without a valid syndrom (for example, > > because of the addressing mode), then we may hand off the fault to > > userspace. When resuming the guest, we unconditionally advance the PC > > by 4 bytes, since ESR_EL2.IL is always 1 for data aborts generated without > > a valid syndrome. This is a bit rubbish, but it's also difficult to see > > how we can fix it without potentially introducing regressions in userspace > > MMIO fault handling. > > Not quite, see below. > > > > > Update the comment when skipping a guest MMIO access instruction so that > > this corner case is at least written down. > > > > Cc: Marc Zyngier > > Cc: Quentin Perret > > Signed-off-by: Will Deacon > > --- > > arch/arm64/kvm/mmio.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/arch/arm64/kvm/mmio.c b/arch/arm64/kvm/mmio.c > > index 4e0366759726..b54ea5aa6c06 100644 > > --- a/arch/arm64/kvm/mmio.c > > +++ b/arch/arm64/kvm/mmio.c > > @@ -113,6 +113,13 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run) > > /* > > * The MMIO instruction is emulated and should not be re-executed > > * in the guest. > > + * > > + * Note: If user space handled the emulation because the abort > > + * symdrome information was not valid (ISV set in the ESR), then > > nits: syndrome, ISV *clear*. Duh, thanks. > > + * this will assume that the faulting instruction was 32-bit. > > + * If the faulting instruction was a 16-bit Thumb instruction, > > + * then userspace would need to rewind the PC by 2 bytes prior to > > + * resuming the vCPU (yuck!). > > */ > > kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu)); > > > > That's not how I read it. On ESR_EL2.ISV being clear, and in the > absence of KVM_CAP_ARM_NISV_TO_USER being set, we return a -ENOSYS from > io_mem_abort(), exiting back to userspace *without* advertising a MMIO > access. The VMM is free to do whatever it can to handle it (i.e. not > much), but crucially we don't go via kvm_handle_mmio_return() on > resuming the vcpu (unless the VMM sets run->exit_reason to > KVM_EXIT_MMIO, but that's clearly its own decision). > > Instead, the expectation is that userspace willing to handle an exit > resulting in ESR_EL2.ISV being clear would instead request a > KVM_EXIT_ARM_NISV exit type (by enabling KVM_CAP_ARM_NISV_TO_USER), > getting extra information in the process such as as the fault > IPA). KVM_EXIT_ARM_NISV clearly states in the documentation: > > "Note that KVM does not skip the faulting instruction as it does for > KVM_EXIT_MMIO, but userspace has to emulate any change to the > processing state if it decides to decode and emulate the instruction." Thanks, I think you're right. I _thought_ we always reported EXIT_MMIO for write faults on read-only memslots (as per the documented behaviour), but actually that goes down the io_mem_abort() path too and so the skipping only ever occurs when the syndrome is valid. I'll drop this patch. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel