From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47872) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2Qz7-0000Ms-7x for qemu-devel@nongnu.org; Tue, 09 Jun 2015 17:23:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z2Qz3-0000zN-9h for qemu-devel@nongnu.org; Tue, 09 Jun 2015 17:23:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50064) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2Qz3-0000z5-2z for qemu-devel@nongnu.org; Tue, 09 Jun 2015 17:22:57 -0400 Message-ID: <1433884975.4927.142.camel@redhat.com> From: Alex Williamson Date: Tue, 09 Jun 2015 15:22:55 -0600 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC v9 10/18] get all affected groups for each device support aer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chen Fan Cc: izumi.taku@jp.fujitsu.com, qemu-devel@nongnu.org On Tue, 2015-06-09 at 11:37 +0800, Chen Fan wrote: > Add the affected groups without any devices into VM, > it can keep the VM ownship the all groups. and use a > reference to make the group visible. > > Signed-off-by: Chen Fan > --- > hw/vfio/pci.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 108 insertions(+), 7 deletions(-) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 7a3fad7..06006ce 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -2822,6 +2822,105 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos) > return 0; > } > > +static bool vfio_pci_host_match(PCIHostDeviceAddress *host1, > + PCIHostDeviceAddress *host2) > +{ > + return (host1->domain == host2->domain && host1->bus == host2->bus && > + host1->slot == host2->slot && host1->function == host2->function); > +} > + > +static int vfio_put_affected_groups(VFIOPCIDevice *vdev) > +{ > + struct vfio_pci_hot_reset_info *info = NULL; > + struct vfio_pci_dependent_device *devices; > + PCIHostDeviceAddress host; > + VFIOGroup *group; > + int ret, i; > + > + ret = vfio_get_hot_reset_info(vdev, &info); > + if (ret) { > + goto out; > + } > + > + /* List all affected devices by bus reset */ > + devices = &info->devices[0]; > + > + /* Verify that we have all the groups required */ > + for (i = 0; i < info->count; i++) { > + host.domain = devices[i].segment; > + host.bus = devices[i].bus; > + host.slot = PCI_SLOT(devices[i].devfn); > + host.function = PCI_FUNC(devices[i].devfn); > + > + /* Skip the current device */ > + if (vfio_pci_host_match(&host, &vdev->host)) { > + continue; > + } > + > + /* Ensure we own the group of the affected device */ > + QLIST_FOREACH(group, &vfio_group_list, next) { > + if (group->groupid == devices[i].group_id) { > + break; > + } > + } > + > + if (!group) > + continue; > + > + group->ref--; > + vfio_put_group(group); > + } > + > + ret = 0; > +out: > + g_free(info); > + return ret; > +} > + > +static int vfio_get_affected_groups(VFIOPCIDevice *vdev) > +{ > + struct vfio_pci_hot_reset_info *info = NULL; > + struct vfio_pci_dependent_device *devices; > + PCIHostDeviceAddress host; > + VFIOGroup *group; > + int ret, i; > + > + ret = vfio_get_hot_reset_info(vdev, &info); > + if (ret) { > + goto out; > + } > + > + /* List all affected devices by bus reset */ > + devices = &info->devices[0]; > + > + /* Verify that we have all the groups required */ > + for (i = 0; i < info->count; i++) { > + host.domain = devices[i].segment; > + host.bus = devices[i].bus; > + host.slot = PCI_SLOT(devices[i].devfn); > + host.function = PCI_FUNC(devices[i].devfn); > + > + /* Skip the current device */ > + if (vfio_pci_host_match(&host, &vdev->host)) { > + continue; > + } > + > + /* Get all affected groups into VM */ > + group = vfio_get_group(devices[i].group_id, NULL); > + if (!group) { > + error_report("vfio: failed to get affected group %d", > + devices[i].group_id); > + ret = -1; > + goto out; There needs to be an unwind here, if the error occurs during hotplug, we've just left QEMU in a state where some groups have extra references. > + } > + group->ref++; If we're going to have a group reference, get_group should increment it and put_group should decrement it, we shouldn't modify it externally from those functions. > + } > + ret = 0; > +out: > + g_free(info); > + return ret; > +} > + > static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver, > int pos, uint16_t size) > { > @@ -2858,6 +2957,12 @@ static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver, > dev_iter = pci_bridge_get_device(dev_iter->bus); > } > > + /* Ensure own all affected groups */ > + ret = vfio_get_affected_groups(vdev); > + if (ret) { > + goto error; > + } > + > errcap = vfio_pci_read_config(pdev, pdev->exp.aer_cap + PCI_ERR_CAP, 4); > /* > * The ability to record multiple headers is depending on > @@ -3013,13 +3118,6 @@ static void vfio_pci_post_reset(VFIOPCIDevice *vdev) > vfio_enable_intx(vdev); > } > > -static bool vfio_pci_host_match(PCIHostDeviceAddress *host1, > - PCIHostDeviceAddress *host2) > -{ > - return (host1->domain == host2->domain && host1->bus == host2->bus && > - host1->slot == host2->slot && host1->function == host2->function); > -} > - > static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single) > { > VFIOGroup *group; > @@ -3851,6 +3949,9 @@ static void vfio_instance_finalize(Object *obj) > g_free(vdev->rom); > vfio_put_device(vdev); > vfio_put_group(group); > + if (vdev->features & VFIO_FEATURE_ENABLE_AER) { > + vfio_put_affected_groups(vdev); > + } > } > > static void vfio_exitfn(PCIDevice *pdev)