From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sheng Yang Subject: Re: [PATCH 1/8] KVM: Add MSI_ACTION flag for assigned irq Date: Wed, 24 Dec 2008 10:24:34 +0800 Message-ID: <200812241024.35361.sheng@linux.intel.com> References: <1230019231-16543-1-git-send-email-sheng@linux.intel.com> <1230019231-16543-2-git-send-email-sheng@linux.intel.com> <20081223173224.GA5449@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Cc: Avi Kivity , kvm@vger.kernel.org To: Marcelo Tosatti Return-path: Received: from mga09.intel.com ([134.134.136.24]:48357 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750894AbYLXCYi (ORCPT ); Tue, 23 Dec 2008 21:24:38 -0500 In-Reply-To: <20081223173224.GA5449@amt.cnet> Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On Wednesday 24 December 2008 01:32:24 Marcelo Tosatti wrote: > On Tue, Dec 23, 2008 at 04:00:24PM +0800, Sheng Yang wrote: > > For MSI disable feature later. > > > > Notice I changed ABI here, but due to no userspace patch, I think it's > > OK. > > > > Signed-off-by: Sheng Yang > > --- > > include/linux/kvm.h | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/include/linux/kvm.h b/include/linux/kvm.h > > index ef7f98e..5b965f6 100644 > > --- a/include/linux/kvm.h > > +++ b/include/linux/kvm.h > > @@ -544,6 +544,7 @@ struct kvm_assigned_irq { > > > > #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0) > > > > -#define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI (1 << 0) > > +#define KVM_DEV_IRQ_ASSIGN_MSI_ACTION (1 << 0) > > +#define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI (1 << 1) > > This is a little confusing. KVM_DEV_IRQ_ASSIGN_MSI_ACTION is assigned > from userspace, and in the patchset used in conjunction with msi2intx > which is a module parameter. > > Is there anything that blocks control of msi2intx translate behaviour > from userspace? > > Perhaps add a KVM_DEV_IRQ_UNASSIGN ioctl, and pass the desired > guest/host irq types on the IRQ_ASSIGN ioctl, thus removing some of the > kernel complexity. Marcelo, Thanks for reviewing. msi2intx module parameter is mostly a debug assist rather than a real option, so we won't want this to be a real option controlled by userspace program. Another simple solution for the conjunction is: diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index ffd261d..35bc81e 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -405,8 +405,7 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm, } } - if ((!msi2intx && - (assigned_irq->flags & KVM_DEV_IRQ_ASSIGN_MSI_ACTION)) || + if ((assigned_irq->flags & KVM_DEV_IRQ_ASSIGN_MSI_ACTION) || (msi2intx && match->dev->msi_enabled)) { So we wouldn't confusion in the semantic. About MSI disable side, maybe leave some judgment to userspace can reduce the complexity, I will try it. -- regards Yang, Sheng