Re: [PATCH v7 0/4] Add Mediated device support

[Alex Williamson <alex.williamson@redhat.com>]

On Thu, 1 Sep 2016 18:47:06 +0200
Michal Privoznik <mprivozn@redhat.com> wrote:

> On 31.08.2016 08:12, Tian, Kevin wrote:
> >> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> >> Sent: Wednesday, August 31, 2016 12:17 AM
> >>
> >> Hi folks,
> >>
> >> At KVM Forum we had a BoF session primarily around the mediated device
> >> sysfs interface.  I'd like to share what I think we agreed on and the
> >> "problem areas" that still need some work so we can get the thoughts
> >> and ideas from those who weren't able to attend.
> >>
> >> DanPB expressed some concern about the mdev_supported_types sysfs
> >> interface, which exposes a flat csv file with fields like "type",
> >> "number of instance", "vendor string", and then a bunch of type
> >> specific fields like "framebuffer size", "resolution", "frame rate
> >> limit", etc.  This is not entirely machine parsing friendly and sort of
> >> abuses the sysfs concept of one value per file.  Example output taken
> >> from Neo's libvirt RFC:
> >>
> >> cat /sys/bus/pci/devices/0000:86:00.0/mdev_supported_types
> >> # vgpu_type_id, vgpu_type, max_instance, num_heads, frl_config, framebuffer,
> >> max_resolution
> >> 11      ,"GRID M60-0B",      16,       2,      45,     512M,    2560x1600
> >> 12      ,"GRID M60-0Q",      16,       2,      60,     512M,    2560x1600
> >> 13      ,"GRID M60-1B",       8,       2,      45,    1024M,    2560x1600
> >> 14      ,"GRID M60-1Q",       8,       2,      60,    1024M,    2560x1600
> >> 15      ,"GRID M60-2B",       4,       2,      45,    2048M,    2560x1600
> >> 16      ,"GRID M60-2Q",       4,       4,      60,    2048M,    2560x1600
> >> 17      ,"GRID M60-4Q",       2,       4,      60,    4096M,    3840x2160
> >> 18      ,"GRID M60-8Q",       1,       4,      60,    8192M,    3840x2160
> >>
> >> The create/destroy then looks like this:
> >>
> >> echo "$mdev_UUID:vendor_specific_argument_list" >
> >> 	/sys/bus/pci/devices/.../mdev_create
> >>
> >> echo "$mdev_UUID:vendor_specific_argument_list" >
> >> 	/sys/bus/pci/devices/.../mdev_destroy
> >>
> >> "vendor_specific_argument_list" is nebulous.
> >>
> >> So the idea to fix this is to explode this into a directory structure,
> >> something like:
> >>
> >> ├── mdev_destroy
> >> └── mdev_supported_types
> >>     ├── 11
> >>     │   ├── create
> >>     │   ├── description
> >>     │   └── max_instances
> >>     ├── 12
> >>     │   ├── create
> >>     │   ├── description
> >>     │   └── max_instances
> >>     └── 13
> >>         ├── create
> >>         ├── description
> >>         └── max_instances
> >>
> >> Note that I'm only exposing the minimal attributes here for simplicity,
> >> the other attributes would be included in separate files and we would
> >> require vendors to create standard attributes for common device classes.  
> > 
> > I like this idea. All standard attributes are reflected into this hierarchy.
> > In the meantime, can we still allow optional vendor string in create 
> > interface? libvirt doesn't need to know the meaning, but allows upper
> > layer to do some vendor specific tweak if necessary.  
> 
> This is not the best idea IMO. Libvirt is there to shadow differences
> between hypervisors. While doing that, we often hide differences between
> various types of HW too. Therefore in order to provide good abstraction
> we should make vendor specific string as small as possible (ideally an
> empty string). I mean I see it as bad idea to expose "vgpu_type_id" from
> example above in domain XML. What I think the better idea is if we let
> users chose resolution and frame buffer size, e.g.: <video
> resolution="1024x768" framebuffer="16"/> (just the first idea that came
> to my mind while writing this e-mail). The point is, XML part is
> completely free of any vendor-specific knobs.

That's not really what you want though, a user actually cares whether
they get an Intel of NVIDIA vGPU, we can't specify it as just a
resolution and framebuffer size.  The user also doesn't want the model
changing each time the VM is started, so not only do you *need* to know
the vendor, you need to know the vendor model.  This is the only way to
provide a consistent VM.  So as we discussed at the BoF, the libvirt
xml will likely reference the vendor string, which will be a unique
identifier that encompasses all the additional attributes we expose.
Really the goal of the attributes is simply so you don't need a per
vendor magic decoder ring to figure out the basic features of a given
vendor string.  Thanks,

Alex