Re: [Qemu-devel] [PATCH v7 2/4] vfio: Add vfio_container_spapr_get_info()

[Gavin Shan <gwshan@linux.vnet.ibm.com>]

On Fri, Jun 06, 2014 at 09:40:56AM +1000, Alexey Kardashevskiy wrote:
>On 06/06/2014 05:27 AM, Alex Williamson wrote:
>> On Thu, 2014-06-05 at 15:49 +1000, Alexey Kardashevskiy wrote:
>>> To perform DMA mapping via TCE table correctly, the guest must
>>> know where DMA window is located on the PCI bus. A hypervisor is
>>> expected to provide such information. Since QEMU has no control
>>> over this setting, we need a way to obtain a start address and size
>>> from the host VFIO driver.
>>>
>>> This adds a helper which returns the default DMA window properties
>>> for the specific IOMMU group. The upstream kernel implements this ioctl
>>> already.
>> 
>> Couldn't this be done with Gavin's vfio_pci_container_ioctl()?  Thanks,
>
>
>Good point, I missed that. I'll merge two helpers into one and repost, then
>Gavin will use it.
>

Sure, I'll rebase my code on top of Alexey's. But Alex raised more comments
about the function. I just copy & paste so that we don't miss the comments:

- "fd == 0" is valid
- In addition to fd 0 being valid, there's some white space issues here.

  Passing an integer option is not very extensible, maybe a void* that
  gets cast to an int* for VFIO_EEH_PE_OP would be better.  It's a qemu
  internal API though, so I'm not going to sweat saving that problem for
  the next user.  Thanks,

Thanks,
Gavin

>> 
>> Alex
>> 
>> 
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> Changes:
>>> 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         | 36 ++++++++++++++++++++++++++++++++++++
>>>  include/hw/misc/vfio.h | 11 +++++++++++
>>>  2 files changed, 47 insertions(+)
>>>  create mode 100644 include/hw/misc/vfio.h
>>>
>>> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
>>> index 7437c2e..99141f3 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,38 @@ static void register_vfio_pci_dev_type(void)
>>>  }
>>>  
>>>  type_init(register_vfio_pci_dev_type)
>>> +
>>> +int vfio_container_spapr_get_info(AddressSpace *as,
>>> +                                  int32_t groupid,
>>> +                                  struct vfio_iommu_spapr_tce_info *info)
>>> +{
>>> +    VFIOGroup *group;
>>> +    VFIOContainer *container;
>>> +    int ret, fd;
>>> +
>>> +    group = vfio_get_group(groupid, as);
>>> +    if (!group) {
>>> +        return -1;
>>> +    }
>>> +    container = group->container;
>>> +    if (!group->container) {
>>> +        goto put_group_exit;
>>> +    }
>>> +    fd = container->fd;
>>> +    if (!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
>>> +        goto put_group_exit;
>>> +    }
>>> +    ret = ioctl(fd, VFIO_IOMMU_SPAPR_TCE_GET_INFO, info);
>>> +    if (ret) {
>>> +        error_report("vfio: failed to get iommu info for container: %s",
>>> +                     strerror(errno));
>>> +        goto put_group_exit;
>>> +    }
>>> +
>>> +    return 0;
>>> +
>>> +put_group_exit:
>>> +    vfio_put_group(group);
>>> +
>>> +    return -1;
>>> +}
>>> diff --git a/include/hw/misc/vfio.h b/include/hw/misc/vfio.h
>>> new file mode 100644
>>> index 0000000..e82f5a3
>>> --- /dev/null
>>> +++ b/include/hw/misc/vfio.h
>>> @@ -0,0 +1,11 @@
>>> +#ifndef VFIO_API_H
>>> +#define VFIO_API_H
>>> +
>>> +#include "qemu/typedefs.h"
>>> +#include <linux/vfio.h>
>>> +
>>> +extern int vfio_container_spapr_get_info(AddressSpace *as,
>>> +                                         int32_t groupid,
>>> +                                         struct vfio_iommu_spapr_tce_info *info);
>>> +
>>> +#endif
>> 
>> 
>> 
>
>
>-- 
>Alexey
>