Hi Jonathan, sorry it took me so long to get to this topic.
On 2026-02-19 at 20:43:03 +0000, Cavitt, Jonathan wrote: > Pinging Krzystof Karas for a second opinion. > -Jonathan Cavitt > > -----Original Message----- > From: Cavitt, Jonathan > Sent: Friday, February 13, 2026 8:28 AM > To: 'Zhenyu Wang' <[email protected]>; [email protected] > Cc: Gupta, Saurabhg <[email protected]>; Zuo, Alex > <[email protected]>; Cavitt, Jonathan <[email protected]> > Subject: RE: [PATCH] drm/i915/gvt: Cast u64 array to u32 array > > > > -----Original Message----- > > From: Zhenyu Wang <[email protected]> > > Sent: Friday, February 13, 2026 2:42 AM > > To: Cavitt, Jonathan <[email protected]>; > > [email protected] > > Cc: Gupta, Saurabhg <[email protected]>; Zuo, Alex > > <[email protected]>; Cavitt, Jonathan <[email protected]> > > Subject: Re: [PATCH] drm/i915/gvt: Cast u64 array to u32 array > > > > > > Jonathan Cavitt <[email protected]> writes: > > > > > > > Static analysis issue: > > > > > > > > The u64 array workload->shadow_mm->ppgtt_mm.shadow_pdps is cast to a > > > > void pointer and passed as a u32 array to set_context_pdp_root_pointer > > > > as a part of update_shadow_pdps. This isn't wrong, per se, but we > > > > should properly cast it to an appropriately-sized u32 array before > > > > submission. > > > > > > > > Signed-off-by: Jonathan Cavitt <[email protected]> > > > > --- > > > > drivers/gpu/drm/i915/gvt/scheduler.c | 6 ++++-- > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c > > > > b/drivers/gpu/drm/i915/gvt/scheduler.c > > > > index 15fdd514ca83..1a95c9f76faa 100644 > > > > --- a/drivers/gpu/drm/i915/gvt/scheduler.c > > > > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c > > > > @@ -72,6 +72,7 @@ static void update_shadow_pdps(struct > > > > intel_vgpu_workload *workload) > > > > { > > > > struct execlist_ring_context *shadow_ring_context; > > > > struct intel_context *ctx = workload->req->context; > > > > + u32 pdp[8]; > > > > > > > > if (WARN_ON(!workload->shadow_mm)) > > > > return; > > > > @@ -79,9 +80,10 @@ static void update_shadow_pdps(struct > > > > intel_vgpu_workload *workload) > > > > if (WARN_ON(!atomic_read(&workload->shadow_mm->pincount))) > > > > return; > > > > > > > > + memcpy(pdp, workload->shadow_mm->ppgtt_mm.shadow_pdps, > > > > + sizeof(u64) * > > > > ARRAY_SIZE(workload->shadow_mm->ppgtt_mm.shadow_pdps)); > > > > shadow_ring_context = (struct execlist_ring_context > > > > *)ctx->lrc_reg_state; > > > > - set_context_pdp_root_pointer(shadow_ring_context, > > > > - (void > > > > *)workload->shadow_mm->ppgtt_mm.shadow_pdps); > > > > + set_context_pdp_root_pointer(shadow_ring_context, pdp); > > > > } > > > > > > > > > > I think we'd better just cast the type instead of extra copy. > > > > I'm not certain that would resolve the static analysis issue. > > > > To specify, the static analyzer is complaining that we're taking a pointer > > to an object > > of type 'unsigned long long' and dereferencing it as an object of type > > 'unsigned int'. > > The analyzer is getting uppity about this causing unexpected results > > depending on > > machine endianness (which... it won't, but the static analyzer doesn't know > > that), > > so I suspect the only way to get it to calm down is to do a direct memory > > copy, as > > seen here. Casting the type would just result in the same static analysis > > issue. Do you have any more details from the static analysis? The first thing I'd be worried about is possible truncation of the values in passed array, so casting it explicitly to u32 would not resolve that issue, if it were the focus of this report from static analyser. Also, seeing that set_context_pdp_root_pointer() function is only called from update_shadow_pdps(), you could change its definition to: static void set_context_pdp_root_pointer( struct execlist_ring_context *ring_context, u64 pdp[8]) so the cast would be done implicitly on value assignment: ring_context->pdps[i].val = pdp[7 - i]; or you could even do that explicitly: ring_context->pdps[i].val = (u32)pdp[7 - i]; Unfortunately, there is a discrepancy between "val" (defined i915/gvt/execlist.h as u32) and "shadow_pdps" (defined as an array of u64 values in i915/gvt/gtt.h). We could try to match these types, but it would be more invasive change than a simple cast you are trying to achieve here. -- Best Regards, Krzysztof
