[PATCH v2] vTPM: Fix missing NULL check
The current code passes the address of tpm_chip as the argument to dev_get_drvdata() without prior NULL check in tpm_ibmvtpm_get_desired_dma. This resulted an oops during kernel boot when vTPM is enabled in Power partition configured in active memory sharing mode. The vio_driver's get_desired_dma() is called before the probe(), which for vtpm is tpm_ibmvtpm_probe, and it's this latter function that initializes the driver and set data. Attempting to get data before the probe() caused the problem. This patch adds a NULL check to the tpm_ibmvtpm_get_desired_dma. fixes: 9e0d39d8a6a0 ("tpm: Remove useless priv field in struct tpm_vendor_specific") Cc: Signed-off-by: Hon Ching(Vicky) Lo --- drivers/char/tpm/tpm_ibmvtpm.c |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 1b9d61f..f01d083 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -299,6 +299,8 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev) } kfree(ibmvtpm); + /* For tpm_ibmvtpm_get_desired_dma */ + dev_set_drvdata(&vdev->dev, NULL); return 0; } @@ -313,14 +315,16 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev) static unsigned long tpm_ibmvtpm_get_desired_dma(struct vio_dev *vdev) { struct tpm_chip *chip = dev_get_drvdata(&vdev->dev); - struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev); + struct ibmvtpm_dev *ibmvtpm; /* * ibmvtpm initializes at probe time, so the data we are * asking for may not be set yet. Estimate that 4K required * for TCE-mapped buffer in addition to CRQ. */ - if (!ibmvtpm) + if (chip) + ibmvtpm = dev_get_drvdata(&chip->dev); + else return CRQ_RES_BUF_SIZE + PAGE_SIZE; return CRQ_RES_BUF_SIZE + ibmvtpm->rtce_size; -- 1.7.1
Re: [tpmdd-devel] [PATCH] vTPM: Fix missing NULL check
On Wed, 2017-03-08 at 13:52 -0700, Jason Gunthorpe wrote: > On Wed, Mar 08, 2017 at 03:28:11PM -0500, Hon Ching(Vicky) Lo wrote: > > On Wed, 2017-03-08 at 10:17 -0700, Jason Gunthorpe wrote: > > > On Tue, Mar 07, 2017 at 11:12:43PM -0500, Hon Ching(Vicky) Lo wrote: > > > > On Mon, 2017-03-06 at 16:19 -0700, Jason Gunthorpe wrote: > > > > > > > > Also, how does locking work here? Does the vio core prevent > > > > > tpm_ibmvtpm_get_desired_dma and tpm_ibmvtpm_remove from running > > > > > concurrently? > > > > > > > > No, vio core doesn't prevent tpm_ibmvtpm_get_desired_dma and > > > > tpm_ibmvtpm_remove > > > > from running concurrently. > > > > > > > > vio_bus_probe calls vio_cmo_bus_probe which calls > > > > tpm_ibmvtpm_get_desired_dma. > > > > tpm_ibmvtpm_get_desired_dma is called before the code enters critical > > > > section. > > > > > > > > There is no locking mechanism around tpm_ibmvtpm_remove in > > > > vio_bus_remove. > > > > > > > > What's the concern here? > > > > > > tpm_ibmvtpm_remove makes the pointer that tpm_ibmvtpm_get_desired_dma > > > is accessing invalid, so some kind of locking is technically required > > > so that the two things do not create a use after free race: > > > > I don't think we need to worry about locking in this specific case. > > tpm_ibmvtpm_get_desired_dma was designed to return a default value > > in the case when the chip is not available. > > You have to worry about it to prevent a use after free race: > > CPU0CPU1 > tpm_ibmvtpm_remove() tpm_ibmvtpm_get_desired_dma() > >chip = dev_get_drvdata(dev); > dev_set_drvdata(&vdev->dev, NULL); > if (chip) > ibmvtpm = dev_get_drvdata(&chip->dev); > kfree(ibmvtpm); > // *ibmvtpm is now a use-after-free > > Jason > I have dug further up along the call stack of tpm_ibmvtpm_get_desired_dma() and found that there is a locking mechanism in place at the bus probe level. 'probe' and 'remove' callbacks are both surrounded by mutex_lock and mutex_unlock on the device. The code is in the really_probe() and device_release_driver_internal() accordingly. Thanks for pointing this out! Vicky
Re: [tpmdd-devel] [PATCH] vTPM: Fix missing NULL check
On Wed, 2017-03-08 at 10:17 -0700, Jason Gunthorpe wrote: > On Tue, Mar 07, 2017 at 11:12:43PM -0500, Hon Ching(Vicky) Lo wrote: > > On Mon, 2017-03-06 at 16:19 -0700, Jason Gunthorpe wrote: > > > > Also, how does locking work here? Does the vio core prevent > > > tpm_ibmvtpm_get_desired_dma and tpm_ibmvtpm_remove from running > > > concurrently? > > > > No, vio core doesn't prevent tpm_ibmvtpm_get_desired_dma and > > tpm_ibmvtpm_remove > > from running concurrently. > > > > vio_bus_probe calls vio_cmo_bus_probe which calls > > tpm_ibmvtpm_get_desired_dma. > > tpm_ibmvtpm_get_desired_dma is called before the code enters critical > > section. > > > > There is no locking mechanism around tpm_ibmvtpm_remove in vio_bus_remove. > > > > What's the concern here? > > tpm_ibmvtpm_remove makes the pointer that tpm_ibmvtpm_get_desired_dma > is accessing invalid, so some kind of locking is technically required > so that the two things do not create a use after free race: > I don't think we need to worry about locking in this specific case. tpm_ibmvtpm_get_desired_dma was designed to return a default value in the case when the chip is not available. There is a locking mechanism between the probe and the remove at vio level. The 'get_desired_dma' is called before acquiring a lock within the probe code is rather a design than a bug. Vicky > > > + /* For tpm_ibmvtpm_get_desired_dma */ > > > + dev_set_drvdata(&vdev->dev, NULL); > > > kfree(ibmvtpm); > > Eg with the kfree above. > > It may be that the driver core prevents probe/remove from running > concurrently and things are fine, but this is something to confirm.. > > Jason >
Re: [tpmdd-devel] [PATCH] vTPM: Fix missing NULL check
On Mon, 2017-03-06 at 16:19 -0700, Jason Gunthorpe wrote: > On Mon, Mar 06, 2017 at 05:32:15PM -0500, Hon Ching(Vicky) Lo wrote: > > The current code passes the address of tpm_chip as the argument to > > dev_get_drvdata() without prior NULL check in > > tpm_ibmvtpm_get_desired_dma. This resulted an oops during kernel > > boot when vTPM is enabled in Power partition configured in active > > memory sharing mode. > > > > The vio_driver's get_desired_dma() is called before the probe(), which > > for vtpm is tpm_ibmvtpm_probe, and it's this latter function that > > initializes the driver and set data. Attempting to get data before > > the probe() caused the problem. > > > > This patch adds a NULL check to the tpm_ibmvtpm_get_desired_dma. > > Does this also need a hunk in tpm_ibmvtpm_remove to null the drvdata > after removal, or does something in the driver code guarentee it is > null'd after remove? The driver does not ganrantee it is null'd after remove. > > We don't want to use-after-free chip on the next probe cycle. > > > static unsigned long tpm_ibmvtpm_get_desired_dma(struct vio_dev *vdev) > > { > > struct tpm_chip *chip = dev_get_drvdata(&vdev->dev); > > - struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev); > > + struct ibmvtpm_dev *ibmvtpm = NULL; > > + > > + if (chip) > > + ibmvtpm = dev_get_drvdata(&chip->dev); > > Maybe just do this, clearer that it is chip that can be null. We do > not want to see drivers testing their chip drvdata against null. > That should do it. > Also, how does locking work here? Does the vio core prevent > tpm_ibmvtpm_get_desired_dma and tpm_ibmvtpm_remove from running > concurrently? No, vio core doesn't prevent tpm_ibmvtpm_get_desired_dma and tpm_ibmvtpm_remove from running concurrently. vio_bus_probe calls vio_cmo_bus_probe which calls tpm_ibmvtpm_get_desired_dma. tpm_ibmvtpm_get_desired_dma is called before the code enters critical section. There is no locking mechanism around tpm_ibmvtpm_remove in vio_bus_remove. What's the concern here? > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c > index 946025a7413b6b..ced6b9f0008dc2 100644 > --- a/drivers/char/tpm/tpm_ibmvtpm.c > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > @@ -294,6 +294,8 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev) > kfree(ibmvtpm->rtce_buf); > } > > + /* For tpm_ibmvtpm_get_desired_dma */ > + dev_set_drvdata(&vdev->dev, NULL); > kfree(ibmvtpm); > > return 0; > @@ -309,15 +311,16 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev) > static unsigned long tpm_ibmvtpm_get_desired_dma(struct vio_dev *vdev) > { > struct tpm_chip *chip = dev_get_drvdata(&vdev->dev); > - struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev); > + struct ibmvtpm_dev *ibmvtpm; > > /* ibmvtpm initializes at probe time, so the data we are > * asking for may not be set yet. Estimate that 4K required > * for TCE-mapped buffer in addition to CRQ. > */ > - if (!ibmvtpm) > + if (!chip) > return CRQ_RES_BUF_SIZE + PAGE_SIZE; > > + ibmvtpm = dev_get_drvdata(&chip->dev); > return CRQ_RES_BUF_SIZE + ibmvtpm->rtce_size; > } > > Thanks, Vicky
[PATCH] vTPM: Fix missing NULL check
The current code passes the address of tpm_chip as the argument to dev_get_drvdata() without prior NULL check in tpm_ibmvtpm_get_desired_dma. This resulted an oops during kernel boot when vTPM is enabled in Power partition configured in active memory sharing mode. The vio_driver's get_desired_dma() is called before the probe(), which for vtpm is tpm_ibmvtpm_probe, and it's this latter function that initializes the driver and set data. Attempting to get data before the probe() caused the problem. This patch adds a NULL check to the tpm_ibmvtpm_get_desired_dma. fixes: 9e0d39d8a6a0 ("tpm: Remove useless priv field in struct tpm_vendor_specific") Cc: Signed-off-by: Hon Ching(Vicky) Lo --- drivers/char/tpm/tpm_ibmvtpm.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 1b9d61f..a88ee25 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -313,7 +313,10 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev) static unsigned long tpm_ibmvtpm_get_desired_dma(struct vio_dev *vdev) { struct tpm_chip *chip = dev_get_drvdata(&vdev->dev); - struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev); + struct ibmvtpm_dev *ibmvtpm = NULL; + + if (chip) + ibmvtpm = dev_get_drvdata(&chip->dev); /* * ibmvtpm initializes at probe time, so the data we are -- 1.7.1
[PATCH v3] vTPM: fix missing error handling for suspend operation
ibmvtpm_send_crq in tpm_ibmvtpm_suspend returns errors in a more granular level than what the existing code does, according to the PAPR standard. This patch adds the missing CRQ transport event code checks to ensure the appropriate actions are taken, in the case that ibmvtpm_send_crq returns H_CLOSED. Signed-off-by: Hon Ching(Vicky) Lo --- drivers/char/tpm/tpm_ibmvtpm.c | 43 --- drivers/char/tpm/tpm_ibmvtpm.h |7 ++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 3e6a226..ea2a970 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -335,17 +335,46 @@ static int tpm_ibmvtpm_suspend(struct device *dev) struct ibmvtpm_crq crq; u64 *buf = (u64 *) &crq; int rc = 0; + int counter = 0; + int sig; - crq.valid = (u8)IBMVTPM_VALID_CMD; - crq.msg = (u8)VTPM_PREPARE_TO_SUSPEND; +suspend: + crq_initialized = 0; + crq.valid = (u8) IBMVTPM_VALID_CMD; + crq.msg = (u8) VTPM_PREPARE_TO_SUSPEND; rc = ibmvtpm_send_crq(ibmvtpm->vdev, cpu_to_be64(buf[0]), cpu_to_be64(buf[1])); + + if ((rc == H_CLOSED) && (crq.valid == (u8) VALID_TRANSPORT_EVENT)) { + if (crq.msg == (u8) PARTNER_PARTITION_FAILED) { + dev_err(ibmvtpm->dev, + "vtpm has terminated fatally; reboot to reinstate a trusted state.\n"); + } + + if ((crq.msg == (u8) PARTNER_PARTITION_DEREG_CRQ) + && (counter < 1)) { + counter++; + + /* The vtpm is in the process of being reloaded by +* firmware and has de-registered CRQ. The client +* must wait for the CRQ INITIALIZATION message and +* respond and must resubmit suspend message. +*/ + sig = + wait_event_interruptible(ibmvtpm->wq, +crq_initialized == 1); + if (sig) + return -EINTR; + goto suspend; + } + } + if (rc != H_SUCCESS) - dev_err(ibmvtpm->dev, - "tpm_ibmvtpm_suspend failed rc=%d\n", rc); + dev_err(ibmvtpm->dev, "tpm_ibmvtpm_suspend failed rc=%d\n", rc); return rc; + } /** @@ -477,6 +506,9 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, case INIT_CRQ_COMP_RES: dev_info(ibmvtpm->dev, "CRQ initialization completed\n"); + /* in case vtpm is being reloaded */ + crq_initialized = 1; + wake_up_interruptible(&ibmvtpm->wq); return; default: dev_err(ibmvtpm->dev, "Unknown crq message type: %d\n", crq->msg); @@ -517,6 +549,9 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, ibmvtpm->res_len = be16_to_cpu(crq->len); wake_up_interruptible(&ibmvtpm->wq); return; + case VTPM_PREPARE_TO_SUSPEND_RES: + dev_info(ibmvtpm->dev, "Prepare to Suspend Response\n"); + return; default: return; } diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h index 6af9289..ed5fd5f 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.h +++ b/drivers/char/tpm/tpm_ibmvtpm.h @@ -73,4 +73,11 @@ struct ibmvtpm_dev { #define VTPM_PREPARE_TO_SUSPEND0x04 #define VTPM_PREPARE_TO_SUSPEND_RES(0x04 | VTPM_MSG_RES) +/* vTPM CRQ Transport Event codes */ +#define VALID_TRANSPORT_EVENT 0xFF +#define PARTNER_PARTITION_FAILED 0x01 +#define PARTNER_PARTITION_DEREG_CRQ0x02 + +int crq_initialized; + #endif -- 1.7.1
[PATCH v2] vTPM: fix missing error handling for suspend operation
ibmvtpm_send_crq in tpm_ibmvtpm_suspend returns errors in a more granular level than what the existing code does. This patch adds the missing CRQ transport event code checks to ensure appropriate action taken, in the case that ibmvtpm_send_crq returns H_CLOSED. Signed-off-by: Hon Ching(Vicky) Lo --- drivers/char/tpm/tpm_ibmvtpm.c | 43 --- drivers/char/tpm/tpm_ibmvtpm.h |7 ++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 3e6a226..ea2a970 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -335,17 +335,46 @@ static int tpm_ibmvtpm_suspend(struct device *dev) struct ibmvtpm_crq crq; u64 *buf = (u64 *) &crq; int rc = 0; + int counter = 0; + int sig; - crq.valid = (u8)IBMVTPM_VALID_CMD; - crq.msg = (u8)VTPM_PREPARE_TO_SUSPEND; +suspend: + crq_initialized = 0; + crq.valid = (u8) IBMVTPM_VALID_CMD; + crq.msg = (u8) VTPM_PREPARE_TO_SUSPEND; rc = ibmvtpm_send_crq(ibmvtpm->vdev, cpu_to_be64(buf[0]), cpu_to_be64(buf[1])); + + if ((rc == H_CLOSED) && (crq.valid == (u8) VALID_TRANSPORT_EVENT)) { + if (crq.msg == (u8) PARTNER_PARTITION_FAILED) { + dev_err(ibmvtpm->dev, + "vtpm has terminated fatally; reboot to reinstate a trusted state.\n"); + } + + if ((crq.msg == (u8) PARTNER_PARTITION_DEREG_CRQ) + && (counter < 1)) { + counter++; + + /* The vtpm is in the process of being reloaded by +* firmware and has de-registered CRQ. The client +* must wait for the CRQ INITIALIZATION message and +* respond and must resubmit suspend message. +*/ + sig = + wait_event_interruptible(ibmvtpm->wq, +crq_initialized == 1); + if (sig) + return -EINTR; + goto suspend; + } + } + if (rc != H_SUCCESS) - dev_err(ibmvtpm->dev, - "tpm_ibmvtpm_suspend failed rc=%d\n", rc); + dev_err(ibmvtpm->dev, "tpm_ibmvtpm_suspend failed rc=%d\n", rc); return rc; + } /** @@ -477,6 +506,9 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, case INIT_CRQ_COMP_RES: dev_info(ibmvtpm->dev, "CRQ initialization completed\n"); + /* in case vtpm is being reloaded */ + crq_initialized = 1; + wake_up_interruptible(&ibmvtpm->wq); return; default: dev_err(ibmvtpm->dev, "Unknown crq message type: %d\n", crq->msg); @@ -517,6 +549,9 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, ibmvtpm->res_len = be16_to_cpu(crq->len); wake_up_interruptible(&ibmvtpm->wq); return; + case VTPM_PREPARE_TO_SUSPEND_RES: + dev_info(ibmvtpm->dev, "Prepare to Suspend Response\n"); + return; default: return; } diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h index 6af9289..ed5fd5f 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.h +++ b/drivers/char/tpm/tpm_ibmvtpm.h @@ -73,4 +73,11 @@ struct ibmvtpm_dev { #define VTPM_PREPARE_TO_SUSPEND0x04 #define VTPM_PREPARE_TO_SUSPEND_RES(0x04 | VTPM_MSG_RES) +/* vTPM CRQ Transport Event codes */ +#define VALID_TRANSPORT_EVENT 0xFF +#define PARTNER_PARTITION_FAILED 0x01 +#define PARTNER_PARTITION_DEREG_CRQ0x02 + +int crq_initialized; + #endif -- 1.7.1
Re: [PATCH] vTPM: fix missing error handling for suspend operation
> > > + } else if (crq.msg == (u8) PARTNER_PARTITION_FAILED) { > > > + dev_err(ibmvtpm->dev, > > > + "vtpm has terminated fatally; reboot to > > > reinstate a trusted state.\n"); > > > + } else if (crq.msg == (u8) PARTNER_PARTITION_DEREG_CRQ) { > > > + /* The vtpm is in the process of being reloaded by > > > + * firmware and has de-registered CRQ. The client > > > + * must wait for the CRQ INITIALIZATION message and > > > + * respond and must resubmit suspend message. > > > + */ > > > + sig = > > > + wait_event_interruptible(ibmvtpm->wq, > > > + crq_initialized == 1); > > > + if (sig) > > > + return -EINTR; > > > + > > > + if (suspend_again_count < 1) { > > > + suspend_again_count++; > > > + goto suspendagain; > > > + } > > > + } else > > > + ; > > > + } > > > + > > > if (rc != H_SUCCESS) > > > - dev_err(ibmvtpm->dev, > > > - "tpm_ibmvtpm_suspend failed rc=%d\n", rc); > > > + dev_err(ibmvtpm->dev, "tpm_ibmvtpm_suspend failed rc=%d\n", rc); > > > > > > return rc; > > > + > > > +suspendagain: > > > + rc = tpm_ibmvtpm_suspend(ibmvtpm->dev); > > > + suspend_again_count = 0; > > > + > > > + if (rc != H_SUCCESS) > > > + dev_err(ibmvtpm->dev, "tpm_ibmvtpm_suspend failed rc=%d\n", rc); > > > + > > > + return rc; > > > + > > > > Get rid of this horrible looking tail recursion thing. > > > > What the heck is suspend_again_count and why it can be module scope > > variable? You could use a local variable instead if you would iterate > > with a loop. > > > > /Jarkko > > > > The reason for the 'goto' statement and the suspend_again_count was to > prevent the suspend function recurse again. In the case if vtpm is in > the process of being reloaded by firmware, we want to wait for the CRQ > INITIALIZATION and resubmit suspend message i.e. recurse only once. > Never mind.. I don't really save any repetitive code by using recursion now. I'll rework and resubmit the patch. Thanks, Vicky
Re: [PATCH] vTPM: fix missing error handling for suspend operation
On Fri, 2016-03-04 at 18:55 +0200, Jarkko Sakkinen wrote: > On Wed, Mar 02, 2016 at 01:23:47AM -0500, Hon Ching(Vicky) Lo wrote: > > ibmvtpm_send_crq in tpm_ibmvtpm_suspend returns errors in a more > > granular level than what the existing code does. This patch adds > > the missing CRQ transport event code checks to ensure appropriate > > action taken, in the case that ibmvtpm_send_crq returns H_CLOSED. > > > > Signed-off-by: Hon Ching(Vicky) Lo > > --- > > drivers/char/tpm/tpm_ibmvtpm.c | 58 > > +--- > > drivers/char/tpm/tpm_ibmvtpm.h |9 ++ > > 2 files changed, 63 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c > > index 3e6a226..5d984af 100644 > > --- a/drivers/char/tpm/tpm_ibmvtpm.c > > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > > @@ -335,17 +335,61 @@ static int tpm_ibmvtpm_suspend(struct device *dev) > > struct ibmvtpm_crq crq; > > u64 *buf = (u64 *) &crq; > > int rc = 0; > > + int sig; > > > > - crq.valid = (u8)IBMVTPM_VALID_CMD; > > - crq.msg = (u8)VTPM_PREPARE_TO_SUSPEND; > > + crq_initialized = 0; > > + crq.valid = (u8) IBMVTPM_VALID_CMD; > > + crq.msg = (u8) VTPM_PREPARE_TO_SUSPEND; > > > > rc = ibmvtpm_send_crq(ibmvtpm->vdev, cpu_to_be64(buf[0]), > > cpu_to_be64(buf[1])); > > + > > + if ((rc == H_CLOSED) && (crq.valid == (u8) VALID_TRANSPORT_EVENT)) { > > What if rc == H_CLOSED and crq.valid != VALID_TRANSPORT_EVENT? If that's the case, the function will return rc as the execution will skip this if block. > > > + if (crq.msg == (u8) PARTNER_PARTITION_SUSPENDED) { > > + /* The "partner partition suspended" transport > > +* event disables the associated CRQ such that > > +* any H_SEND_CRQ hcall() to the associated CRQ > > +* returns H_Closed until CRQ has been explicitly > > +* enabled using the H_ENABLED_CRQ hcall. > > +*/ > > + return H_SUCCESS; > > I'm having trouble to understand when the suspend happens through this > route and when you just get H_SUCCESS from ibmvtpm_send_crq(). It > seems that there are two ways how suspend can happen. > > I don't understand the big picture. You're right. This is not a valid case. As I revisited it, I realized that "partner partition suspended" transport event was handled in the rtas calls; vtpm doesn't have to take that into account. I'll get rid of this if-block. So, upon receiving H_CLOSED only the following two events are expected to be handled: 1) vtpm has terminated fatally. 2) partner partition preregistered CRQ. > > + } else if (crq.msg == (u8) PARTNER_PARTITION_FAILED) { > > + dev_err(ibmvtpm->dev, > > + "vtpm has terminated fatally; reboot to > > reinstate a trusted state.\n"); > > + } else if (crq.msg == (u8) PARTNER_PARTITION_DEREG_CRQ) { > > + /* The vtpm is in the process of being reloaded by > > +* firmware and has de-registered CRQ. The client > > +* must wait for the CRQ INITIALIZATION message and > > +* respond and must resubmit suspend message. > > +*/ > > + sig = > > + wait_event_interruptible(ibmvtpm->wq, > > +crq_initialized == 1); > > + if (sig) > > + return -EINTR; > > + > > + if (suspend_again_count < 1) { > > + suspend_again_count++; > > + goto suspendagain; > > + } > > + } else > > + ; > > + } > > + > > if (rc != H_SUCCESS) > > - dev_err(ibmvtpm->dev, > > - "tpm_ibmvtpm_suspend failed rc=%d\n", rc); > > + dev_err(ibmvtpm->dev, "tpm_ibmvtpm_suspend failed rc=%d\n", rc); > > > > return rc; > > + > > +suspendagain: > > + rc = tpm_ibmvtpm_suspend(ibmvtpm->dev); > > + suspend_again_count = 0; > > + > > + if (rc != H_SUCCESS) > > + dev_err(ibmvtpm->dev, "tpm_ibmvtpm_suspend failed rc=%d\n", rc); > > + > > + return rc; > > + > > Get rid of this horrible looking tail recursion thing. > > What the heck is suspend_again_count and why it can be module scope > variable? You could use a local variable instead if you would iterate > with a loop. > The reason for the 'goto' statement and the suspend_again_count was to prevent the suspend function recurse again. In the case if vtpm is in the process of being reloaded by firmware, we want to wait for the CRQ INITIALIZATION and resubmit suspend message i.e. recurse only once. > /Jarkko >
[PATCH] vTPM: fix missing error handling for suspend operation
ibmvtpm_send_crq in tpm_ibmvtpm_suspend returns errors in a more granular level than what the existing code does. This patch adds the missing CRQ transport event code checks to ensure appropriate action taken, in the case that ibmvtpm_send_crq returns H_CLOSED. Signed-off-by: Hon Ching(Vicky) Lo --- drivers/char/tpm/tpm_ibmvtpm.c | 58 +--- drivers/char/tpm/tpm_ibmvtpm.h |9 ++ 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 3e6a226..5d984af 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -335,17 +335,61 @@ static int tpm_ibmvtpm_suspend(struct device *dev) struct ibmvtpm_crq crq; u64 *buf = (u64 *) &crq; int rc = 0; + int sig; - crq.valid = (u8)IBMVTPM_VALID_CMD; - crq.msg = (u8)VTPM_PREPARE_TO_SUSPEND; + crq_initialized = 0; + crq.valid = (u8) IBMVTPM_VALID_CMD; + crq.msg = (u8) VTPM_PREPARE_TO_SUSPEND; rc = ibmvtpm_send_crq(ibmvtpm->vdev, cpu_to_be64(buf[0]), cpu_to_be64(buf[1])); + + if ((rc == H_CLOSED) && (crq.valid == (u8) VALID_TRANSPORT_EVENT)) { + if (crq.msg == (u8) PARTNER_PARTITION_SUSPENDED) { + /* The "partner partition suspended" transport +* event disables the associated CRQ such that +* any H_SEND_CRQ hcall() to the associated CRQ +* returns H_Closed until CRQ has been explicitly +* enabled using the H_ENABLED_CRQ hcall. +*/ + return H_SUCCESS; + } else if (crq.msg == (u8) PARTNER_PARTITION_FAILED) { + dev_err(ibmvtpm->dev, + "vtpm has terminated fatally; reboot to reinstate a trusted state.\n"); + } else if (crq.msg == (u8) PARTNER_PARTITION_DEREG_CRQ) { + /* The vtpm is in the process of being reloaded by +* firmware and has de-registered CRQ. The client +* must wait for the CRQ INITIALIZATION message and +* respond and must resubmit suspend message. +*/ + sig = + wait_event_interruptible(ibmvtpm->wq, +crq_initialized == 1); + if (sig) + return -EINTR; + + if (suspend_again_count < 1) { + suspend_again_count++; + goto suspendagain; + } + } else + ; + } + if (rc != H_SUCCESS) - dev_err(ibmvtpm->dev, - "tpm_ibmvtpm_suspend failed rc=%d\n", rc); + dev_err(ibmvtpm->dev, "tpm_ibmvtpm_suspend failed rc=%d\n", rc); return rc; + +suspendagain: + rc = tpm_ibmvtpm_suspend(ibmvtpm->dev); + suspend_again_count = 0; + + if (rc != H_SUCCESS) + dev_err(ibmvtpm->dev, "tpm_ibmvtpm_suspend failed rc=%d\n", rc); + + return rc; + } /** @@ -477,6 +521,9 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, case INIT_CRQ_COMP_RES: dev_info(ibmvtpm->dev, "CRQ initialization completed\n"); + /* in case vtpm is being reloaded */ + crq_initialized = 1; + wake_up_interruptible(&ibmvtpm->wq); return; default: dev_err(ibmvtpm->dev, "Unknown crq message type: %d\n", crq->msg); @@ -517,6 +564,9 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, ibmvtpm->res_len = be16_to_cpu(crq->len); wake_up_interruptible(&ibmvtpm->wq); return; + case VTPM_PREPARE_TO_SUSPEND_RES: + dev_info(ibmvtpm->dev, "Prepare to Suspend Response\n"); + return; default: return; } diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h index 6af9289..1990d3c 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.h +++ b/drivers/char/tpm/tpm_ibmvtpm.h @@ -73,4 +73,13 @@ struct ibmvtpm_dev { #define VTPM_PREPARE_TO_SUSPEND0x04 #define VTPM_PREPARE_TO_SUSPEND_RES(0x04 | VTPM_MSG_RES) +/* vTPM CRQ Transport Event codes */ +#define VALID_TRANSPORT_EVENT 0xFF +#define PARTNER_PARTITION_FAILED 0x01 +
Re: [PATCH v2 2/3] vTPM: reformat event log to be byte-aligned
On Tue, 2015-10-13 at 13:43 -0500, Ashley Lai wrote: > > On 10/07/2015 07:11 PM, Hon Ching(Vicky) Lo wrote: > > The event log generated by OpenFirmware in PowerPC is 4-byte aligned. > > This patch reformats the log to be byte-aligned for the Linux client. > > > > Signed-off-by: Hon Ching(Vicky) Lo > > --- > > arch/powerpc/kernel/prom_init.c | 13 - > > 1 files changed, 12 insertions(+), 1 deletions(-) > > > > diff --git a/arch/powerpc/kernel/prom_init.c > > b/arch/powerpc/kernel/prom_init.c > > index b9b6bb1..8a5c248 100644 > > --- a/arch/powerpc/kernel/prom_init.c > > +++ b/arch/powerpc/kernel/prom_init.c > > @@ -1417,8 +1417,9 @@ static void __init prom_instantiate_sml(void) > > { > > phandle ibmvtpm_node; > > ihandle ibmvtpm_inst; > > - u32 entry = 0, size = 0; > > + u32 entry = 0, size = 0, succ = 0; > > u64 base; > > + __be32 val; > > > > prom_debug("prom_instantiate_sml: start...\n"); > > > > @@ -1433,6 +1434,16 @@ static void __init prom_instantiate_sml(void) > > return; > > } > > > > + if (prom_getprop(ibmvtpm_node, "ibm,sml-efi-reformat-supported", > > +&val, sizeof(val)) != PROM_ERROR) { > > + if (call_prom_ret("call-method", 2, 2, &succ, > > + ADDR("reformat-sml-to-efi-alignment"), > > + ibmvtpm_inst) != 0 || succ == 0) { > > reformat-sml-to-efi-alignment is something new just added in the firmware? I > don't remember seeing it before. Yes, it's new. Our new firmware version will support it. > > > > + prom_printf("Reformat SML to EFI alignment failed\n"); > > + return; > > + } > > + } > > + > > if (call_prom_ret("call-method", 2, 2, &size, > > ADDR("sml-get-handover-size"), > > ibmvtpm_inst) != 0 || size == 0) { > -- 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/
[PATCH v2 2/3] vTPM: reformat event log to be byte-aligned
The event log generated by OpenFirmware in PowerPC is 4-byte aligned. This patch reformats the log to be byte-aligned for the Linux client. Signed-off-by: Hon Ching(Vicky) Lo --- arch/powerpc/kernel/prom_init.c | 13 - 1 files changed, 12 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index b9b6bb1..8a5c248 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -1417,8 +1417,9 @@ static void __init prom_instantiate_sml(void) { phandle ibmvtpm_node; ihandle ibmvtpm_inst; - u32 entry = 0, size = 0; + u32 entry = 0, size = 0, succ = 0; u64 base; + __be32 val; prom_debug("prom_instantiate_sml: start...\n"); @@ -1433,6 +1434,16 @@ static void __init prom_instantiate_sml(void) return; } + if (prom_getprop(ibmvtpm_node, "ibm,sml-efi-reformat-supported", +&val, sizeof(val)) != PROM_ERROR) { + if (call_prom_ret("call-method", 2, 2, &succ, + ADDR("reformat-sml-to-efi-alignment"), + ibmvtpm_inst) != 0 || succ == 0) { + prom_printf("Reformat SML to EFI alignment failed\n"); + return; + } + } + if (call_prom_ret("call-method", 2, 2, &size, ADDR("sml-get-handover-size"), ibmvtpm_inst) != 0 || size == 0) { -- 1.7.1 -- 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/
[PATCH v2 1/3] vTPM: fix searching for the right vTPM node in device tree
Replace all occurrences of '/ibm,vtpm' with '/vdevice/vtpm', as only the latter is ganranteed to be available for the client OS. The '/ibm,vtpm' node should only be used by Open Firmware, which is susceptible to changes. Signed-off-by: Hon Ching(Vicky) Lo --- arch/powerpc/kernel/prom_init.c |8 drivers/char/tpm/tpm_of.c |2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index 1a85d8f..b9b6bb1 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -1422,12 +1422,12 @@ static void __init prom_instantiate_sml(void) prom_debug("prom_instantiate_sml: start...\n"); - ibmvtpm_node = call_prom("finddevice", 1, 1, ADDR("/ibm,vtpm")); + ibmvtpm_node = call_prom("finddevice", 1, 1, ADDR("/vdevice/vtpm")); prom_debug("ibmvtpm_node: %x\n", ibmvtpm_node); if (!PHANDLE_VALID(ibmvtpm_node)) return; - ibmvtpm_inst = call_prom("open", 1, 1, ADDR("/ibm,vtpm")); + ibmvtpm_inst = call_prom("open", 1, 1, ADDR("/vdevice/vtpm")); if (!IHANDLE_VALID(ibmvtpm_inst)) { prom_printf("opening vtpm package failed (%x)\n", ibmvtpm_inst); return; @@ -1456,9 +1456,9 @@ static void __init prom_instantiate_sml(void) reserve_mem(base, size); - prom_setprop(ibmvtpm_node, "/ibm,vtpm", "linux,sml-base", + prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-base", &base, sizeof(base)); - prom_setprop(ibmvtpm_node, "/ibm,vtpm", "linux,sml-size", + prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-size", &size, sizeof(size)); prom_debug("sml base = 0x%x\n", base); diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c index 62a22ce..6c73199 100644 --- a/drivers/char/tpm/tpm_of.c +++ b/drivers/char/tpm/tpm_of.c @@ -31,7 +31,7 @@ int read_log(struct tpm_bios_log *log) return -EFAULT; } - np = of_find_node_by_name(NULL, "ibm,vtpm"); + np = of_find_node_by_name(NULL, "vtpm"); if (!np) { pr_err("%s: ERROR - IBMVTPM not supported\n", __func__); return -ENODEV; -- 1.7.1 -- 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/
[PATCH v2 3/3] vTPM: get the buffer allocated for event log instead of the actual log
The OS should ask Power Firmware (PFW) for the size of the buffer allocated for the event log, instead of the size of the actual event log. It then passes the buffer adddress and size to PFW in the handover process, into which PFW copies the log. Signed-off-by: Hon Ching(Vicky) Lo --- arch/powerpc/kernel/prom_init.c | 21 +++-- 1 files changed, 15 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index 8a5c248..08fe5e7 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -1442,13 +1442,20 @@ static void __init prom_instantiate_sml(void) prom_printf("Reformat SML to EFI alignment failed\n"); return; } - } - if (call_prom_ret("call-method", 2, 2, &size, - ADDR("sml-get-handover-size"), - ibmvtpm_inst) != 0 || size == 0) { - prom_printf("SML get handover size failed\n"); - return; + if (call_prom_ret("call-method", 2, 2, &size, + ADDR("sml-get-allocated-size"), + ibmvtpm_inst) != 0 || size == 0) { + prom_printf("SML get allocated size failed\n"); + return; + } + } else { + if (call_prom_ret("call-method", 2, 2, &size, + ADDR("sml-get-handover-size"), + ibmvtpm_inst) != 0 || size == 0) { + prom_printf("SML get handover size failed\n"); + return; + } } base = alloc_down(size, PAGE_SIZE, 0); @@ -1457,6 +1464,8 @@ static void __init prom_instantiate_sml(void) prom_printf("instantiating sml at 0x%x...", base); + memset((void *)base, 0, size); + if (call_prom_ret("call-method", 4, 2, &entry, ADDR("sml-handover"), ibmvtpm_inst, size, base) != 0 || entry == 0) { -- 1.7.1 -- 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/
[PATCH] vTPM: fix memory allocation flag for rtce buffer at kernel boot
At ibm vtpm initialzation, tpm_ibmvtpm_probe() registers its interrupt handler, ibmvtpm_interrupt, which calls ibmvtpm_crq_process to allocate memory for rtce buffer. The current code uses 'GFP_KERNEL' as the type of kernel memory allocation, which resulted a warning at kernel/lockdep.c. This patch uses 'GFP_ATOMIC' instead so that the allocation is high-priority and does not sleep. Signed-off-by: Hon Ching(Vicky) Lo --- drivers/char/tpm/tpm_ibmvtpm.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 27ebf95..3e6a226 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -491,7 +491,7 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, } ibmvtpm->rtce_size = be16_to_cpu(crq->len); ibmvtpm->rtce_buf = kmalloc(ibmvtpm->rtce_size, - GFP_KERNEL); + GFP_ATOMIC); if (!ibmvtpm->rtce_buf) { dev_err(ibmvtpm->dev, "Failed to allocate memory for rtce buffer\n"); return; -- 1.7.1 -- 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/
[PATCH v2 1/3] vTPM: fix searching for the right vTPM node in device tree
Replace all occurrences of '/ibm,vtpm' with '/vdevice/vtpm', as only the latter is ganranteed to be available for the client OS. The '/ibm,vtpm' node should only be used by Open Firmware, which is susceptible to changes. Signed-off-by: Hon Ching(Vicky) Lo --- arch/powerpc/kernel/prom_init.c |8 drivers/char/tpm/tpm_of.c |2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index 1a85d8f..b9b6bb1 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -1422,12 +1422,12 @@ static void __init prom_instantiate_sml(void) prom_debug("prom_instantiate_sml: start...\n"); - ibmvtpm_node = call_prom("finddevice", 1, 1, ADDR("/ibm,vtpm")); + ibmvtpm_node = call_prom("finddevice", 1, 1, ADDR("/vdevice/vtpm")); prom_debug("ibmvtpm_node: %x\n", ibmvtpm_node); if (!PHANDLE_VALID(ibmvtpm_node)) return; - ibmvtpm_inst = call_prom("open", 1, 1, ADDR("/ibm,vtpm")); + ibmvtpm_inst = call_prom("open", 1, 1, ADDR("/vdevice/vtpm")); if (!IHANDLE_VALID(ibmvtpm_inst)) { prom_printf("opening vtpm package failed (%x)\n", ibmvtpm_inst); return; @@ -1456,9 +1456,9 @@ static void __init prom_instantiate_sml(void) reserve_mem(base, size); - prom_setprop(ibmvtpm_node, "/ibm,vtpm", "linux,sml-base", + prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-base", &base, sizeof(base)); - prom_setprop(ibmvtpm_node, "/ibm,vtpm", "linux,sml-size", + prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-size", &size, sizeof(size)); prom_debug("sml base = 0x%x\n", base); diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c index 62a22ce..6c73199 100644 --- a/drivers/char/tpm/tpm_of.c +++ b/drivers/char/tpm/tpm_of.c @@ -31,7 +31,7 @@ int read_log(struct tpm_bios_log *log) return -EFAULT; } - np = of_find_node_by_name(NULL, "ibm,vtpm"); + np = of_find_node_by_name(NULL, "vtpm"); if (!np) { pr_err("%s: ERROR - IBMVTPM not supported\n", __func__); return -ENODEV; -- 1.7.1 -- 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/
[PATCH v2 3/3] vTPM: get the buffer allocated for event log instead of the actual log
The OS should ask Power Firmware (PFW) for the size of the buffer allocated for the event log, instead of the size of the actual event log. It then passes the buffer adddress and size to PFW in the handover process, into which PFW copies the log. Signed-off-by: Hon Ching(Vicky) Lo --- arch/powerpc/kernel/prom_init.c | 21 +++-- 1 files changed, 15 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index 8a5c248..08fe5e7 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -1442,13 +1442,20 @@ static void __init prom_instantiate_sml(void) prom_printf("Reformat SML to EFI alignment failed\n"); return; } - } - if (call_prom_ret("call-method", 2, 2, &size, - ADDR("sml-get-handover-size"), - ibmvtpm_inst) != 0 || size == 0) { - prom_printf("SML get handover size failed\n"); - return; + if (call_prom_ret("call-method", 2, 2, &size, + ADDR("sml-get-allocated-size"), + ibmvtpm_inst) != 0 || size == 0) { + prom_printf("SML get allocated size failed\n"); + return; + } + } else { + if (call_prom_ret("call-method", 2, 2, &size, + ADDR("sml-get-handover-size"), + ibmvtpm_inst) != 0 || size == 0) { + prom_printf("SML get handover size failed\n"); + return; + } } base = alloc_down(size, PAGE_SIZE, 0); @@ -1457,6 +1464,8 @@ static void __init prom_instantiate_sml(void) prom_printf("instantiating sml at 0x%x...", base); + memset((void *)base, 0, size); + if (call_prom_ret("call-method", 4, 2, &entry, ADDR("sml-handover"), ibmvtpm_inst, size, base) != 0 || entry == 0) { -- 1.7.1 -- 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/
[PATCH] vTPM: fix memory allocation flag for rtce buffer at kernel boot
At ibm vtpm initialzation, tpm_ibmvtpm_probe() registers its interrupt handler, ibmvtpm_interrupt, which calls ibmvtpm_crq_process to allocate memory for rtce buffer. The current code uses 'GFP_KERNEL' as the type of kernel memory allocation, which resulted a warning at kernel/lockdep.c. This patch uses 'GFP_ATOMIC' instead so that the allocation is high-priority and does not sleep. Signed-off-by: Hon Ching(Vicky) Lo --- drivers/char/tpm/tpm_ibmvtpm.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 27ebf95..3e6a226 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -491,7 +491,7 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, } ibmvtpm->rtce_size = be16_to_cpu(crq->len); ibmvtpm->rtce_buf = kmalloc(ibmvtpm->rtce_size, - GFP_KERNEL); + GFP_ATOMIC); if (!ibmvtpm->rtce_buf) { dev_err(ibmvtpm->dev, "Failed to allocate memory for rtce buffer\n"); return; -- 1.7.1 -- 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/
[PATCH v2 2/3] vTPM: reformat event log to be byte-aligned
The event log generated by OpenFirmware in PowerPC is 4-byte aligned. This patch reformats the log to be byte-aligned for the Linux client. Signed-off-by: Hon Ching(Vicky) Lo --- arch/powerpc/kernel/prom_init.c | 13 - 1 files changed, 12 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index b9b6bb1..8a5c248 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -1417,8 +1417,9 @@ static void __init prom_instantiate_sml(void) { phandle ibmvtpm_node; ihandle ibmvtpm_inst; - u32 entry = 0, size = 0; + u32 entry = 0, size = 0, succ = 0; u64 base; + __be32 val; prom_debug("prom_instantiate_sml: start...\n"); @@ -1433,6 +1434,16 @@ static void __init prom_instantiate_sml(void) return; } + if (prom_getprop(ibmvtpm_node, "ibm,sml-efi-reformat-supported", +&val, sizeof(val)) != PROM_ERROR) { + if (call_prom_ret("call-method", 2, 2, &succ, + ADDR("reformat-sml-to-efi-alignment"), + ibmvtpm_inst) != 0 || succ == 0) { + prom_printf("Reformat SML to EFI alignment failed\n"); + return; + } + } + if (call_prom_ret("call-method", 2, 2, &size, ADDR("sml-get-handover-size"), ibmvtpm_inst) != 0 || size == 0) { -- 1.7.1 -- 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/
[Fwd: Re: [PATCH v4 1/2] vTPM: support little endian guests]
Hi Peter, Did you the explanations in the following reply make sense to you? If you needed more clarifications, please advice. Thanks! Forwarded Message From: Hon Ching(Vicky) Lo To: Peter Hüwe Cc: tpmdd-de...@lists.sourceforge.net, Ashley Lai , Vicky Lo , linux-kernel@vger.kernel.org, Joy Latten Subject: Re: [PATCH v4 1/2] vTPM: support little endian guests Date: Fri, 17 Jul 2015 16:07:50 -0400 On Thu, 2015-07-16 at 20:43 +0200, Peter Hüwe wrote: > Hi Vicky, > Am Donnerstag, 16. Juli 2015, 19:54:15 schrieb Hon Ching(Vicky) Lo: > > Hi Peter, > > > > On Mon, 2015-07-13 at 23:08 +0200, Peter Hüwe wrote: > > > Hi Vicky, > > > > > > sorry for the late reply > > > > > > > This patch makes the code endianness independent. We defined a > > > > macro do_endian_conversion to apply endianness to raw integers > > > > in the event entries so that they will be displayed properly. > > > > tpm_binary_bios_measurements_show() is modified for the display. > > > > > > > > Signed-off-by: Hon Ching(Vicky) Lo > > > > Signed-off-by: Joy Latten > > > > > > > > b/drivers/char/tpm/tpm_eventlog.h index e7da086..267bfbd 100644 > > > > --- a/drivers/char/tpm/tpm_eventlog.h > > > > +++ b/drivers/char/tpm/tpm_eventlog.h > > > > @@ -6,6 +6,12 @@ > > > > > > > > #define MAX_TEXT_EVENT 1000/* Max event string length */ > > > > #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */ > > > > > > > > +#ifdef CONFIG_PPC64 > > > > +#define do_endian_conversion(x) be32_to_cpu(x) > > > > +#else > > > > +#define do_endian_conversion(x) x > > > > +#endif > > > > > > Why is this macro needed? > > > shouldn't the be32_to_cpu macro already do correct thing? > > > Or am I missing something here? > > > > > > > > > Thanks, > > > Peter > > > > The macro is defined to not do the conversion in the architecture > > that does not need it. > > Unfortunately I'm still not convinced this is needed? > be32_to_cpu(x) > should already do the right thing if no conversion is needed ? (being defined > as (x)) > Or is it not? > > > > Peter > include/linux/byteorder/generic.h: #define be32_to_cpu __be32_to_cpu include/uapi/linux/byteorder/little_endian.h: #define __be32_to_cpu(x) __swab32((__force __u32)(__be32)(x)) include/uapi/linux/byteorder/big_endian.h: #define __be32_to_cpu(x) ((__force __u32)(__be32)(x)) The above defines show that be32_to_cpu(x) would do a byte swap on x. The defines for __be32_to_cpu(x) in both little_endian.h and big_endian.h are doing the "right thing" that you mentioned. However, with the architecture differences between ppc64 and x86 for instance, we need the new macro to not do the conversion. The power firmware will always be in BE. Therefore, firmware will pass BE data to an LE operating system, so the conversion is needed. Whereas for an x86 system, BIOS gives LE data to the LE OS. (i.e. the macro is needed for LE support. Does it make sense? Vicky -- 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/
Re: [PATCH v4 1/2] vTPM: support little endian guests
On Thu, 2015-07-16 at 20:43 +0200, Peter Hüwe wrote: > Hi Vicky, > Am Donnerstag, 16. Juli 2015, 19:54:15 schrieb Hon Ching(Vicky) Lo: > > Hi Peter, > > > > On Mon, 2015-07-13 at 23:08 +0200, Peter Hüwe wrote: > > > Hi Vicky, > > > > > > sorry for the late reply > > > > > > > This patch makes the code endianness independent. We defined a > > > > macro do_endian_conversion to apply endianness to raw integers > > > > in the event entries so that they will be displayed properly. > > > > tpm_binary_bios_measurements_show() is modified for the display. > > > > > > > > Signed-off-by: Hon Ching(Vicky) Lo > > > > Signed-off-by: Joy Latten > > > > > > > > b/drivers/char/tpm/tpm_eventlog.h index e7da086..267bfbd 100644 > > > > --- a/drivers/char/tpm/tpm_eventlog.h > > > > +++ b/drivers/char/tpm/tpm_eventlog.h > > > > @@ -6,6 +6,12 @@ > > > > > > > > #define MAX_TEXT_EVENT 1000/* Max event string length */ > > > > #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */ > > > > > > > > +#ifdef CONFIG_PPC64 > > > > +#define do_endian_conversion(x) be32_to_cpu(x) > > > > +#else > > > > +#define do_endian_conversion(x) x > > > > +#endif > > > > > > Why is this macro needed? > > > shouldn't the be32_to_cpu macro already do correct thing? > > > Or am I missing something here? > > > > > > > > > Thanks, > > > Peter > > > > The macro is defined to not do the conversion in the architecture > > that does not need it. > > Unfortunately I'm still not convinced this is needed? > be32_to_cpu(x) > should already do the right thing if no conversion is needed ? (being defined > as (x)) > Or is it not? > > > > Peter > include/linux/byteorder/generic.h: #define be32_to_cpu __be32_to_cpu include/uapi/linux/byteorder/little_endian.h: #define __be32_to_cpu(x) __swab32((__force __u32)(__be32)(x)) include/uapi/linux/byteorder/big_endian.h: #define __be32_to_cpu(x) ((__force __u32)(__be32)(x)) The above defines show that be32_to_cpu(x) would do a byte swap on x. The defines for __be32_to_cpu(x) in both little_endian.h and big_endian.h are doing the "right thing" that you mentioned. However, with the architecture differences between ppc64 and x86 for instance, we need the new macro to not do the conversion. The power firmware will always be in BE. Therefore, firmware will pass BE data to an LE operating system, so the conversion is needed. Whereas for an x86 system, BIOS gives LE data to the LE OS. (i.e. the macro is needed for LE support. Does it make sense? Vicky -- 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/
Re: [PATCH v4 1/2] vTPM: support little endian guests
Hi Peter, On Mon, 2015-07-13 at 23:08 +0200, Peter Hüwe wrote: > Hi Vicky, > > sorry for the late reply > > > > This patch makes the code endianness independent. We defined a > > macro do_endian_conversion to apply endianness to raw integers > > in the event entries so that they will be displayed properly. > > tpm_binary_bios_measurements_show() is modified for the display. > > > > Signed-off-by: Hon Ching(Vicky) Lo > > Signed-off-by: Joy Latten > > > b/drivers/char/tpm/tpm_eventlog.h index e7da086..267bfbd 100644 > > --- a/drivers/char/tpm/tpm_eventlog.h > > +++ b/drivers/char/tpm/tpm_eventlog.h > > @@ -6,6 +6,12 @@ > > #define MAX_TEXT_EVENT 1000/* Max event string length */ > > #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */ > > > > +#ifdef CONFIG_PPC64 > > +#define do_endian_conversion(x) be32_to_cpu(x) > > +#else > > +#define do_endian_conversion(x) x > > +#endif > > > Why is this macro needed? > shouldn't the be32_to_cpu macro already do correct thing? > Or am I missing something here? > > > Thanks, > Peter > The macro is defined to not do the conversion in the architecture that does not need it. Regards, Vicky -- 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/
Re: [PATCH v4 2/2] TPM: remove unnecessary little endian conversion
Hi Peter, Please also commit this patch, if you accept it as well. Thanks, Vicky Forwarded Message From: Ashley Lai To: Hon Ching(Vicky) Lo Cc: tpmdd-de...@lists.sourceforge.net, Peter Huewe , Ashley Lai , Vicky Lo , linux-kernel@vger.kernel.org, Joy Latten >Subject: Re: [PATCH v4 2/2] TPM: remove unnecessary little endian conversion > Date: Thu, 18 Jun 2015 08:23:33 -0500 (CDT) > Looks good. > Reviewed-by: Ashley Lai > Ashley Lai On Wed, 2015-06-17 at 18:17 -0400, Hon Ching(Vicky) Lo wrote: > The base pointer for the event log is allocated in the local > kernel (in prom_instantiate_sml()), therefore it is already in > the host's endian byte order and requires no conversion. > > The content of the 'basep' pointer in read_log() stores the > base address of the log. This patch ensures that it is correctly > implemented. > > Signed-off-by: Hon Ching(Vicky) Lo > Signed-off-by: Joy Latten > --- > drivers/char/tpm/tpm_of.c |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c > index c002d1b..62a22ce 100644 > --- a/drivers/char/tpm/tpm_of.c > +++ b/drivers/char/tpm/tpm_of.c > @@ -24,7 +24,7 @@ int read_log(struct tpm_bios_log *log) > { > struct device_node *np; > const u32 *sizep; > - const __be64 *basep; > + const u64 *basep; > > if (log->bios_event_log != NULL) { > pr_err("%s: ERROR - Eventlog already initialized\n", __func__); > @@ -63,7 +63,7 @@ int read_log(struct tpm_bios_log *log) > > log->bios_event_log_end = log->bios_event_log + *sizep; > > - memcpy(log->bios_event_log, __va(be64_to_cpup(basep)), *sizep); > + memcpy(log->bios_event_log, __va(*basep), *sizep); > > return 0; > -- 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/
Re: [PATCH v4 1/2] vTPM: support little endian guests
Hi Peter, Can you please commit the patch in the next open window, if you accept it as well? Thanks! Regards, Vicky Forwarded Message From: Ashley Lai To: Hon Ching(Vicky) Lo Cc: tpmdd-de...@lists.sourceforge.net, Peter Huewe , Ashley Lai , Vicky Lo , linux-kernel@vger.kernel.org, Joy Latten Subject: Re: [PATCH v4 1/2] vTPM: support little endian guests Date: Thu, 18 Jun 2015 08:23:01 -0500 (CDT) Looks better. Thanks. Reviewed-by: Ashley Lai Cheers, Ashley Lai On Wed, 2015-06-17 at 18:17 -0400, Hon Ching(Vicky) Lo wrote: > This patch makes the code endianness independent. We defined a > macro do_endian_conversion to apply endianness to raw integers > in the event entries so that they will be displayed properly. > tpm_binary_bios_measurements_show() is modified for the display. > > Signed-off-by: Hon Ching(Vicky) Lo > Signed-off-by: Joy Latten > --- > drivers/char/tpm/tpm_eventlog.c | 78 > --- > drivers/char/tpm/tpm_eventlog.h |6 +++ > 2 files changed, 62 insertions(+), 22 deletions(-) > > diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c > index 3a56a13..bd72fb0 100644 > --- a/drivers/char/tpm/tpm_eventlog.c > +++ b/drivers/char/tpm/tpm_eventlog.c > @@ -76,15 +76,25 @@ static void *tpm_bios_measurements_start(struct seq_file > *m, loff_t *pos) > void *addr = log->bios_event_log; > void *limit = log->bios_event_log_end; > struct tcpa_event *event; > + u32 converted_event_size; > + u32 converted_event_type; > + > > /* read over *pos measurements */ > for (i = 0; i < *pos; i++) { > event = addr; > > + converted_event_size = > + do_endian_conversion(event->event_size); > + converted_event_type = > + do_endian_conversion(event->event_type); > + > if ((addr + sizeof(struct tcpa_event)) < limit) { > - if (event->event_type == 0 && event->event_size == 0) > + if ((converted_event_type == 0) && > + (converted_event_size == 0)) > return NULL; > - addr += sizeof(struct tcpa_event) + event->event_size; > + addr += (sizeof(struct tcpa_event) + > + converted_event_size); > } > } > > @@ -94,8 +104,12 @@ static void *tpm_bios_measurements_start(struct seq_file > *m, loff_t *pos) > > event = addr; > > - if ((event->event_type == 0 && event->event_size == 0) || > - ((addr + sizeof(struct tcpa_event) + event->event_size) >= limit)) > + converted_event_size = do_endian_conversion(event->event_size); > + converted_event_type = do_endian_conversion(event->event_type); > + > + if (((converted_event_type == 0) && (converted_event_size == 0)) > + || ((addr + sizeof(struct tcpa_event) + converted_event_size) > + >= limit)) > return NULL; > > return addr; > @@ -107,8 +121,12 @@ static void *tpm_bios_measurements_next(struct seq_file > *m, void *v, > struct tcpa_event *event = v; > struct tpm_bios_log *log = m->private; > void *limit = log->bios_event_log_end; > + u32 converted_event_size; > + u32 converted_event_type; > > - v += sizeof(struct tcpa_event) + event->event_size; > + converted_event_size = do_endian_conversion(event->event_size); > + > + v += sizeof(struct tcpa_event) + converted_event_size; > > /* now check if current entry is valid */ > if ((v + sizeof(struct tcpa_event)) >= limit) > @@ -116,11 +134,11 @@ static void *tpm_bios_measurements_next(struct seq_file > *m, void *v, > > event = v; > > - if (event->event_type == 0 && event->event_size == 0) > - return NULL; > + converted_event_size = do_endian_conversion(event->event_size); > + converted_event_type = do_endian_conversion(event->event_type); > > - if ((event->event_type == 0 && event->event_size == 0) || > - ((v + sizeof(struct tcpa_event) + event->event_size) >= limit)) > + if (((converted_event_type == 0) && (converted_event_size == 0)) || > + ((v + sizeof(struct tcpa_event) + converted_event_size) >= limit)) > return NULL; > > (*pos)++; > @@ -140,7 +158,7 @@ static int get_event_name(char *dest, struct tcpa_event > *event, > int i, n_len = 0, d_len = 0; > struct tcpa_pc_eve
Re: [PATCH v4 1/2] vTPM: support little endian guests
Hi Peter, Can you please commit the patch in the next open window, if you accept it as well? Thanks, Vicky On Thu, 2015-06-18 at 08:23 -0500, Ashley Lai wrote: Looks better. Thanks. > > Reviewed-by: Ashley Lai > > Cheers, > Ashley Lai > > On Wed, 2015-06-17 at 18:17 -0400, Hon Ching(Vicky) Lo wrote: > This patch makes the code endianness independent. We defined a > macro do_endian_conversion to apply endianness to raw integers > in the event entries so that they will be displayed properly. > tpm_binary_bios_measurements_show() is modified for the display. > > Signed-off-by: Hon Ching(Vicky) Lo > Signed-off-by: Joy Latten > --- > drivers/char/tpm/tpm_eventlog.c | 78 > --- > drivers/char/tpm/tpm_eventlog.h |6 +++ > 2 files changed, 62 insertions(+), 22 deletions(-) > > diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c > index 3a56a13..bd72fb0 100644 > --- a/drivers/char/tpm/tpm_eventlog.c > +++ b/drivers/char/tpm/tpm_eventlog.c > @@ -76,15 +76,25 @@ static void *tpm_bios_measurements_start(struct seq_file > *m, loff_t *pos) > void *addr = log->bios_event_log; > void *limit = log->bios_event_log_end; > struct tcpa_event *event; > + u32 converted_event_size; > + u32 converted_event_type; > + > > /* read over *pos measurements */ > for (i = 0; i < *pos; i++) { > event = addr; > > + converted_event_size = > + do_endian_conversion(event->event_size); > + converted_event_type = > + do_endian_conversion(event->event_type); > + > if ((addr + sizeof(struct tcpa_event)) < limit) { > - if (event->event_type == 0 && event->event_size == 0) > + if ((converted_event_type == 0) && > + (converted_event_size == 0)) > return NULL; > - addr += sizeof(struct tcpa_event) + event->event_size; > + addr += (sizeof(struct tcpa_event) + > + converted_event_size); > } > } > > @@ -94,8 +104,12 @@ static void *tpm_bios_measurements_start(struct seq_file > *m, loff_t *pos) > > event = addr; > > - if ((event->event_type == 0 && event->event_size == 0) || > - ((addr + sizeof(struct tcpa_event) + event->event_size) >= limit)) > + converted_event_size = do_endian_conversion(event->event_size); > + converted_event_type = do_endian_conversion(event->event_type); > + > + if (((converted_event_type == 0) && (converted_event_size == 0)) > + || ((addr + sizeof(struct tcpa_event) + converted_event_size) > + >= limit)) > return NULL; > > return addr; > @@ -107,8 +121,12 @@ static void *tpm_bios_measurements_next(struct seq_file > *m, void *v, > struct tcpa_event *event = v; > struct tpm_bios_log *log = m->private; > void *limit = log->bios_event_log_end; > + u32 converted_event_size; > + u32 converted_event_type; > > - v += sizeof(struct tcpa_event) + event->event_size; > + converted_event_size = do_endian_conversion(event->event_size); > + > + v += sizeof(struct tcpa_event) + converted_event_size; > > /* now check if current entry is valid */ > if ((v + sizeof(struct tcpa_event)) >= limit) > @@ -116,11 +134,11 @@ static void *tpm_bios_measurements_next(struct seq_file > *m, void *v, > > event = v; > > - if (event->event_type == 0 && event->event_size == 0) > - return NULL; > + converted_event_size = do_endian_conversion(event->event_size); > + converted_event_type = do_endian_conversion(event->event_type); > > - if ((event->event_type == 0 && event->event_size == 0) || > - ((v + sizeof(struct tcpa_event) + event->event_size) >= limit)) > + if (((converted_event_type == 0) && (converted_event_size == 0)) || > + ((v + sizeof(struct tcpa_event) + converted_event_size) >= limit)) > return NULL; > > (*pos)++; > @@ -140,7 +158,7 @@ static int get_event_name(char *dest, struct tcpa_event > *event, > int i, n_len = 0, d_len = 0; > struct tcpa_pc_event *pc_event; > > - switch(event->event_type) { > + switch (do_endian_conversion(event->event_type)) { > case PREBOOT: > case POST_CODE: > case UNUSED: > @@ -156,14 +174,16 @@ static int get_
[PATCH v4 1/2] vTPM: support little endian guests
This patch makes the code endianness independent. We defined a macro do_endian_conversion to apply endianness to raw integers in the event entries so that they will be displayed properly. tpm_binary_bios_measurements_show() is modified for the display. Signed-off-by: Hon Ching(Vicky) Lo Signed-off-by: Joy Latten --- drivers/char/tpm/tpm_eventlog.c | 78 --- drivers/char/tpm/tpm_eventlog.h |6 +++ 2 files changed, 62 insertions(+), 22 deletions(-) diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c index 3a56a13..bd72fb0 100644 --- a/drivers/char/tpm/tpm_eventlog.c +++ b/drivers/char/tpm/tpm_eventlog.c @@ -76,15 +76,25 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos) void *addr = log->bios_event_log; void *limit = log->bios_event_log_end; struct tcpa_event *event; + u32 converted_event_size; + u32 converted_event_type; + /* read over *pos measurements */ for (i = 0; i < *pos; i++) { event = addr; + converted_event_size = + do_endian_conversion(event->event_size); + converted_event_type = + do_endian_conversion(event->event_type); + if ((addr + sizeof(struct tcpa_event)) < limit) { - if (event->event_type == 0 && event->event_size == 0) + if ((converted_event_type == 0) && + (converted_event_size == 0)) return NULL; - addr += sizeof(struct tcpa_event) + event->event_size; + addr += (sizeof(struct tcpa_event) + +converted_event_size); } } @@ -94,8 +104,12 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos) event = addr; - if ((event->event_type == 0 && event->event_size == 0) || - ((addr + sizeof(struct tcpa_event) + event->event_size) >= limit)) + converted_event_size = do_endian_conversion(event->event_size); + converted_event_type = do_endian_conversion(event->event_type); + + if (((converted_event_type == 0) && (converted_event_size == 0)) + || ((addr + sizeof(struct tcpa_event) + converted_event_size) + >= limit)) return NULL; return addr; @@ -107,8 +121,12 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v, struct tcpa_event *event = v; struct tpm_bios_log *log = m->private; void *limit = log->bios_event_log_end; + u32 converted_event_size; + u32 converted_event_type; - v += sizeof(struct tcpa_event) + event->event_size; + converted_event_size = do_endian_conversion(event->event_size); + + v += sizeof(struct tcpa_event) + converted_event_size; /* now check if current entry is valid */ if ((v + sizeof(struct tcpa_event)) >= limit) @@ -116,11 +134,11 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v, event = v; - if (event->event_type == 0 && event->event_size == 0) - return NULL; + converted_event_size = do_endian_conversion(event->event_size); + converted_event_type = do_endian_conversion(event->event_type); - if ((event->event_type == 0 && event->event_size == 0) || - ((v + sizeof(struct tcpa_event) + event->event_size) >= limit)) + if (((converted_event_type == 0) && (converted_event_size == 0)) || + ((v + sizeof(struct tcpa_event) + converted_event_size) >= limit)) return NULL; (*pos)++; @@ -140,7 +158,7 @@ static int get_event_name(char *dest, struct tcpa_event *event, int i, n_len = 0, d_len = 0; struct tcpa_pc_event *pc_event; - switch(event->event_type) { + switch (do_endian_conversion(event->event_type)) { case PREBOOT: case POST_CODE: case UNUSED: @@ -156,14 +174,16 @@ static int get_event_name(char *dest, struct tcpa_event *event, case NONHOST_CODE: case NONHOST_CONFIG: case NONHOST_INFO: - name = tcpa_event_type_strings[event->event_type]; + name = tcpa_event_type_strings[do_endian_conversion + (event->event_type)]; n_len = strlen(name); break; case SEPARATOR: case ACTION: - if (MAX_TEXT_EVENT > event->event_size) { + if (MAX_TEXT_EVENT > + do_endian_conversion(event->event_size)) { name = event_entry; - n_len = event->event_size; +
[PATCH v4 2/2] TPM: remove unnecessary little endian conversion
The base pointer for the event log is allocated in the local kernel (in prom_instantiate_sml()), therefore it is already in the host's endian byte order and requires no conversion. The content of the 'basep' pointer in read_log() stores the base address of the log. This patch ensures that it is correctly implemented. Signed-off-by: Hon Ching(Vicky) Lo Signed-off-by: Joy Latten --- drivers/char/tpm/tpm_of.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c index c002d1b..62a22ce 100644 --- a/drivers/char/tpm/tpm_of.c +++ b/drivers/char/tpm/tpm_of.c @@ -24,7 +24,7 @@ int read_log(struct tpm_bios_log *log) { struct device_node *np; const u32 *sizep; - const __be64 *basep; + const u64 *basep; if (log->bios_event_log != NULL) { pr_err("%s: ERROR - Eventlog already initialized\n", __func__); @@ -63,7 +63,7 @@ int read_log(struct tpm_bios_log *log) log->bios_event_log_end = log->bios_event_log + *sizep; - memcpy(log->bios_event_log, __va(be64_to_cpup(basep)), *sizep); + memcpy(log->bios_event_log, __va(*basep), *sizep); return 0; -- 1.7.1 -- 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/
Re: [PATCH v2 1/2] vTPM: support little endian guests
Hi Ashley, Ah, good catch. I think I can only join the first two lines (where the assignments are) and will have to leave the rest splitted. I'll resubmit this one soon. Thanks for the review! Vicky On Tue, 2015-06-16 at 20:17 -0500, Ashley Lai wrote: > Just a small comment otherwise it looks good. > > On Tue, 9 Jun 2015, Hon Ching(Vicky) Lo wrote: > > case NONHOST_INFO: > > - name = tcpa_event_type_strings[event->event_type]; > > + name = > > + tcpa_event_type_strings[do_endian_conversion > > + (event->event_type)]; > Not being picky but if it does not exceed 80 characters it looks better > to join the line above. > name = tcpa_event_type_strings[do_endian_conversion >(event->event_type)]; > > > case POST_CONTENTS: > > - name = tcpa_pc_event_id_strings[pc_event->event_id]; > > + name = > > + tcpa_pc_event_id_strings[do_endian_conversion > > +(pc_event->event_id)]; > Same as above. > > Thanks, > --Ashley Lai > -- 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/
Re: [PATCH] vTPM: set virtual device before passing to ibmvtpm_reset_crq
Hi Peter, Yes, it's a fix to a kernel dump caused by enabling both vtpm and kdump. On Tue, 2015-06-16 at 22:37 +0200, Peter Hüwe wrote: > Hey, > > Am Freitag, 22. Mai 2015, 19:23:02 schrieb Hon Ching(Vicky) Lo: > > tpm_ibmvtpm_probe() calls ibmvtpm_reset_crq(ibmvtpm) without having yet > > set the virtual device in the ibmvtpm structure. So in ibmvtpm_reset_crq, > > the phype call contains empty unit addresses, ibmvtpm->vdev->unit_address. > > > > Signed-off-by: Hon Ching(Vicky) Lo > > Signed-off-by: Joy Latten > > --- > > drivers/char/tpm/tpm_ibmvtpm.c |5 +++-- > > 1 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c > > b/drivers/char/tpm/tpm_ibmvtpm.c index 42ffa5e..27ebf95 100644 > > --- a/drivers/char/tpm/tpm_ibmvtpm.c > > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > > @@ -578,6 +578,9 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, > > goto cleanup; > > } > > > > + ibmvtpm->dev = dev; > > + ibmvtpm->vdev = vio_dev; > > + > > crq_q = &ibmvtpm->crq_queue; > > crq_q->crq_addr = (struct ibmvtpm_crq *)get_zeroed_page(GFP_KERNEL); > > if (!crq_q->crq_addr) { > > @@ -622,8 +625,6 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, > > > > crq_q->index = 0; > > > > - ibmvtpm->dev = dev; > > - ibmvtpm->vdev = vio_dev; > > TPM_VPRIV(chip) = (void *)ibmvtpm; > > > > spin_lock_init(&ibmvtpm->rtce_lock); > > Is this a fix for something? > does it need to go through stable? > > Thanks, > Peter > -- 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/
[PATCH v2 2/2] TPM: remove unnecessary little endian conversion
The base pointer for the event log is allocated in the local kernel (in prom_instantiate_sml()), therefore it is already in the host's endian byte order and requires no conversion. The content of the 'basep' pointer in read_log() stores the base address of the log. This patch ensures that it is correctly implemented. Signed-off-by: Hon Ching(Vicky) Lo Signed-off-by: Joy Latten --- drivers/char/tpm/tpm_of.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c index c002d1b..62a22ce 100644 --- a/drivers/char/tpm/tpm_of.c +++ b/drivers/char/tpm/tpm_of.c @@ -24,7 +24,7 @@ int read_log(struct tpm_bios_log *log) { struct device_node *np; const u32 *sizep; - const __be64 *basep; + const u64 *basep; if (log->bios_event_log != NULL) { pr_err("%s: ERROR - Eventlog already initialized\n", __func__); @@ -63,7 +63,7 @@ int read_log(struct tpm_bios_log *log) log->bios_event_log_end = log->bios_event_log + *sizep; - memcpy(log->bios_event_log, __va(be64_to_cpup(basep)), *sizep); + memcpy(log->bios_event_log, __va(*basep), *sizep); return 0; -- 1.7.1 -- 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/
[PATCH v2 1/2] vTPM: support little endian guests
This patch makes the code endianness independent. We defined a macro do_endian_conversion to apply endianness to raw integers in the event entries so that they will be displayed properly. tpm_binary_bios_measurements_show() is modified for the display. Signed-off-by: Hon Ching(Vicky) Lo Signed-off-by: Joy Latten --- drivers/char/tpm/tpm_eventlog.c | 81 -- drivers/char/tpm/tpm_eventlog.h |6 +++ 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c index 3a56a13..8689957 100644 --- a/drivers/char/tpm/tpm_eventlog.c +++ b/drivers/char/tpm/tpm_eventlog.c @@ -76,15 +76,25 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos) void *addr = log->bios_event_log; void *limit = log->bios_event_log_end; struct tcpa_event *event; + u32 converted_event_size; + u32 converted_event_type; + /* read over *pos measurements */ for (i = 0; i < *pos; i++) { event = addr; + converted_event_size = + do_endian_conversion(event->event_size); + converted_event_type = + do_endian_conversion(event->event_type); + if ((addr + sizeof(struct tcpa_event)) < limit) { - if (event->event_type == 0 && event->event_size == 0) + if ((converted_event_type == 0) && + (converted_event_size == 0)) return NULL; - addr += sizeof(struct tcpa_event) + event->event_size; + addr += (sizeof(struct tcpa_event) + +converted_event_size); } } @@ -94,8 +104,12 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos) event = addr; - if ((event->event_type == 0 && event->event_size == 0) || - ((addr + sizeof(struct tcpa_event) + event->event_size) >= limit)) + converted_event_size = do_endian_conversion(event->event_size); + converted_event_type = do_endian_conversion(event->event_type); + + if (((converted_event_type == 0) && (converted_event_size == 0)) + || ((addr + sizeof(struct tcpa_event) + converted_event_size) + >= limit)) return NULL; return addr; @@ -107,8 +121,12 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v, struct tcpa_event *event = v; struct tpm_bios_log *log = m->private; void *limit = log->bios_event_log_end; + u32 converted_event_size; + u32 converted_event_type; - v += sizeof(struct tcpa_event) + event->event_size; + converted_event_size = do_endian_conversion(event->event_size); + + v += sizeof(struct tcpa_event) + converted_event_size; /* now check if current entry is valid */ if ((v + sizeof(struct tcpa_event)) >= limit) @@ -116,11 +134,11 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v, event = v; - if (event->event_type == 0 && event->event_size == 0) - return NULL; + converted_event_size = do_endian_conversion(event->event_size); + converted_event_type = do_endian_conversion(event->event_type); - if ((event->event_type == 0 && event->event_size == 0) || - ((v + sizeof(struct tcpa_event) + event->event_size) >= limit)) + if (((converted_event_type == 0) && (converted_event_size == 0)) || + ((v + sizeof(struct tcpa_event) + converted_event_size) >= limit)) return NULL; (*pos)++; @@ -140,7 +158,7 @@ static int get_event_name(char *dest, struct tcpa_event *event, int i, n_len = 0, d_len = 0; struct tcpa_pc_event *pc_event; - switch(event->event_type) { + switch (do_endian_conversion(event->event_type)) { case PREBOOT: case POST_CODE: case UNUSED: @@ -156,14 +174,17 @@ static int get_event_name(char *dest, struct tcpa_event *event, case NONHOST_CODE: case NONHOST_CONFIG: case NONHOST_INFO: - name = tcpa_event_type_strings[event->event_type]; + name = + tcpa_event_type_strings[do_endian_conversion + (event->event_type)]; n_len = strlen(name); break; case SEPARATOR: case ACTION: - if (MAX_TEXT_EVENT > event->event_size) { + if (MAX_TEXT_EVENT > + do_endian_conversion(event->event_size)) { name = event_entry; - n_len = event->event_s
[Fwd: Re: [PATCH] vTPM: set virtual device before passing to ibmvtpm_reset_crq]
Hi Peter, Would it be possible for you to review and commit the following patch at your earliest convenience? Thanks in advance! Forwarded Message From: Ashley Lai To: Hon Ching(Vicky) Lo Cc: tpmdd-de...@lists.sourceforge.net, Peter Huewe , Ashley Lai , Vicky Lo , linux-kernel@vger.kernel.org, Joy Latten Subject: Re: [PATCH] vTPM: set virtual device before passing to ibmvtpm_reset_crq Date: Tue, 2 Jun 2015 00:50:43 -0500 (CDT) Thanks for the patch. Looks good to me. Reviewed-by: Ashley Lai --Ashley Lai On Fri, 22 May 2015, Hon Ching(Vicky) Lo wrote: > tpm_ibmvtpm_probe() calls ibmvtpm_reset_crq(ibmvtpm) without having yet > set the virtual device in the ibmvtpm structure. So in ibmvtpm_reset_crq, > the phype call contains empty unit addresses, ibmvtpm->vdev->unit_address. > > Signed-off-by: Hon Ching(Vicky) Lo > Signed-off-by: Joy Latten > --- > drivers/char/tpm/tpm_ibmvtpm.c |5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c > index 42ffa5e..27ebf95 100644 > --- a/drivers/char/tpm/tpm_ibmvtpm.c > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > @@ -578,6 +578,9 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, > goto cleanup; > } > > + ibmvtpm->dev = dev; > + ibmvtpm->vdev = vio_dev; > + > crq_q = &ibmvtpm->crq_queue; > crq_q->crq_addr = (struct ibmvtpm_crq *)get_zeroed_page(GFP_KERNEL); > if (!crq_q->crq_addr) { > @@ -622,8 +625,6 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, > > crq_q->index = 0; > > - ibmvtpm->dev = dev; > - ibmvtpm->vdev = vio_dev; > TPM_VPRIV(chip) = (void *)ibmvtpm; > > spin_lock_init(&ibmvtpm->rtce_lock); > -- > 1.7.1 > > Best Regards, Vicky -- 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/
[PATCH] vTPM: set virtual device before passing to ibmvtpm_reset_crq
tpm_ibmvtpm_probe() calls ibmvtpm_reset_crq(ibmvtpm) without having yet set the virtual device in the ibmvtpm structure. So in ibmvtpm_reset_crq, the phype call contains empty unit addresses, ibmvtpm->vdev->unit_address. Signed-off-by: Hon Ching(Vicky) Lo Signed-off-by: Joy Latten --- drivers/char/tpm/tpm_ibmvtpm.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 42ffa5e..27ebf95 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -578,6 +578,9 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, goto cleanup; } + ibmvtpm->dev = dev; + ibmvtpm->vdev = vio_dev; + crq_q = &ibmvtpm->crq_queue; crq_q->crq_addr = (struct ibmvtpm_crq *)get_zeroed_page(GFP_KERNEL); if (!crq_q->crq_addr) { @@ -622,8 +625,6 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, crq_q->index = 0; - ibmvtpm->dev = dev; - ibmvtpm->vdev = vio_dev; TPM_VPRIV(chip) = (void *)ibmvtpm; spin_lock_init(&ibmvtpm->rtce_lock); -- 1.7.1 -- 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/
Re: [PATCH 3/3] vTPM: support little endian guests
On Tue, 2015-05-19 at 16:08 -0500, Ashley Lai wrote: > Thank you Vicky and Joy for the clarification. This patch mainly > converts the fields in the tcpa_event structure. I see the code converts > everytime it accesses the event fields. Would it be more efficient if you > do the conversion once and reuse them when needed? Could > convert_to_host_format(x) takes x as a tcpa_event structure? > If not you still can convert individual fields and reuse them. I'm aware > that the pcr_value field is type u8 and it does not need the conversion > but if convert_to_host_format() can convert the structure it shouldn't > convert > u8 type I think. > > > static const char* tcpa_event_type_strings[] = { > > "PREBOOT", > > @@ -82,9 +87,11 @@ static void *tpm_bios_measurements_start(struct seq_file > > *m, loff_t *pos) > > event = addr; > > > > if ((addr + sizeof(struct tcpa_event)) < limit) { > > - if (event->event_type == 0 && event->event_size == 0) > > + if ((convert_to_host_format(event->event_type) == 0) && > > + (convert_to_host_format(event->event_size) == 0)) > > return NULL; > > - addr += sizeof(struct tcpa_event) + event->event_size; > > + addr += (sizeof(struct tcpa_event) + > > +convert_to_host_format(event->event_size)); > > } > > } > > > > @@ -94,8 +101,11 @@ static void *tpm_bios_measurements_start(struct > > seq_file *m, loff_t *pos) > > > > event = addr; > > > > - if ((event->event_type == 0 && event->event_size == 0) || > > - ((addr + sizeof(struct tcpa_event) + event->event_size) >= limit)) > > + if (((convert_to_host_format(event->event_type) == 0) && > > +(convert_to_host_format(event->event_size) == 0)) > > + || > > + ((addr + sizeof(struct tcpa_event) + > > + convert_to_host_format(event->event_size)) >= limit)) > > return NULL; > > > > return addr; > > In this function, event_type and event_size can be converted > once and reuse. > > > case SEPARATOR: > > case ACTION: > > - if (MAX_TEXT_EVENT > event->event_size) { > > + if (MAX_TEXT_EVENT > > > + convert_to_host_format(event->event_size)) { > > name = event_entry; > > - n_len = event->event_size; > > + n_len = convert_to_host_format(event->event_size); > > } > > break; > > case EVENT_TAG: > > Same here. > Agree. It's more efficient to do the conversion once and reuse. convert_to_host_format(x) cannot take tcpa_event structure, as be64_to_cpu() only converts raw integers. > > @@ -208,11 +229,43 @@ static int tpm_binary_bios_measurements_show(struct > > seq_file *m, void *v) > > struct tcpa_event *event = v; > > char *data = v; > > int i; > > - > > - for (i = 0; i < sizeof(struct tcpa_event) + event->event_size; i++) > > + u32 x; > > + char tmp[4]; > > + > > + /* PCR */ > > + x = convert_to_host_format(event->pcr_index); > > + memcpy(tmp, &x, 4); > > + for (i = 0; i < 4; i++) > > + seq_putc(m, tmp[i]); > > + data += 4; > > + > > + /* Event Type */ > > + x = convert_to_host_format(event->event_type); > > + memcpy(tmp, &x, 4); > > + for (i = 0; i < 4; i++) > > + seq_putc(m, tmp[i]); > > + data += 4; > > + > > + /* HASH */ > > + for (i = 0; i < 20; i++) > > seq_putc(m, data[i]); > > + data += 20; > > + > > + /* Size */ > > + x = convert_to_host_format(event->event_size); > > + memcpy(tmp, &x, 4); > > + for (i = 0; i < 4; i++) > > + seq_putc(m, tmp[i]); > > + data += 4; > > + > > + /* Data */ > > + if (convert_to_host_format(event->event_size)) { > > + for (i = 0; i < convert_to_host_format(event->event_size); i++) > > + seq_putc(m, data[i]); > > + } > > > > return 0; > > + > > } > If the tcpa_event structure is converted, you may be able to get away > with memcpy and the for loop. > > Thanks, > --Ashley Lai > To simplify the code, we can try creating a struct that contains the first 4 fields in the tcpa_event so to only copy over the header part. Then we do conversion on it. That way, we can use a loop to print byte-by-byte on the new struct. Then, we can use a small loop to print byte-by-byte of the data part of the original event structure. I'll try this and re-submit patch. Thanks! Vicky -- 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/
Re: [PATCH 3/3] vTPM: support little endian guests
Thanks Ashley! > > The event log in ppc64 arch is always in big endian format. PowerPC > > supports both little endian and big endian guests. This patch converts > > the event log entries to guest format. > > I'm a little confused here. If this patch is to convert the event log > entries why are we convert in the conditional statements? One example > below: > > + if (((convert_to_host_format(event->event_type) == 0) && > +(convert_to_host_format(event->event_size) == 0)) > + || > + ((v + sizeof(struct tcpa_event) + > + convert_to_host_format(event->event_size)) > limit)) > > > > > We defined a macro to convert to guest format. In addition, > > tpm_binary_bios_measurements_show() is modified to parse the event > > and print each field individually. > We do not convert the whole event entry. Instead, we're converting only what's necessary such as pcr_index, event_type and event_size. pcr_value and event_data are of type u8. They do not need LE conversion. > It's nice to have human readable format but it may break existing tools > that parse or understand the machine readable format. Any comments on > this anyone? I got comments on the format, so I tried to make that conditional statement all in one line, but the 'Lindent' tool puts the lines back to the above format.. Regards, Vicky -- 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/
[PATCH 2/3] TPM: remove unnecessary little endian conversion
prom_instantiate_sml() already converted the base pointer to little endian. This patch removes this unnecessary additional conversion. Signed-off-by: Hon Ching(Vicky) Lo Signed-off-by: Joy Latten --- drivers/char/tpm/tpm_of.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c index c002d1b..62a22ce 100644 --- a/drivers/char/tpm/tpm_of.c +++ b/drivers/char/tpm/tpm_of.c @@ -24,7 +24,7 @@ int read_log(struct tpm_bios_log *log) { struct device_node *np; const u32 *sizep; - const __be64 *basep; + const u64 *basep; if (log->bios_event_log != NULL) { pr_err("%s: ERROR - Eventlog already initialized\n", __func__); @@ -63,7 +63,7 @@ int read_log(struct tpm_bios_log *log) log->bios_event_log_end = log->bios_event_log + *sizep; - memcpy(log->bios_event_log, __va(be64_to_cpup(basep)), *sizep); + memcpy(log->bios_event_log, __va(*basep), *sizep); return 0; -- 1.7.1 -- 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/
[PATCH 1/3] vTPM: fixed the limit checking
Do not skip the last entry of the event log. Signed-off-by: Hon Ching(Vicky) Lo Signed-off-by: Joy Latten Changelog: - remove redundant code --- drivers/char/tpm/tpm_eventlog.c |5 + 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c index 3a56a13..e77d8c1 100644 --- a/drivers/char/tpm/tpm_eventlog.c +++ b/drivers/char/tpm/tpm_eventlog.c @@ -116,11 +116,8 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v, event = v; - if (event->event_type == 0 && event->event_size == 0) - return NULL; - if ((event->event_type == 0 && event->event_size == 0) || - ((v + sizeof(struct tcpa_event) + event->event_size) >= limit)) + ((v + sizeof(struct tcpa_event) + event->event_size) > limit)) return NULL; (*pos)++; -- 1.7.1 -- 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/
[PATCH 0/3] additional little endian support
Hi, The patch set converts big endian event log entries to guest format in PowerPC, which supports both little endian and big endian guests. It also contains a fix to make sure the last event entry wasn't skipped. Hon Ching(Vicky) Lo (3): vTPM: fixed the limit checking TPM: remove unnecessary little endian conversion vTPM: support little endian guests drivers/char/tpm/tpm_eventlog.c | 95 ++- drivers/char/tpm/tpm_of.c |4 +- 2 files changed, 75 insertions(+), 24 deletions(-) -- 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/
[PATCH 3/3] vTPM: support little endian guests
The event log in ppc64 arch is always in big endian format. PowerPC supports both little endian and big endian guests. This patch converts the event log entries to guest format. We defined a macro to convert to guest format. In addition, tpm_binary_bios_measurements_show() is modified to parse the event and print each field individually. Signed-off-by: Hon Ching(Vicky) Lo Signed-off-by: Joy Latten --- drivers/char/tpm/tpm_eventlog.c | 92 +++ 1 files changed, 73 insertions(+), 19 deletions(-) diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c index e77d8c1..1b62c52 100644 --- a/drivers/char/tpm/tpm_eventlog.c +++ b/drivers/char/tpm/tpm_eventlog.c @@ -28,6 +28,11 @@ #include "tpm.h" #include "tpm_eventlog.h" +#ifdef CONFIG_PPC64 +#define convert_to_host_format(x) be32_to_cpu(x) +#else +#define convert_to_host_format(x) x +#endif static const char* tcpa_event_type_strings[] = { "PREBOOT", @@ -82,9 +87,11 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos) event = addr; if ((addr + sizeof(struct tcpa_event)) < limit) { - if (event->event_type == 0 && event->event_size == 0) + if ((convert_to_host_format(event->event_type) == 0) && + (convert_to_host_format(event->event_size) == 0)) return NULL; - addr += sizeof(struct tcpa_event) + event->event_size; + addr += (sizeof(struct tcpa_event) + +convert_to_host_format(event->event_size)); } } @@ -94,8 +101,11 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos) event = addr; - if ((event->event_type == 0 && event->event_size == 0) || - ((addr + sizeof(struct tcpa_event) + event->event_size) >= limit)) + if (((convert_to_host_format(event->event_type) == 0) && +(convert_to_host_format(event->event_size) == 0)) + || + ((addr + sizeof(struct tcpa_event) + + convert_to_host_format(event->event_size)) >= limit)) return NULL; return addr; @@ -108,7 +118,8 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v, struct tpm_bios_log *log = m->private; void *limit = log->bios_event_log_end; - v += sizeof(struct tcpa_event) + event->event_size; + v += (sizeof(struct tcpa_event) + + convert_to_host_format(event->event_size)); /* now check if current entry is valid */ if ((v + sizeof(struct tcpa_event)) >= limit) @@ -116,8 +127,11 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v, event = v; - if ((event->event_type == 0 && event->event_size == 0) || - ((v + sizeof(struct tcpa_event) + event->event_size) > limit)) + if (((convert_to_host_format(event->event_type) == 0) && +(convert_to_host_format(event->event_size) == 0)) + || + ((v + sizeof(struct tcpa_event) + + convert_to_host_format(event->event_size)) > limit)) return NULL; (*pos)++; @@ -137,7 +151,7 @@ static int get_event_name(char *dest, struct tcpa_event *event, int i, n_len = 0, d_len = 0; struct tcpa_pc_event *pc_event; - switch(event->event_type) { + switch(convert_to_host_format(event->event_type)) { case PREBOOT: case POST_CODE: case UNUSED: @@ -153,14 +167,17 @@ static int get_event_name(char *dest, struct tcpa_event *event, case NONHOST_CODE: case NONHOST_CONFIG: case NONHOST_INFO: - name = tcpa_event_type_strings[event->event_type]; + name = + tcpa_event_type_strings[convert_to_host_format + (event->event_type)]; n_len = strlen(name); break; case SEPARATOR: case ACTION: - if (MAX_TEXT_EVENT > event->event_size) { + if (MAX_TEXT_EVENT > + convert_to_host_format(event->event_size)) { name = event_entry; - n_len = event->event_size; + n_len = convert_to_host_format(event->event_size); } break; case EVENT_TAG: @@ -168,7 +185,7 @@ static int get_event_name(char *dest, struct tcpa_event *event, /* ToDo Row data -> Base64 */ - switch (pc_event->event_id) { + switch(convert_to_host_format(pc_event->event_id)) { case SMBIOS:
Re: [PATCH 1/2] tpm/tpm_ibmvtpm: Fail in ibmvtpm_get_data if driver_data is bad
Hi Anton, vio_bus_probe now calls vio_cmo_bus_probe before calling probe. This results in calling get_desired_dma to get rtce buf size before we have called probe which initializes vtpm driver and sets up the tpm data, i.e. rtce buf size. ibmvtpm_get_data returns NULL in get_desired_dma is a special case where the device is not initialized. Regards, Vicky On Wed, 2014-12-03 at 08:20 +1100, Anton Blanchard wrote: > Hi, > > > is this patchset still needed after Vicky's patch > > "[tpmdd-devel] Fix NULL return in tpm_ibmvtpm_get_desired_dma" > > https://patchwork.ozlabs.org/patch/402315/ > > > > Ashley raised some concerns. > > > > Since merge window is coming up, a fast reply is appreciated. > > We definitely need a way to catch an invalid driver data pointer, but it > looks like that needs to be reworked after Vicky's patch. > > Vicky could you look at all uses of ibmvtpm_get_data and make > sure we don't blindly dereference it? eg: > > static int tpm_ibmvtpm_resume(struct device *dev) > { > struct ibmvtpm_dev *ibmvtpm = ibmvtpm_get_data(dev); > ... > > rc = plpar_hcall_norets(H_ENABLE_CRQ, > ibmvtpm->vdev->unit_address); > > Anton > -- 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/