On Wed, 28 Jan 2026, Andi Shyti <[email protected]> wrote:
> Hi Krzysztof,
>
> nice catch, but the fix looks a bit messy to me.
>
> ...
>
>> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c 
>> b/drivers/gpu/drm/i915/i915_vgpu.c
>> index d29a06ea51a5..362282b20f7b 100644
>> --- a/drivers/gpu/drm/i915/i915_vgpu.c
>> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
>> @@ -67,6 +67,7 @@ void intel_vgpu_detect(struct drm_i915_private *dev_priv)
>>      u64 magic;
>>      u16 version_major;
>>      void __iomem *shared_area;
>> +    INIT_LIST_HEAD(&dev_priv->vgpu.entry);
>
> Despite what Sebastian is suggesting, I think the place is right
> (or almost right).
>
> But I wouldn't fix it this way. Initializing virtual GPUs is not
> mandatory, indeed it fails only during delete that is the only
> function that doesn't check whether the list is initialized.
>
> I would rather check whether the list is initialized before
> trying to delete it, with a nice comment saying that the list
> might not have been initialized and we want to avoid accessing an
> invalid list.

Nope, none of this.

The problem is intel_gvt_init() and intel_gvt_driver_remove() happening
at different abstraction levels in i915_driver.c, with the calls also
happening at different abstraction levels in the error path.

Basically when i915_driver_hw_probe() returns with an error, the caller
has no way of knowing whether intel_gvt_init() succeeded or not, and any
call to intel_gvt_driver_remove() is bound to be wrong.

The fix is not to make intel_gvt_driver_remove() "gracefully" handle
broken probe/cleanup calls, but to fix the probe/cleanup calls.

Michał is actually looking into this, Cc'd.


BR,
Jani.


>
> Andi
>
>>      BUILD_BUG_ON(sizeof(struct vgt_if) != VGT_PVINFO_SIZE);
>>  
>> -- 
>> 2.43.0
>> 
>> 
>> -- 
>> Best Regards,
>> Krzysztof

-- 
Jani Nikula, Intel

Reply via email to