Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
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

  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