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

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 06/07/2014 03:35 AM, Alex Williamson wrote:
> 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,


if (req != CHECK_EXTENSION) &&
   (req != ENABLE) &&
   (reg != EEH_OP)
	return -1;

Like that (roughly)?




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


-- 
Alexey