[Nouveau] [PATCH RESEND] drm/nouveau: add locking around instobj list operations
Fixes memory corruptions, oopses, etc. when multiple gpuobjs are
simultaneously created or destroyed.
Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/nouveau/core/subdev/instmem/base.c | 35 +-
1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/core/subdev/instmem/base.c
b/drivers/gpu/drm/nouveau/core/subdev/instmem/base.c
index 1188227..6565f3d 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/instmem/base.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/instmem/base.c
@@ -40,15 +40,21 @@ nouveau_instobj_create_(struct nouveau_object *parent,
if (ret)
return ret;
+ mutex_lock(imem-base.mutex);
list_add(iobj-head, imem-list);
+ mutex_unlock(imem-base.mutex);
return 0;
}
void
nouveau_instobj_destroy(struct nouveau_instobj *iobj)
{
- if (iobj-head.prev)
- list_del(iobj-head);
+ struct nouveau_subdev *subdev = nv_subdev(iobj-base.engine);
+
+ mutex_lock(subdev-mutex);
+ list_del(iobj-head);
+ mutex_unlock(subdev-mutex);
+
return nouveau_object_destroy(iobj-base);
}
@@ -88,6 +94,8 @@ nouveau_instmem_init(struct nouveau_instmem *imem)
if (ret)
return ret;
+ mutex_lock(imem-base.mutex);
+
list_for_each_entry(iobj, imem-list, head) {
if (iobj-suspend) {
for (i = 0; i iobj-size; i += 4)
@@ -97,6 +105,8 @@ nouveau_instmem_init(struct nouveau_instmem *imem)
}
}
+ mutex_unlock(imem-base.mutex);
+
return 0;
}
@@ -104,17 +114,26 @@ int
nouveau_instmem_fini(struct nouveau_instmem *imem, bool suspend)
{
struct nouveau_instobj *iobj;
- int i;
+ int i, ret = 0;
if (suspend) {
+ mutex_lock(imem-base.mutex);
+
list_for_each_entry(iobj, imem-list, head) {
iobj-suspend = vmalloc(iobj-size);
- if (iobj-suspend) {
- for (i = 0; i iobj-size; i += 4)
- iobj-suspend[i / 4] = nv_ro32(iobj, i);
- } else
- return -ENOMEM;
+ if (!iobj-suspend) {
+ ret = -ENOMEM;
+ break;
+ }
+
+ for (i = 0; i iobj-size; i += 4)
+ iobj-suspend[i / 4] = nv_ro32(iobj, i);
}
+
+ mutex_unlock(imem-base.mutex);
+
+ if (ret)
+ return ret;
}
return nouveau_subdev_fini(imem-base, suspend);
--
1.8.0.2
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH] drm/nouveau/vm: fix memory corruption when pgt allocation fails
If we return freed vm, nouveau_drm_open will happily call nouveau_cli_destroy, which will try to free it again. Reported-by: Peter Hurley pe...@hurleysoftware.com Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com --- drivers/gpu/drm/nouveau/core/subdev/vm/base.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/core/subdev/vm/base.c b/drivers/gpu/drm/nouveau/core/subdev/vm/base.c index 082c11b..77c67fc 100644 --- a/drivers/gpu/drm/nouveau/core/subdev/vm/base.c +++ b/drivers/gpu/drm/nouveau/core/subdev/vm/base.c @@ -352,7 +352,7 @@ nouveau_vm_create(struct nouveau_vmmgr *vmm, u64 offset, u64 length, u64 mm_length = (offset + length) - mm_offset; int ret; - vm = *pvm = kzalloc(sizeof(*vm), GFP_KERNEL); + vm = kzalloc(sizeof(*vm), GFP_KERNEL); if (!vm) return -ENOMEM; @@ -376,6 +376,8 @@ nouveau_vm_create(struct nouveau_vmmgr *vmm, u64 offset, u64 length, return ret; } + *pvm = vm; + return 0; } -- 1.8.0.2 ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH] drm/nouveau: don't return freed object from nouveau_handle_create
Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com --- drivers/gpu/drm/nouveau/core/core/handle.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/core/core/handle.c b/drivers/gpu/drm/nouveau/core/core/handle.c index b8d2cbf..264c2b3 100644 --- a/drivers/gpu/drm/nouveau/core/core/handle.c +++ b/drivers/gpu/drm/nouveau/core/core/handle.c @@ -109,7 +109,7 @@ nouveau_handle_create(struct nouveau_object *parent, u32 _parent, u32 _handle, while (!nv_iclass(namedb, NV_NAMEDB_CLASS)) namedb = namedb-parent; - handle = *phandle = kzalloc(sizeof(*handle), GFP_KERNEL); + handle = kzalloc(sizeof(*handle), GFP_KERNEL); if (!handle) return -ENOMEM; @@ -146,6 +146,9 @@ nouveau_handle_create(struct nouveau_object *parent, u32 _parent, u32 _handle, } hprintk(handle, TRACE, created\n); + + *phandle = handle; + return 0; } -- 1.8.0.2 ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH] drm/nouveau: fix nouveau_client allocation failure path
Depending on the point of failure, freed object would be returned
or memory leak would happen.
Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
---
drivers/gpu/drm/nouveau/core/core/client.c | 4 +---
drivers/gpu/drm/nouveau/core/include/core/client.h | 3 +++
drivers/gpu/drm/nouveau/nouveau_drm.c | 7 ++-
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/core/core/client.c
b/drivers/gpu/drm/nouveau/core/core/client.c
index 762e4fc..295c221 100644
--- a/drivers/gpu/drm/nouveau/core/core/client.c
+++ b/drivers/gpu/drm/nouveau/core/core/client.c
@@ -66,10 +66,8 @@ nouveau_client_create_(const char *name, u64 devname, const
char *cfg,
ret = nouveau_handle_create(nv_object(client), ~0, ~0,
nv_object(client), client-root);
- if (ret) {
- nouveau_namedb_destroy(client-base);
+ if (ret)
return ret;
- }
/* prevent init/fini being called, os in in charge of this */
atomic_set(nv_object(client)-usecount, 2);
diff --git a/drivers/gpu/drm/nouveau/core/include/core/client.h
b/drivers/gpu/drm/nouveau/core/include/core/client.h
index b8d0457..c66eac5 100644
--- a/drivers/gpu/drm/nouveau/core/include/core/client.h
+++ b/drivers/gpu/drm/nouveau/core/include/core/client.h
@@ -36,6 +36,9 @@ nouveau_client(void *obj)
int nouveau_client_create_(const char *name, u64 device, const char *cfg,
const char *dbg, int, void **);
+#define nouveau_client_destroy(p)
\
+ nouveau_namedb_destroy((p)-base)
+
int nouveau_client_init(struct nouveau_client *);
int nouveau_client_fini(struct nouveau_client *, bool suspend);
const char *nouveau_client_name(void *obj);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index ab85a89..83a38d8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -84,11 +84,16 @@ nouveau_cli_create(struct pci_dev *pdev, const char *name,
struct nouveau_cli *cli;
int ret;
+ *pcli = NULL;
ret = nouveau_client_create_(name, nouveau_name(pdev), nouveau_config,
nouveau_debug, size, pcli);
cli = *pcli;
- if (ret)
+ if (ret) {
+ if (cli)
+ nouveau_client_destroy(cli-base);
+ *pcli = NULL;
return ret;
+ }
mutex_init(cli-mutex);
return 0;
--
1.8.0.2
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH] drm/nouveau: fix ramht wraparound
When hash collision occurs and it's near ramht object boundary, we could read and possibly overwrite some memory after ramht object. Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com Cc: sta...@vger.kernel.org --- drivers/gpu/drm/nouveau/core/core/ramht.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/core/core/ramht.c b/drivers/gpu/drm/nouveau/core/core/ramht.c index 86a6404..6da314c 100644 --- a/drivers/gpu/drm/nouveau/core/core/ramht.c +++ b/drivers/gpu/drm/nouveau/core/core/ramht.c @@ -59,7 +59,7 @@ nouveau_ramht_insert(struct nouveau_ramht *ramht, int chid, } co += 8; - if (co = nv_gpuobj(ramht)-size) + if (co + 8 nv_gpuobj(ramht)-size) co = 0; } while (co != ho); -- 1.8.0.2 ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH] drm/nouveau: fix blank LVDS screen regression on pre-nv50 cards
Commit 2a44e499 (drm/nouveau/disp: introduce proper init/fini, separate
from create/destroy) started to call display init routines on pre-nv50
hardware on module load. But LVDS init code sets driver state in a way
which prevents modesetting code from operating properly.
nv04_display_init calls nv04_dfp_restore, which sets encoder-last_dpms to
NV_DPMS_CLEARED.
drm_crtc_helper_set_mode
nv04_dfp_prepare
nv04_lvds_dpms(DRM_MODE_DPMS_OFF)
nv04_lvds_dpms checks last_dpms mode (which is NV_DPMS_CLEARED) and wrongly
assumes it's a powersaving mode, the new one (DRM_MODE_DPMS_OFF) is too,
so it skips calling some crucial lvds scripts.
Reported-by: Chris Paulson-Ellis ch...@edesix.com
Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/nouveau/nv04_dfp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nv04_dfp.c
b/drivers/gpu/drm/nouveau/nv04_dfp.c
index 184cdf8..39ffc07 100644
--- a/drivers/gpu/drm/nouveau/nv04_dfp.c
+++ b/drivers/gpu/drm/nouveau/nv04_dfp.c
@@ -505,7 +505,7 @@ static void nv04_dfp_update_backlight(struct drm_encoder
*encoder, int mode)
static inline bool is_powersaving_dpms(int mode)
{
- return (mode != DRM_MODE_DPMS_ON);
+ return mode != DRM_MODE_DPMS_ON mode != NV_DPMS_CLEARED;
}
static void nv04_lvds_dpms(struct drm_encoder *encoder, int mode)
--
1.8.0.2
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH] drm/nouveau: improve reporting of fifo errors
Note: bar faults and semaphore errors were previously silently acked.
Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
---
drivers/gpu/drm/nouveau/core/engine/fifo/nv04.c | 142 ++--
drivers/gpu/drm/nouveau/core/engine/fifo/nv04.h | 5 +-
2 files changed, 139 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/core/engine/fifo/nv04.c
b/drivers/gpu/drm/nouveau/core/engine/fifo/nv04.c
index e34ab40..9c73bc1 100644
--- a/drivers/gpu/drm/nouveau/core/engine/fifo/nv04.c
+++ b/drivers/gpu/drm/nouveau/core/engine/fifo/nv04.c
@@ -25,6 +25,7 @@
#include core/os.h
#include core/class.h
#include core/engctx.h
+#include core/enum.h
#include core/namedb.h
#include core/handle.h
#include core/ramht.h
@@ -398,6 +399,73 @@ out:
return handled;
}
+static struct nouveau_bitfield
+nv04_cache1_pull0_bits[] = {
+ {0x0001, ACCESS},
+ {0x0010, HASH_FAILED},
+ {0x0100, DEVICE_SOFTWARE},
+ {0x1000, HASH_BUSY},
+ {0x0001, ACQUIRE_BUSY},
+ {}
+};
+
+static struct nouveau_bitfield
+nv50_cache1_pull0_bits[] = {
+ {0x0001, ACCESS},
+ {0x0010, HASH_FAILED},
+ {0x0100, DEVICE_SOFTWARE},
+ {0x1000, HASH_BUSY},
+ {0x0001, ACQUIRE_BUSY},
+ {0x0100, INVALID_ENGINE},
+ {}
+};
+
+static const char * const
+nv11_sem_error_codes[] = {
+ NONE, INVALID_OPERAND, INVALID_STATE
+};
+
+static const char * const
+nv50_sem_error_codes[] = {
+ NONE, ADDRESS_UNALIGNED, INVALID_STATE,
+ ADDRESS_TOO_LARGE, MEM_FAULT
+};
+
+static void
+nv_decode_cache1_pull0(struct nouveau_device *device, u32 reg)
+{
+ const char * const *sem_error_codes;
+ int error_codes_size;
+ struct nouveau_bitfield *pull0_bits;
+ u8 sem_error;
+
+ pr_cont( cache1_pull0 0x%08x, reg);
+
+ sem_error = (reg 0x00F0) 20;
+ reg = ~0x00F0;
+
+ if (device-card_type == NV_50) {
+ pull0_bits = nv50_cache1_pull0_bits;
+ sem_error_codes = nv50_sem_error_codes;
+ error_codes_size = ARRAY_SIZE(nv50_sem_error_codes);
+ } else {
+ pull0_bits = nv04_cache1_pull0_bits;
+ sem_error_codes = nv11_sem_error_codes;
+ error_codes_size = ARRAY_SIZE(nv11_sem_error_codes);
+ }
+
+ nouveau_bitfield_print(pull0_bits, reg);
+
+ if (sem_error) {
+ pr_cont( SEMAPHORE_ERROR=);
+
+ if (sem_error error_codes_size)
+ pr_cont(%s, sem_error_codes[sem_error]);
+ else
+ pr_cont(UNK%d, sem_error);
+ }
+}
+
static void
nv04_fifo_cache_error(struct nouveau_device *device,
struct nv04_fifo_priv *priv, u32 chid, u32 get)
@@ -423,10 +491,16 @@ nv04_fifo_cache_error(struct nouveau_device *device,
if (!nv04_fifo_swmthd(priv, chid, mthd, data)) {
const char *client_name =
nouveau_client_name_for_fifo_chid(priv-base, chid);
+ u32 c1p0 = nv_rd32(priv, NV04_PFIFO_CACHE1_PULL0);
nv_error(priv,
-CACHE_ERROR - ch %d [%s] subc %d mthd 0x%04x data
0x%08x\n,
+CACHE_ERROR - ch %d [%s] subc %d mthd 0x%04x data
0x%08x,
chid, client_name, (mthd 13) 7, mthd 0x1ffc,
data);
+ nv_decode_cache1_pull0(device, c1p0);
+ if (c1p0 0x0010) /* HASH_FAILED */
+ pr_cont( cache1_hash 0x%08x,
+ nv_rd32(priv, NV04_PFIFO_CACHE1_HASH));
+ pr_cont(\n);
}
nv_wr32(priv, NV04_PFIFO_CACHE1_DMA_PUSH, 0);
@@ -496,6 +570,7 @@ nv04_fifo_intr(struct nouveau_subdev *subdev)
struct nouveau_device *device = nv_device(subdev);
struct nv04_fifo_priv *priv = (void *)subdev;
uint32_t status, reassign;
+ const char *client_name = NULL;
int cnt = 0;
reassign = nv_rd32(priv, NV03_PFIFO_CACHES) 1;
@@ -517,9 +592,19 @@ nv04_fifo_intr(struct nouveau_subdev *subdev)
status = ~NV_PFIFO_INTR_DMA_PUSHER;
}
+ if (status)
+ client_name = nouveau_client_name_for_fifo_chid(
+ priv-base, chid);
+
if (status NV_PFIFO_INTR_SEMAPHORE) {
uint32_t sem;
+ nv_error(priv, SEM_ERROR - ch %d [%s], chid,
+client_name);
+ nv_decode_cache1_pull0(device,
+ nv_rd32(priv, NV04_PFIFO_CACHE1_PULL0));
+ pr_cont(\n);
+
status = ~NV_PFIFO_INTR_SEMAPHORE;
nv_wr32(priv, NV03_PFIFO_INTR_0,
NV_PFIFO_INTR_SEMAPHORE);
@@ -532,15 +617,56 @@
[Nouveau] reproducible CACHE_ERRORS
Hi I found a way to reliably reproduce PFIFO CACHE_ERRORS, but I don't see why. How to reproduce them: 1) Run a couple of glxinfo loops - 3 is usually enough. while [ true ]; do nvgl glxinfo /dev/null 2/dev/null; done 2) Run glxgears. 3) If you can't see it now, resize glxgears window or add more glxinfos. Note: you need at least 2 CPUs. Usually the error looks like this (with a bit improved logging): nouveau E[ PFIFO][:02:00.0] CACHE_ERROR - ch 6 [glxgears[15559]] subc 0 mthd 0x0060 data 0x800f c1p0 0x2010 HASH_FAILED (unknown bits 0x2000) c1_hash 0x0436 What I found so far: 1) It's triggered by setting of NV11_SUBCHAN_DMA_SEMAPHORE to NvSema (0x800f) in nv84_fence_emit. Hw tells us it cannot find ramht entry for NvSema object (NV04_PFIFO_CACHE1_PULL0 == HASH_FAILED, frequently unknown 30th bit is set) 2) In 95% cases CACHE_ERRORs are triggered on glxgears channel. 3) RAMHT entry was definitely created and used many times before reporting an error. Next use of NvSema usually does NOT trigger another CACHE_ERROR. 4) NV04_PFIFO_CACHE1_HASH has the same value as was written to ramht. 5) I can replace glxinfo loops with application which creates and destroys PGRAPH objects. PCRYPT, PMPEG and SW objects do NOT provoke this bug. (program attached) 6) There are no interrupts between CACHE_ERRORs, so it's not caused by race in cache_error/software method handling. Any ideas how to debug it? Marcin test_objects.tar.gz Description: Binary data ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH] libdrm_nouveau.pc: don't include I${includedir}/nouveau
Nouveau related headers are installed in I${includedir}/libdrm now.
---
nouveau/libdrm_nouveau.pc.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/nouveau/libdrm_nouveau.pc.in b/nouveau/libdrm_nouveau.pc.in
index 6170613..454fc87 100644
--- a/nouveau/libdrm_nouveau.pc.in
+++ b/nouveau/libdrm_nouveau.pc.in
@@ -7,5 +7,5 @@ Name: libdrm_nouveau
Description: Userspace interface to nouveau kernel DRM services
Version: 2.4.33
Libs: -L${libdir} -ldrm_nouveau
-Cflags: -I${includedir} -I${includedir}/libdrm -I${includedir}/nouveau
+Cflags: -I${includedir} -I${includedir}/libdrm
Requires.private: libdrm
--
1.8.0.2
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH] drm/nouveau: fix ramht wraparound
On Thu, Dec 20, 2012 at 11:37:12PM +0100, Marcin Slusarz wrote: When hash collision occurs and it's near ramht object boundary, we could read and possibly overwrite some memory after ramht object. Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com Cc: sta...@vger.kernel.org --- drivers/gpu/drm/nouveau/core/core/ramht.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/core/core/ramht.c b/drivers/gpu/drm/nouveau/core/core/ramht.c index 86a6404..6da314c 100644 --- a/drivers/gpu/drm/nouveau/core/core/ramht.c +++ b/drivers/gpu/drm/nouveau/core/core/ramht.c @@ -59,7 +59,7 @@ nouveau_ramht_insert(struct nouveau_ramht *ramht, int chid, } co += 8; - if (co = nv_gpuobj(ramht)-size) + if (co + 8 nv_gpuobj(ramht)-size) I might just be really tired, but, how exactly is the original wrong? The original could even just be (co == size) and still work correctly as far as I can tell. Ben. co = 0; } while (co != ho); -- 1.8.0.2 ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
