Re: [PATCH v9 2/2] kvm: On Ack, De-assert Notify KVM_IRQFD extension

2012-09-17 Thread Alex Williamson
On Wed, 2012-09-05 at 17:57 +0300, Avi Kivity wrote:
 On 08/21/2012 10:29 PM, Alex Williamson wrote:
  For VFIO based device assignment we'd like a mechanism to allow level
  triggered interrutps to be directly injected into KVM.  KVM_IRQFD
  already allows this for edge triggered interrupts, but for level, we
  need to watch for acknowledgement of the interrupt from the guest to
  provide us a hint when to test the device and allow it to re-assert
  if necessary.  To do this, we create a new KVM_IRQFD mode called
  On Ack, De-assert  Notify, or OADN.  In this mode, an interrupt
  injection provides only a gsi assertion.  We then hook into the IRQ
  ACK notifier, which when triggered de-asserts the gsi and notifies
  via another eventfd.  It's then the responsibility of the user to
  re-assert the interrupt is service is still required.
  
  
  diff --git a/Documentation/virtual/kvm/api.txt 
  b/Documentation/virtual/kvm/api.txt
  index bf33aaa..87d7321 100644
  --- a/Documentation/virtual/kvm/api.txt
  +++ b/Documentation/virtual/kvm/api.txt
  @@ -1946,6 +1946,19 @@ the guest using the specified gsi pin.  The irqfd is 
  removed using
   the KVM_IRQFD_FLAG_DEASSIGN flag, specifying both kvm_irqfd.fd
   and kvm_irqfd.gsi.
   
  +With KVM_CAP_IRQFD_OADN, KVM_IRQFD supports an On Ack, De-assert 
  +Notify option that allows emulation of level-triggered interrupts.
  +When kvm_irqfd.fd is triggered, the requested gsi is asserted and
  +remains asserted until interaction with the irqchip indicates the
  +VM has acknowledged the interrupt, such as an EOI.  On acknoledgement
  +the gsi is automatically de-asserted and the user is notified via
  +kvm_irqfd.notifyfd.  The user is then required to re-assert the
  +interrupt if the associated device still requires service.  To enable
  +this mode, configure the KVM_IRQFD using the KVM_IRQFD_FLAG_OADN flag
  +and specify kvm_irqfd.notifyfd.  Note that closing kvm_irqfd.notifyfd
  +while configured in this mode does not disable the irqfd.  The
  +KVM_IRQFD_FLAG_OADN flag is only necessary on assignment.
 
 Under my suggested naming, this would be called a resampling irqfd,
 with resampling requested via kvm_irqfd.resamplefd.
 
  diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
  index 2245cfa..dfdb5b2 100644
  --- a/virt/kvm/eventfd.c
  +++ b/virt/kvm/eventfd.c
  @@ -43,6 +43,23 @@
* 
*/
   
  +/*
  + * OADN irqfds (On Ack, De-assert  Notify) are a special variety of
  + * irqfds that assert an interrupt to the irqchip on eventfd trigger,
  + * receieve notification when userspace acknowledges the interrupt,
  + * automatically de-asserts the irqchip level, and notifies userspace
  + * via the oadn_eventfd.  This object helps to provide one-to-many
  + * deassert-to-notify so we can share a single irq source ID per OADN.
  + */
  +struct _irqfd_oadn {
  +   struct kvm *kvm;
  +   int irq_source_id; /* IRQ source ID shared by these irqfds */
  +   struct list_head irqfds; /* list of irqfds using this object */
  +   struct kvm_irq_ack_notifier notifier; /* IRQ ACK notification */
  +   struct kref kref; /* Race-free removal */
  +   struct list_head list;
  +};
 
 
 Why do you need per-gsi irq source IDs?  irq source ids only matter
 within a gsi.  For example KVM_IRQ_LINE shares one source ID for all
 lines (with result that userspace is forced to manage the ORing of
 shared inputs itself).

Right, but locking makes it difficult to tear down a resample irqfd
without potentially racing creation of a new one, which I tried to
explain here:

http://www.spinics.net/lists/kvm/msg78460.html

This can cause a de-assert w/o ack as we briefly have multiple resample
irqfds on the same gsi, irq source id pair.  That can dead lock a vfio
device.  Using a new irq source ID ensures that the old resample irqfd
doesn't interfere with the new one.  We count on the final clear or the
gsi assertion when releasing the irq source id, so we can't share it
among other resample irqfds on other gsis with different life cycles.

Michael has suggested re-architecting the locking around some structure,
but I'm not sure it's worth it.  AFAICT we have more irq source IDs than
we could consume if resample irqfds on the same gsi share an irq source
id.  Thanks,

Alex

--
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 v9 2/2] kvm: On Ack, De-assert Notify KVM_IRQFD extension

2012-09-05 Thread Avi Kivity
On 08/21/2012 10:29 PM, Alex Williamson wrote:
 For VFIO based device assignment we'd like a mechanism to allow level
 triggered interrutps to be directly injected into KVM.  KVM_IRQFD
 already allows this for edge triggered interrupts, but for level, we
 need to watch for acknowledgement of the interrupt from the guest to
 provide us a hint when to test the device and allow it to re-assert
 if necessary.  To do this, we create a new KVM_IRQFD mode called
 On Ack, De-assert  Notify, or OADN.  In this mode, an interrupt
 injection provides only a gsi assertion.  We then hook into the IRQ
 ACK notifier, which when triggered de-asserts the gsi and notifies
 via another eventfd.  It's then the responsibility of the user to
 re-assert the interrupt is service is still required.
 
 
 diff --git a/Documentation/virtual/kvm/api.txt 
 b/Documentation/virtual/kvm/api.txt
 index bf33aaa..87d7321 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -1946,6 +1946,19 @@ the guest using the specified gsi pin.  The irqfd is 
 removed using
  the KVM_IRQFD_FLAG_DEASSIGN flag, specifying both kvm_irqfd.fd
  and kvm_irqfd.gsi.
  
 +With KVM_CAP_IRQFD_OADN, KVM_IRQFD supports an On Ack, De-assert 
 +Notify option that allows emulation of level-triggered interrupts.
 +When kvm_irqfd.fd is triggered, the requested gsi is asserted and
 +remains asserted until interaction with the irqchip indicates the
 +VM has acknowledged the interrupt, such as an EOI.  On acknoledgement
 +the gsi is automatically de-asserted and the user is notified via
 +kvm_irqfd.notifyfd.  The user is then required to re-assert the
 +interrupt if the associated device still requires service.  To enable
 +this mode, configure the KVM_IRQFD using the KVM_IRQFD_FLAG_OADN flag
 +and specify kvm_irqfd.notifyfd.  Note that closing kvm_irqfd.notifyfd
 +while configured in this mode does not disable the irqfd.  The
 +KVM_IRQFD_FLAG_OADN flag is only necessary on assignment.

Under my suggested naming, this would be called a resampling irqfd,
with resampling requested via kvm_irqfd.resamplefd.

 diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
 index 2245cfa..dfdb5b2 100644
 --- a/virt/kvm/eventfd.c
 +++ b/virt/kvm/eventfd.c
 @@ -43,6 +43,23 @@
   * 
   */
  
 +/*
 + * OADN irqfds (On Ack, De-assert  Notify) are a special variety of
 + * irqfds that assert an interrupt to the irqchip on eventfd trigger,
 + * receieve notification when userspace acknowledges the interrupt,
 + * automatically de-asserts the irqchip level, and notifies userspace
 + * via the oadn_eventfd.  This object helps to provide one-to-many
 + * deassert-to-notify so we can share a single irq source ID per OADN.
 + */
 +struct _irqfd_oadn {
 + struct kvm *kvm;
 + int irq_source_id; /* IRQ source ID shared by these irqfds */
 + struct list_head irqfds; /* list of irqfds using this object */
 + struct kvm_irq_ack_notifier notifier; /* IRQ ACK notification */
 + struct kref kref; /* Race-free removal */
 + struct list_head list;
 +};


Why do you need per-gsi irq source IDs?  irq source ids only matter
within a gsi.  For example KVM_IRQ_LINE shares one source ID for all
lines (with result that userspace is forced to manage the ORing of
shared inputs itself).




-- 
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 v9 2/2] kvm: On Ack, De-assert Notify KVM_IRQFD extension

2012-08-21 Thread Alex Williamson
For VFIO based device assignment we'd like a mechanism to allow level
triggered interrutps to be directly injected into KVM.  KVM_IRQFD
already allows this for edge triggered interrupts, but for level, we
need to watch for acknowledgement of the interrupt from the guest to
provide us a hint when to test the device and allow it to re-assert
if necessary.  To do this, we create a new KVM_IRQFD mode called
On Ack, De-assert  Notify, or OADN.  In this mode, an interrupt
injection provides only a gsi assertion.  We then hook into the IRQ
ACK notifier, which when triggered de-asserts the gsi and notifies
via another eventfd.  It's then the responsibility of the user to
re-assert the interrupt is service is still required.

Signed-off-by: Alex Williamson alex.william...@redhat.com
---

 Documentation/virtual/kvm/api.txt |   13 ++
 arch/x86/kvm/x86.c|1 
 include/linux/kvm.h   |6 +
 include/linux/kvm_host.h  |1 
 virt/kvm/eventfd.c|  193 -
 5 files changed, 210 insertions(+), 4 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt 
b/Documentation/virtual/kvm/api.txt
index bf33aaa..87d7321 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1946,6 +1946,19 @@ the guest using the specified gsi pin.  The irqfd is 
removed using
 the KVM_IRQFD_FLAG_DEASSIGN flag, specifying both kvm_irqfd.fd
 and kvm_irqfd.gsi.
 
+With KVM_CAP_IRQFD_OADN, KVM_IRQFD supports an On Ack, De-assert 
+Notify option that allows emulation of level-triggered interrupts.
+When kvm_irqfd.fd is triggered, the requested gsi is asserted and
+remains asserted until interaction with the irqchip indicates the
+VM has acknowledged the interrupt, such as an EOI.  On acknoledgement
+the gsi is automatically de-asserted and the user is notified via
+kvm_irqfd.notifyfd.  The user is then required to re-assert the
+interrupt if the associated device still requires service.  To enable
+this mode, configure the KVM_IRQFD using the KVM_IRQFD_FLAG_OADN flag
+and specify kvm_irqfd.notifyfd.  Note that closing kvm_irqfd.notifyfd
+while configured in this mode does not disable the irqfd.  The
+KVM_IRQFD_FLAG_OADN flag is only necessary on assignment.
+
 4.76 KVM_PPC_ALLOCATE_HTAB
 
 Capability: KVM_CAP_PPC_ALLOC_HTAB
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cd98673..fde7b66 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2175,6 +2175,7 @@ int kvm_dev_ioctl_check_extension(long ext)
case KVM_CAP_PCI_2_3:
case KVM_CAP_KVMCLOCK_CTRL:
case KVM_CAP_IRQFD_IRQ_SOURCE_ID:
+   case KVM_CAP_IRQFD_OADN:
r = 1;
break;
case KVM_CAP_COALESCED_MMIO:
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index ae66b9c..ec0f1d8 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -619,6 +619,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_S390_COW 79
 #define KVM_CAP_PPC_ALLOC_HTAB 80
 #define KVM_CAP_IRQFD_IRQ_SOURCE_ID 81
+#define KVM_CAP_IRQFD_OADN 82
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -684,12 +685,15 @@ struct kvm_xen_hvm_config {
 #endif
 
 #define KVM_IRQFD_FLAG_DEASSIGN (1  0)
+/* Availabie with KVM_CAP_IRQFD_OADN */
+#define KVM_IRQFD_FLAG_OADN (1  1)
 
 struct kvm_irqfd {
__u32 fd;
__u32 gsi;
__u32 flags;
-   __u8  pad[20];
+   __u32 notifyfd;
+   __u8  pad[16];
 };
 
 struct kvm_clock_data {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index b763230..d502d08 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -284,6 +284,7 @@ struct kvm {
struct {
spinlock_tlock;
struct list_head  items;
+   struct list_head  oadns;
} irqfds;
struct list_head ioeventfds;
 #endif
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 2245cfa..dfdb5b2 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -43,6 +43,23 @@
  * 
  */
 
+/*
+ * OADN irqfds (On Ack, De-assert  Notify) are a special variety of
+ * irqfds that assert an interrupt to the irqchip on eventfd trigger,
+ * receieve notification when userspace acknowledges the interrupt,
+ * automatically de-asserts the irqchip level, and notifies userspace
+ * via the oadn_eventfd.  This object helps to provide one-to-many
+ * deassert-to-notify so we can share a single irq source ID per OADN.
+ */
+struct _irqfd_oadn {
+   struct kvm *kvm;
+   int irq_source_id; /* IRQ source ID shared by these irqfds */
+   struct list_head irqfds; /* list of irqfds using this object */
+   struct kvm_irq_ack_notifier notifier; /* IRQ ACK notification */
+   struct kref kref; /* Race-free removal */
+   struct list_head list;
+};
+
 struct _irqfd {
/* Used for MSI fast-path */
struct kvm *kvm;
@@ -52,6 +69,10 @@ struct _irqfd {
  

Re: [PATCH v9 2/2] kvm: On Ack, De-assert Notify KVM_IRQFD extension

2012-08-21 Thread Alex Williamson
On Tue, 2012-08-21 at 23:37 +0300, Michael S. Tsirkin wrote:
 On Tue, Aug 21, 2012 at 01:29:14PM -0600, Alex Williamson wrote:
  For VFIO based device assignment we'd like a mechanism to allow level
  triggered interrutps to be directly injected into KVM.  KVM_IRQFD
  already allows this for edge triggered interrupts, but for level, we
  need to watch for acknowledgement of the interrupt from the guest to
  provide us a hint when to test the device and allow it to re-assert
  if necessary.  To do this, we create a new KVM_IRQFD mode called
  On Ack, De-assert  Notify, or OADN.  In this mode, an interrupt
  injection provides only a gsi assertion.  We then hook into the IRQ
  ACK notifier, which when triggered de-asserts the gsi and notifies
  via another eventfd.  It's then the responsibility of the user to
  re-assert the interrupt is service is still required.
  
  Signed-off-by: Alex Williamson alex.william...@redhat.com
 
 Naming aside, looks good.
 I think I see some minor bugs, and I added some improvement
 suggestions below.
 
 Thanks!
 
  ---
  
   Documentation/virtual/kvm/api.txt |   13 ++
   arch/x86/kvm/x86.c|1 
   include/linux/kvm.h   |6 +
   include/linux/kvm_host.h  |1 
   virt/kvm/eventfd.c|  193 
  -
   5 files changed, 210 insertions(+), 4 deletions(-)
  
  diff --git a/Documentation/virtual/kvm/api.txt 
  b/Documentation/virtual/kvm/api.txt
  index bf33aaa..87d7321 100644
  --- a/Documentation/virtual/kvm/api.txt
  +++ b/Documentation/virtual/kvm/api.txt
  @@ -1946,6 +1946,19 @@ the guest using the specified gsi pin.  The irqfd is 
  removed using
   the KVM_IRQFD_FLAG_DEASSIGN flag, specifying both kvm_irqfd.fd
   and kvm_irqfd.gsi.
   
  +With KVM_CAP_IRQFD_OADN, KVM_IRQFD supports an On Ack, De-assert 
  +Notify option that allows emulation of level-triggered interrupts.
  +When kvm_irqfd.fd is triggered, the requested gsi is asserted and
  +remains asserted until interaction with the irqchip indicates the
  +VM has acknowledged the interrupt, such as an EOI.  On acknoledgement
  +the gsi is automatically de-asserted and the user is notified via
  +kvm_irqfd.notifyfd.  The user is then required to re-assert the
  +interrupt if the associated device still requires service.  To enable
  +this mode, configure the KVM_IRQFD using the KVM_IRQFD_FLAG_OADN flag
  +and specify kvm_irqfd.notifyfd.  Note that closing kvm_irqfd.notifyfd
  +while configured in this mode does not disable the irqfd.  The
  +KVM_IRQFD_FLAG_OADN flag is only necessary on assignment.
  +
   4.76 KVM_PPC_ALLOCATE_HTAB
   
   Capability: KVM_CAP_PPC_ALLOC_HTAB
  diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
  index cd98673..fde7b66 100644
  --- a/arch/x86/kvm/x86.c
  +++ b/arch/x86/kvm/x86.c
  @@ -2175,6 +2175,7 @@ int kvm_dev_ioctl_check_extension(long ext)
  case KVM_CAP_PCI_2_3:
  case KVM_CAP_KVMCLOCK_CTRL:
  case KVM_CAP_IRQFD_IRQ_SOURCE_ID:
  +   case KVM_CAP_IRQFD_OADN:
  r = 1;
  break;
  case KVM_CAP_COALESCED_MMIO:
  diff --git a/include/linux/kvm.h b/include/linux/kvm.h
  index ae66b9c..ec0f1d8 100644
  --- a/include/linux/kvm.h
  +++ b/include/linux/kvm.h
  @@ -619,6 +619,7 @@ struct kvm_ppc_smmu_info {
   #define KVM_CAP_S390_COW 79
   #define KVM_CAP_PPC_ALLOC_HTAB 80
   #define KVM_CAP_IRQFD_IRQ_SOURCE_ID 81
  +#define KVM_CAP_IRQFD_OADN 82
   
   #ifdef KVM_CAP_IRQ_ROUTING
   
  @@ -684,12 +685,15 @@ struct kvm_xen_hvm_config {
   #endif
   
   #define KVM_IRQFD_FLAG_DEASSIGN (1  0)
  +/* Availabie with KVM_CAP_IRQFD_OADN */
 
 Need to also explain what it is.

Beyond Documentation/virtual/kvm/api.txt?  I don't see much else getting
documented here.  Or maybe you mean

/* On Ack, De-assert  Notify */

  +#define KVM_IRQFD_FLAG_OADN (1  1)
   
   struct kvm_irqfd {
  __u32 fd;
  __u32 gsi;
  __u32 flags;
  -   __u8  pad[20];
  +   __u32 notifyfd;
 
 Document that this is only valid with OADN flag.  Might be a good idea
 to rename this to deassert_on_ack_notifyfd or oadn_notifyfd
 to avoid confusion.

I'll add a /* only valid with KVM_IRQFD_FLAG_OADN */

I can change the name if you prefer, but it seems pretty clear to me how
a notifyfd might relate to a On Ack, De-assert  Notify irqfd without
pulling longer names into userspace.

  +   __u8  pad[16];
   };
   
   struct kvm_clock_data {
  diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
  index b763230..d502d08 100644
  --- a/include/linux/kvm_host.h
  +++ b/include/linux/kvm_host.h
  @@ -284,6 +284,7 @@ struct kvm {
  struct {
  spinlock_tlock;
  struct list_head  items;
  +   struct list_head  oadns;
  } irqfds;
  struct list_head ioeventfds;
   #endif
  diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
  index 2245cfa..dfdb5b2 100644
  --- a/virt/kvm/eventfd.c
  +++ b/virt/kvm/eventfd.c
  @@ -43,6 +43,23 @@
* 

Re: [PATCH v9 2/2] kvm: On Ack, De-assert Notify KVM_IRQFD extension

2012-08-21 Thread Michael S. Tsirkin
On Tue, Aug 21, 2012 at 03:11:41PM -0600, Alex Williamson wrote:
 On Tue, 2012-08-21 at 23:37 +0300, Michael S. Tsirkin wrote:
  On Tue, Aug 21, 2012 at 01:29:14PM -0600, Alex Williamson wrote:
   For VFIO based device assignment we'd like a mechanism to allow level
   triggered interrutps to be directly injected into KVM.  KVM_IRQFD
   already allows this for edge triggered interrupts, but for level, we
   need to watch for acknowledgement of the interrupt from the guest to
   provide us a hint when to test the device and allow it to re-assert
   if necessary.  To do this, we create a new KVM_IRQFD mode called
   On Ack, De-assert  Notify, or OADN.  In this mode, an interrupt
   injection provides only a gsi assertion.  We then hook into the IRQ
   ACK notifier, which when triggered de-asserts the gsi and notifies
   via another eventfd.  It's then the responsibility of the user to
   re-assert the interrupt is service is still required.
   
   Signed-off-by: Alex Williamson alex.william...@redhat.com
  
  Naming aside, looks good.
  I think I see some minor bugs, and I added some improvement
  suggestions below.
  
  Thanks!
  
   ---
   
Documentation/virtual/kvm/api.txt |   13 ++
arch/x86/kvm/x86.c|1 
include/linux/kvm.h   |6 +
include/linux/kvm_host.h  |1 
virt/kvm/eventfd.c|  193 
   -
5 files changed, 210 insertions(+), 4 deletions(-)
   
   diff --git a/Documentation/virtual/kvm/api.txt 
   b/Documentation/virtual/kvm/api.txt
   index bf33aaa..87d7321 100644
   --- a/Documentation/virtual/kvm/api.txt
   +++ b/Documentation/virtual/kvm/api.txt
   @@ -1946,6 +1946,19 @@ the guest using the specified gsi pin.  The irqfd 
   is removed using
the KVM_IRQFD_FLAG_DEASSIGN flag, specifying both kvm_irqfd.fd
and kvm_irqfd.gsi.

   +With KVM_CAP_IRQFD_OADN, KVM_IRQFD supports an On Ack, De-assert 
   +Notify option that allows emulation of level-triggered interrupts.
   +When kvm_irqfd.fd is triggered, the requested gsi is asserted and
   +remains asserted until interaction with the irqchip indicates the
   +VM has acknowledged the interrupt, such as an EOI.  On acknoledgement
   +the gsi is automatically de-asserted and the user is notified via
   +kvm_irqfd.notifyfd.  The user is then required to re-assert the
   +interrupt if the associated device still requires service.  To enable
   +this mode, configure the KVM_IRQFD using the KVM_IRQFD_FLAG_OADN flag
   +and specify kvm_irqfd.notifyfd.  Note that closing kvm_irqfd.notifyfd
   +while configured in this mode does not disable the irqfd.  The
   +KVM_IRQFD_FLAG_OADN flag is only necessary on assignment.
   +
4.76 KVM_PPC_ALLOCATE_HTAB

Capability: KVM_CAP_PPC_ALLOC_HTAB
   diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
   index cd98673..fde7b66 100644
   --- a/arch/x86/kvm/x86.c
   +++ b/arch/x86/kvm/x86.c
   @@ -2175,6 +2175,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 case KVM_CAP_PCI_2_3:
 case KVM_CAP_KVMCLOCK_CTRL:
 case KVM_CAP_IRQFD_IRQ_SOURCE_ID:
   + case KVM_CAP_IRQFD_OADN:
 r = 1;
 break;
 case KVM_CAP_COALESCED_MMIO:
   diff --git a/include/linux/kvm.h b/include/linux/kvm.h
   index ae66b9c..ec0f1d8 100644
   --- a/include/linux/kvm.h
   +++ b/include/linux/kvm.h
   @@ -619,6 +619,7 @@ struct kvm_ppc_smmu_info {
#define KVM_CAP_S390_COW 79
#define KVM_CAP_PPC_ALLOC_HTAB 80
#define KVM_CAP_IRQFD_IRQ_SOURCE_ID 81
   +#define KVM_CAP_IRQFD_OADN 82

#ifdef KVM_CAP_IRQ_ROUTING

   @@ -684,12 +685,15 @@ struct kvm_xen_hvm_config {
#endif

#define KVM_IRQFD_FLAG_DEASSIGN (1  0)
   +/* Availabie with KVM_CAP_IRQFD_OADN */
  
  Need to also explain what it is.
 
 Beyond Documentation/virtual/kvm/api.txt?  I don't see much else getting
 documented here.  Or maybe you mean
 
 /* On Ack, De-assert  Notify */

 yes


   +#define KVM_IRQFD_FLAG_OADN (1  1)

struct kvm_irqfd {
 __u32 fd;
 __u32 gsi;
 __u32 flags;
   - __u8  pad[20];
   + __u32 notifyfd;
  
  Document that this is only valid with OADN flag.  Might be a good idea
  to rename this to deassert_on_ack_notifyfd or oadn_notifyfd
  to avoid confusion.
 
 I'll add a /* only valid with KVM_IRQFD_FLAG_OADN */
 
 I can change the name if you prefer, but it seems pretty clear to me how
 a notifyfd might relate to a On Ack, De-assert  Notify irqfd without
 pulling longer names into userspace.

Ideally you can figure out stuff without reading docs.
So eschew abbreviations, make it as long and clear as possible
is my advise.
But it is up to Avi.

   + __u8  pad[16];
};

struct kvm_clock_data {
   diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
   index b763230..d502d08 100644
   --- a/include/linux/kvm_host.h
   +++ b/include/linux/kvm_host.h
   @@ -284,6 +284,7 @@ struct kvm {
 struct {
 spinlock_tlock;

Re: [PATCH v9 2/2] kvm: On Ack, De-assert Notify KVM_IRQFD extension

2012-08-21 Thread Alex Williamson
On Wed, 2012-08-22 at 03:14 +0300, Michael S. Tsirkin wrote:
 On Tue, Aug 21, 2012 at 03:11:41PM -0600, Alex Williamson wrote:
  On Tue, 2012-08-21 at 23:37 +0300, Michael S. Tsirkin wrote:
   On Tue, Aug 21, 2012 at 01:29:14PM -0600, Alex Williamson wrote:
+   }
+
irq_rt = rcu_dereference_protected(kvm-irq_routing,
   
lockdep_is_held(kvm-irqfds.lock));
irqfd_update(kvm, irqfd, irq_rt);
@@ -267,6 +426,26 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd 
*args)
 
spin_unlock_irq(kvm-irqfds.lock);
 
+   if (oadn) {
+   synchronize_rcu();
   
   Seems unexpected on assign.
   What does this synchronize_rcu do?
  
  Because we force the notify below and we need the irqfd we just added to
  the list to be visible on the list for that.
 
 I do not see it yet - maybe add a comment?

It's entirely possible I'm wrong too.  This was an after thought and now
that I look at it again, the below irqfd_oadn_notify obviously starts
after the list_add, so there shouldn't be any need to wait.  Thanks,

Alex

--
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