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: 9634299 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 2BA4C602D6 for ; Mon, 20 Mar 2017 14:27:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1DEED27F10 for ; Mon, 20 Mar 2017 14:27:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0E1692843F; Mon, 20 Mar 2017 14:27:44 +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=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4D35527F10 for ; Mon, 20 Mar 2017 14:27:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754253AbdCTOZz (ORCPT ); Mon, 20 Mar 2017 10:25:55 -0400 Received: from mail-wr0-f174.google.com ([209.85.128.174]:35383 "EHLO mail-wr0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753594AbdCTOY3 (ORCPT ); Mon, 20 Mar 2017 10:24:29 -0400 Received: by mail-wr0-f174.google.com with SMTP id g10so93572956wrg.2 for ; Mon, 20 Mar 2017 07:24:27 -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=ngyrkU/ZMZVSX8fxKF3WmLwHJQRbH0x0O8rJS2At0+UsSPWH59LQu2dXOugmcPJ5tF UPYzo3aPYtNsl3nAt1GSUa5BaiungJnDexI8hOcuhwDfLWC9KTfVmNg+w9a/2ElZUomI 203TGL/6Gd4Z8clZIfhvDHiTd4FuxlXbTxjI7i9Xfwon7oU4QtPBEVfJdS9AGBDmD9Zb 0vf12wQyXM6SlDwSAEbT4Fi8OSN0HqbwA6LFHDHgwKG1OPVY4MvrpU0lf9d9wnCIvZDD ShsURoSxHpVFx97jnR38+TVZKjxbHJtpy4E15H4nBCLOinFLOgc8Q0vYe2vpnV7Sa72M RPbQ== X-Gm-Message-State: AFeK/H1q/QatiRy8Gi4tmpPzAl5EAdegnLb5t+XxHCpZVgUINHTGvPlV+CLQfq7MgZxcer8B 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 Cc: Christoffer Dall , Peter Maydell , Eric Auger , Andre Przywara , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org 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) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.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;