From patchwork Mon Mar 20 14:24:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 9634291 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 C5B73602D6 for ; Mon, 20 Mar 2017 14:24:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B9F9127F85 for ; Mon, 20 Mar 2017 14:24:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id AEE6328449; Mon, 20 Mar 2017 14:24:56 +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 3BF4527F85 for ; Mon, 20 Mar 2017 14:24:56 +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: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=UyjXPObmkEzOCgjISNWeBSXAKa7e565urGy7rD0FWT4=; b=E7KMkAe6oJIGLe Y3KKOQF4NS6MEAsIKeM5dqe3csOMMq37VQ6hLrq2w0SOmvDddg6mnTJhn6uSU5HxJ/ZkWbmqBWLbF HxSNmftrW0Nu20wuFI6fby8+ZyOY7EGrzyMcocdPBpj6MVpWSQDvxIweviTB1QkFFArQnL96hIsZl bxiBldG9Vw1c+x9h7AzUpQ3oG8jf1q+VzJil5DzUTHRjnN7mpe8pCH5Vzl4yJqlVX0AqbGcwK1a3Y fugMmSVm8NROmaqw8NaZnqZ+S61FuUuTwls8WNV66Rd5iDVuH2hCszqJrVJuvkFePVDZLZFGr+FL4 rK/V78qoyesjHyqujCZw==; 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 1cpyEw-0003qo-Lx; Mon, 20 Mar 2017 14:24:54 +0000 Received: from mail-wr0-x22d.google.com ([2a00:1450:400c:c0c::22d]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cpyEt-0003oI-4I for linux-arm-kernel@lists.infradead.org; Mon, 20 Mar 2017 14:24:52 +0000 Received: by mail-wr0-x22d.google.com with SMTP id l37so93662885wrc.1 for ; Mon, 20 Mar 2017 07:24:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=mS2Vn+S7EfdrGnbKcZqwlBvezTkjR08z80tu2gqkXt8=; b=S9u2AVnc2zbY1dyrxMbmPfvNuJ6jo/zdTW+QYRxmrlboliVzCjuizH9AUnM1fWrnfd HvAhhxAClBJC9XWcI+/eTh2l0w2r/9bv+K2Vonq6INGC78YjOlbaDVfu/l4lgFkS5WjZ CVIUOHlDSQNvaSQLKnrLTmkZiqSE/xQw2WSXM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=mS2Vn+S7EfdrGnbKcZqwlBvezTkjR08z80tu2gqkXt8=; b=eoD7VXiYuIe8X2sjU3o/rux/aLEwCkCXPdYJQxcKaF9uWSsL2DjQKcsqkuOVq/T+s4 MtBMsh5K9Gg9L4LlS2LyI1CeNaXkxGhW5mA58Qij5zBaIW8offrSuk39Tglosz4iOps7 CR3QhBV1WVzeelrU5Fv6E9LY4+Ie/y1pAtQuklR+nd90kDdulM8Yr/+n6tVNmo1FzoEj aDB4uNQT2nqEpM2IwHUPlh4VXmZ+yA28OPQJIp08tXEdYqXf+iQ8vBOLw/hKIhi5kTa/ NwcYvBk7OXrJhowFfXppAIW+xtfoQw3mVRLuNi8nI1tOHbHuVoPuJ/Cn9XNY+K1J9hy+ +brA== X-Gm-Message-State: AFeK/H1aLSyHzPsX31PLp/wNw3eeikpLAXDqGYP89Nu+Jw/4jflvsGIaFqrajSN0m90f7Ef9 X-Received: by 10.223.169.171 with SMTP id b40mr27541126wrd.132.1490019866944; Mon, 20 Mar 2017 07:24:26 -0700 (PDT) Received: from localhost (xd93ddc2d.cust.hiper.dk. [217.61.220.45]) by smtp.gmail.com with ESMTPSA id i133sm13750455wmg.26.2017.03.20.07.24.25 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 20 Mar 2017 07:24:26 -0700 (PDT) Date: Mon, 20 Mar 2017 15:24:25 +0100 From: Christoffer Dall To: Marc Zyngier Subject: Re: [PATCH 1/2] KVM: arm/arm64: vgic-v2: Expose the correct GICC_PMR values to userspace Message-ID: <20170320142425.GB20711@cbox> References: <20170316114535.25233-1-marc.zyngier@arm.com> <20170316114535.25233-2-marc.zyngier@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170316114535.25233-2-marc.zyngier@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170320_072451_333557_BC383520 X-CRM114-Status: GOOD ( 18.78 ) 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: Peter Maydell , kvm@vger.kernel.org, Andre Przywara , Eric Auger , Christoffer Dall , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org 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 On Thu, Mar 16, 2017 at 11:45:34AM +0000, Marc Zyngier wrote: > We allow userspace to save/restore the GICC_PMR values in order > to allow migration. This value is extracted from GICH_PMCR, where > it occupies a 5 bit field. But the canonical PMR is an 8 bit > value and we fail to shift the virtual priority, resulting in > a non-sensical value being reported to userspace. > > Fixing it once and for all would be ideal, but that would break > migration of guest from old to new kernels. We thus introduce > a new GICv2 attribute (KVM_DEV_ARM_VGIC_CTRL_CANONICAL_PMR) > that allows userspace to register its interest for the one true > representation of PMR. Thinking about this some more, I think we should just leave the ABI as is without introducing the flag, because we do not loose any information by doing so, and we can completely leave it up to userspace to work around our funny ABI. In the end, considering my comments on the next patch, the result would be amusing, and look something like this patch instead: Let me know what you think. Thanks, -Christoffer diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt index 76e61c8..b2f60ca 100644 --- a/Documentation/virtual/kvm/devices/arm-vgic.txt +++ b/Documentation/virtual/kvm/devices/arm-vgic.txt @@ -83,6 +83,12 @@ Groups: Bits for undefined preemption levels are RAZ/WI. + For historical reasons and to provide ABI compatibility with userspace we + export the GICC_PMR register in the format of the GICH_VMCR.VMPriMask + field in the lower 5 bits of a word, meaning that userspace must always + use the lower 5 bits to communicate with the KVM device and must shift the + value left by 3 places to obtain the actual priority mask level. + Limitations: - Priorities are not implemented, and registers are RAZ/WI - Currently only implemented for KVM_DEV_TYPE_ARM_VGIC_V2. diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c index a3ad7ff..7b7ecac 100644 --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c @@ -229,7 +229,14 @@ static unsigned long vgic_mmio_read_vcpuif(struct kvm_vcpu *vcpu, val = vmcr.ctlr; break; case GIC_CPU_PRIMASK: - val = vmcr.pmr; + /* + * Our KVM_DEV_TYPE_ARM_VGIC_V2 device ABI exports the + * the PMR field as GICH_VMCR.VMPriMask rather than + * GICC_PMR.Priority, so we expose the upper five bits of + * priority mask to userspace using the lower bits in the + * unsigned long. + */ + val = vmcr.pmr >> 3; break; case GIC_CPU_BINPOINT: val = vmcr.bpr; @@ -262,7 +269,14 @@ static void vgic_mmio_write_vcpuif(struct kvm_vcpu *vcpu, vmcr.ctlr = val; break; case GIC_CPU_PRIMASK: - vmcr.pmr = val; + /* + * Our KVM_DEV_TYPE_ARM_VGIC_V2 device ABI exports the + * the PMR field as GICH_VMCR.VMPriMask rather than + * GICC_PMR.Priority, so we expose the upper five bits of + * priority mask to userspace using the lower bits in the + * unsigned long. + */ + vmcr.pmr = val << 3; break; case GIC_CPU_BINPOINT: vmcr.bpr = val; diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c index b834ecd..95739cc 100644 --- a/virt/kvm/arm/vgic/vgic-v2.c +++ b/virt/kvm/arm/vgic/vgic-v2.c @@ -191,7 +191,7 @@ void vgic_v2_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) GICH_VMCR_ALIAS_BINPOINT_MASK; vmcr |= (vmcrp->bpr << GICH_VMCR_BINPOINT_SHIFT) & GICH_VMCR_BINPOINT_MASK; - vmcr |= (vmcrp->pmr << GICH_VMCR_PRIMASK_SHIFT) & + vmcr |= ((vmcrp->pmr >> 3) << GICH_VMCR_PRIMASK_SHIFT) & GICH_VMCR_PRIMASK_MASK; vcpu->arch.vgic_cpu.vgic_v2.vgic_vmcr = vmcr; @@ -207,8 +207,8 @@ void vgic_v2_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) GICH_VMCR_ALIAS_BINPOINT_SHIFT; vmcrp->bpr = (vmcr & GICH_VMCR_BINPOINT_MASK) >> GICH_VMCR_BINPOINT_SHIFT; - vmcrp->pmr = (vmcr & GICH_VMCR_PRIMASK_MASK) >> - GICH_VMCR_PRIMASK_SHIFT; + vmcrp->pmr = ((vmcr & GICH_VMCR_PRIMASK_MASK) >> + GICH_VMCR_PRIMASK_SHIFT) << 3; } void vgic_v2_enable(struct kvm_vcpu *vcpu) diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h index db28f7c..64b70b4 100644 --- a/virt/kvm/arm/vgic/vgic.h +++ b/virt/kvm/arm/vgic/vgic.h @@ -85,7 +85,8 @@ struct vgic_vmcr { u32 ctlr; u32 abpr; u32 bpr; - u32 pmr; + u32 pmr; /* Priority mask field in the GICC_PMR and + * ICC_PMR_EL1 priority field format */ /* Below member variable are valid only for GICv3 */ u32 grpen0; u32 grpen1;