Re: [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl()

[Alex Williamson <alex.williamson@redhat.com>]

On Sat, 2014-06-07 at 02:54 +1000, Alexey Kardashevskiy wrote:
> On 06/07/2014 02:48 AM, Alex Williamson wrote:
> > On Fri, 2014-06-06 at 13:34 +1000, Alexey Kardashevskiy wrote:
> >> While most operations with VFIO IOMMU driver are generic and used inside
> >> vfio.c, there are still some operations which only specific VFIO IOMMU
> >> drivers implement. The first example of it will be reading a DMA window
> >> start from the host.
> >>
> >> This adds a helper which passes an ioctl request to the container's fd.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >> ---
> >> Changes:
> >> v8:
> >> * s/vfio_container_spapr_get_info/vfio_container_ioctl/ - now it is
> >> generalized
> >>
> >> v7:
> >> * do not return a group fd from the helper
> >>
> >> v6:
> >> * added dup() to protect group_fd from accidental disposal
> >>
> >> v5:
> >> * reworked to reflect change in vfio_get_group() from one
> >> of previous patches change
> >>
> >> v4:
> >> * fixed possible leaks on error paths
> >> ---
> >>  hw/misc/vfio.c         | 28 ++++++++++++++++++++++++++++
> >>  include/hw/misc/vfio.h |  9 +++++++++
> >>  2 files changed, 37 insertions(+)
> >>  create mode 100644 include/hw/misc/vfio.h
> >>
> >> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> >> index 7437c2e..bb77934 100644
> >> --- a/hw/misc/vfio.c
> >> +++ b/hw/misc/vfio.c
> >> @@ -39,6 +39,7 @@
> >>  #include "qemu/range.h"
> >>  #include "sysemu/kvm.h"
> >>  #include "sysemu/sysemu.h"
> >> +#include "hw/misc/vfio.h"
> >>  
> >>  /* #define DEBUG_VFIO */
> >>  #ifdef DEBUG_VFIO
> >> @@ -4318,3 +4319,30 @@ static void register_vfio_pci_dev_type(void)
> >>  }
> >>  
> >>  type_init(register_vfio_pci_dev_type)
> >> +
> >> +int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> >> +                         int req, void *param)
> >> +{
> >> +    VFIOGroup *group;
> >> +    VFIOContainer *container;
> >> +    int ret = -1;
> >> +
> >> +    group = vfio_get_group(groupid, as);
> >> +    if (!group) {
> >> +        error_report("vfio: group %d not registered", groupid);
> >> +        return ret;
> >> +    }
> > 
> > Gavin's version of this walked existing groups and returned an error if
> > not found.  vfio_get_group() will go out and create a group and connect
> > it to a container.  I'm not sure we really want to expose that sort of
> > capability out to the reset of QEMU.  Thanks,
> 
> 
> If it is not created here, then it can only be created from vfio_initfn()
> which is too late as I need group to get DMA window properties for PHB when
> PHB is being realized.
> 
> Add another helper?

Urgh.  No, let's not make another helper.  Can we at least keep the
filtering Gavin had so we don't just allow any ioctl through here
though?  At least we can make vfio.c involved if someone what's to do
something crazy here.  Thanks,

Alex

> >> +
> >> +    container = group->container;
> >> +    if (group->container) {
> >> +        ret = ioctl(container->fd, req, param);
> >> +        if (ret < 0) {
> >> +            error_report("vfio: failed to ioctl container: ret=%d, %s",
> >> +                         ret, strerror(errno));
> >> +        }
> >> +    }
> >> +
> >> +    vfio_put_group(group);
> >> +
> >> +    return ret;
> >> +}
> >> diff --git a/include/hw/misc/vfio.h b/include/hw/misc/vfio.h
> >> new file mode 100644
> >> index 0000000..0b26cd8
> >> --- /dev/null
> >> +++ b/include/hw/misc/vfio.h
> >> @@ -0,0 +1,9 @@
> >> +#ifndef VFIO_API_H
> >> +#define VFIO_API_H
> >> +
> >> +#include "qemu/typedefs.h"
> >> +
> >> +extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> >> +                                int req, void *param);
> >> +
> >> +#endif
> > 
> > 
> > 
> 
>