Re: [PATCH 2/6] drm/xe: Allow to assign GGTT region to the VF

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3841 bytes --]


On 15-04-2024 00:31, Michal Wajdeczko wrote:
> VF's drivers can't modify GGTT PTEs except the range explicitly
> assigned by the PF driver. To allow hardware enforcement of this
> requirement, each GGTT PTE has a field with the VF number that
> identifies which VF can modify that particular GGTT PTE entry.
>
> Only PF driver can modify this field and PF driver shall do that
> before VF drivers will be loaded. Add function to prepare PTEs.
> Since it will be used only by the PF driver, make it available
> only for CONFIG_PCI_IOV=y.
>
> Bspec: 45015
> Signed-off-by: Michal Wajdeczko<michal.wajdeczko@intel.com>
> ---
>   drivers/gpu/drm/xe/regs/xe_gtt_defs.h |  2 ++
>   drivers/gpu/drm/xe/xe_ggtt.c          | 44 +++++++++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_ggtt.h          |  4 +++
>   3 files changed, 50 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/regs/xe_gtt_defs.h b/drivers/gpu/drm/xe/regs/xe_gtt_defs.h
> index 558519ce48c7..4389e5a76f89 100644
> --- a/drivers/gpu/drm/xe/regs/xe_gtt_defs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_gtt_defs.h
> @@ -9,6 +9,8 @@
>   #define XELPG_GGTT_PTE_PAT0	BIT_ULL(52)
>   #define XELPG_GGTT_PTE_PAT1	BIT_ULL(53)
>   
> +#define GGTT_PTE_VFID		GENMASK_ULL(11, 2)

Patch looks good to me.

Just to confrim, based on BSPEC, this MASK applies to GRAPHICS_VER >= 
12.50. For versions before this, the applicable mask is (4, 2). I infer 
that since SRIOV support on XE is expected beyond GRAPHICS_VER version 
12.50, you are using mask (11, 2).

> +
>   #define GUC_GGTT_TOP		0xFEE00000
>   
>   #define XELPG_PPGTT_PTE_PAT3		BIT_ULL(62)
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index ff2239c0eda5..f090cab065b8 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -460,6 +460,50 @@ void xe_ggtt_remove_bo(struct xe_ggtt *ggtt, struct xe_bo *bo)
>   			    bo->flags & XE_BO_FLAG_GGTT_INVALIDATE);
>   }
>   
> +#ifdef CONFIG_PCI_IOV
> +static u64 xe_encode_vfid_pte(u16 vfid)
> +{
> +	return FIELD_PREP(GGTT_PTE_VFID, vfid) | XE_PAGE_PRESENT;
> +}
> +
> +static void xe_ggtt_assign_locked(struct xe_ggtt *ggtt, const struct drm_mm_node *node, u16 vfid)
> +{
> +	u64 start = node->start;
> +	u64 size = node->size;
> +	u64 end = start + size - 1;
> +	u64 pte = xe_encode_vfid_pte(vfid);
> +
> +	lockdep_assert_held(&ggtt->lock);
> +
> +	if (!drm_mm_node_allocated(node))
> +		return;
> +
> +	while (start < end) {
> +		xe_ggtt_set_pte(ggtt, start, pte);
> +		start += XE_PAGE_SIZE;
> +	}
> +
> +	xe_ggtt_invalidate(ggtt);
> +}
> +
> +/**
> + * xe_ggtt_assign - assign a GGTT region to the VF
> + * @ggtt: the &xe_ggtt where the node belongs
> + * @node: the &drm_mm_node to update
> + * @vfid: the VF identifier
> + *
> + * This function is used by the PF driver to assign a GGTT region to the VF.
> + * In addition to PTE's VFID bits 11:2 also PRESENT bit 0 is set as on some
> + * platforms VFs can't modify that either.

This info regarding bit 0 not being modifiable by VF's on some platforms 
is missing in Bspec.

> + */
> +void xe_ggtt_assign(struct xe_ggtt *ggtt, const struct drm_mm_node *node, u16 vfid)
> +{
> +	mutex_lock(&ggtt->lock);
> +	xe_ggtt_assign_locked(ggtt, node, vfid);
> +	mutex_unlock(&ggtt->lock);
> +}
> +#endif
> +
>   int xe_ggtt_dump(struct xe_ggtt *ggtt, struct drm_printer *p)
>   {
>   	int err;
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h
> index 8306ef74abc6..4a41a1762358 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.h
> +++ b/drivers/gpu/drm/xe/xe_ggtt.h
> @@ -33,4 +33,8 @@ void xe_ggtt_remove_bo(struct xe_ggtt *ggtt, struct xe_bo *bo);
>   
>   int xe_ggtt_dump(struct xe_ggtt *ggtt, struct drm_printer *p);
>   
> +#ifdef CONFIG_PCI_IOV
> +void xe_ggtt_assign(struct xe_ggtt *ggtt, const struct drm_mm_node *node, u16 vfid);
> +#endif
> +
>   #endif

[-- Attachment #2: Type: text/html, Size: 5573 bytes --]