[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: <sta...@vger.kernel.org>
Signed-off-by: Hon Ching(Vicky) Lo <hon...@linux.vnet.ibm.com>
---
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(>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(>dev);
- struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(>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(>dev);
+ else
return CRQ_RES_BUF_SIZE + PAGE_SIZE;
return CRQ_RES_BUF_SIZE + ibmvtpm->rtce_size;
--
1.7.1
[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(>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(>dev);
- struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(>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(>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(>dev, NULL); > if (chip) > ibmvtpm = dev_get_drvdata(>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 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(>dev, NULL); > if (chip) > ibmvtpm = dev_get_drvdata(>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(>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 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(>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(>dev);
> > - struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(>dev);
> > + struct ibmvtpm_dev *ibmvtpm = NULL;
> > +
> > + if (chip)
> > + ibmvtpm = dev_get_drvdata(>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(>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(>dev);
> - struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(>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(>dev);
> return CRQ_RES_BUF_SIZE + ibmvtpm->rtce_size;
> }
>
>
Thanks,
Vicky
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(>dev);
> > - struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(>dev);
> > + struct ibmvtpm_dev *ibmvtpm = NULL;
> > +
> > + if (chip)
> > + ibmvtpm = dev_get_drvdata(>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(>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(>dev);
> - struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(>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(>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: <sta...@vger.kernel.org>
Signed-off-by: Hon Ching(Vicky) Lo <hon...@linux.vnet.ibm.com>
---
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(>dev);
- struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(>dev);
+ struct ibmvtpm_dev *ibmvtpm = NULL;
+
+ if (chip)
+ ibmvtpm = dev_get_drvdata(>dev);
/*
* ibmvtpm initializes at probe time, so the data we are
--
1.7.1
[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(>dev);
- struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(>dev);
+ struct ibmvtpm_dev *ibmvtpm = NULL;
+
+ if (chip)
+ ibmvtpm = dev_get_drvdata(>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 <hon...@linux.vnet.ibm.com>
---
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 *)
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(>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(>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 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 *)
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(>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(>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 <hon...@linux.vnet.ibm.com>
---
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 *)
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(>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(>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 *)
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(>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(>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
> > > + } 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 <hon...@linux.vnet.ibm.com>
> > ---
> > 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 *)
> > 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
>
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 *)
> > 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 <hon...@linux.vnet.ibm.com>
---
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 *)
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(>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(>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 0
[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 *)
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(>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(>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
+#define PARTNER_PARTITION_DEREG_CRQ0x02
+#define PARTNER_PARTITION_SUSPENDED0x06
+
+int crq_initialized;
+int suspend_again_count;
+
#endif
--
1.7.1
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",
> > +, sizeof(val)) != PROM_ERROR) {
> > + if (call_prom_ret("call-method", 2, 2, ,
> > + 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, ,
> > 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/
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 <hon...@linux.vnet.ibm.com>
> > ---
> > 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",
> > +, sizeof(val)) != PROM_ERROR) {
> > + if (call_prom_ret("call-method", 2, 2, ,
> > + 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, ,
> > 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",
+, sizeof(val)) != PROM_ERROR) {
+ if (call_prom_ret("call-method", 2, 2, ,
+ 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, ,
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",
, sizeof(base));
- prom_setprop(ibmvtpm_node, "/ibm,vtpm", "linux,sml-size",
+ prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-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, ,
- 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, ,
+ 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, ,
+ 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, ,
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",
, sizeof(base));
- prom_setprop(ibmvtpm_node, "/ibm,vtpm", "linux,sml-size",
+ prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-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, ,
- 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, ,
+ 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, ,
+ 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, ,
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",
+, sizeof(val)) != PROM_ERROR) {
+ if (call_prom_ret("call-method", 2, 2, ,
+ 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, ,
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 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 <hon...@linux.vnet.ibm.com>
---
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, ,
- 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, ,
+ 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, ,
+ 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, ,
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 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 <hon...@linux.vnet.ibm.com>
---
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",
, sizeof(base));
- prom_setprop(ibmvtpm_node, "/ibm,vtpm", "linux,sml-size",
+ prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-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] 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 <hon...@linux.vnet.ibm.com>
---
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 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 <hon...@linux.vnet.ibm.com>
---
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, ,
- 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, ,
+ 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, ,
+ 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, ,
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 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 <hon...@linux.vnet.ibm.com>
---
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",
+, sizeof(val)) != PROM_ERROR) {
+ if (call_prom_ret("call-method", 2, 2, ,
+ 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, ,
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 <hon...@linux.vnet.ibm.com>
---
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",
, sizeof(base));
- prom_setprop(ibmvtpm_node, "/ibm,vtpm", "linux,sml-size",
+ prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-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] 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 <hon...@linux.vnet.ibm.com>
---
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 <hon...@linux.vnet.ibm.com>
---
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",
+, sizeof(val)) != PROM_ERROR) {
+ if (call_prom_ret("call-method", 2, 2, ,
+ 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, ,
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/
[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 hon...@linux.vnet.ibm.com To: Peter Hüwe peterhu...@gmx.de Cc: tpmdd-de...@lists.sourceforge.net, Ashley Lai ash...@ashleylai.com, Vicky Lo honclo2...@gmail.com, linux-kernel@vger.kernel.org, Joy Latten jmlat...@linux.vnet.ibm.com 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 hon...@linux.vnet.ibm.com Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com 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
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 hon...@linux.vnet.ibm.com Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com 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 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 hon...@linux.vnet.ibm.com Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com 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_
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 ash...@ahsleylai.com
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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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
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 ash...@ashleylai.com
To: Hon Ching(Vicky) Lo hon...@linux.vnet.ibm.com
Cc: tpmdd-de...@lists.sourceforge.net, Peter Huewe peterhu...@gmx.de,
Ashley Lai ash...@ashleylai.com, Vicky Lo honclo2...@gmail.com,
linux-kernel@vger.kernel.org, Joy Latten jmlat...@linux.vnet.ibm.com
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 ash...@ahsleylai.com
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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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
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 ash...@ashleylai.com
To: Hon Ching(Vicky) Lo hon...@linux.vnet.ibm.com
Cc: tpmdd-de...@lists.sourceforge.net, Peter Huewe peterhu...@gmx.de,
Ashley Lai ash...@ashleylai.com, Vicky Lo honclo2...@gmail.com,
linux-kernel@vger.kernel.org, Joy Latten jmlat...@linux.vnet.ibm.com
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 ash...@ahsleylai.com
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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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/
[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/
[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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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 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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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;
+ n_len = do_endian_conversion(event-event_size);
}
break;
case EVENT_TAG:
@@ -171,7 +191,7 @@ static int get_event_name(char
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 = >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(>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/
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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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
[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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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_size;
+ n_len = do_endian_conversion(event-event_size);
}
break;
case EVENT_TAG:
@@ -171,7 +192,7 @@ static int
[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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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/
[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 = >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(>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/
[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 ash...@ashleylai.com
To: Hon Ching(Vicky) Lo hon...@linux.vnet.ibm.com
Cc: tpmdd-de...@lists.sourceforge.net, Peter Huewe peterhu...@gmx.de,
Ashley Lai ash...@ashleylai.com, Vicky Lo honclo2...@gmail.com,
linux-kernel@vger.kernel.org, Joy Latten jmlat...@linux.vnet.ibm.com
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 ash...@ahsleylai.com
--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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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 = >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(>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/
[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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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, , 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, , 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, , 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
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/
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:
[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 hon...@linux.vnet.ibm.com Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com 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 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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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 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 hon...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
---
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:
case BIS_CERT:
case CMOS:
@@ -176,7 +193,9 @@ static int get_event_name(char *dest, struct tcpa_event
*event,
case OPTION_ROM_EXEC:
case
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/
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/
