On Wed, May 27, 2020 at 05:53:16PM +0200, Philippe Mathieu-Daudé wrote:
> >>> +    for (i = 0; i < ARRAY_SIZE(iommu); i++) {
> >>> +        if (ioctl(container->fd, VFIO_CHECK_EXTENSION, iommu[i].type)) {
> >>> +            trace_vfio_get_iommu_type(iommu[i].type, iommu[i].name);
> >> Just wondering why you want to trace the type as you now have the name
> >> string.
> > 
> > You are right :)
> > 
> >>> +            return iommu[i].type;
> >>>          }
> >>>      }
> >>> +    trace_vfio_get_iommu_type(-1, "Not available or not supported");
> >> nit: from a debugging pov, this may be not needed as
> >> vfio_get_group/vfio_connect_container() fails and this leads to an error
> >> output.
> 
> But you can reach this for example using No-IOMMU. If you don't mind, I
> find having this information in the trace log clearer.

I kinda agree with Eric - AFAICT QEMU vfio-pci don't work with no-iommu, then
it seems meaningless to trace it...

I'm not sure whether this trace is extremely helpful because syscalls like this
could be easily traced by things like strace or bpftrace as general tools (and
this information should be a one-time thing rather than dynamically changing),
no strong opinion though.  Also, if we want to dump something, maybe it's
better to do in vfio_init_container() after vfio_get_iommu_type() succeeded, so
we dump which container is enabled with what type of iommu.

Thanks,

-- 
Peter Xu


Reply via email to