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

[Michal Privoznik]

On 01.09.2016 18:59, Alex Williamson wrote:
> On Thu, 1 Sep 2016 18:47:06 +0200
> Michal Privoznik <address@hidden> wrote:
> 
>> On 31.08.2016 08:12, Tian, Kevin wrote:
>>>> From: Alex Williamson [mailto:address@hidden
>>>> 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,

Okay, maybe I'm misunderstanding something. I just thought that users
will consult libvirt's nodedev driver (e.g. virsh nodedev-list && virsh
nodedev-dumpxml $id) to fetch vGPU capabilities and then use that info
to construct domain XML.
Also, I guess libvirt will need some sort of understanding of vGPUs in
sense that if there are two vGPUs in the system (say both INTEL and
NVIDIA) libvirt must create mdev on the right one. I guess we can't rely
solely on vgpu_type_id uniqueness here, can we.

Michal