From: James Morris <jmorris@namei.org>
To: Ashley Lai <adlai@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
tpmdd-devel@lists.sourceforge.net, benh@kernel.crashing.org,
linuxppc-dev@lists.ozlabs.org, rcj@linux.vnet.ibm.com,
key@linux.vnet.ibm.com, adlai@us.ibm.com
Subject: Re: [PATCH 1/1] drivers/char/tpm: remove tasklet and cleanup
Date: Mon, 24 Sep 2012 12:26:05 +1000 (EST) [thread overview]
Message-ID: <alpine.LRH.2.02.1209241225230.678@tundra.namei.org> (raw)
In-Reply-To: <1347472190.27058.5.camel@footlong>
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 <adlai@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 majordomo@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
<jmorris@namei.org>
next prev parent reply other threads:[~2012-09-24 2:37 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-14 23:23 [PATCH V3 0/3] tpm: Add new vTPM device driver for PPC64 Ashley Lai
2012-08-14 23:34 ` [PATCH V3 1/3] drivers/char/tpm: Add new device driver to support IBM vTPM Ashley Lai
2012-08-22 21:17 ` Ashley Lai
2012-08-22 21:42 ` Kent Yoder
2012-09-05 3:40 ` Benjamin Herrenschmidt
2012-09-05 15:46 ` Kent Yoder
2012-09-05 21:11 ` Benjamin Herrenschmidt
2012-09-07 17:38 ` Kent Yoder
2012-09-12 3:16 ` James Morris
2012-09-05 17:25 ` Ashley Lai
2012-09-12 17:49 ` [PATCH 1/1] drivers/char/tpm: remove tasklet and cleanup Ashley Lai
2012-09-24 2:26 ` James Morris [this message]
2012-09-24 14:10 ` key
2012-09-24 15:48 ` key
2012-09-26 13:55 ` key
2012-09-27 14:46 ` Ashley Lai
2012-08-14 23:34 ` [PATCH V3 2/3] PPC64: Add support for instantiating SML from Open Firmware Ashley Lai
2012-08-14 23:35 ` [PATCH V3 3/3] drivers/char/tpm: Add securityfs support for event log Ashley Lai
2012-08-15 19:59 ` [PATCH V3 0/3] tpm: Add new vTPM device driver for PPC64 Kent Yoder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LRH.2.02.1209241225230.678@tundra.namei.org \
--to=jmorris@namei.org \
--cc=adlai@linux.vnet.ibm.com \
--cc=adlai@us.ibm.com \
--cc=benh@kernel.crashing.org \
--cc=key@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=rcj@linux.vnet.ibm.com \
--cc=tpmdd-devel@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).