Re: [PATCH 1/8] KVM: Add MSI_ACTION flag for assigned irq

[Sheng Yang <sheng@linux.intel.com>]

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 <sheng@linux.intel.com>
> > ---
> >  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