On Wed, 12 Sep 2012, Ashley Lai wrote:

> This patch removed the tasklet and moved the wait queue into the
> private structure.  It also cleaned up the response CRQ path.
> 
> Signed-off-by: Ashley Lai <ad...@us.ibm.com>


Kent: any comment on this?  You should probably push this to me via your 
tree.


> ---
> James,
> 
> This patch is based on your "next" branch.
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git
> 
> Thanks,
> Ashley Lai
> ---
>  drivers/char/tpm/tpm_ibmvtpm.c | 81 
> +++++++++++++++---------------------------
>  drivers/char/tpm/tpm_ibmvtpm.h |  5 ++-
>  2 files changed, 30 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> index efc4ab3..88a95ea 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.c
> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> @@ -38,8 +38,6 @@ static struct vio_device_id tpm_ibmvtpm_device_table[] 
> __devinitdata = {
>  };
>  MODULE_DEVICE_TABLE(vio, tpm_ibmvtpm_device_table);
>  
> -DECLARE_WAIT_QUEUE_HEAD(wq);
> -
>  /**
>   * ibmvtpm_send_crq - Send a CRQ request
>   * @vdev:    vio device struct
> @@ -83,6 +81,7 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, 
> size_t count)
>  {
>       struct ibmvtpm_dev *ibmvtpm;
>       u16 len;
> +     int sig;
>  
>       ibmvtpm = (struct ibmvtpm_dev *)chip->vendor.data;
>  
> @@ -91,22 +90,23 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 
> *buf, size_t count)
>               return 0;
>       }
>  
> -     wait_event_interruptible(wq, ibmvtpm->crq_res.len != 0);
> +     sig = wait_event_interruptible(ibmvtpm->wq, ibmvtpm->res_len != 0);
> +     if (sig)
> +             return -EINTR;
> +
> +     len = ibmvtpm->res_len;
>  
> -     if (count < ibmvtpm->crq_res.len) {
> +     if (count < len) {
>               dev_err(ibmvtpm->dev,
>                       "Invalid size in recv: count=%ld, crq_size=%d\n",
> -                     count, ibmvtpm->crq_res.len);
> +                     count, len);
>               return -EIO;
>       }
>  
>       spin_lock(&ibmvtpm->rtce_lock);
> -     memcpy((void *)buf, (void *)ibmvtpm->rtce_buf, ibmvtpm->crq_res.len);
> -     memset(ibmvtpm->rtce_buf, 0, ibmvtpm->crq_res.len);
> -     ibmvtpm->crq_res.valid = 0;
> -     ibmvtpm->crq_res.msg = 0;
> -     len = ibmvtpm->crq_res.len;
> -     ibmvtpm->crq_res.len = 0;
> +     memcpy((void *)buf, (void *)ibmvtpm->rtce_buf, len);
> +     memset(ibmvtpm->rtce_buf, 0, len);
> +     ibmvtpm->res_len = 0;
>       spin_unlock(&ibmvtpm->rtce_lock);
>       return len;
>  }
> @@ -273,7 +273,6 @@ static int __devexit tpm_ibmvtpm_remove(struct vio_dev 
> *vdev)
>       int rc = 0;
>  
>       free_irq(vdev->irq, ibmvtpm);
> -     tasklet_kill(&ibmvtpm->tasklet);
>  
>       do {
>               if (rc)
> @@ -372,7 +371,6 @@ static int ibmvtpm_reset_crq(struct ibmvtpm_dev *ibmvtpm)
>  static int tpm_ibmvtpm_resume(struct device *dev)
>  {
>       struct ibmvtpm_dev *ibmvtpm = ibmvtpm_get_data(dev);
> -     unsigned long flags;
>       int rc = 0;
>  
>       do {
> @@ -387,10 +385,11 @@ static int tpm_ibmvtpm_resume(struct device *dev)
>               return rc;
>       }
>  
> -     spin_lock_irqsave(&ibmvtpm->lock, flags);
> -     vio_disable_interrupts(ibmvtpm->vdev);
> -     tasklet_schedule(&ibmvtpm->tasklet);
> -     spin_unlock_irqrestore(&ibmvtpm->lock, flags);
> +     rc = vio_enable_interrupts(ibmvtpm->vdev);
> +     if (rc) {
> +             dev_err(dev, "Error vio_enable_interrupts rc=%d\n", rc);
> +             return rc;
> +     }
>  
>       rc = ibmvtpm_crq_send_init(ibmvtpm);
>       if (rc)
> @@ -467,7 +466,7 @@ static struct ibmvtpm_crq *ibmvtpm_crq_get_next(struct 
> ibmvtpm_dev *ibmvtpm)
>       if (crq->valid & VTPM_MSG_RES) {
>               if (++crq_q->index == crq_q->num_entry)
>                       crq_q->index = 0;
> -             rmb();
> +             smp_rmb();
>       } else
>               crq = NULL;
>       return crq;
> @@ -535,11 +534,9 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq,
>                       ibmvtpm->vtpm_version = crq->data;
>                       return;
>               case VTPM_TPM_COMMAND_RES:
> -                     ibmvtpm->crq_res.valid = crq->valid;
> -                     ibmvtpm->crq_res.msg = crq->msg;
> -                     ibmvtpm->crq_res.len = crq->len;
> -                     ibmvtpm->crq_res.data = crq->data;
> -                     wake_up_interruptible(&wq);
> +                     /* len of the data in rtce buffer */
> +                     ibmvtpm->res_len = crq->len;
> +                     wake_up_interruptible(&ibmvtpm->wq);
>                       return;
>               default:
>                       return;
> @@ -559,38 +556,19 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq,
>  static irqreturn_t ibmvtpm_interrupt(int irq, void *vtpm_instance)
>  {
>       struct ibmvtpm_dev *ibmvtpm = (struct ibmvtpm_dev *) vtpm_instance;
> -     unsigned long flags;
> -
> -     spin_lock_irqsave(&ibmvtpm->lock, flags);
> -     vio_disable_interrupts(ibmvtpm->vdev);
> -     tasklet_schedule(&ibmvtpm->tasklet);
> -     spin_unlock_irqrestore(&ibmvtpm->lock, flags);
> -
> -     return IRQ_HANDLED;
> -}
> -
> -/**
> - * ibmvtpm_tasklet - Interrupt handler tasklet
> - * @data:    ibm vtpm device struct
> - *
> - * Returns:
> - *   Nothing
> - **/
> -static void ibmvtpm_tasklet(void *data)
> -{
> -     struct ibmvtpm_dev *ibmvtpm = data;
>       struct ibmvtpm_crq *crq;
> -     unsigned long flags;
>  
> -     spin_lock_irqsave(&ibmvtpm->lock, flags);
> +     /* while loop is needed for initial setup (get version and
> +      * get rtce_size). There should be only one tpm request at any
> +      * given time.
> +      */
>       while ((crq = ibmvtpm_crq_get_next(ibmvtpm)) != NULL) {
>               ibmvtpm_crq_process(crq, ibmvtpm);
>               crq->valid = 0;
> -             wmb();
> +             smp_wmb();
>       }
>  
> -     vio_enable_interrupts(ibmvtpm->vdev);
> -     spin_unlock_irqrestore(&ibmvtpm->lock, flags);
> +     return IRQ_HANDLED;
>  }
>  
>  /**
> @@ -650,9 +628,6 @@ static int __devinit tpm_ibmvtpm_probe(struct vio_dev 
> *vio_dev,
>               goto reg_crq_cleanup;
>       }
>  
> -     tasklet_init(&ibmvtpm->tasklet, (void *)ibmvtpm_tasklet,
> -                  (unsigned long)ibmvtpm);
> -
>       rc = request_irq(vio_dev->irq, ibmvtpm_interrupt, 0,
>                        tpm_ibmvtpm_driver_name, ibmvtpm);
>       if (rc) {
> @@ -666,13 +641,14 @@ static int __devinit tpm_ibmvtpm_probe(struct vio_dev 
> *vio_dev,
>               goto init_irq_cleanup;
>       }
>  
> +     init_waitqueue_head(&ibmvtpm->wq);
> +
>       crq_q->index = 0;
>  
>       ibmvtpm->dev = dev;
>       ibmvtpm->vdev = vio_dev;
>       chip->vendor.data = (void *)ibmvtpm;
>  
> -     spin_lock_init(&ibmvtpm->lock);
>       spin_lock_init(&ibmvtpm->rtce_lock);
>  
>       rc = ibmvtpm_crq_send_init(ibmvtpm);
> @@ -689,7 +665,6 @@ static int __devinit tpm_ibmvtpm_probe(struct vio_dev 
> *vio_dev,
>  
>       return rc;
>  init_irq_cleanup:
> -     tasklet_kill(&ibmvtpm->tasklet);
>       do {
>               rc1 = plpar_hcall_norets(H_FREE_CRQ, vio_dev->unit_address);
>       } while (rc1 == H_BUSY || H_IS_LONG_BUSY(rc1));
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
> index 4296eb4..bd82a79 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.h
> +++ b/drivers/char/tpm/tpm_ibmvtpm.h
> @@ -38,13 +38,12 @@ struct ibmvtpm_dev {
>       struct vio_dev *vdev;
>       struct ibmvtpm_crq_queue crq_queue;
>       dma_addr_t crq_dma_handle;
> -     spinlock_t lock;
> -     struct tasklet_struct tasklet;
>       u32 rtce_size;
>       void __iomem *rtce_buf;
>       dma_addr_t rtce_dma_handle;
>       spinlock_t rtce_lock;
> -     struct ibmvtpm_crq crq_res;
> +     wait_queue_head_t wq;
> +     u16 res_len;
>       u32 vtpm_version;
>  };
>  
> -- 
> 1.7.11.2
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
James Morris
<jmor...@namei.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to