[PATCH] drm/xe/vf: Track writes to inaccessible registers from VF

[Gustavo Sousa]

Quoting Michal Wajdeczko (2024-06-24 08:45:26-03:00)
>Only limited set of registers is accessible for the VF driver.
>The hardware will silently drop writes to inaccessible registers,
>but to improve our driver lets track all such unexpected writes
>on debug builds.
>
>We will explicitly ignore writes to SOFTWARE_FLAGS_SPR33 since it

Drive-by comment: I might be missing something, but it looks like the
new code would actually try to do the write as PF instead of ignoring
it?

--
Gustavo Sousa

>is used by the driver just to mimic wmb and we do not have any
>similar unused scratch register accessible from the VF.
>
>Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>---
> drivers/gpu/drm/xe/xe_gt_sriov_vf.c | 22 ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_gt_sriov_vf.h |  1 +
> drivers/gpu/drm/xe/xe_mmio.c        |  6 +++++-
> 3 files changed, 28 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>index 41e46a00c01e..36cefe3161e1 100644
>--- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>+++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>@@ -892,6 +892,28 @@ u32 xe_gt_sriov_vf_read32(struct xe_gt *gt, struct xe_reg reg)
>         return rr->value;
> }
> 
>+/**
>+ * xe_gt_sriov_vf_write32 - Track writes to an inaccessible registers.
>+ * @gt: the &xe_gt
>+ * @reg: the register to write
>+ * @val: value to write
>+ *
>+ * This function is for VF use only.
>+ * This function is dedicated for registers that VFs can't write directly.
>+ * It will trigger a WARN if running on debug build.
>+ */
>+void xe_gt_sriov_vf_write32(struct xe_gt *gt, struct xe_reg reg, u32 val)
>+{
>+        u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>+
>+        xe_gt_assert(gt, IS_SRIOV_VF(gt_to_xe(gt)));
>+        xe_gt_assert(gt, !reg.vf);
>+
>+        xe_gt_WARN(gt, IS_ENABLED(CONFIG_DRM_XE_DEBUG),
>+                   "VF is trying to write %#x to an inaccessible register %#x+%#x\n",
>+                   val, reg.addr, addr - reg.addr);
>+}
>+
> /**
>  * xe_gt_sriov_vf_print_config - Print VF self config.
>  * @gt: the &xe_gt
>diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.h b/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
>index 0de7f8cbcfa6..e541ce57bec2 100644
>--- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
>+++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
>@@ -22,6 +22,7 @@ u32 xe_gt_sriov_vf_gmdid(struct xe_gt *gt);
> u16 xe_gt_sriov_vf_guc_ids(struct xe_gt *gt);
> u64 xe_gt_sriov_vf_lmem(struct xe_gt *gt);
> u32 xe_gt_sriov_vf_read32(struct xe_gt *gt, struct xe_reg reg);
>+void xe_gt_sriov_vf_write32(struct xe_gt *gt, struct xe_reg reg, u32 val);
> 
> void xe_gt_sriov_vf_print_config(struct xe_gt *gt, struct drm_printer *p);
> void xe_gt_sriov_vf_print_runtime(struct xe_gt *gt, struct drm_printer *p);
>diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
>index f92faad4b96d..ff72afd79272 100644
>--- a/drivers/gpu/drm/xe/xe_mmio.c
>+++ b/drivers/gpu/drm/xe/xe_mmio.c
>@@ -151,7 +151,11 @@ void xe_mmio_write32(struct xe_gt *gt, struct xe_reg reg, u32 val)
>         u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
> 
>         trace_xe_reg_rw(gt, true, addr, val, sizeof(val));
>-        writel(val, (reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>+
>+        if (!reg.vf && IS_SRIOV_VF(gt_to_xe(gt)) && reg.addr != SOFTWARE_FLAGS_SPR33.addr)
>+                xe_gt_sriov_vf_write32(gt, reg, val);
>+        else
>+                writel(val, (reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
> }
> 
> u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
>-- 
>2.43.0
>