From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751501AbdE3ODb (ORCPT ); Tue, 30 May 2017 10:03:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37646 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751028AbdE3OD3 (ORCPT ); Tue, 30 May 2017 10:03:29 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E61A580F6D Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eric.auger@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E61A580F6D Subject: Re: [PATCH 09/10] KVM: arm/arm64: vgic: Implement forwarding setting To: Marc Zyngier References: <1495656803-28011-1-git-send-email-eric.auger@redhat.com> <1495656803-28011-10-git-send-email-eric.auger@redhat.com> <877f14lp76.fsf@arm.com> <9192256c-157b-2112-b8ef-e7815ad14e7d@redhat.com> <534fb830-4b38-e146-ed77-d64b7c6a8c22@arm.com> Cc: eric.auger.pro@gmail.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, alex.williamson@redhat.com, pbonzini@redhat.com, christoffer.dall@linaro.org, drjones@redhat.com, wei@redhat.com From: Auger Eric Message-ID: <6f1d4d51-c35f-7ec7-48b9-a38e2b0196b6@redhat.com> Date: Tue, 30 May 2017 16:03:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <534fb830-4b38-e146-ed77-d64b7c6a8c22@arm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 30 May 2017 14:03:28 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Marc, On 30/05/2017 15:17, Marc Zyngier wrote: > On 30/05/17 13:54, Auger Eric wrote: >> Hi, >> >> On 25/05/2017 21:19, Marc Zyngier wrote: >>> On Wed, May 24 2017 at 10:13:22 pm BST, Eric Auger wrote: >>>> Implements kvm_vgic_[set|unset]_forwarding. >>>> >>>> Handle low-level VGIC programming and consistent irqchip >>>> programming. >>>> >>>> Signed-off-by: Eric Auger >>>> >>>> --- >>>> --- >>>> include/kvm/arm_vgic.h | 5 +++ >>>> virt/kvm/arm/vgic/vgic.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 110 insertions(+) >>>> >>>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h >>>> index 695ebc7..7ddac8a 100644 >>>> --- a/include/kvm/arm_vgic.h >>>> +++ b/include/kvm/arm_vgic.h >>>> @@ -343,4 +343,9 @@ int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi); >>>> */ >>>> int kvm_vgic_setup_default_irq_routing(struct kvm *kvm); >>>> >>>> +int kvm_vgic_set_forwarding(struct kvm *kvm, unsigned int irq, >>>> + unsigned int virt_irq); >>>> +void kvm_vgic_unset_forwarding(struct kvm *kvm, unsigned int irq, >>>> + unsigned int virt_irq); >>> >>> nit: the name of the variables do not match that of the function >>> definition, and are much clearer there. >>> >>>> + >>>> #endif /* __KVM_ARM_VGIC_H */ >>>> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c >>>> index aa0618c..c2add8d 100644 >>>> --- a/virt/kvm/arm/vgic/vgic.c >>>> +++ b/virt/kvm/arm/vgic/vgic.c >>>> @@ -17,6 +17,8 @@ >>>> #include >>>> #include >>>> #include >>>> +#include >>>> +#include >>>> >>>> #include "vgic.h" >>>> >>>> @@ -771,3 +773,106 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int virt_irq) >>>> return map_is_active; >>>> } >>>> >>>> +/** >>>> + * kvm_vgic_set_forwarding - Set IRQ forwarding >>>> + * >>>> + * @kvm: kvm handle >>>> + * @host_irq: the host linux IRQ >>>> + * @vintid: the virtual INTID >>>> + * >>>> + * This function must be called when the IRQ is not active: >>>> + * ie. not active at GIC level and not currently under injection >>>> + * into the guest using the unforwarded mode. The physical IRQ must >>>> + * be disabled and all vCPUs must have been exited and prevented >>>> + * from being re-entered. >>>> + */ >>>> +int kvm_vgic_set_forwarding(struct kvm *kvm, unsigned int host_irq, >>>> + unsigned int vintid) >>>> +{ >>>> + struct kvm_vcpu *vcpu; >>>> + struct vgic_irq *irq; >>>> + struct irq_desc *desc; >>>> + struct irq_data *data; >>>> + unsigned int pintid; >>>> + int ret = 0; >>>> + >>>> + >>>> + kvm_debug("%s host linux irq=%d vintid=%d\n", >>>> + __func__, host_irq, vintid); >>>> + >>>> + if (!vgic_valid_spi(kvm, vintid)) >>>> + return 0; >>>> + >>>> + /* find the INTID corresponding to @host_irq */ >>>> + desc = irq_to_desc(host_irq); >>>> + if (!desc) { >>>> + kvm_err("%s: no interrupt descriptor\n", __func__); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + data = irq_desc_get_irq_data(desc); >>>> + while (data->parent_data) >>>> + data = data->parent_data; >>>> + >>>> + pintid = data->hwirq; >>>> + >>>> + irq = vgic_get_irq(kvm, NULL, vintid); >>>> + >>>> + spin_lock(&irq->irq_lock); >>>> + >>>> + vcpu = irq->target_vcpu; >>>> + >>>> + if (!vcpu) { >>>> + ret = -EAGAIN; >>>> + goto unlock; >>>> + } >>>> + >>>> + irq_set_vcpu_affinity(host_irq, vcpu); >>>> + >>>> + irq->hw = true; >>>> + irq->hwintid = pintid; >>>> + irq->host_irq = host_irq; >>> >>> This feels like a duplication of kvm_vgic_map_phys_irq(), specially if >>> you move the pintid discovery there. Can we somehow unify them? >> Sure. At the beginning it was just a matter of irq_lock I did not want >> to release. >> >> I was somehow embarrassed by the vcpu param of irq_set_vcpu_affinity. >> Shall we really test target_vcpu. The actual value is unused for SPI so >> shouldn't we simply use something != NULL. > > I guess that for the time being, this would be good enough. But GICv4 > requires some actual tracking of the affinity, so we may have to bite > the bullet already, and decide that the interrupt is always affine to a > vcpu. > > Does this have any userspace visible impact? I don't see any. Thanks Eric > > Thanks, > > M. >