[PATCH] spufs: fix mailbox polling

Handling mailbox interrupts was broken in multiple respects,
the combination of which was hiding the bugs most of the time.

- The ibox interrupt mask was open initially even though there
  are no waiters on a newly created SPU.

- Acknowledging the mailbox interrupt did not work because
  it is level triggered and the mailbox data is never retrieved
  from inside the interrupt handler.

- The interrupt handler delivered interrupts with a disabled
  mask if another interrupt is triggered for the same class
  but a different mask.

- The poll function did not enable the interrupt if it had not
  been enabled, so we might run into the poll timeout if none of
  the other bugs saved us and no signal was delivered.

We probably still have a similar problem with blocking
read/write on mailbox files, but that will result in extra
wakeup in the worst case, not in incorrect behaviour.

Signed-off-by: Arnd Bergmann <arndb@de.ibm.com>
Signed-off-by: Paul Mackerras <paulus@samba.org>

diff --git a/arch/powerpc/platforms/cell/spu_base.c b/arch/powerpc/platforms/cell/spu_base.c
index 8abd4bd196654da503d9ccd79e0531858064b303..f9da79eb3db03608492da050104f426d166b8cbe 100644 (file)
--- a/arch/powerpc/platforms/cell/spu_base.c
+++ b/arch/powerpc/platforms/cell/spu_base.c
@@ -202,12 +202,15 @@ spu_irq_class_0(int irq, void *data, struct pt_regs *regs)
 int
 spu_irq_class_0_bottom(struct spu *spu)
 {
-       unsigned long stat;
+       unsigned long stat, mask;
 
        spu->class_0_pending = 0;
 
+       mask = in_be64(&spu->priv1->int_mask_class0_RW);
        stat = in_be64(&spu->priv1->int_stat_class0_RW);
 
+       stat &= mask;
+
        if (stat & 1) /* invalid MFC DMA */
                __spu_trap_invalid_dma(spu);
 
@@ -263,13 +266,15 @@ spu_irq_class_2(int irq, void *data, struct pt_regs *regs)
 {
        struct spu *spu;
        unsigned long stat;
+       unsigned long mask;
 
        spu = data;
        stat = in_be64(&spu->priv1->int_stat_class2_RW);
+       mask = in_be64(&spu->priv1->int_mask_class2_RW);
 
-       pr_debug("class 2 interrupt %d, %lx, %lx\n", irq, stat,
-               in_be64(&spu->priv1->int_mask_class2_RW));
+       pr_debug("class 2 interrupt %d, %lx, %lx\n", irq, stat, mask);
 
+       stat &= mask;
 
        if (stat & 1)  /* PPC core mailbox */
                __spu_trap_mailbox(spu);

diff --git a/arch/powerpc/platforms/cell/spufs/backing_ops.c b/arch/powerpc/platforms/cell/spufs/backing_ops.c
index 66567c1099653344a8712b3c8d4fd3ef308f60f6..a5c489a53c61cdd679040d1d40ff304fe91bf9e7 100644 (file)
--- a/arch/powerpc/platforms/cell/spufs/backing_ops.c
+++ b/arch/powerpc/platforms/cell/spufs/backing_ops.c
@@ -32,6 +32,7 @@
 #include <linux/smp_lock.h>
 #include <linux/stddef.h>
 #include <linux/unistd.h>
+#include <linux/poll.h>
 
 #include <asm/io.h>
 #include <asm/spu.h>
@@ -87,6 +88,41 @@ static u32 spu_backing_mbox_stat_read(struct spu_context *ctx)
        return ctx->csa.prob.mb_stat_R;
 }
 
+static unsigned int spu_backing_mbox_stat_poll(struct spu_context *ctx,
+                                         unsigned int events)
+{
+       int ret;
+       u32 stat;
+
+       ret = 0;
+       spin_lock_irq(&ctx->csa.register_lock);
+       stat = ctx->csa.prob.mb_stat_R;
+
+       /* if the requested event is there, return the poll
+          mask, otherwise enable the interrupt to get notified,
+          but first mark any pending interrupts as done so
+          we don't get woken up unnecessarily */
+
+       if (events & (POLLIN | POLLRDNORM)) {
+               if (stat & 0xff0000)
+                       ret |= POLLIN | POLLRDNORM;
+               else {
+                       ctx->csa.priv1.int_stat_class0_RW &= ~0x1;
+                       ctx->csa.priv1.int_mask_class2_RW |= 0x1;
+               }
+       }
+       if (events & (POLLOUT | POLLWRNORM)) {
+               if (stat & 0x00ff00)
+                       ret = POLLOUT | POLLWRNORM;
+               else {
+                       ctx->csa.priv1.int_stat_class0_RW &= ~0x10;
+                       ctx->csa.priv1.int_mask_class2_RW |= 0x10;
+               }
+       }
+       spin_unlock_irq(&ctx->csa.register_lock);
+       return ret;
+}
+
 static int spu_backing_ibox_read(struct spu_context *ctx, u32 * data)
 {
        int ret;
@@ -252,6 +288,7 @@ static void spu_backing_runcntl_stop(struct spu_context *ctx)
 struct spu_context_ops spu_backing_ops = {
        .mbox_read = spu_backing_mbox_read,
        .mbox_stat_read = spu_backing_mbox_stat_read,
+       .mbox_stat_poll = spu_backing_mbox_stat_poll,
        .ibox_read = spu_backing_ibox_read,
        .wbox_write = spu_backing_wbox_write,
        .signal1_read = spu_backing_signal1_read,

diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
index af5adc37222416db953e01607aff9e24857aece9..9738de727f3222dfca189de58632a4c11e172a3b 100644 (file)
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -389,20 +389,13 @@ static ssize_t spufs_ibox_read(struct file *file, char __user *buf,
 static unsigned int spufs_ibox_poll(struct file *file, poll_table *wait)
 {
        struct spu_context *ctx = file->private_data;
-       u32 mbox_stat;
        unsigned int mask;
 
-       spu_acquire(ctx);
-
-       mbox_stat = ctx->ops->mbox_stat_read(ctx);
-
-       spu_release(ctx);
-
        poll_wait(file, &ctx->ibox_wq, wait);
 
-       mask = 0;
-       if (mbox_stat & 0xff0000)
-               mask |= POLLIN | POLLRDNORM;
+       spu_acquire(ctx);
+       mask = ctx->ops->mbox_stat_poll(ctx, POLLIN | POLLRDNORM);
+       spu_release(ctx);
 
        return mask;
 }
@@ -494,18 +487,13 @@ static ssize_t spufs_wbox_write(struct file *file, const char __user *buf,
 static unsigned int spufs_wbox_poll(struct file *file, poll_table *wait)
 {
        struct spu_context *ctx = file->private_data;
-       u32 mbox_stat;
        unsigned int mask;
 
-       spu_acquire(ctx);
-       mbox_stat = ctx->ops->mbox_stat_read(ctx);
-       spu_release(ctx);
-
        poll_wait(file, &ctx->wbox_wq, wait);
 
-       mask = 0;
-       if (mbox_stat & 0x00ff00)
-               mask = POLLOUT | POLLWRNORM;
+       spu_acquire(ctx);
+       mask = ctx->ops->mbox_stat_poll(ctx, POLLOUT | POLLWRNORM);
+       spu_release(ctx);
 
        return mask;
 }

diff --git a/arch/powerpc/platforms/cell/spufs/hw_ops.c b/arch/powerpc/platforms/cell/spufs/hw_ops.c
index 68812415ee2978888d42446d723377b1e5caa08f..9a53e29f9d7ec9b5a8712835e60021b4c850c5c8 100644 (file)
--- a/arch/powerpc/platforms/cell/spufs/hw_ops.c
+++ b/arch/powerpc/platforms/cell/spufs/hw_ops.c
@@ -24,7 +24,7 @@
 #include <linux/sched.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
-#include <linux/vmalloc.h>
+#include <linux/poll.h>
 #include <linux/smp.h>
 #include <linux/smp_lock.h>
 #include <linux/stddef.h>
@@ -58,6 +58,44 @@ static u32 spu_hw_mbox_stat_read(struct spu_context *ctx)
        return in_be32(&ctx->spu->problem->mb_stat_R);
 }
 
+static unsigned int spu_hw_mbox_stat_poll(struct spu_context *ctx,
+                                         unsigned int events)
+{
+       struct spu *spu = ctx->spu;
+       struct spu_priv1 __iomem *priv1 = spu->priv1;
+       int ret = 0;
+       u32 stat;
+
+       spin_lock_irq(&spu->register_lock);
+       stat = in_be32(&spu->problem->mb_stat_R);
+
+       /* if the requested event is there, return the poll
+          mask, otherwise enable the interrupt to get notified,
+          but first mark any pending interrupts as done so
+          we don't get woken up unnecessarily */
+
+       if (events & (POLLIN | POLLRDNORM)) {
+               if (stat & 0xff0000)
+                       ret |= POLLIN | POLLRDNORM;
+               else {
+                       out_be64(&priv1->int_stat_class2_RW, 0x1);
+                       out_be64(&priv1->int_mask_class2_RW,
+                                in_be64(&priv1->int_mask_class2_RW) | 0x1);
+               }
+       }
+       if (events & (POLLOUT | POLLWRNORM)) {
+               if (stat & 0x00ff00)
+                       ret = POLLOUT | POLLWRNORM;
+               else {
+                       out_be64(&priv1->int_stat_class2_RW, 0x10);
+                       out_be64(&priv1->int_mask_class2_RW,
+                                in_be64(&priv1->int_mask_class2_RW) | 0x10);
+               }
+       }
+       spin_unlock_irq(&spu->register_lock);
+       return ret;
+}
+
 static int spu_hw_ibox_read(struct spu_context *ctx, u32 * data)
 {
        struct spu *spu = ctx->spu;
@@ -204,6 +242,7 @@ static void spu_hw_runcntl_stop(struct spu_context *ctx)
 struct spu_context_ops spu_hw_ops = {
        .mbox_read = spu_hw_mbox_read,
        .mbox_stat_read = spu_hw_mbox_stat_read,
+       .mbox_stat_poll = spu_hw_mbox_stat_poll,
        .ibox_read = spu_hw_ibox_read,
        .wbox_write = spu_hw_wbox_write,
        .signal1_read = spu_hw_signal1_read,

diff --git a/arch/powerpc/platforms/cell/spufs/spufs.h b/arch/powerpc/platforms/cell/spufs/spufs.h
index 5bb75f22f7221c8b1a143cced758060023c030be..17cae5e5fdf51991d8ace480e59152260d3404ee 100644 (file)
--- a/arch/powerpc/platforms/cell/spufs/spufs.h
+++ b/arch/powerpc/platforms/cell/spufs/spufs.h
@@ -66,6 +66,8 @@ struct spu_context {
 struct spu_context_ops {
        int (*mbox_read) (struct spu_context * ctx, u32 * data);
         u32(*mbox_stat_read) (struct spu_context * ctx);
+       unsigned int (*mbox_stat_poll)(struct spu_context *ctx,
+                                       unsigned int events);
        int (*ibox_read) (struct spu_context * ctx, u32 * data);
        int (*wbox_write) (struct spu_context * ctx, u32 data);
         u32(*signal1_read) (struct spu_context * ctx);

diff --git a/arch/powerpc/platforms/cell/spufs/switch.c b/arch/powerpc/platforms/cell/spufs/switch.c
index 51266257b0a543b10a3d4570834f8f5368f119d5..010a9fe55ef85ab878380432ec6ed1054b73a167 100644 (file)
--- a/arch/powerpc/platforms/cell/spufs/switch.c
+++ b/arch/powerpc/platforms/cell/spufs/switch.c
@@ -2155,8 +2155,8 @@ static void init_priv1(struct spu_state *csa)
            CLASS0_ENABLE_SPU_ERROR_INTR;
        csa->priv1.int_mask_class1_RW = CLASS1_ENABLE_SEGMENT_FAULT_INTR |
            CLASS1_ENABLE_STORAGE_FAULT_INTR;
-       csa->priv1.int_mask_class2_RW = CLASS2_ENABLE_MAILBOX_INTR |
-           CLASS2_ENABLE_SPU_STOP_INTR | CLASS2_ENABLE_SPU_HALT_INTR;
+       csa->priv1.int_mask_class2_RW = CLASS2_ENABLE_SPU_STOP_INTR |
+           CLASS2_ENABLE_SPU_HALT_INTR;
 }
 
 static void init_priv2(struct spu_state *csa)