On Mon, 2022-10-17 at 10:03 -0700, Alan Previn Teres Alexis wrote:
> >
> > On Thu, 2022-10-13 at 13:48 -0700, Ceraolo Spurio, Daniele wrote:
> > > >
> > > > On 10/5/2022 9:38 PM, Alan Previn wrote:
> > > > > > In preparation for future MTL-PXP feature support, PXP control
> > > > > > context should only valid on the correct gt tile. Depending on the
> > > > > > device-info this mat not necessarily be the root GT tile and
> > > > > > depends on which tile owns the VEBOX and KCR.
> > > > > >
> > Alan:[snip]
> > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> > > > > > b/drivers/gpu/drm/i915/gt/intel_gt.c
> > > > > > index b367cfff48d5..e61f6c5ed440 100644
> > > > > > --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> > > > > > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> > > > > > @@ -850,6 +850,10 @@ int intel_gt_probe_all(struct drm_i915_private
> > > > > > *i915)
> > > > > > gt->name = "Primary GT";
> > > > > > gt->info.engine_mask = RUNTIME_INFO(i915)->platform_engine_mask;
> > > > > >
> > > > > > + /* device config determines which GT owns the global pxp-tee
> > > > > > context */
> > > > > > + if (VDBOX_MASK(gt) && !INTEL_INFO(i915)->has_nonroot_pxpgt)
> > > > > > + gt->pxptee_iface_owner = true;
> > > > > > +
> > > >
> > > > I'm not convinced that we need dedicated has_nonroot_pxpgt and
> > > > pxptee_iface_owner flags. MTL moves the GSC inside a GT and the owner
> > > > of
> > > > PXP is the GT where the GSC engine resides. So we could have a checker
> > > > like:
> > > >
> > > > bool intel_pxp_supported(struct intel_gt *gt)
> > > > {
> > > > /* we only support HECI PXP from the root GT */
> > > > if (HAS_HECI_PXP(gt->i915))
> > > > return gt_is_root(gt);
> > > >
> > > > return HAS_ENGINE(gt, GSC);
> > > > }
> > > >
> > > > I'm aware that the GSC engine code is still not available, but we can
> > > > special case for MTL for now and then replace it when the GSC code
> > > > lands:
> > > >
> > > > bool intel_pxp_supported(struct intel_gt *gt)
> > > > {
> > > > /* we only support HECI PXP from the root GT */
> > > > if (HAS_HECI_PXP(gt->i915))
> > > > return gt_is_root(gt);
> > > >
> > > > /* TODO: replace with GSC check */
> > > > return IS_METEORLAKE(gt->i915) && !gt_is_root(gt);
> > > > }
> > > >
> > > > Then we can use intel_pxp_supported(gt) instead of
> > > > gt->pxptee_iface_owner and we can drop has_nonroot_pxpgt. Might also be
> > > > worth merging this with HAS_PXP for a unified check, but that can come
> > > > later.
> > > >
> > > > Daniele
> >
> > As per offline conversations, we know above combination may not work for
> > the DG2 case, but i'll go ahead and re-rev this
> > after i look for another way to avoid creating another device info
> > variable- i'll try to get a karnaugh map going to
> > ensure we have a good combination of existing device-config info that are
> > reliable for all current and MTL usages else
> > we may need a new device-config after all (maybe a better named one if
> > needed).
Alan: Looks like the original intel_pxp_init (with only the change to HAS_PXP
to take in gt as input) might be
sufficient for all cases we have with today's hw - without the need for the
pxptee_iface_owner or has_nonroot_pxpgt:
* for HW without PXP/GSC-HuC-Authent: will fail on either HAS_PXP and
_gt_needs_teelink
* for adl/tgl: HAS_PXP will PASS
* for dg2: HAS_PXP will fail, but _gt_needs_teelink will pass
* for mtl: HAS_PXP will pass for 2nd tile only due to VDBOX mask
the only open would be if kernel was being built without CONFIG_MEI_PXP and we
are running on MTL
so that might require some "TODO .. MTL..." comments
