On Sat, 11 Jun 2016, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
> On Sat, Jun 11, 2016 at 06:22:00PM +0300, Jani Nikula wrote:
>> On Sat, 11 Jun 2016, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
>> > One of the first steps in triaging memory corruption on module load is
>> > to disable stolen. (It helps reduce the impact of the BIOS clobbering
>> > our memory since we allocate our ringbuffers and initial objects from
>> > stolen, if they are clobbered we get an immediate hang and garbage on
>> > screen.) Rather than requesting bug reporters patch their kernel to test
>> > the theory, allow us to disable stolen through a module option.
>> >
>> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>> > ---
>> > drivers/gpu/drm/i915/i915_gem_stolen.c | 5 +++++
>> > drivers/gpu/drm/i915/i915_params.c | 6 ++++++
>> > drivers/gpu/drm/i915/i915_params.h | 1 +
>> > 3 files changed, 12 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> > index e9cd82290408..bb8567b1d6a4 100644
>> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> > @@ -411,6 +411,11 @@ int i915_gem_init_stolen(struct drm_device *dev)
>> > }
>> > #endif
>> >
>> > + if (!i915.enable_stolen) {
>> > + DRM_INFO("Disabling use of stolen memory at user request.\n");
>> > + return 0;
>> > + }
>> > +
>> > if (ggtt->stolen_size == 0)
>> > return 0;
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>> > index 573e78723fc5..511528199aec 100644
>> > --- a/drivers/gpu/drm/i915/i915_params.c
>> > +++ b/drivers/gpu/drm/i915/i915_params.c
>> > @@ -60,6 +60,7 @@ struct i915_params i915 __read_mostly = {
>> > .enable_dp_mst = true,
>> > .inject_load_failure = 0,
>> > .enable_dpcd_backlight = false,
>> > + .enable_stolen = true,
>> > };
>> >
>> > module_param_named(modeset, i915.modeset, int, 0400);
>> > @@ -119,6 +120,11 @@ MODULE_PARM_DESC(enable_hangcheck,
>> > "WARNING: Disabling this can cause system wide hangs. "
>> > "(default: true)");
>> >
>> > +module_param_named(enable_stolen, i915.enable_stolen, bool, 0400);
>>
>> _unsafe, to make it clear that this is for debugging?
>
> There shouldn't be a negative consequence, some secondary effects but
> nothing that I would consider unsafe. It would be the same as tainting
> the kernel because of a BIOS setting.
It's not so much about it being unsafe or not, really, but a statement
that this is not ABI and people shouldn't rely on it. If someone works
around an issue in their system by toggling this, removing the option
becomes a regression.
If it were as convenient to add debugfs flags, and you could set and
have them available at module reload time, that's what we'd do. But
instead we have these debug-but-ABI-flags just because it's the
convenient thing to do.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
