Re: [Qemu-devel] [RFC PATCH 2/5] vfio/quirks: Add generic support for ioveventfds

2018-02-08 Thread Auger Eric
Hi Alex,

On 08/02/18 19:33, Alex Williamson wrote:
> On Thu, 8 Feb 2018 12:11:57 +0100
> Auger Eric  wrote:
> 
>> Hi Alex,
>>
>> On 07/02/18 01:26, Alex Williamson wrote:
>>> We might wish to handle some quirks via ioeventfds, add a list of
>>> ioeventfds to the quirk.  
>> The commit title is a bit misleading as we only add the data type and
>> deletion function.
> 
> Unfortunately that's as much as I can add here or else QEMU will fail
> to build because of unused functions :-\  So I pulled as much
> infrastructure as I could here but had to leave some meat for when it
> gets called.  Suggestions welcome for title/commit updates.

Understood. Something like "vfio/quirks: Add a VFIOIOEventFD list in
quirks"?
> 
>>>
>>> Signed-off-by: Alex Williamson 
>>> ---
>>>  hw/vfio/pci-quirks.c |   17 +
>>>  hw/vfio/pci.h|   11 +++
>>>  2 files changed, 28 insertions(+)
>>>
>>> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
>>> index 10af23217292..e4cf4ea2dd9c 100644
>>> --- a/hw/vfio/pci-quirks.c
>>> +++ b/hw/vfio/pci-quirks.c
>>> @@ -12,6 +12,7 @@
>>>  
>>>  #include "qemu/osdep.h"
>>>  #include "qemu/error-report.h"
>>> +#include "qemu/main-loop.h"
>>>  #include "qemu/range.h"
>>>  #include "qapi/error.h"
>>>  #include "qapi/visitor.h"
>>> @@ -278,12 +279,24 @@ static const MemoryRegionOps vfio_ati_3c3_quirk = {
>>>  static VFIOQuirk *vfio_quirk_alloc(int nr_mem)
>>>  {
>>>  VFIOQuirk *quirk = g_malloc0(sizeof(*quirk));
>>> +QLIST_INIT(>ioeventfds);
>>>  quirk->mem = g_new0(MemoryRegion, nr_mem);
>>>  quirk->nr_mem = nr_mem;
>>>  
>>>  return quirk;
>>>  }
>>>  
>>> +static void vfio_ioeventfd_exit(VFIOIOEventFD *ioeventfd)
>>> +{
>>> +QLIST_REMOVE(ioeventfd, next);
>>> +memory_region_del_eventfd(ioeventfd->mr, ioeventfd->addr, 
>>> ioeventfd->size,
>>> +  ioeventfd->match_data, ioeventfd->data,
>>> +  >e);
>>> +qemu_set_fd_handler(event_notifier_get_fd(>e), NULL, NULL, 
>>> NULL);
>>> +event_notifier_cleanup(>e);
>>> +g_free(ioeventfd);
>>> +}
>>> +
>>>  static void vfio_vga_probe_ati_3c3_quirk(VFIOPCIDevice *vdev)
>>>  {
>>>  VFIOQuirk *quirk;
>>> @@ -1668,6 +1681,10 @@ void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr)
>>>  int i;
>>>  
>>>  QLIST_FOREACH(quirk, >quirks, next) {
>>> +while (!QLIST_EMPTY(>ioeventfds)) {
>>> +vfio_ioeventfd_exit(QLIST_FIRST(>ioeventfds));
>>> +}
>>> +
>>>  for (i = 0; i < quirk->nr_mem; i++) {
>>>  memory_region_del_subregion(bar->region.mem, >mem[i]);
>>>  }
>>> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
>>> index f4aa13e021fa..146065c2f715 100644
>>> --- a/hw/vfio/pci.h
>>> +++ b/hw/vfio/pci.h
>>> @@ -24,9 +24,20 @@
>>>  
>>>  struct VFIOPCIDevice;
>>>  
>>> +typedef struct VFIOIOEventFD {
>>> +QLIST_ENTRY(VFIOIOEventFD) next;
>>> +MemoryRegion *mr;
>>> +hwaddr addr;
>>> +unsigned size;
>>> +bool match_data;  
>> Shouldn't you add the match_data field also in the kernel uapi?
> 
> In the vfio uapi?  vfio isn't matching anything and has no ability to
> match anything, it's just triggering a pre-programmed write via
> eventfd.  Maybe I'm not understanding the comment.  Thanks,

Yes that not sensible. We have a KVM ioeventfd that triggers an fd only
if the address/data matches and this VFIO "ioeventfd" listens to this fd
and writes the fixed address/data. At the beginning I was thinking about
having the VFIO handler taking in charge any value written at this
address but there is no way to retrieve the value besides doing a fixed
assignment from userspace. Sorry for the noise.

Thanks

Eric
> 
> Alex
> 



Re: [Qemu-devel] [RFC PATCH 2/5] vfio/quirks: Add generic support for ioveventfds

2018-02-08 Thread Alex Williamson
On Thu, 8 Feb 2018 12:11:57 +0100
Auger Eric  wrote:

> Hi Alex,
> 
> On 07/02/18 01:26, Alex Williamson wrote:
> > We might wish to handle some quirks via ioeventfds, add a list of
> > ioeventfds to the quirk.  
> The commit title is a bit misleading as we only add the data type and
> deletion function.

Unfortunately that's as much as I can add here or else QEMU will fail
to build because of unused functions :-\  So I pulled as much
infrastructure as I could here but had to leave some meat for when it
gets called.  Suggestions welcome for title/commit updates.

> > 
> > Signed-off-by: Alex Williamson 
> > ---
> >  hw/vfio/pci-quirks.c |   17 +
> >  hw/vfio/pci.h|   11 +++
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> > index 10af23217292..e4cf4ea2dd9c 100644
> > --- a/hw/vfio/pci-quirks.c
> > +++ b/hw/vfio/pci-quirks.c
> > @@ -12,6 +12,7 @@
> >  
> >  #include "qemu/osdep.h"
> >  #include "qemu/error-report.h"
> > +#include "qemu/main-loop.h"
> >  #include "qemu/range.h"
> >  #include "qapi/error.h"
> >  #include "qapi/visitor.h"
> > @@ -278,12 +279,24 @@ static const MemoryRegionOps vfio_ati_3c3_quirk = {
> >  static VFIOQuirk *vfio_quirk_alloc(int nr_mem)
> >  {
> >  VFIOQuirk *quirk = g_malloc0(sizeof(*quirk));
> > +QLIST_INIT(>ioeventfds);
> >  quirk->mem = g_new0(MemoryRegion, nr_mem);
> >  quirk->nr_mem = nr_mem;
> >  
> >  return quirk;
> >  }
> >  
> > +static void vfio_ioeventfd_exit(VFIOIOEventFD *ioeventfd)
> > +{
> > +QLIST_REMOVE(ioeventfd, next);
> > +memory_region_del_eventfd(ioeventfd->mr, ioeventfd->addr, 
> > ioeventfd->size,
> > +  ioeventfd->match_data, ioeventfd->data,
> > +  >e);
> > +qemu_set_fd_handler(event_notifier_get_fd(>e), NULL, NULL, 
> > NULL);
> > +event_notifier_cleanup(>e);
> > +g_free(ioeventfd);
> > +}
> > +
> >  static void vfio_vga_probe_ati_3c3_quirk(VFIOPCIDevice *vdev)
> >  {
> >  VFIOQuirk *quirk;
> > @@ -1668,6 +1681,10 @@ void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr)
> >  int i;
> >  
> >  QLIST_FOREACH(quirk, >quirks, next) {
> > +while (!QLIST_EMPTY(>ioeventfds)) {
> > +vfio_ioeventfd_exit(QLIST_FIRST(>ioeventfds));
> > +}
> > +
> >  for (i = 0; i < quirk->nr_mem; i++) {
> >  memory_region_del_subregion(bar->region.mem, >mem[i]);
> >  }
> > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> > index f4aa13e021fa..146065c2f715 100644
> > --- a/hw/vfio/pci.h
> > +++ b/hw/vfio/pci.h
> > @@ -24,9 +24,20 @@
> >  
> >  struct VFIOPCIDevice;
> >  
> > +typedef struct VFIOIOEventFD {
> > +QLIST_ENTRY(VFIOIOEventFD) next;
> > +MemoryRegion *mr;
> > +hwaddr addr;
> > +unsigned size;
> > +bool match_data;  
> Shouldn't you add the match_data field also in the kernel uapi?

In the vfio uapi?  vfio isn't matching anything and has no ability to
match anything, it's just triggering a pre-programmed write via
eventfd.  Maybe I'm not understanding the comment.  Thanks,

Alex



Re: [Qemu-devel] [RFC PATCH 2/5] vfio/quirks: Add generic support for ioveventfds

2018-02-08 Thread Auger Eric
Hi Alex,

On 07/02/18 01:26, Alex Williamson wrote:
> We might wish to handle some quirks via ioeventfds, add a list of
> ioeventfds to the quirk.
The commit title is a bit misleading as we only add the data type and
deletion function.
> 
> Signed-off-by: Alex Williamson 
> ---
>  hw/vfio/pci-quirks.c |   17 +
>  hw/vfio/pci.h|   11 +++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index 10af23217292..e4cf4ea2dd9c 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -12,6 +12,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/error-report.h"
> +#include "qemu/main-loop.h"
>  #include "qemu/range.h"
>  #include "qapi/error.h"
>  #include "qapi/visitor.h"
> @@ -278,12 +279,24 @@ static const MemoryRegionOps vfio_ati_3c3_quirk = {
>  static VFIOQuirk *vfio_quirk_alloc(int nr_mem)
>  {
>  VFIOQuirk *quirk = g_malloc0(sizeof(*quirk));
> +QLIST_INIT(>ioeventfds);
>  quirk->mem = g_new0(MemoryRegion, nr_mem);
>  quirk->nr_mem = nr_mem;
>  
>  return quirk;
>  }
>  
> +static void vfio_ioeventfd_exit(VFIOIOEventFD *ioeventfd)
> +{
> +QLIST_REMOVE(ioeventfd, next);
> +memory_region_del_eventfd(ioeventfd->mr, ioeventfd->addr, 
> ioeventfd->size,
> +  ioeventfd->match_data, ioeventfd->data,
> +  >e);
> +qemu_set_fd_handler(event_notifier_get_fd(>e), NULL, NULL, 
> NULL);
> +event_notifier_cleanup(>e);
> +g_free(ioeventfd);
> +}
> +
>  static void vfio_vga_probe_ati_3c3_quirk(VFIOPCIDevice *vdev)
>  {
>  VFIOQuirk *quirk;
> @@ -1668,6 +1681,10 @@ void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr)
>  int i;
>  
>  QLIST_FOREACH(quirk, >quirks, next) {
> +while (!QLIST_EMPTY(>ioeventfds)) {
> +vfio_ioeventfd_exit(QLIST_FIRST(>ioeventfds));
> +}
> +
>  for (i = 0; i < quirk->nr_mem; i++) {
>  memory_region_del_subregion(bar->region.mem, >mem[i]);
>  }
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index f4aa13e021fa..146065c2f715 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -24,9 +24,20 @@
>  
>  struct VFIOPCIDevice;
>  
> +typedef struct VFIOIOEventFD {
> +QLIST_ENTRY(VFIOIOEventFD) next;
> +MemoryRegion *mr;
> +hwaddr addr;
> +unsigned size;
> +bool match_data;
Shouldn't you add the match_data field also in the kernel uapi?

Thanks

Eric
> +uint64_t data;
> +EventNotifier e;
> +} VFIOIOEventFD;.
> +
>  typedef struct VFIOQuirk {
>  QLIST_ENTRY(VFIOQuirk) next;
>  void *data;
> +QLIST_HEAD(, VFIOIOEventFD) ioeventfds;
>  int nr_mem;
>  MemoryRegion *mem;
>  } VFIOQuirk;
> 



[Qemu-devel] [RFC PATCH 2/5] vfio/quirks: Add generic support for ioveventfds

2018-02-06 Thread Alex Williamson
We might wish to handle some quirks via ioeventfds, add a list of
ioeventfds to the quirk.

Signed-off-by: Alex Williamson 
---
 hw/vfio/pci-quirks.c |   17 +
 hw/vfio/pci.h|   11 +++
 2 files changed, 28 insertions(+)

diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 10af23217292..e4cf4ea2dd9c 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -12,6 +12,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
+#include "qemu/main-loop.h"
 #include "qemu/range.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
@@ -278,12 +279,24 @@ static const MemoryRegionOps vfio_ati_3c3_quirk = {
 static VFIOQuirk *vfio_quirk_alloc(int nr_mem)
 {
 VFIOQuirk *quirk = g_malloc0(sizeof(*quirk));
+QLIST_INIT(>ioeventfds);
 quirk->mem = g_new0(MemoryRegion, nr_mem);
 quirk->nr_mem = nr_mem;
 
 return quirk;
 }
 
+static void vfio_ioeventfd_exit(VFIOIOEventFD *ioeventfd)
+{
+QLIST_REMOVE(ioeventfd, next);
+memory_region_del_eventfd(ioeventfd->mr, ioeventfd->addr, ioeventfd->size,
+  ioeventfd->match_data, ioeventfd->data,
+  >e);
+qemu_set_fd_handler(event_notifier_get_fd(>e), NULL, NULL, 
NULL);
+event_notifier_cleanup(>e);
+g_free(ioeventfd);
+}
+
 static void vfio_vga_probe_ati_3c3_quirk(VFIOPCIDevice *vdev)
 {
 VFIOQuirk *quirk;
@@ -1668,6 +1681,10 @@ void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr)
 int i;
 
 QLIST_FOREACH(quirk, >quirks, next) {
+while (!QLIST_EMPTY(>ioeventfds)) {
+vfio_ioeventfd_exit(QLIST_FIRST(>ioeventfds));
+}
+
 for (i = 0; i < quirk->nr_mem; i++) {
 memory_region_del_subregion(bar->region.mem, >mem[i]);
 }
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index f4aa13e021fa..146065c2f715 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -24,9 +24,20 @@
 
 struct VFIOPCIDevice;
 
+typedef struct VFIOIOEventFD {
+QLIST_ENTRY(VFIOIOEventFD) next;
+MemoryRegion *mr;
+hwaddr addr;
+unsigned size;
+bool match_data;
+uint64_t data;
+EventNotifier e;
+} VFIOIOEventFD;
+
 typedef struct VFIOQuirk {
 QLIST_ENTRY(VFIOQuirk) next;
 void *data;
+QLIST_HEAD(, VFIOIOEventFD) ioeventfds;
 int nr_mem;
 MemoryRegion *mem;
 } VFIOQuirk;