Commit 568944af authored by John Harrison's avatar John Harrison

drm/i915/guc: Limit scheduling properties to avoid overflow

GuC converts the pre-emption timeout and timeslice quantum values into
clock ticks internally. That significantly reduces the point of 32bit
overflow. On current platforms, worst case scenario is approximately
110 seconds. Rather than allowing the user to set higher values and
then get confused by early timeouts, add limits when setting these
values.

v2: Add helper functions for clamping (review feedback from Tvrtko).
v3: Add a bunch of BUG_ON range checks in addition to the checks
already in the clamping functions (Tvrtko)
Signed-off-by: default avatarJohn Harrison <John.C.Harrison@Intel.com>
Reviewed-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Acked-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221006213813.1563435-2-John.C.Harrison@Intel.com
parent c61aa740
......@@ -348,4 +348,10 @@ intel_engine_get_hung_context(struct intel_engine_cs *engine)
return engine->hung_ce;
}
u64 intel_clamp_heartbeat_interval_ms(struct intel_engine_cs *engine, u64 value);
u64 intel_clamp_max_busywait_duration_ns(struct intel_engine_cs *engine, u64 value);
u64 intel_clamp_preempt_timeout_ms(struct intel_engine_cs *engine, u64 value);
u64 intel_clamp_stop_timeout_ms(struct intel_engine_cs *engine, u64 value);
u64 intel_clamp_timeslice_duration_ms(struct intel_engine_cs *engine, u64 value);
#endif /* _INTEL_RINGBUFFER_H_ */
......@@ -512,6 +512,26 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id,
engine->flags |= I915_ENGINE_HAS_EU_PRIORITY;
}
/* Cap properties according to any system limits */
#define CLAMP_PROP(field) \
do { \
u64 clamp = intel_clamp_##field(engine, engine->props.field); \
if (clamp != engine->props.field) { \
drm_notice(&engine->i915->drm, \
"Warning, clamping %s to %lld to prevent overflow\n", \
#field, clamp); \
engine->props.field = clamp; \
} \
} while (0)
CLAMP_PROP(heartbeat_interval_ms);
CLAMP_PROP(max_busywait_duration_ns);
CLAMP_PROP(preempt_timeout_ms);
CLAMP_PROP(stop_timeout_ms);
CLAMP_PROP(timeslice_duration_ms);
#undef CLAMP_PROP
engine->defaults = engine->props; /* never to change again */
engine->context_size = intel_engine_context_size(gt, engine->class);
......@@ -534,6 +554,55 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id,
return 0;
}
u64 intel_clamp_heartbeat_interval_ms(struct intel_engine_cs *engine, u64 value)
{
value = min_t(u64, value, jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT));
return value;
}
u64 intel_clamp_max_busywait_duration_ns(struct intel_engine_cs *engine, u64 value)
{
value = min(value, jiffies_to_nsecs(2));
return value;
}
u64 intel_clamp_preempt_timeout_ms(struct intel_engine_cs *engine, u64 value)
{
/*
* NB: The GuC API only supports 32bit values. However, the limit is further
* reduced due to internal calculations which would otherwise overflow.
*/
if (intel_guc_submission_is_wanted(&engine->gt->uc.guc))
value = min_t(u64, value, guc_policy_max_preempt_timeout_ms());
value = min_t(u64, value, jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT));
return value;
}
u64 intel_clamp_stop_timeout_ms(struct intel_engine_cs *engine, u64 value)
{
value = min_t(u64, value, jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT));
return value;
}
u64 intel_clamp_timeslice_duration_ms(struct intel_engine_cs *engine, u64 value)
{
/*
* NB: The GuC API only supports 32bit values. However, the limit is further
* reduced due to internal calculations which would otherwise overflow.
*/
if (intel_guc_submission_is_wanted(&engine->gt->uc.guc))
value = min_t(u64, value, guc_policy_max_exec_quantum_ms());
value = min_t(u64, value, jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT));
return value;
}
static void __setup_engine_capabilities(struct intel_engine_cs *engine)
{
struct drm_i915_private *i915 = engine->i915;
......
......@@ -144,7 +144,7 @@ max_spin_store(struct kobject *kobj, struct kobj_attribute *attr,
const char *buf, size_t count)
{
struct intel_engine_cs *engine = kobj_to_engine(kobj);
unsigned long long duration;
unsigned long long duration, clamped;
int err;
/*
......@@ -168,7 +168,8 @@ max_spin_store(struct kobject *kobj, struct kobj_attribute *attr,
if (err)
return err;
if (duration > jiffies_to_nsecs(2))
clamped = intel_clamp_max_busywait_duration_ns(engine, duration);
if (duration != clamped)
return -EINVAL;
WRITE_ONCE(engine->props.max_busywait_duration_ns, duration);
......@@ -203,7 +204,7 @@ timeslice_store(struct kobject *kobj, struct kobj_attribute *attr,
const char *buf, size_t count)
{
struct intel_engine_cs *engine = kobj_to_engine(kobj);
unsigned long long duration;
unsigned long long duration, clamped;
int err;
/*
......@@ -218,7 +219,8 @@ timeslice_store(struct kobject *kobj, struct kobj_attribute *attr,
if (err)
return err;
if (duration > jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT))
clamped = intel_clamp_timeslice_duration_ms(engine, duration);
if (duration != clamped)
return -EINVAL;
WRITE_ONCE(engine->props.timeslice_duration_ms, duration);
......@@ -256,7 +258,7 @@ stop_store(struct kobject *kobj, struct kobj_attribute *attr,
const char *buf, size_t count)
{
struct intel_engine_cs *engine = kobj_to_engine(kobj);
unsigned long long duration;
unsigned long long duration, clamped;
int err;
/*
......@@ -272,7 +274,8 @@ stop_store(struct kobject *kobj, struct kobj_attribute *attr,
if (err)
return err;
if (duration > jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT))
clamped = intel_clamp_stop_timeout_ms(engine, duration);
if (duration != clamped)
return -EINVAL;
WRITE_ONCE(engine->props.stop_timeout_ms, duration);
......@@ -306,7 +309,7 @@ preempt_timeout_store(struct kobject *kobj, struct kobj_attribute *attr,
const char *buf, size_t count)
{
struct intel_engine_cs *engine = kobj_to_engine(kobj);
unsigned long long timeout;
unsigned long long timeout, clamped;
int err;
/*
......@@ -322,7 +325,8 @@ preempt_timeout_store(struct kobject *kobj, struct kobj_attribute *attr,
if (err)
return err;
if (timeout > jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT))
clamped = intel_clamp_preempt_timeout_ms(engine, timeout);
if (timeout != clamped)
return -EINVAL;
WRITE_ONCE(engine->props.preempt_timeout_ms, timeout);
......@@ -362,7 +366,7 @@ heartbeat_store(struct kobject *kobj, struct kobj_attribute *attr,
const char *buf, size_t count)
{
struct intel_engine_cs *engine = kobj_to_engine(kobj);
unsigned long long delay;
unsigned long long delay, clamped;
int err;
/*
......@@ -379,7 +383,8 @@ heartbeat_store(struct kobject *kobj, struct kobj_attribute *attr,
if (err)
return err;
if (delay >= jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT))
clamped = intel_clamp_heartbeat_interval_ms(engine, delay);
if (delay != clamped)
return -EINVAL;
err = intel_engine_set_heartbeat(engine, delay);
......
......@@ -327,6 +327,27 @@ struct guc_update_scheduling_policy {
#define GLOBAL_POLICY_DEFAULT_DPC_PROMOTE_TIME_US 500000
/*
* GuC converts the timeout to clock ticks internally. Different platforms have
* different GuC clocks. Thus, the maximum value before overflow is platform
* dependent. Current worst case scenario is about 110s. So, the spec says to
* limit to 100s to be safe.
*/
#define GUC_POLICY_MAX_EXEC_QUANTUM_US (100 * 1000 * 1000UL)
#define GUC_POLICY_MAX_PREEMPT_TIMEOUT_US (100 * 1000 * 1000UL)
static inline u32 guc_policy_max_exec_quantum_ms(void)
{
BUILD_BUG_ON(GUC_POLICY_MAX_EXEC_QUANTUM_US >= UINT_MAX);
return GUC_POLICY_MAX_EXEC_QUANTUM_US / 1000;
}
static inline u32 guc_policy_max_preempt_timeout_ms(void)
{
BUILD_BUG_ON(GUC_POLICY_MAX_PREEMPT_TIMEOUT_US >= UINT_MAX);
return GUC_POLICY_MAX_PREEMPT_TIMEOUT_US / 1000;
}
struct guc_policies {
u32 submission_queue_depth[GUC_MAX_ENGINE_CLASSES];
/* In micro seconds. How much time to allow before DPC processing is
......
......@@ -2430,6 +2430,10 @@ static int guc_context_policy_init_v70(struct intel_context *ce, bool loop)
int ret;
/* NB: For both of these, zero means disabled. */
GEM_BUG_ON(overflows_type(engine->props.timeslice_duration_ms * 1000,
execution_quantum));
GEM_BUG_ON(overflows_type(engine->props.preempt_timeout_ms * 1000,
preemption_timeout));
execution_quantum = engine->props.timeslice_duration_ms * 1000;
preemption_timeout = engine->props.preempt_timeout_ms * 1000;
......@@ -2463,6 +2467,10 @@ static void guc_context_policy_init_v69(struct intel_engine_cs *engine,
desc->policy_flags |= CONTEXT_POLICY_FLAG_PREEMPT_TO_IDLE_V69;
/* NB: For both of these, zero means disabled. */
GEM_BUG_ON(overflows_type(engine->props.timeslice_duration_ms * 1000,
desc->execution_quantum));
GEM_BUG_ON(overflows_type(engine->props.preempt_timeout_ms * 1000,
desc->preemption_timeout));
desc->execution_quantum = engine->props.timeslice_duration_ms * 1000;
desc->preemption_timeout = engine->props.preempt_timeout_ms * 1000;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment