queue-4.4/drm-nouveau-i2c-disable-i2c-bus-access-after-fini.patch

   1 From 342406e4fbba9a174125fbfe6aeac3d64ef90f76 Mon Sep 17 00:00:00 2001
   2 From: Lyude Paul <lyude@redhat.com>
   3 Date: Tue, 9 Apr 2019 16:23:30 -0400
   4 Subject: drm/nouveau/i2c: Disable i2c bus access after ->fini()
   5
   6 From: Lyude Paul <lyude@redhat.com>
   7
   8 commit 342406e4fbba9a174125fbfe6aeac3d64ef90f76 upstream.
   9
  10 For a while, we've had the problem of i2c bus access not grabbing
  11 a runtime PM ref when it's being used in userspace by i2c-dev, resulting
  12 in nouveau spamming the kernel log with errors if anything attempts to
  13 access the i2c bus while the GPU is in runtime suspend. An example:
  14
  15 [  130.078386] nouveau 0000:01:00.0: i2c: aux 000d: begin idle timeout ffffffff
  16
  17 Since the GPU is in runtime suspend, the MMIO region that the i2c bus is
  18 on isn't accessible. On x86, the standard behavior for accessing an
  19 unavailable MMIO region is to just return ~0.
  20
  21 Except, that turned out to be a lie. While computers with a clean
  22 concious will return ~0 in this scenario, some machines will actually
  23 completely hang a CPU on certian bad MMIO accesses. This was witnessed
  24 with someone's Lenovo ThinkPad P50, where sensors-detect attempting to
  25 access the i2c bus while the GPU was suspended would result in a CPU
  26 hang:
  27
  28   CPU: 5 PID: 12438 Comm: sensors-detect Not tainted 5.0.0-0.rc4.git3.1.fc30.x86_64 #1
  29   Hardware name: LENOVO 20EQS64N17/20EQS64N17, BIOS N1EET74W (1.47 ) 11/21/2017
  30   RIP: 0010:ioread32+0x2b/0x30
  31   Code: 81 ff ff ff 03 00 77 20 48 81 ff 00 00 01 00 76 05 0f b7 d7 ed c3
  32   48 c7 c6 e1 0c 36 96 e8 2d ff ff ff b8 ff ff ff ff c3 8b 07 <c3> 0f 1f
  33   40 00 49 89 f0 48 81 fe ff ff 03 00 76 04 40 88 3e c3 48
  34   RSP: 0018:ffffaac3c5007b48 EFLAGS: 00000292 ORIG_RAX: ffffffffffffff13
  35   RAX: 0000000001111000 RBX: 0000000001111000 RCX: 0000043017a97186
  36   RDX: 0000000000000aaa RSI: 0000000000000005 RDI: ffffaac3c400e4e4
  37   RBP: ffff9e6443902c00 R08: ffffaac3c400e4e4 R09: ffffaac3c5007be7
  38   R10: 0000000000000004 R11: 0000000000000001 R12: ffff9e6445dd0000
  39   R13: 000000000000e4e4 R14: 00000000000003c4 R15: 0000000000000000
  40   FS:  00007f253155a740(0000) GS:ffff9e644f600000(0000) knlGS:0000000000000000
  41   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  42   CR2: 00005630d1500358 CR3: 0000000417c44006 CR4: 00000000003606e0
  43   DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
  44   DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
  45   Call Trace:
  46    g94_i2c_aux_xfer+0x326/0x850 [nouveau]
  47    nvkm_i2c_aux_i2c_xfer+0x9e/0x140 [nouveau]
  48    __i2c_transfer+0x14b/0x620
  49    i2c_smbus_xfer_emulated+0x159/0x680
  50    ? _raw_spin_unlock_irqrestore+0x1/0x60
  51    ? rt_mutex_slowlock.constprop.0+0x13d/0x1e0
  52    ? __lock_is_held+0x59/0xa0
  53    __i2c_smbus_xfer+0x138/0x5a0
  54    i2c_smbus_xfer+0x4f/0x80
  55    i2cdev_ioctl_smbus+0x162/0x2d0 [i2c_dev]
  56    i2cdev_ioctl+0x1db/0x2c0 [i2c_dev]
  57    do_vfs_ioctl+0x408/0x750
  58    ksys_ioctl+0x5e/0x90
  59    __x64_sys_ioctl+0x16/0x20
  60    do_syscall_64+0x60/0x1e0
  61    entry_SYSCALL_64_after_hwframe+0x49/0xbe
  62   RIP: 0033:0x7f25317f546b
  63   Code: 0f 1e fa 48 8b 05 1d da 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff
  64   ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01
  65   f0 ff ff 73 01 c3 48 8b 0d ed d9 0c 00 f7 d8 64 89 01 48
  66   RSP: 002b:00007ffc88caab68 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
  67   RAX: ffffffffffffffda RBX: 00005630d0fe7260 RCX: 00007f25317f546b
  68   RDX: 00005630d1598e80 RSI: 0000000000000720 RDI: 0000000000000003
  69   RBP: 00005630d155b968 R08: 0000000000000001 R09: 00005630d15a1da0
  70   R10: 0000000000000070 R11: 0000000000000246 R12: 00005630d1598e80
  71   R13: 00005630d12f3d28 R14: 0000000000000720 R15: 00005630d12f3ce0
  72   watchdog: BUG: soft lockup - CPU#5 stuck for 23s! [sensors-detect:12438]
  73
  74 Yikes! While I wanted to try to make it so that accessing an i2c bus on
  75 nouveau would wake up the GPU as needed, airlied pointed out that pretty
  76 much any usecase for userspace accessing an i2c bus on a GPU (mainly for
  77 the DDC brightness control that some displays have) is going to only be
  78 useful while there's at least one display enabled on the GPU anyway, and
  79 the GPU never sleeps while there's displays running.
  80
  81 Since teaching the i2c bus to wake up the GPU on userspace accesses is a
  82 good deal more difficult than it might seem, mostly due to the fact that
  83 we have to use the i2c bus during runtime resume of the GPU, we instead
  84 opt for the easiest solution: don't let userspace access i2c busses on
  85 the GPU at all while it's in runtime suspend.
  86
  87 Changes since v1:
  88 * Also disable i2c busses that run over DP AUX
  89
  90 Signed-off-by: Lyude Paul <lyude@redhat.com>
  91 Cc: stable@vger.kernel.org
  92 Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
  93 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  94
  95 ---
  96  drivers/gpu/drm/nouveau/include/nvkm/subdev/i2c.h |    2 +
  97  drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c     |   26 +++++++++++++++++++++-
  98  drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.h     |    2 +
  99  drivers/gpu/drm/nouveau/nvkm/subdev/i2c/base.c    |   15 ++++++++++++
 100  drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c     |   21 ++++++++++++++++-
 101  drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.h     |    1
 102  6 files changed, 65 insertions(+), 2 deletions(-)
 103
 104 --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/i2c.h
 105 +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/i2c.h
 106 @@ -37,6 +37,7 @@ struct nvkm_i2c_bus {
 107         struct mutex mutex;
 108         struct list_head head;
 109         struct i2c_adapter i2c;
 110 +       u8 enabled;
 111  };
 112
 113  int nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *);
 114 @@ -56,6 +57,7 @@ struct nvkm_i2c_aux {
 115         struct mutex mutex;
 116         struct list_head head;
 117         struct i2c_adapter i2c;
 118 +       u8 enabled;
 119
 120         u32 intr;
 121  };
 122 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c
 123 +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c
 124 @@ -105,9 +105,15 @@ nvkm_i2c_aux_acquire(struct nvkm_i2c_aux
 125  {
 126         struct nvkm_i2c_pad *pad = aux->pad;
 127         int ret;
 128 +
 129         AUX_TRACE(aux, "acquire");
 130         mutex_lock(&aux->mutex);
 131 -       ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_AUX);
 132 +
 133 +       if (aux->enabled)
 134 +               ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_AUX);
 135 +       else
 136 +               ret = -EIO;
 137 +
 138         if (ret)
 139                 mutex_unlock(&aux->mutex);
 140         return ret;
 141 @@ -141,6 +147,24 @@ nvkm_i2c_aux_del(struct nvkm_i2c_aux **p
 142         }
 143  }
 144
 145 +void
 146 +nvkm_i2c_aux_init(struct nvkm_i2c_aux *aux)
 147 +{
 148 +       AUX_TRACE(aux, "init");
 149 +       mutex_lock(&aux->mutex);
 150 +       aux->enabled = true;
 151 +       mutex_unlock(&aux->mutex);
 152 +}
 153 +
 154 +void
 155 +nvkm_i2c_aux_fini(struct nvkm_i2c_aux *aux)
 156 +{
 157 +       AUX_TRACE(aux, "fini");
 158 +       mutex_lock(&aux->mutex);
 159 +       aux->enabled = false;
 160 +       mutex_unlock(&aux->mutex);
 161 +}
 162 +
 163  int
 164  nvkm_i2c_aux_ctor(const struct nvkm_i2c_aux_func *func,
 165                   struct nvkm_i2c_pad *pad, int id,
 166 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.h
 167 +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.h
 168 @@ -14,6 +14,8 @@ int nvkm_i2c_aux_ctor(const struct nvkm_
 169  int nvkm_i2c_aux_new_(const struct nvkm_i2c_aux_func *, struct nvkm_i2c_pad *,
 170                       int id, struct nvkm_i2c_aux **);
 171  void nvkm_i2c_aux_del(struct nvkm_i2c_aux **);
 172 +void nvkm_i2c_aux_init(struct nvkm_i2c_aux *);
 173 +void nvkm_i2c_aux_fini(struct nvkm_i2c_aux *);
 174  int nvkm_i2c_aux_xfer(struct nvkm_i2c_aux *, bool retry, u8 type,
 175                       u32 addr, u8 *data, u8 size);
 176
 177 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/base.c
 178 +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/base.c
 179 @@ -160,8 +160,18 @@ nvkm_i2c_fini(struct nvkm_subdev *subdev
 180  {
 181         struct nvkm_i2c *i2c = nvkm_i2c(subdev);
 182         struct nvkm_i2c_pad *pad;
 183 +       struct nvkm_i2c_bus *bus;
 184 +       struct nvkm_i2c_aux *aux;
 185         u32 mask;
 186
 187 +       list_for_each_entry(aux, &i2c->aux, head) {
 188 +               nvkm_i2c_aux_fini(aux);
 189 +       }
 190 +
 191 +       list_for_each_entry(bus, &i2c->bus, head) {
 192 +               nvkm_i2c_bus_fini(bus);
 193 +       }
 194 +
 195         if ((mask = (1 << i2c->func->aux) - 1), i2c->func->aux_stat) {
 196                 i2c->func->aux_mask(i2c, NVKM_I2C_ANY, mask, 0);
 197                 i2c->func->aux_stat(i2c, &mask, &mask, &mask, &mask);
 198 @@ -180,6 +190,7 @@ nvkm_i2c_init(struct nvkm_subdev *subdev
 199         struct nvkm_i2c *i2c = nvkm_i2c(subdev);
 200         struct nvkm_i2c_bus *bus;
 201         struct nvkm_i2c_pad *pad;
 202 +       struct nvkm_i2c_aux *aux;
 203
 204         list_for_each_entry(pad, &i2c->pad, head) {
 205                 nvkm_i2c_pad_init(pad);
 206 @@ -189,6 +200,10 @@ nvkm_i2c_init(struct nvkm_subdev *subdev
 207                 nvkm_i2c_bus_init(bus);
 208         }
 209
 210 +       list_for_each_entry(aux, &i2c->aux, head) {
 211 +               nvkm_i2c_aux_init(aux);
 212 +       }
 213 +
 214         return 0;
 215  }
 216
 217 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
 218 +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
 219 @@ -110,6 +110,19 @@ nvkm_i2c_bus_init(struct nvkm_i2c_bus *b
 220         BUS_TRACE(bus, "init");
 221         if (bus->func->init)
 222                 bus->func->init(bus);
 223 +
 224 +       mutex_lock(&bus->mutex);
 225 +       bus->enabled = true;
 226 +       mutex_unlock(&bus->mutex);
 227 +}
 228 +
 229 +void
 230 +nvkm_i2c_bus_fini(struct nvkm_i2c_bus *bus)
 231 +{
 232 +       BUS_TRACE(bus, "fini");
 233 +       mutex_lock(&bus->mutex);
 234 +       bus->enabled = false;
 235 +       mutex_unlock(&bus->mutex);
 236  }
 237
 238  void
 239 @@ -126,9 +139,15 @@ nvkm_i2c_bus_acquire(struct nvkm_i2c_bus
 240  {
 241         struct nvkm_i2c_pad *pad = bus->pad;
 242         int ret;
 243 +
 244         BUS_TRACE(bus, "acquire");
 245         mutex_lock(&bus->mutex);
 246 -       ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C);
 247 +
 248 +       if (bus->enabled)
 249 +               ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C);
 250 +       else
 251 +               ret = -EIO;
 252 +
 253         if (ret)
 254                 mutex_unlock(&bus->mutex);
 255         return ret;
 256 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.h
 257 +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.h
 258 @@ -17,6 +17,7 @@ int nvkm_i2c_bus_new_(const struct nvkm_
 259                       int id, struct nvkm_i2c_bus **);
 260  void nvkm_i2c_bus_del(struct nvkm_i2c_bus **);
 261  void nvkm_i2c_bus_init(struct nvkm_i2c_bus *);
 262 +void nvkm_i2c_bus_fini(struct nvkm_i2c_bus *);
 263
 264  int nvkm_i2c_bit_xfer(struct nvkm_i2c_bus *, struct i2c_msg *, int);
 265