[PATCH v3] drm/xe/pf: Don't allow LMEM provisioning if LMTT isn't available on the device

[Piotr Piórkowski]

Michal Wajdeczko <michal.wajdeczko at intel.com> wrote on czw [2025-kwi-17 21:32:40 +0200]:
> 
> 
> On 17.04.2025 20:18, Summers, Stuart wrote:
> > On Thu, 2025-04-17 at 16:21 +0200, Piórkowski, Piotr wrote:
> >> From: Piotr Piórkowski <piotr.piorkowski at intel.com>
> >>
> >> The LMEM provisioning is applicable only on platforms with LMTT.
> >>
> >> v2:
> >>  - new commit description
> >>  - use xe_gt_assert in xe_gt_sriov_pf_config_set_lmem instead return
> >>    error,
> >>  - disable pf_lmem_info if LMTT is not available
> >> v3: fix condition in xe_gt_assert
> >>
> >> Signed-off-by: Piotr Piórkowski <piotr.piorkowski at intel.com>
> >> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c  | 6 ++++--
> >>  drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c | 4 ++--
> >>  2 files changed, 6 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> >> b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> >> index 2420a548cacc..3556c41c041b 100644
> >> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> >> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> >> @@ -1520,6 +1520,8 @@ int xe_gt_sriov_pf_config_set_lmem(struct xe_gt
> >> *gt, unsigned int vfid, u64 size
> >>  {
> >>         int err;
> >>  
> >> +       xe_gt_assert(gt, xe_device_has_lmtt(gt_to_xe(gt)));
> >> +
> >>         mutex_lock(xe_gt_sriov_pf_master_mutex(gt));
> >>         if (vfid)
> >>                 err = pf_provision_vf_lmem(gt, vfid, size);
> >> @@ -1629,7 +1631,7 @@ int xe_gt_sriov_pf_config_set_fair_lmem(struct
> >> xe_gt *gt, unsigned int vfid,
> >>         xe_gt_assert(gt, num_vfs);
> >>         xe_gt_assert(gt, !xe_gt_is_media_type(gt));
> >>  
> >> -       if (!IS_DGFX(gt_to_xe(gt)))
> >> +       if (!xe_device_has_lmtt(gt_to_xe(gt)))
> >>                 return 0;
> >>  
> >>         mutex_lock(xe_gt_sriov_pf_master_mutex(gt));
> >> @@ -2163,7 +2165,7 @@ static int pf_validate_vf_config(struct xe_gt
> >> *gt, unsigned int vfid)
> >>         valid_all = valid_all && valid_ggtt;
> >>         valid_any = valid_any || (valid_ggtt && is_primary);
> >>  
> >> -       if (IS_DGFX(xe)) {
> >> +       if (xe_device_has_lmtt(xe)) {
> > 
> > I see a few other calls like pf_sanitize_vf_resources() and
> 
> from old offline discussion - there is/was a plan to reuse lmem_obj also
> when we will not let to configure LMEM manually, not sure if that stands
> still, though
> 
> > pf_validate_vf_config() 
> 
> it's touched in this patch right here ;)
> 
> > and pf_restore_vf_config_klv() and
> > pf_get_ggtt_alignment() in this same file. Is there a reason not to
> > just change all these at once?
> 
> if we want to allow the LMEM configuration only with LMTT then agree
> 
> then maybe we should drop new explicit assert in:
> 
> 	xe_gt_sriov_pf_config_set_lmem
> 
> and instead replace legacy IS_DGFX in:
> 
> 	pf_release_vf_config_lmem
> 	pf_provision_vf_lmem
> 	pf_release_vf_config
> 
> but let Piotr speak here

As Michal rightly pointed out, this was a intentional decision on my
side. I would like to disable LMEM provisioning in the current way
for platforms that do not have LMTT, but I would like to keep
the possibility of using lmem_obj in the future without LMTT.
That's why I left IS_DGFX for the following functions:

pf_release_vf_config_lmem
pf_release_vf_config
pf_sanitize_vf_resources
pf_provision_vf_lmem

Note that all calls related to LMTT in these functions already
have the appropriate condition.

> 
> > 
> > Also right now has_lmtt just calls IS_DGFX, and I confirmed this looks
> > right from the specs, so at a high level:
> > Reviewed-by: Stuart Summers <stuart.summers at intel.com>
> 

As you rightly pointed out, currently has_lmtt is just a call to IS_DGFX,
so from a functional point of view nothing changes

> I'll wait with my r-b until Piotr reply
> 
> > 
> > Thanks,
> > Stuart
> > 

Thanks for the review.
Piotr


> >>                 bool valid_lmem = pf_get_vf_config_lmem(primary_gt,
> >> vfid);
> >>  
> >>                 valid_any = valid_any || (valid_lmem && is_primary);
> >> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c
> >> b/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c
> >> index 0fe47f41b63c..13970d5a2867 100644
> >> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c
> >> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c
> >> @@ -308,7 +308,7 @@ static void pf_add_config_attrs(struct xe_gt *gt,
> >> struct dentry *parent, unsigne
> >>         if (!xe_gt_is_media_type(gt)) {
> >>                 debugfs_create_file_unsafe(vfid ? "ggtt_quota" :
> >> "ggtt_spare",
> >>                                            0644, parent, parent,
> >> &ggtt_fops);
> >> -               if (IS_DGFX(gt_to_xe(gt)))
> >> +               if (xe_device_has_lmtt(gt_to_xe(gt)))
> >>                         debugfs_create_file_unsafe(vfid ?
> >> "lmem_quota" : "lmem_spare",
> >>                                                    0644, parent,
> >> parent, &lmem_fops);
> >>         }
> >> @@ -558,7 +558,7 @@ void xe_gt_sriov_pf_debugfs_register(struct xe_gt
> >> *gt, struct dentry *root)
> >>                 drm_debugfs_create_files(pf_ggtt_info,
> >>                                          ARRAY_SIZE(pf_ggtt_info),
> >>                                          pfdentry, minor);
> >> -               if (IS_DGFX(gt_to_xe(gt)))
> >> +               if (xe_device_has_lmtt(gt_to_xe(gt)))
> >>                         drm_debugfs_create_files(pf_lmem_info,
> >>                                                 
> >> ARRAY_SIZE(pf_lmem_info),
> >>                                                  pfdentry, minor);
> > 
> 

--