On 27/09/2024 1.56, Jason Gunthorpe wrote:
> On Sun, Sep 22, 2024 at 05:49:40AM -0700, Zhi Wang wrote:
>
>> diff --git a/include/drm/nvkm_vgpu_mgr_vfio.h b/include/drm/nvkm_vgpu_mgr_vfio.h
>> index d9ed2cd202ff..5c2c650c2df9 100644
>> --- a/include/drm/nvkm_vgpu_mgr_vfio.h
>> +++ b/include/drm/nvkm_vgpu_mgr_vfio.h
>> @@ -6,8 +6,13 @@
>> #ifndef __NVKM_VGPU_MGR_VFIO_H__
>> #define __NVKM_VGPU_MGR_VFIO_H__
>>
>> +enum {
>> + NVIDIA_VGPU_EVENT_PCI_SRIOV_CONFIGURE = 0,
>> +};
>> +
>> struct nvidia_vgpu_vfio_handle_data {
>> void *priv;
>> + struct notifier_block notifier;
>> };
>
> Nothing references this? Why would you need it?
>
Oops, these are the leftovers of the discard changes. Will remove them
accordingly in the next iteration. Thanks so much for catching this.
> It looks approx correct to me to just directly put your function in
> the sriov_configure callback.
>
> This is the callback that indicates the admin has decided to turn on
> the SRIOV feature.
Turning on the SRIOV feature is just a part of the process enabling a
vGPU. The VF is not instantly usable before a vGPU type is chosen via
another userspace interface (e.g. fwctl).
Besides, admin has to enable the vGPU support by some means (e.g. a
kernel parameter is just one candidate) and GSP firmware needs to be
configured accordingly when being loaded.
As this is related to user space interface, I am leaning towards putting
some restriction/checks for the pre-condition in the
driver.sriov_configure(), so admin would know there is something wrong
in his configuration as early as possible, instead of he failed to
creating vGPUs again and again, then he found he forgot to enable the
vGPU support.
Thanks,
Zhi.
> Jason
