Re: [PATCH] drm/xe/pf: Fix MMIO access using PF view instead of VF view during migration

["Summers, Stuart" <stuart.summers@intel.com>]

On Wed, 2026-04-29 at 22:43 +0200, Michal Wajdeczko wrote:
> 
> 
> On 4/29/2026 10:25 PM, Summers, Stuart wrote:
> > On Wed, 2026-04-29 at 19:22 +0000, Shuicheng Lin wrote:
> > > pf_migration_mmio_save() and pf_migration_mmio_restore()
> > > initialize a
> > > local VF-specific MMIO view via xe_mmio_init_vf_view() but then
> > > pass
> > > &gt->mmio (the PF base) to all xe_mmio_read32()/xe_mmio_write32()
> > > calls instead of the local &mmio. This causes the PF own SW flag
> > > registers to be saved/restored rather than the target VF
> > > registers,
> > > silently corrupting migration state.
> > > 
> > > Use the VF MMIO view for all register accesses, matching the
> > > correct
> > > pattern used in pf_clear_vf_scratch_regs().
> > > 
> > > Fixes: b7c1b990f719 ("drm/xe/pf: Handle MMIO migration data as
> > > part
> > > of PF control")
> > > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > > Assisted-by: Claude:claude-opus-4.6
> > > Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> 
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> 
> > > ---
> > >  drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c
> > > b/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c
> > > index 87a164efcc33..01fe03b9efe8 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c
> > > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c
> > > @@ -385,10 +385,10 @@ static int pf_migration_mmio_save(struct
> > > xe_gt
> > > *gt, unsigned int vfid, void *buf
> > >  
> > >         if (xe_gt_is_media_type(gt))
> > >                 for (n = 0; n < MED_VF_SW_FLAG_COUNT; n++)
> > > -                       regs[n] = xe_mmio_read32(&gt->mmio,
> > > MED_VF_SW_FLAG(n));
> > > +                       regs[n] = xe_mmio_read32(&mmio,
> > > MED_VF_SW_FLAG(n));
> > 
> > Good to get feedback from Michal Wa/Michal Wi here, but I don't see
> > any
> > usage of these MMIOs from the VF in the driver. Are these even
> > exposed
> > to the VF? This seems unsafe from what I can see...
> 
> see xe_guc_mmio_send_recv()

Ok got it and makes sense. I was seeing also the gt->mmio in
xe_guc_mmio_send_recv() but I see that is called from the VF as well,
so good to align there.

Reviewed-by: Stuart Summers <stuart.summers@intel.com>

> 
> > 
> > Can you show the error you are seeing specifically?
> 
> it might be hard to catch the VF driver in the middle of the
> MMIO communication, which usually is done only during probe

Yeah makes sense.. was also wondering based on the commit message which
indicated corruption.

Thanks,
Stuart

> 
> > 
> > Thanks,
> > Stuart
> > 
> > >         else
> > >                 for (n = 0; n < VF_SW_FLAG_COUNT; n++)
> > > -                       regs[n] = xe_mmio_read32(&gt->mmio,
> > > VF_SW_FLAG(n));
> > > +                       regs[n] = xe_mmio_read32(&mmio,
> > > VF_SW_FLAG(n));
> > >  
> > >         return 0;
> > >  }
> > > @@ -407,10 +407,10 @@ static int pf_migration_mmio_restore(struct
> > > xe_gt *gt, unsigned int vfid,
> > >  
> > >         if (xe_gt_is_media_type(gt))
> > >                 for (n = 0; n < MED_VF_SW_FLAG_COUNT; n++)
> > > -                       xe_mmio_write32(&gt->mmio,
> > > MED_VF_SW_FLAG(n),
> > > regs[n]);
> > > +                       xe_mmio_write32(&mmio, MED_VF_SW_FLAG(n),
> > > regs[n]);
> > >         else
> > >                 for (n = 0; n < VF_SW_FLAG_COUNT; n++)
> > > -                       xe_mmio_write32(&gt->mmio, VF_SW_FLAG(n),
> > > regs[n]);
> > > +                       xe_mmio_write32(&mmio, VF_SW_FLAG(n),
> > > regs[n]);
> > >  
> > >         return 0;
> > >  }
> > 
>