Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1282284 > unrolled thread

[PATCH] tpm_crb: Use the common ACPI definition of struct acpi_tpm2

Back to article view | Back to linux.kernel

Contents

  [PATCH] tpm_crb: Use the common ACPI definition of struct acpi_tpm2 Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-02 20:20 +0100
    Re: [PATCH] tpm_crb: Use the common ACPI definition of struct  acpi_tpm2 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-03 09:30 +0100
      Re: [PATCH] tpm_crb: Use the common ACPI definition of struct  acpi_tpm2 Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-03 18:10 +0100
        Re: [PATCH] tpm_crb: Use the common ACPI definition of struct  acpi_tpm2 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-06 02:30 +0100

#1282284 — [PATCH] tpm_crb: Use the common ACPI definition of struct acpi_tpm2

include/acpi/actbl2.h is the proper place for these definitions
and the needed TPM2 ones have been there since
commit 413d4a6defe0 ("ACPICA: Update TPM2 ACPI table")

This also drops the le32_to_cpu for members of this table,
consistent with other ACPI stuff.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/char/tpm/tpm.h     |  7 -------
 drivers/char/tpm/tpm_crb.c | 24 ++++++++----------------
 drivers/char/tpm/tpm_tis.c |  2 +-
 3 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 542a80cbfd9c..28b477e8da6a 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -128,13 +128,6 @@ enum tpm2_startup_types {
 	TPM2_SU_STATE	= 0x0001,
 };
 
-enum tpm2_start_method {
-	TPM2_START_ACPI = 2,
-	TPM2_START_FIFO = 6,
-	TPM2_START_CRB = 7,
-	TPM2_START_CRB_WITH_ACPI = 8,
-};
-
 struct tpm_chip;
 
 struct tpm_vendor_specific {
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 8342cf51ffdc..5b74ad8ca7fd 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -34,14 +34,6 @@ enum crb_defaults {
 	CRB_ACPI_START_INDEX = 1,
 };
 
-struct acpi_tpm2 {
-	struct acpi_table_header hdr;
-	u16 platform_class;
-	u16 reserved;
-	u64 control_area_pa;
-	u32 start_method;
-} __packed;
-
 enum crb_ca_request {
 	CRB_CA_REQ_GO_IDLE	= BIT(0),
 	CRB_CA_REQ_CMD_READY	= BIT(1),
@@ -207,7 +199,7 @@ static const struct tpm_class_ops tpm_crb = {
 static int crb_acpi_add(struct acpi_device *device)
 {
 	struct tpm_chip *chip;
-	struct acpi_tpm2 *buf;
+	struct acpi_table_tpm2 *buf;
 	struct crb_priv *priv;
 	struct device *dev = &device->dev;
 	acpi_status status;
@@ -223,7 +215,8 @@ static int crb_acpi_add(struct acpi_device *device)
 	}
 
 	/* Should the FIFO driver handle this? */
-	if (buf->start_method == TPM2_START_FIFO)
+	sm = buf->start_method;
+	if (sm == ACPI_TPM2_MEMORY_MAPPED)
 		return -ENODEV;
 
 	chip = tpmm_chip_alloc(dev, &tpm_crb);
@@ -232,7 +225,7 @@ static int crb_acpi_add(struct acpi_device *device)
 
 	chip->flags = TPM_CHIP_FLAG_TPM2;
 
-	if (buf->hdr.length < sizeof(struct acpi_tpm2)) {
+	if (buf->header.length < sizeof(*buf)) {
 		dev_err(dev, "TPM2 ACPI table has wrong size");
 		return -EINVAL;
 	}
@@ -244,21 +237,20 @@ static int crb_acpi_add(struct acpi_device *device)
 		return -ENOMEM;
 	}
 
-	sm = le32_to_cpu(buf->start_method);
-
 	/* The reason for the extra quirk is that the PTT in 4th Gen Core CPUs
 	 * report only ACPI start but in practice seems to require both
 	 * ACPI start and CRB start.
 	 */
-	if (sm == TPM2_START_CRB || sm == TPM2_START_FIFO ||
+	if (sm == ACPI_TPM2_COMMAND_BUFFER || sm == ACPI_TPM2_MEMORY_MAPPED ||
 	    !strcmp(acpi_device_hid(device), "MSFT0101"))
 		priv->flags |= CRB_FL_CRB_START;
 
-	if (sm == TPM2_START_ACPI || sm == TPM2_START_CRB_WITH_ACPI)
+	if (sm == ACPI_TPM2_START_METHOD ||
+	    sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
 		priv->flags |= CRB_FL_ACPI_START;
 
 	priv->cca = (struct crb_control_area __iomem *)
-		devm_ioremap_nocache(dev, buf->control_area_pa, 0x1000);
+		devm_ioremap_nocache(dev, buf->control_address, 0x1000);
 	if (!priv->cca) {
 		dev_err(dev, "ioremap of the control area failed\n");
 		return -ENOMEM;
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 8a3509cb10da..304323bdcaaa 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -135,7 +135,7 @@ static inline int is_fifo(struct acpi_device *dev)
 		return 0;
 	}
 
-	if (le32_to_cpu(tbl->start_method) != TPM2_START_FIFO)
+	if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
 		return 0;
 
 	/* TPM 2.0 FIFO */
-- 
2.1.4

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

[toc] | [next] | [standalone]

#1282777 — Re: [PATCH] tpm_crb: Use the common ACPI definition of struct acpi_tpm2

On Wed, Dec 02, 2015 at 12:19:00PM -0700, Jason Gunthorpe wrote:
> include/acpi/actbl2.h is the proper place for these definitions
> and the needed TPM2 ones have been there since
> commit 413d4a6defe0 ("ACPICA: Update TPM2 ACPI table")
> 
> This also drops the le32_to_cpu for members of this table,
> consistent with other ACPI stuff.

Why le32_to_cpu() is not needed?

/Jarkko

> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  drivers/char/tpm/tpm.h     |  7 -------
>  drivers/char/tpm/tpm_crb.c | 24 ++++++++----------------
>  drivers/char/tpm/tpm_tis.c |  2 +-
>  3 files changed, 9 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 542a80cbfd9c..28b477e8da6a 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -128,13 +128,6 @@ enum tpm2_startup_types {
>  	TPM2_SU_STATE	= 0x0001,
>  };
>  
> -enum tpm2_start_method {
> -	TPM2_START_ACPI = 2,
> -	TPM2_START_FIFO = 6,
> -	TPM2_START_CRB = 7,
> -	TPM2_START_CRB_WITH_ACPI = 8,
> -};
> -
>  struct tpm_chip;
>  
>  struct tpm_vendor_specific {
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 8342cf51ffdc..5b74ad8ca7fd 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -34,14 +34,6 @@ enum crb_defaults {
>  	CRB_ACPI_START_INDEX = 1,
>  };
>  
> -struct acpi_tpm2 {
> -	struct acpi_table_header hdr;
> -	u16 platform_class;
> -	u16 reserved;
> -	u64 control_area_pa;
> -	u32 start_method;
> -} __packed;
> -
>  enum crb_ca_request {
>  	CRB_CA_REQ_GO_IDLE	= BIT(0),
>  	CRB_CA_REQ_CMD_READY	= BIT(1),
> @@ -207,7 +199,7 @@ static const struct tpm_class_ops tpm_crb = {
>  static int crb_acpi_add(struct acpi_device *device)
>  {
>  	struct tpm_chip *chip;
> -	struct acpi_tpm2 *buf;
> +	struct acpi_table_tpm2 *buf;
>  	struct crb_priv *priv;
>  	struct device *dev = &device->dev;
>  	acpi_status status;
> @@ -223,7 +215,8 @@ static int crb_acpi_add(struct acpi_device *device)
>  	}
>  
>  	/* Should the FIFO driver handle this? */
> -	if (buf->start_method == TPM2_START_FIFO)
> +	sm = buf->start_method;
> +	if (sm == ACPI_TPM2_MEMORY_MAPPED)
>  		return -ENODEV;
>  
>  	chip = tpmm_chip_alloc(dev, &tpm_crb);
> @@ -232,7 +225,7 @@ static int crb_acpi_add(struct acpi_device *device)
>  
>  	chip->flags = TPM_CHIP_FLAG_TPM2;
>  
> -	if (buf->hdr.length < sizeof(struct acpi_tpm2)) {
> +	if (buf->header.length < sizeof(*buf)) {
>  		dev_err(dev, "TPM2 ACPI table has wrong size");
>  		return -EINVAL;
>  	}
> @@ -244,21 +237,20 @@ static int crb_acpi_add(struct acpi_device *device)
>  		return -ENOMEM;
>  	}
>  
> -	sm = le32_to_cpu(buf->start_method);
> -
>  	/* The reason for the extra quirk is that the PTT in 4th Gen Core CPUs
>  	 * report only ACPI start but in practice seems to require both
>  	 * ACPI start and CRB start.
>  	 */
> -	if (sm == TPM2_START_CRB || sm == TPM2_START_FIFO ||
> +	if (sm == ACPI_TPM2_COMMAND_BUFFER || sm == ACPI_TPM2_MEMORY_MAPPED ||
>  	    !strcmp(acpi_device_hid(device), "MSFT0101"))
>  		priv->flags |= CRB_FL_CRB_START;
>  
> -	if (sm == TPM2_START_ACPI || sm == TPM2_START_CRB_WITH_ACPI)
> +	if (sm == ACPI_TPM2_START_METHOD ||
> +	    sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
>  		priv->flags |= CRB_FL_ACPI_START;
>  
>  	priv->cca = (struct crb_control_area __iomem *)
> -		devm_ioremap_nocache(dev, buf->control_area_pa, 0x1000);
> +		devm_ioremap_nocache(dev, buf->control_address, 0x1000);
>  	if (!priv->cca) {
>  		dev_err(dev, "ioremap of the control area failed\n");
>  		return -ENOMEM;
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 8a3509cb10da..304323bdcaaa 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -135,7 +135,7 @@ static inline int is_fifo(struct acpi_device *dev)
>  		return 0;
>  	}
>  
> -	if (le32_to_cpu(tbl->start_method) != TPM2_START_FIFO)
> +	if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
>  		return 0;
>  
>  	/* TPM 2.0 FIFO */
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283197 — Re: [PATCH] tpm_crb: Use the common ACPI definition of struct acpi_tpm2

On Thu, Dec 03, 2015 at 10:27:53AM +0200, Jarkko Sakkinen wrote:
> On Wed, Dec 02, 2015 at 12:19:00PM -0700, Jason Gunthorpe wrote:
> > include/acpi/actbl2.h is the proper place for these definitions
> > and the needed TPM2 ones have been there since
> > commit 413d4a6defe0 ("ACPICA: Update TPM2 ACPI table")
> > 
> > This also drops the le32_to_cpu for members of this table,
> > consistent with other ACPI stuff.
> 
> Why le32_to_cpu() is not needed?

Why is it needed?

None of the fields are marked le32 in the struct and no other ACPI
thing I could find swaps.

If swapping is needed then the struct fields must be declared as 'le'

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

[toc] | [prev] | [next] | [standalone]

#1284679 — Re: [PATCH] tpm_crb: Use the common ACPI definition of struct acpi_tpm2

On Thu, Dec 03, 2015 at 10:02:17AM -0700, Jason Gunthorpe wrote:
> On Thu, Dec 03, 2015 at 10:27:53AM +0200, Jarkko Sakkinen wrote:
> > On Wed, Dec 02, 2015 at 12:19:00PM -0700, Jason Gunthorpe wrote:
> > > include/acpi/actbl2.h is the proper place for these definitions
> > > and the needed TPM2 ones have been there since
> > > commit 413d4a6defe0 ("ACPICA: Update TPM2 ACPI table")
> > > 
> > > This also drops the le32_to_cpu for members of this table,
> > > consistent with other ACPI stuff.
> > 
> > Why le32_to_cpu() is not needed?
> 
> Why is it needed?
> 
> None of the fields are marked le32 in the struct and no other ACPI
> thing I could find swaps.
> 
> If swapping is needed then the struct fields must be declared as 'le'

I'm just puzzling with three things:

1. Does it do conversion to the CPU format internally if you use the
   tables in a big-endian environment?
2. Isn't that code import from ACPICA? That would explains why it does
   not have tagged fields.
3. Could the reason be for such conversions not widely applied in the
   kernel that ACPI has been previously mostly used only in the PC/x86
   ecosystem?

AFAIK ACPI exist also for ARM64 servers, which can be a big-endian
environment.

> Jason

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web