On 11.03.2022 10:40, Lucas De Marchi wrote:
> On Tue, Mar 08, 2022 at 10:17:42PM +0530, Balasubramani Vivekanandan wrote:
> > This patch is continuation of the effort to move all pointers in i915,
> > which at any point may be pointing to device memory or system memory, to
> > iosys_map interface.
> > More details about the need of this change is explained in the patch
> > series which initiated this task
> > https://patchwork.freedesktop.org/series/99711/
> > 
> > This patch converts all access to the lrc_desc through iosys_map
> > interfaces.
> > 
> > Cc: Lucas De Marchi <lucas.demar...@intel.com>
> > Cc: John Harrison <john.c.harri...@intel.com>
> > Cc: Matthew Brost <matthew.br...@intel.com>
> > Cc: Umesh Nerlige Ramappa <umesh.nerlige.rama...@intel.com>
> > Signed-off-by: Balasubramani Vivekanandan 
> > <balasubramani.vivekanan...@intel.com>
> > ---
> 
> ...
> 
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
> > b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > @@ -2245,13 +2256,13 @@ static void 
> > prepare_context_registration_info(struct intel_context *ce)
> >     GEM_BUG_ON(i915_gem_object_is_lmem(guc->ct.vma->obj) !=
> >                i915_gem_object_is_lmem(ce->ring->vma->obj));
> > 
> > -   desc = __get_lrc_desc(guc, ctx_id);
> > -   desc->engine_class = engine_class_to_guc_class(engine->class);
> > -   desc->engine_submit_mask = engine->logical_mask;
> > -   desc->hw_context_desc = ce->lrc.lrca;
> > -   desc->priority = ce->guc_state.prio;
> > -   desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
> > -   guc_context_policy_init(engine, desc);
> > +   memset(&desc, 0, sizeof(desc));
> 
> previously we would re-use whatever was left in
> guc->lrc_desc_pool_vaddr. Here we are changing it to always zero
> everything and set the fields we are interested in.
> 
> As I'm not too familiar with this part and I see us traversing child 
> guc_process_desc
> which may point to the same id, it doesn't _feel_ safe. Did you check if
> this is not zero'ing what it shouldn't?
> 
> Matt Brost / John / Daniele, could you clarify?
> 
> thanks
> Lucas De Marchi

I verified that struct guc_lrc_desc is not updated anywhere else in the
driver other than in prepare_context_registration_info. So I went ahead
with clearing it before updating the fields.
But I will still wait for comments from Matt Brost/ John / Daniele for
their confirmation.

Thanks
Bala

Reply via email to