On 09/06/2013 05:01 AM, Alex Williamson wrote: > On Fri, 2013-08-30 at 20:15 +1000, Alexey Kardashevskiy wrote: >> As sPAPR platform supports DMA windows on a PCI bus, the information >> about their location and size should be passed into the guest via >> the device tree. >> >> The patch adds a helper to read this info from the container fd. >> >> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru> >> --- >> Changes: >> v4: >> * fixed possible leaks on error paths >> --- >> hw/misc/vfio.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/misc/vfio.h | 11 +++++++++++ >> 2 files changed, 56 insertions(+) >> create mode 100644 include/hw/misc/vfio.h >> >> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c >> index 53791fb..4210471 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 >> @@ -3490,3 +3491,47 @@ 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, >> + int *group_fd) >> +{ >> + VFIOAddressSpace *space; >> + VFIOGroup *group; >> + VFIOContainer *container; >> + int ret, fd; >> + >> + space = vfio_get_address_space(as); >> + if (!space) { >> + return -1; >> + } >> + group = vfio_get_group(groupid, space); >> + if (!group) { >> + goto put_as_exit; >> + } >> + 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; >> + } >> + *group_fd = group->fd; > > The above gets don't actually increment a reference count, so copying > the fd seems risky here.
If fd is gone while I am carrying it to my "external VFIO user" to call kvmppc_vfio_group_get_external_user() on it, then the guest just shut itself in a foot, no? And I do not see how I would make it no risky, do you? >> + >> + return 0; >> + >> +put_group_exit: >> + vfio_put_group(group); >> + >> +put_as_exit: >> + vfio_put_address_space(space); > > But put_group calls disconnect_container which calls > put_address_space... so it get's put twice. The lack of symmetry > already bites us with a bug. True. This will be fixed by moving vfio_get_address_space() into vfio_get_group(). -- Alexey