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

2008-12-30 Thread Sheng Yang
On Tuesday 30 December 2008 18:41:54 Avi Kivity wrote:
> Sheng Yang wrote:
> >>> MASK_MSIX, and UNMASK, every two action are in pairs but we have to use
> >>> twice bits to store them. So I'd like to use MSI_ACTION approach...
> >>
> >> Well, it you have flags without ENABLE_MSI, doesn't it imply
> >> DISABLE_MSI?
> >>
> >> The structure contains the state we want to reach, not a command we wish
> >> the kernel to perform.
> >
> > Yes, that's what I want. But check more than one flags(for MSI-X) to
> > determine where to go is not that clear. So I add a flag here to indicate
> > the operation type which I think is a little more clear.
>
> Don't understand.  Do you mean MSI and MSI-X are mutually exclusive?
>
> If so, we can add a comment.

Yes, MSI/MSI-X are mutually exclusive.

OK, I will try to keep this, just hope the logic of code won't become too 
complicate..

-- 
regards
Yang, Sheng

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2008-12-30 Thread Avi Kivity

Sheng Yang wrote:

MASK_MSIX, and UNMASK, every two action are in pairs but we have to use
twice bits to store them. So I'd like to use MSI_ACTION approach...
  

Well, it you have flags without ENABLE_MSI, doesn't it imply DISABLE_MSI?

The structure contains the state we want to reach, not a command we wish
the kernel to perform.



Yes, that's what I want. But check more than one flags(for MSI-X) to determine 
where to go is not that clear. So I add a flag here to indicate the operation 
type which I think is a little more clear.
  


Don't understand.  Do you mean MSI and MSI-X are mutually exclusive?

If so, we can add a comment.

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2008-12-30 Thread Sheng Yang
On Tuesday 30 December 2008 18:31:16 Avi Kivity wrote:
> Sheng Yang wrote:
> > On Tuesday 30 December 2008 18:19:29 Avi Kivity wrote:
> >> Sheng Yang wrote:
> >>> For MSI disable feature later.
> >>>
> >>> Notice I changed ABI here, but due to no userspace patch, I think it's
> >>> OK.
> >>
> >> It's not okay, since eventually we will have userspace and it will have
> >> to work with older kernels as well.
> >>
> >> No released kernel has KVM_DEV_IRQ_ASSIGN_ENABLE_MSI, so it's fine,
> >> provided I fold this into the 2.6.29 submission.  However, why do this
> >> at all?  It can only cause confusion.
> >
> > If we have ENABLE_MSI, and DISABLE, and ENABLE_MSIX, and DISABLE, and
> > MASK_MSIX, and UNMASK, every two action are in pairs but we have to use
> > twice bits to store them. So I'd like to use MSI_ACTION approach...
>
> Well, it you have flags without ENABLE_MSI, doesn't it imply DISABLE_MSI?
>
> The structure contains the state we want to reach, not a command we wish
> the kernel to perform.

Yes, that's what I want. But check more than one flags(for MSI-X) to determine 
where to go is not that clear. So I add a flag here to indicate the operation 
type which I think is a little more clear.

-- 
regards
Yang, Sheng

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2008-12-30 Thread Avi Kivity

Sheng Yang wrote:

On Tuesday 30 December 2008 18:19:29 Avi Kivity wrote:
  

Sheng Yang wrote:


For MSI disable feature later.

Notice I changed ABI here, but due to no userspace patch, I think it's
OK.
  

It's not okay, since eventually we will have userspace and it will have
to work with older kernels as well.

No released kernel has KVM_DEV_IRQ_ASSIGN_ENABLE_MSI, so it's fine,
provided I fold this into the 2.6.29 submission.  However, why do this
at all?  It can only cause confusion.



If we have ENABLE_MSI, and DISABLE, and ENABLE_MSIX, and DISABLE, and 
MASK_MSIX, and UNMASK, every two action are in pairs but we have to use twice 
bits to store them. So I'd like to use MSI_ACTION approach...
  


Well, it you have flags without ENABLE_MSI, doesn't it imply DISABLE_MSI?

The structure contains the state we want to reach, not a command we wish 
the kernel to perform.


--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2008-12-30 Thread Sheng Yang
On Tuesday 30 December 2008 18:19:29 Avi Kivity wrote:
> Sheng Yang wrote:
> > For MSI disable feature later.
> >
> > Notice I changed ABI here, but due to no userspace patch, I think it's
> > OK.
>
> It's not okay, since eventually we will have userspace and it will have
> to work with older kernels as well.
>
> No released kernel has KVM_DEV_IRQ_ASSIGN_ENABLE_MSI, so it's fine,
> provided I fold this into the 2.6.29 submission.  However, why do this
> at all?  It can only cause confusion.

If we have ENABLE_MSI, and DISABLE, and ENABLE_MSIX, and DISABLE, and 
MASK_MSIX, and UNMASK, every two action are in pairs but we have to use twice 
bits to store them. So I'd like to use MSI_ACTION approach...

-- 
regards
Yang, Sheng

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2008-12-30 Thread Avi Kivity

Sheng Yang wrote:

For MSI disable feature later.

Notice I changed ABI here, but due to no userspace patch, I think it's OK.
  


It's not okay, since eventually we will have userspace and it will have 
to work with older kernels as well.


No released kernel has KVM_DEV_IRQ_ASSIGN_ENABLE_MSI, so it's fine, 
provided I fold this into the 2.6.29 submission.  However, why do this 
at all?  It can only cause confusion.



--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2008-12-29 Thread Sheng Yang
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 42f51dc..c24f207 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -546,6 +546,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)
 
 #endif
-- 
1.5.4.5

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html