Re: [PATCH] drm/xe/pf: Access VF's register using dedicated MMIO view

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 10/27/2025 5:42 PM, Lucas De Marchi wrote:
> On Fri, Oct 24, 2025 at 10:58:25PM +0200, Michal Wajdeczko wrote:
>> Instead of creating ad-hoc new register definitions with altered
>> register addresses to mimic the VF's access to these registers,
>> prepare new MMIO instance per required VF, with shifted internal
>> location of the register map.  This will allow to use unmodified
>> register definitions in all calls to xe_mmio() functions.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> ---
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_gt_sriov_pf.c | 36 +++++++----------------------
>> drivers/gpu/drm/xe/xe_mmio.c        | 29 +++++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_mmio.h        |  4 ++++
>> 3 files changed, 41 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
>> index c4dda87b47cc..0714c758b9c1 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
>> @@ -158,39 +158,19 @@ void xe_gt_sriov_pf_init_hw(struct xe_gt *gt)
>>     xe_gt_sriov_pf_service_update(gt);
>> }
>>
>> -static u32 pf_get_vf_regs_stride(struct xe_device *xe)
>> -{
>> -    return GRAPHICS_VERx100(xe) > 1200 ? 0x400 : 0x1000;
>> -}
>> -
>> -static struct xe_reg xe_reg_vf_to_pf(struct xe_reg vf_reg, unsigned int vfid, u32 stride)
>> -{
>> -    struct xe_reg pf_reg = vf_reg;
>> -
>> -    pf_reg.vf = 0;
>> -    pf_reg.addr += stride * vfid;
>> -
>> -    return pf_reg;
>> -}
>> -
>> static void pf_clear_vf_scratch_regs(struct xe_gt *gt, unsigned int vfid)
>> {
>> -    u32 stride = pf_get_vf_regs_stride(gt_to_xe(gt));
>> -    struct xe_reg scratch;
>> -    int n, count;
>> +    struct xe_mmio mmio;
>> +    int n;
>> +
>> +    xe_mmio_init_vf_view(&mmio, &gt->mmio, vfid);
> 
> 
> Wouldn't it make more sense to keep the VF-specific bits in this file
> rather than propagate that knowledge outside? Something like this:
> 
>     xe_mmio_init_view(&mmio, &gt->mmio,
>               vf_regs_stride(xe) * vfid);
> 
> and then xe_mmio_init_view() receives an offset rather than the vfid.

but the location of the mirrored VF registers in the full MMIO space is
still part of the a MMIO layout - so IMO it fits in xe_mmio.c even better
then it was previously here, in the PF code

Michal

> 
> Lucas De Marchi
> 
>>
>>     if (xe_gt_is_media_type(gt)) {
>> -        count = MED_VF_SW_FLAG_COUNT;
>> -        for (n = 0; n < count; n++) {
>> -            scratch = xe_reg_vf_to_pf(MED_VF_SW_FLAG(n), vfid, stride);
>> -            xe_mmio_write32(&gt->mmio, scratch, 0);
>> -        }
>> +        for (n = 0; n < MED_VF_SW_FLAG_COUNT; n++)
>> +            xe_mmio_write32(&mmio, MED_VF_SW_FLAG(n), 0);
>>     } else {
>> -        count = VF_SW_FLAG_COUNT;
>> -        for (n = 0; n < count; n++) {
>> -            scratch = xe_reg_vf_to_pf(VF_SW_FLAG(n), vfid, stride);
>> -            xe_mmio_write32(&gt->mmio, scratch, 0);
>> -        }
>> +        for (n = 0; n < VF_SW_FLAG_COUNT; n++)
>> +            xe_mmio_write32(&mmio, VF_SW_FLAG(n), 0);
>>     }
>> }
>>
>> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
>> index ef6f3ea573a2..350dca1f0925 100644
>> --- a/drivers/gpu/drm/xe/xe_mmio.c
>> +++ b/drivers/gpu/drm/xe/xe_mmio.c
>> @@ -379,3 +379,32 @@ int xe_mmio_wait32_not(struct xe_mmio *mmio, struct xe_reg reg, u32 mask, u32 va
>> {
>>     return __xe_mmio_wait32(mmio, reg, mask, val, timeout_us, out_val, atomic, false);
>> }
>> +
>> +#ifdef CONFIG_PCI_IOV
>> +static size_t vf_regs_stride(struct xe_device *xe)
>> +{
>> +    return GRAPHICS_VERx100(xe) > 1200 ? 0x400 : 0x1000;
>> +}
>> +
>> +/**
>> + * xe_mmio_init_vf_view() - Initialize an MMIO instance for accesses like the VF
>> + * @mmio: the target &xe_mmio to initialize as VF's view
>> + * @base: the source &xe_mmio to initialize from
>> + * @vfid: the VF identifier
>> + */
>> +void xe_mmio_init_vf_view(struct xe_mmio *mmio, const struct xe_mmio *base, unsigned int vfid)
>> +{
>> +    struct xe_tile *tile = base->tile;
>> +    struct xe_device *xe = tile->xe;
>> +    size_t offset = vf_regs_stride(xe) * vfid;
>> +
>> +    xe_assert(xe, IS_SRIOV_PF(xe));
>> +    xe_assert(xe, vfid);
>> +    xe_assert(xe, !base->sriov_vf_gt);
>> +    xe_assert(xe, base->regs_size > offset);
>> +
>> +    *mmio = *base;
>> +    mmio->regs += offset;
>> +    mmio->regs_size -= offset;
>> +}
>> +#endif
>> diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
>> index c151ba569003..15362789ab99 100644
>> --- a/drivers/gpu/drm/xe/xe_mmio.h
>> +++ b/drivers/gpu/drm/xe/xe_mmio.h
>> @@ -42,4 +42,8 @@ static inline struct xe_mmio *xe_root_tile_mmio(struct xe_device *xe)
>>     return &xe->tiles[0].mmio;
>> }
>>
>> +#ifdef CONFIG_PCI_IOV
>> +void xe_mmio_init_vf_view(struct xe_mmio *mmio, const struct xe_mmio *base, unsigned int vfid);
>> +#endif
>> +
>> #endif
>> -- 
>> 2.47.1
>>