diff mbox series

drm/i915/selftests: Ensure we don't clamp a random offset to 32b

Message ID 20190710143054.610-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series drm/i915/selftests: Ensure we don't clamp a random offset to 32b | expand

Commit Message

Chris Wilson July 10, 2019, 2:30 p.m. UTC
Specify that we do want a 64b value for sizeof(u32) as we want to
compute the mask of the upper 62bits.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mika Kuoppala July 10, 2019, 3:34 p.m. UTC | #1
Chris Wilson <chris@chris-wilson.co.uk> writes:

> Specify that we do want a 64b value for sizeof(u32) as we want to
> compute the mask of the upper 62bits.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> index 3abe15a08b6d..275ec1bfc2be 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> @@ -1539,7 +1539,7 @@ static int igt_vm_isolation(void *arg)
>  
>  			div64_u64_rem(i915_prandom_u64_state(&prng),
>  				      vm_total, &offset);
> -			offset &= -sizeof(u32);
> +			offset &= -(u64)sizeof(u32);

Taking a sizeof of something we know the size of.
Ok enough! Back to business:

should be the same for 64bit, but make sure for 32.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>


>  			offset += I915_GTT_PAGE_SIZE;
>  
>  			err = write_to_scratch(ctx_a, engine,
> -- 
> 2.22.0
Chris Wilson July 10, 2019, 3:37 p.m. UTC | #2
Quoting Mika Kuoppala (2019-07-10 16:34:23)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Specify that we do want a 64b value for sizeof(u32) as we want to
> > compute the mask of the upper 62bits.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> > index 3abe15a08b6d..275ec1bfc2be 100644
> > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> > @@ -1539,7 +1539,7 @@ static int igt_vm_isolation(void *arg)
> >  
> >                       div64_u64_rem(i915_prandom_u64_state(&prng),
> >                                     vm_total, &offset);
> > -                     offset &= -sizeof(u32);
> > +                     offset &= -(u64)sizeof(u32);
> 
> Taking a sizeof of something we know the size of.

u32 is a proxy for dword, and I was just trying to be more expressive
that we want the address aligned to a dword for the MI_STORE_DWORD_IMM
used. alignof_dword maybe?
-Chris
Mika Kuoppala July 10, 2019, 3:45 p.m. UTC | #3
Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-07-10 16:34:23)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Specify that we do want a 64b value for sizeof(u32) as we want to
>> > compute the mask of the upper 62bits.
>> >
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
>> > index 3abe15a08b6d..275ec1bfc2be 100644
>> > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
>> > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
>> > @@ -1539,7 +1539,7 @@ static int igt_vm_isolation(void *arg)
>> >  
>> >                       div64_u64_rem(i915_prandom_u64_state(&prng),
>> >                                     vm_total, &offset);
>> > -                     offset &= -sizeof(u32);
>> > +                     offset &= -(u64)sizeof(u32);
>> 
>> Taking a sizeof of something we know the size of.
>
> u32 is a proxy for dword, and I was just trying to be more expressive
> that we want the address aligned to a dword for the MI_STORE_DWORD_IMM
> used. alignof_dword maybe?

I was being a bit snarky. It is more expressive than -4 indeed.
Compromise as we dont have a define for natural aligment for
command streamer memory access.

-Mika
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 3abe15a08b6d..275ec1bfc2be 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -1539,7 +1539,7 @@  static int igt_vm_isolation(void *arg)
 
 			div64_u64_rem(i915_prandom_u64_state(&prng),
 				      vm_total, &offset);
-			offset &= -sizeof(u32);
+			offset &= -(u64)sizeof(u32);
 			offset += I915_GTT_PAGE_SIZE;
 
 			err = write_to_scratch(ctx_a, engine,