From: Jani Nikula <jani.nikula@linux.intel.com>
To: Andi Shyti <andi.shyti@linux.intel.com>,
Krzysztof Karas <krzysztof.karas@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
"Andi Shyti" <andi.shyti@linux.intel.com>,
"Sebastian Brzezinka" <sebastian.brzezinka@intel.com>,
"Krzysztof Niemiec" <krzysztof.niemiec@intel.com>,
"Janusz Krzysztofik" <janusz.krzysztofik@linux.intel.com>,
"Michał Grzelak" <michal.grzelak@intel.com>
Subject: Re: [PATCH] drm/i915: Add vgpu.entry list initialization
Date: Wed, 28 Jan 2026 19:38:35 +0200 [thread overview]
Message-ID: <3f47d40ba15db7ace414d68934b84b9da8bd69c3@intel.com> (raw)
In-Reply-To: <aXo_t0nqu-TMow4-@ashyti-mobl2.lan>
On Wed, 28 Jan 2026, Andi Shyti <andi.shyti@linux.intel.com> 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
next prev parent reply other threads:[~2026-01-28 17:38 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-23 10:11 [PATCH] drm/i915: Add vgpu.entry list initialization Krzysztof Karas
2026-01-23 11:49 ` Sebastian Brzezinka
2026-01-23 11:59 ` ✓ i915.CI.BAT: success for " Patchwork
2026-01-23 15:09 ` [PATCH] " Janusz Krzysztofik
2026-01-23 19:40 ` ✓ i915.CI.Full: success for " Patchwork
2026-01-28 16:56 ` [PATCH] " Andi Shyti
2026-01-28 17:38 ` Jani Nikula [this message]
2026-01-28 22:39 ` Andi Shyti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3f47d40ba15db7ace414d68934b84b9da8bd69c3@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=andi.shyti@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=janusz.krzysztofik@linux.intel.com \
--cc=krzysztof.karas@intel.com \
--cc=krzysztof.niemiec@intel.com \
--cc=michal.grzelak@intel.com \
--cc=sebastian.brzezinka@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox