"Cavitt, Jonathan" <[email protected]> writes:
> -----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.
>
> This is the part of the email that I'd throw around terms like "strict
> aliasing" and
> "type punning" if I thought they were relevant. They probably aren't, though.
>
I really don't want to do extra copy as this is hot path for every exec
submission.
How about below change?
Btw, which static analysis you're using? Looks I don't get such warning
with either sparse or smatch...
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
b/drivers/gpu/drm/i915/gvt/scheduler.c
index 63ad1fed525a..3f09d6440827 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -1,3 +1,4 @@
+
/*
* Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
*
@@ -54,7 +55,7 @@
static void set_context_pdp_root_pointer(
struct execlist_ring_context *ring_context,
- u32 pdp[8])
+ u32 *pdp)
{
int i;
@@ -75,7 +76,7 @@ static void update_shadow_pdps(struct intel_vgpu_workload
*workload)
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);
+ (u32
*)workload->shadow_mm->ppgtt_mm.shadow_pdps);
}
/*
