[Intel-gfx] drm/i915: Watchdog timeout: DRM kernel interface to set the timeout

[Tvrtko Ursulin]

On 07/01/2019 12:50, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-01-07 12:38:47)
>> On 05/01/2019 02:39, Carlos Santa wrote:
>>> +/* Return the timer count threshold in microseconds. */
>>> +int i915_gem_context_get_watchdog(struct i915_gem_context *ctx,
>>> +                               struct drm_i915_gem_context_param *args)
>>> +     if (__copy_to_user(u64_to_user_ptr(args->value),
>>> +                        &threshold_in_us,
>>> +                        sizeof(threshold_in_us))) {
>>
>> I think user pointer hasn't been verified for write access yet so
>> standard copy_to_user should be used.
> 
> The starting point here is that this bakes in OTHER_CLASS as uABI when
> clearly it is not uABI. The array must be variable size and so the first
> pass is to report the size to userspace (if they pass args->size = 0)
> and then to copy up to the user requested size. Since it is a variable
> array with uncertain indexing, the output should be along the lines of
> [class, instance, watchdog]. But what about virtual engine? Good
> question. (Remember set must follow the same pattern as get, so you can
> always do a rmw cleanly.)
> 
> I guess we just have to accept class == -1 as meaning use instance as an
> index into ctx->engines[]. Tvrtko? Virtual engine would be reported as
> (-1, 0, watchdog), and settable similarly with the other engines being
> addressable either by index or by class-instance.

Or use index based addressing mode if engine map is set? Index 0 only 
being valid if load balancing is enabled on top.

So an array of structs like:

struct drm_i915_watchdog_timeout {
	union {
		struct {
			u16 class;
			u16 instance;
		};
		u32 index;
	};
	u32 timeout_us;
};

?

We can allow standalone set param and via extension as well.

Regards,

Tvrtko