On Tue, 2015-06-09 at 11:37 +0800, Chen Fan wrote:
> Pre-adding all affected groups for aer devices, it could
> ensure the affected groups are owned in VM.
>
> Signed-off-by: Chen Fan <chen.fan.f...@cn.fujitsu.com>
> ---
> hw/vfio/common.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index b1045da..4230f83 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -793,8 +793,15 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as)
>
> QLIST_FOREACH(group, &vfio_group_list, next) {
> if (group->groupid == groupid) {
> + if (as && !group->container) {
> + if (vfio_connect_container(group, as)) {
> + error_report("vfio: failed to setup container for group
> %d",
> + groupid);
> + return NULL;
> + }
> + }
> /* Found it. Now is it already in the right context? */
> - if (group->container->space->as == as) {
> + if (!as || group->container->space->as == as) {
> return group;
> } else {
> error_report("vfio: group %d used in multiple address
> spaces",
> @@ -828,7 +835,7 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as)
> group->groupid = groupid;
> QLIST_INIT(&group->device_list);
>
> - if (vfio_connect_container(group, as)) {
> + if (as && vfio_connect_container(group, as)) {
> error_report("vfio: failed to setup container for group %d",
> groupid);
> goto close_fd_exit;
> }
> @@ -859,7 +866,9 @@ void vfio_put_group(VFIOGroup *group)
> }
>
> vfio_kvm_device_del_group(group);
> - vfio_disconnect_container(group);
> + if (group->container) {
> + vfio_disconnect_container(group);
> + }
> QLIST_REMOVE(group, next);
> trace_vfio_put_group(group->fd);
> close(group->fd);
It's an interesting idea, but should we instead separate the container
from vfio_get/put_group() to be done as a separate step? Using the
AddressSpace pointer is a bit obfuscated.
Also, this gets you the group, but the vfio_group_get_external_user()
interface used by vfio-pci to do a bus reset requires that the group is
active, with an iommu. If it didn't, we could potentially be resetting
a non-viable group, with devices still owned by the host or not isolated
by the iommu. So you're a step closer, but if we go back to the example
of a dual port NIC with isolated functions, QEMU may be able to get the
group for the second port, it may even be a viable group, but if the
second port doesn't eventually get connected to the VM, the bus reset
won't be allowed.
