Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Wed, Feb 17, 2016 at 06:02:36AM +0000, Tian, Kevin wrote:
> > From: Neo Jia
> > Sent: Wednesday, February 17, 2016 1:38 PM
> > > > >
> > > > >
> > > >
> > > > Hi Kevin,
> > > >
> > > > The answer is simple, having a UUID as part of the device name will 
> > > > give you a
> > > > unique sysfs path that will be opened by QEMU.
> > > >
> > > > vgpu-vendor-0 and vgpu-vendor-1 will not be unique as we can have 
> > > > multiple
> > > > virtual gpu devices per VM coming from same or different physical 
> > > > devices.
> > >
> > > That is not a problem. We can add physical device info too like 
> > > vgpu-vendor-0-0,
> > > vgpu-vendor-1-0, ...
> > >
> > > Please note Qemu doesn't care about the actual name. It just accepts a 
> > > sysfs path
> > > to open.
> > 
> > Hi Kevin,
> > 
> > No, I think you are making things even more complicated than it is required,
> > also it is not generic anymore as you are requiring the QEMU to know more 
> > than
> > he needs to.
> > 
> > The way you name those devices will require QEMU to know the relation
> > between virtual devices and physical devices. I don't think that is good.
> 
> I don't think you get my point. Look at how device is assigned in Qemu today:
> 
> -device vfio-pci,host=02:00.0
> 
> Then in a recent patch from Alex, Qemu will accept sysfsdev as well:
> 
> -device vfio-pci,sysfsdev=/sys/devices/pci0000:00/0000:00:1c.0/0000:02:00.0
> 
> Then with vgu (one example from Alex's original post):
> 
> -device 
> vfio-pci,sysfsdev=/sys/devices/virtual/intel-vgpu/address@hidden:00:02.0

Hi Kevin,

I am fully aware of Alex's patch, that is just an example, but he doesn't
exclude the cases of using UUID as the device path as he mentioned in the
beginning of this long email thread, actually he has already agreed with this
UUID-$vgpu_idx path. :-)

Also, you should note that with the proposal we have, it doesn't require to have
anything like either intel-vgpu or nvidia-vgpu as the path, having a non-vendor
specific information within the vgpu device path is one of the requirement IIRC.

-device vfio-pci,sysfsdev=/sys/devices/virtual/intel-vgpu/address@hidden:00:02.0

> 
> Qemu doesn't need to know the relation between virtual/physical devices at
> all. It's just a path regardless of how vgpu name is created (either with your
> UUID proposal or my descriptive string proposal)

No, with path like above, QEMU needs to know the virtual device is created from
that physical device 0000:00:02.0 right? (you have mentioned this yourself
actually below.) If QEMU doesn't want to know that, then he will transfer the
burden to the upper layer stack such as libvirt, who has to figure out the right
path of this new VM, as the vgpu<$id>, the <%id> will become another generic
number generated by the vgpu core driver. 

So why not just use UUID here?

> 
> > 
> > My flow is like this:
> > 
> > libvirt creats a VM object, it will have a UUID. then it will use the UUID 
> > to
> > create virtual gpu devices, then it will pass the UUID to the QEMU (actually
> > QEMU already has the VM UUID), then it will just open up the unique path.
> 
> If you look at above example, it's not UUID itself being passed to Qemu. It's
> the sysfsdev path.

UUID is always sent to QEMU, if you look at your QEMU command line, please.

Yes, it is a sysfs path, even with UUID, it is a sysfs path, the only difference
is that we have a uuid embedded within the device name.

> 
> > 
> > Also, you need to consider those 0-0 numbers are not generic as the UUID.
> 
> that encoding could be flexible to include any meaningful string. libvirt can
> itself manages how UUID is mapped to an actual vgpu name.
> 
> > >
> > > >
> > > > If you are worried about losing meaningful name here, we can create a 
> > > > sysfs file
> > > > to capture the vendor device description if you like.
> > > >
> > >
> > > Having the vgpu name descriptive is more informative imo. User can simply 
> > > check
> > > sysfs names to know raw information w/o relying on 3rd party agent to 
> > > query
> > > information around an opaque UUID.
> > >
> > 
> > You are actually arguing against your own design here, unfortunately. If you
> > look at your design carefully, it is your design actually require to have a 
> > 3rd
> > party code to figure out the VM and virtual gpu device relation as it is
> > never documented in the sysfs.
> 
> No. It's not about figuring out the relation between VM and vGPU. It's about
> figuring out some raw information about vGPU itself, such as:
> 
> address@hidden:00:02.0, I can immediately know it's the 1st instance created 
> on
> 0000:00:02.0 device. While with an UUID, it doesn't speak anything useful 
> here.

Right, I get your point, but this requires the qemu to know this virtual device
is created on that physical device, isn't it, given you already know that the
virtual device is created on the physical device 0000:00:02.0, right?

> 
> This is a typical way how device nodes are created within sysfs, e.g. on my
> platform:
> 
> $ ls /sys/class/drm/
> card0/          card0-DP-2/     card0-HDMI-A-2/ controlD64/
> card0-DP-1/     card0-HDMI-A-1/ card0-VGA-1/    version
> 
> $ ls /sys/bus/pci/devices/
> 0000:00:00.0  0000:00:14.0  0000:00:1a.0  0000:00:1c.1  0000:00:1f.2
> 0000:00:02.0  0000:00:16.0  0000:00:1b.0  0000:00:1d.0  0000:00:1f.3
> 0000:00:03.0  0000:00:19.0  0000:00:1c.0  0000:00:1f.0  0000:02:00.0
> 
> We'd better keep such style when creating vgpu nodes in sysfs. UUID is
> at most anther info suffixed to the default string (or in another file), if 
> necessary.

It doesn't apply here, your above example are all physical devices.

The reason I want to have UUID is it to match the instance of problem domain
here - VM.

> 
> > 
> > In our current design, it doesn't require any 3rd party agent as the VM 
> > UUID is
> > part of the QEMU command already, and the VM UUID is already embedded 
> > within the
> > virtual device path.
> > 
> > Also, it doesn't require 3rd party to retrieve information as the virtual 
> > device
> > will just be a directory, we will have another file within each virtual gpu
> > device directory, you can always cat the file to retrieve vendor 
> > information.
> > 
> > Let's use the UUID-$vgpu_idx as the virtual device directory name plus a 
> > vendor
> > description file within that directory, so we don't lose any additional
> > information, also capture the VM and virtual device relation.
> > 
> 
> I'm fine to have another node to provide more vendor specific information. But
> I don't want to make UUID mandatory when creating a vGPU instance, as 
> explained above. Today we can create VMs in KVM w/o using libvirt, and w/o 
> the need of allocating any UUID. Same thing should be supported for vgpu
> feature too.

So, I really don't see any drawback of using UUID as part of the virtual device
directory, it is easy, simple and organically reflecting the relation between
virtual device and the owner.

Each QEMU process is representing a VM, and a UUID is associated with it. The
virtual gpu device is just a virtual device of this owner, so the path is:

-device vfio-pci,sysfsdev=/sys/devices/virtual/vgpu/$UUID-$vgpu_idx/

You can have multiple virtual device per VM and QEMU doesn't need to know which
physical device it comes from, especially it will automatically know which
virtual device it owns, so the -device vfio-pci path will be setup for free.

If your most concern is having this kind of path doesn't provide enough
information of the virtual device, we can add more sysfs attributes within the
directory of /sys/devices/virtual/vgpu/$UUID-$vgpu_idx/ to reflect the
information you want.

Even with UUID, you don't need libvirt at all. you can get uuid by running
uuidgen command, I don't need libvirt to code up and test the RFC that I have
sent out early. :-)

Thanks,
Neo

> 
> Thanks
> Kevin
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to address@hidden
> More majordomo info at  http://vger.kernel.org/majordomo-info.html