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

[Marcelo Tosatti <mtosatti@redhat.com>]

On Wed, Dec 24, 2008 at 10:24:34AM +0800, Sheng Yang wrote:
> 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.

It seems msi2intx (as a module parameter) is not necessary.
You can achieve the same from userspace by configuring
KVM_DEV_IRQ_ASSIGN_ENABLE_MSI/MSI_ACTION, no? The module parameter seems
a weird interface to me (even if it is a debug one).

But its not a big deal, if you think it is worthwhile to keep it that
way.