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

[Tian, Kevin]

> From: Kirti Wankhede
> Sent: Wednesday, February 03, 2016 9:21 PM
> 
> 
> On 2/3/2016 11:26 AM, Tian, Kevin wrote:
> [...]
> >>>>>     * @vgpu_create:        Called to allocate basic resouces in graphics
> >>>>>     *                      driver for a particular vgpu.
> >>>>>     *                      @dev: physical pci device structure on which 
> >>>>> vgpu
> >>>>>     *                            should be created
> >>>>>     *                      @uuid: uuid for which VM it is intended to
> >>>>>     *                      @instance: vgpu instance in that VM
> >>>>>     *                      @vgpu_id: This represents the type of vgpu 
> >>>>> to be
> >>>>>     *                                created
> >>>>>     *                      Returns integer: success (0) or error (< 0)
> >>>
> >>> Specifically for Intel GVT-g we didn't hard partition resource among 
> >>> vGPUs.
> >>> Instead we allow user to accurately control how many physical resources
> >>> are allocated to a vGPU. So this interface should be extensible to allow
> >>> vendor specific resource control.
> >>>
> >>
> >> This interface forwards the create request to vendor/GPU driver
> >> informing about which physical GPU this request is intended for and the
> >> type of vGPU. Then its vendor/GPU driver's responsibility to do
> >> resources allocation and manage resources in their own driver.
> >
> > However the current parameter definition disallows resource configuration
> > information passed from user. As I said, Intel GVT-g doesn't do static
> > allocation based on type. We provide flexibility to user for fine-grained
> > resource management.
> >
> 
>          int     (*vgpu_create)(struct pci_dev *dev, uuid_le uuid,
> -                              uint32_t instance, uint32_t vgpu_id);
> +                              uint32_t instance, char *vgpu_params);
> 
> If we change integer vgpu_id parameter to char *vgpu_param then GPU
> driver can have multiple parameters.
> 
> Suppose there is a GPU at 0000:85:00.0, then to create vgpu:
> # echo "<uuid>:<vgpu_instance>:<vgpu_params string>" >
> /sys/bus/pci/devices/0000\:85\:00.0/vgpu_create
> 
> Common vgpu module will not parse vgpu_params string, it will be
> forwarded to GPU driver, then its GPU driver's responsibility to parse
> the string and act accordingly. This should give flexibility to have
> multiple parameters for GPU driver.

Yes, this way it would work.

> 
> >>>>>     *
> >>>>>     * Physical GPU that support vGPU should be register with vgpu 
> >>>>> module with
> >>>>>     * gpu_device_ops structure.
> >>>>>     */
> >>>>>
> >>>
> >>> Also it'd be good design to allow extensible usages, such as statistics, 
> >>> and
> >>> other vendor specific control knobs (e.g. foreground/background VM switch
> >>> in Intel GVT-g, etc.)
> >>>
> >>
> >> Can you elaborate on what other control knobs that would be needed?
> >>
> >
> > Some examples:
> >
> > - foreground/background VM switch
> > - resource query
> > - various statistics info
> > - virtual monitor configuration
> > - ...
> >
> > Since this vgpu core driver will become the central point for all vgpu
> > management, we need provide an easy way for vendor specific extension,
> > e.g. exposing above callbacks as sysfs nodes and then vendor can create
> > its own extensions under subdirectory (/intel, /nvidia, ...).
> >
> >
> 
> Ok. Makes sense.
> 
> Are these parameters per physical device or per vgpu device?

some for physical, others for vgpu.

> 
> Adding attribute groups to gpu_device_ops structure would provide a way
> to add vendor specific extensions. I think we need two types of
> attributes here, per physical device and per vgpu device. Right?
> 
>           const struct attribute_group **dev_groups;
>           const struct attribute_group **vgpu_groups;
> 
> 
> 

Agree.

Thanks
Kevin