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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to