Re: [PATCH] drm/xe/pf: Keep VF LMEM BAR size low if no VFs enabled

["Bernatowicz, Marcin" <marcin.bernatowicz@linux.intel.com>]

On 9/18/2025 8:24 PM, Michal Wajdeczko wrote:
> 
> 
> On 9/18/2025 6:43 PM, Marcin Bernatowicz wrote:
>> When VFs are enabled on dGFX the driver resizes the PF VF_LMEM_BAR to
>> fit the requested layout. After VFs are disabled the PF VF BAR
>> size is left as-is. On platforms with tight MMIO apertures a
>> subsequent unplug/rescan followed by another enable may fail with:
>>
>>    "VF BAR …: can't assign; no space"
>>
>> because the PCI core reserves address space based on the (now large) VF
>> template, often multiplied by totalvfs.
>>
>> v2: Use pci.total_vfs in helper (Michał Wajdeczko)
> 
> internal reviews don't count

Ack. I’ll drop the v2 note tied to internal review.

> 
>>
>> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/5937
>> Fixes: 94eae6ee4c2d ("drm/xe/pf: Set VF LMEM BAR size")
>> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
>> Cc: Michał Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_pci_sriov.c | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_pci_sriov.c b/drivers/gpu/drm/xe/xe_pci_sriov.c
>> index af05db07162e..ff003a650f79 100644
>> --- a/drivers/gpu/drm/xe/xe_pci_sriov.c
>> +++ b/drivers/gpu/drm/xe/xe_pci_sriov.c
>> @@ -144,11 +144,26 @@ static int resize_vf_vram_bar(struct xe_device *xe, int num_vfs)
>>   	return pci_iov_vf_bar_set_size(pdev, VF_LMEM_BAR, __fls(sizes));
>>   }
>>   
>> +static void reduce_vf_vram_bar_size(struct xe_device *xe)
> 
> is it "reduce" or "restore" ?

Agree — “restore” is clearer. We’re resetting the VF LMEM BAR template 
to the order that fits total_vfs, not just shrinking it. I’ll rename to 
restore_vf_vram_bar_size()

> 
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>> +	int err;
>> +
>> +	if (!IS_DGFX(xe))
>> +		return;
>> +
>> +	err = resize_vf_vram_bar(xe, pci_sriov_get_totalvfs(pdev));
> 
> pci_sriov_get_totalvfs() might also return tweaked value based on xe.max_vfs modparam
> 
> https://elixir.bootlin.com/linux/v6.17-rc6/source/drivers/pci/iov.c#L1276
> 

Good catch. I’ll use the driver’s cached device total VFs:
xe->sriov.pf.device_total_vfs

>> +	if (err)
>> +		xe_sriov_info(xe, "Failed to reduce VF LMEM BAR size: %d\n",
>> +			      err);
> 
> we try to show nice error codes using (%pe)

Ack
> 
>> +}
>> +
>>   static int pf_enable_vfs(struct xe_device *xe, int num_vfs)
>>   {
>>   	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>   	int total_vfs = xe_sriov_pf_get_totalvfs(xe);
>>   	int err;
>> +	bool vf_vram_bar_resized = false;
> 
> please try to keep vars in reverse-xmas-tree order

Ack>
>>   
>>   	xe_assert(xe, IS_SRIOV_PF(xe));
>>   	xe_assert(xe, num_vfs > 0);
>> @@ -178,6 +193,8 @@ static int pf_enable_vfs(struct xe_device *xe, int num_vfs)
>>   		err = resize_vf_vram_bar(xe, num_vfs);
>>   		if (err)
>>   			xe_sriov_info(xe, "Failed to set VF LMEM BAR size: %d\n", err);
>> +		else
>> +			vf_vram_bar_resized = true;
>>   	}
>>   
>>   	err = pci_enable_sriov(pdev, num_vfs);
>> @@ -194,6 +211,9 @@ static int pf_enable_vfs(struct xe_device *xe, int num_vfs)
>>   	return num_vfs;
>>   
>>   failed:
>> +	if (vf_vram_bar_resized)
>> +		reduce_vf_vram_bar_size(xe);
> 
> in pf_disable_vfs() below it's called unconditionally
> why can't we do the same here? or it's broken there?
>
Makes sense, I do not see a reason to not call it unconditionally here 
and drop vf_vram_bar_resized

>> +
>>   	pf_unprovision_vfs(xe, num_vfs);
>>   	xe_pm_runtime_put(xe);
>>   out:
>> @@ -218,6 +238,8 @@ static int pf_disable_vfs(struct xe_device *xe)
>>   
>>   	pci_disable_sriov(pdev);
>>   
>> +	reduce_vf_vram_bar_size(xe);
>> +
>>   	pf_reset_vfs(xe, num_vfs);
>>   
>>   	pf_unprovision_vfs(xe, num_vfs);
>