linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8] tpm_crb: fix fTPM on AMD Zen+ CPUs
@ 2019-10-16 18:28 ivan.lazeev
  2019-10-21 15:57 ` Jarkko Sakkinen
  0 siblings, 1 reply; 8+ messages in thread
From: ivan.lazeev @ 2019-10-16 18:28 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, linux-kernel, Ivan Lazeev

From: Ivan Lazeev <ivan.lazeev@gmail.com>

Bug link: https://bugzilla.kernel.org/show_bug.cgi?id=195657

cmd/rsp buffers are expected to be in the same ACPI region.
For Zen+ CPUs BIOS's might report two different regions, some of
them also report region sizes inconsistent with values from TPM
registers.

Memory configuration on ASRock x470 ITX:

db0a0000-dc59efff : Reserved
        dc57e000-dc57efff : MSFT0101:00
        dc582000-dc582fff : MSFT0101:00

Work around the issue by storing ACPI regions declared for the
device in a fixed array and adding an array for pointers to
corresponding possibly allocated resources in crb_map_io function.
This data was previously held for a single resource
in struct crb_priv (iobase field) and local variable io_res in
crb_map_io function. ACPI resources array is used to find index of
corresponding region for each buffer and make the buffer size
consistent with region's length. Array of pointers to allocated
resources is used to map the region at most once.

Signed-off-by: Ivan Lazeev <ivan.lazeev@gmail.com>
---
Changelog:
v8:
	- fix sparse warning

v7:
	- use terminator entry in iores_array

v6:
	- get rid of new structures
	- open code helper functions
	- remove incorrect FW_BUG

v5:
	- prefix new symbols with tpm_crb_
	- get rid of dynamic allocation in ACPI walker

v4:
	- rename struct crb_resource fields (style change)
	- improve commit message

v3:
	- make crb_containing_resource search for address only,
	  because buffer sizes aren't trusted anyway
	- improve commit message

v2:
	- use list_for_each_safe

 drivers/char/tpm/tpm_crb.c | 123 +++++++++++++++++++++++++++----------
 1 file changed, 90 insertions(+), 33 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index e59f1f91d7f3..a9dcf31eadd2 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -22,6 +22,7 @@
 #include "tpm.h"
 
 #define ACPI_SIG_TPM2 "TPM2"
+#define TPM_CRB_MAX_RESOURCES 3
 
 static const guid_t crb_acpi_start_guid =
 	GUID_INIT(0x6BBF6CAB, 0x5463, 0x4714,
@@ -91,7 +92,6 @@ enum crb_status {
 struct crb_priv {
 	u32 sm;
 	const char *hid;
-	void __iomem *iobase;
 	struct crb_regs_head __iomem *regs_h;
 	struct crb_regs_tail __iomem *regs_t;
 	u8 __iomem *cmd;
@@ -434,21 +434,27 @@ static const struct tpm_class_ops tpm_crb = {
 
 static int crb_check_resource(struct acpi_resource *ares, void *data)
 {
-	struct resource *io_res = data;
+	struct resource *iores_array = data;
 	struct resource_win win;
 	struct resource *res = &(win.res);
+	int i;
 
 	if (acpi_dev_resource_memory(ares, res) ||
 	    acpi_dev_resource_address_space(ares, &win)) {
-		*io_res = *res;
-		io_res->name = NULL;
+		for (i = 0; i < TPM_CRB_MAX_RESOURCES + 1; ++i) {
+			if (resource_type(iores_array + i) != IORESOURCE_MEM) {
+				iores_array[i] = *res;
+				iores_array[i].name = NULL;
+				break;
+			}
+		}
 	}
 
 	return 1;
 }
 
-static void __iomem *crb_map_res(struct device *dev, struct crb_priv *priv,
-				 struct resource *io_res, u64 start, u32 size)
+static void __iomem *crb_map_res(struct device *dev, struct resource *iores,
+				 void __iomem **iobase_ptr, u64 start, u32 size)
 {
 	struct resource new_res = {
 		.start	= start,
@@ -460,10 +466,16 @@ static void __iomem *crb_map_res(struct device *dev, struct crb_priv *priv,
 	if (start != new_res.start)
 		return (void __iomem *) ERR_PTR(-EINVAL);
 
-	if (!resource_contains(io_res, &new_res))
+	if (!iores)
 		return devm_ioremap_resource(dev, &new_res);
 
-	return priv->iobase + (new_res.start - io_res->start);
+	if (!*iobase_ptr) {
+		*iobase_ptr = devm_ioremap_resource(dev, iores);
+		if (IS_ERR(*iobase_ptr))
+			return *iobase_ptr;
+	}
+
+	return *iobase_ptr + (new_res.start - iores->start);
 }
 
 /*
@@ -490,9 +502,13 @@ static u64 crb_fixup_cmd_size(struct device *dev, struct resource *io_res,
 static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 		      struct acpi_table_tpm2 *buf)
 {
-	struct list_head resources;
-	struct resource io_res;
+	struct list_head acpi_resource_list;
+	struct resource iores_array[TPM_CRB_MAX_RESOURCES + 1] = { {0} };
+	void __iomem *iobase_array[TPM_CRB_MAX_RESOURCES] = {NULL};
 	struct device *dev = &device->dev;
+	struct resource *iores;
+	void __iomem **iobase_ptr;
+	int i;
 	u32 pa_high, pa_low;
 	u64 cmd_pa;
 	u32 cmd_size;
@@ -501,21 +517,41 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 	u32 rsp_size;
 	int ret;
 
-	INIT_LIST_HEAD(&resources);
-	ret = acpi_dev_get_resources(device, &resources, crb_check_resource,
-				     &io_res);
+	INIT_LIST_HEAD(&acpi_resource_list);
+	ret = acpi_dev_get_resources(device, &acpi_resource_list,
+				     crb_check_resource, iores_array);
 	if (ret < 0)
 		return ret;
-	acpi_dev_free_resource_list(&resources);
+	acpi_dev_free_resource_list(&acpi_resource_list);
 
-	if (resource_type(&io_res) != IORESOURCE_MEM) {
+	if (resource_type(iores_array) != IORESOURCE_MEM) {
 		dev_err(dev, FW_BUG "TPM2 ACPI table does not define a memory resource\n");
 		return -EINVAL;
+	} else if (resource_type(iores_array + TPM_CRB_MAX_RESOURCES) ==
+		IORESOURCE_MEM) {
+		dev_warn(dev, "TPM2 ACPI table defines too many memory resources\n");
+		memset(iores_array + TPM_CRB_MAX_RESOURCES,
+		       0, sizeof(*iores_array));
+		iores_array[TPM_CRB_MAX_RESOURCES].flags = 0;
 	}
 
-	priv->iobase = devm_ioremap_resource(dev, &io_res);
-	if (IS_ERR(priv->iobase))
-		return PTR_ERR(priv->iobase);
+	iores = NULL;
+	iobase_ptr = NULL;
+	for (i = 0; resource_type(iores_array + i) == IORESOURCE_MEM; ++i) {
+		if (buf->control_address >= iores_array[i].start &&
+		    buf->control_address + sizeof(struct crb_regs_tail) - 1 <=
+		    iores_array[i].end) {
+			iores = iores_array + i;
+			iobase_ptr = iobase_array + i;
+			break;
+		}
+	}
+
+	priv->regs_t = crb_map_res(dev, iores, iobase_ptr, buf->control_address,
+				   sizeof(struct crb_regs_tail));
+
+	if (IS_ERR(priv->regs_t))
+		return PTR_ERR(priv->regs_t);
 
 	/* The ACPI IO region starts at the head area and continues to include
 	 * the control area, as one nice sane region except for some older
@@ -523,9 +559,10 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 	 */
 	if ((priv->sm == ACPI_TPM2_COMMAND_BUFFER) ||
 	    (priv->sm == ACPI_TPM2_MEMORY_MAPPED)) {
-		if (buf->control_address == io_res.start +
+		if (iores &&
+		    buf->control_address == iores->start +
 		    sizeof(*priv->regs_h))
-			priv->regs_h = priv->iobase;
+			priv->regs_h = *iobase_ptr;
 		else
 			dev_warn(dev, FW_BUG "Bad ACPI memory layout");
 	}
@@ -534,13 +571,6 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 	if (ret)
 		return ret;
 
-	priv->regs_t = crb_map_res(dev, priv, &io_res, buf->control_address,
-				   sizeof(struct crb_regs_tail));
-	if (IS_ERR(priv->regs_t)) {
-		ret = PTR_ERR(priv->regs_t);
-		goto out_relinquish_locality;
-	}
-
 	/*
 	 * PTT HW bug w/a: wake up the device to access
 	 * possibly not retained registers.
@@ -552,13 +582,26 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 	pa_high = ioread32(&priv->regs_t->ctrl_cmd_pa_high);
 	pa_low  = ioread32(&priv->regs_t->ctrl_cmd_pa_low);
 	cmd_pa = ((u64)pa_high << 32) | pa_low;
-	cmd_size = crb_fixup_cmd_size(dev, &io_res, cmd_pa,
-				      ioread32(&priv->regs_t->ctrl_cmd_size));
+	cmd_size = ioread32(&priv->regs_t->ctrl_cmd_size);
+
+	iores = NULL;
+	iobase_ptr = NULL;
+	for (i = 0; iores_array[i].end; ++i) {
+		if (cmd_pa >= iores_array[i].start &&
+		    cmd_pa <= iores_array[i].end) {
+			iores = iores_array + i;
+			iobase_ptr = iobase_array + i;
+			break;
+		}
+	}
+
+	if (iores)
+		cmd_size = crb_fixup_cmd_size(dev, iores, cmd_pa, cmd_size);
 
 	dev_dbg(dev, "cmd_hi = %X cmd_low = %X cmd_size %X\n",
 		pa_high, pa_low, cmd_size);
 
-	priv->cmd = crb_map_res(dev, priv, &io_res, cmd_pa, cmd_size);
+	priv->cmd = crb_map_res(dev, iores, iobase_ptr,	cmd_pa, cmd_size);
 	if (IS_ERR(priv->cmd)) {
 		ret = PTR_ERR(priv->cmd);
 		goto out;
@@ -566,11 +609,25 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 
 	memcpy_fromio(&__rsp_pa, &priv->regs_t->ctrl_rsp_pa, 8);
 	rsp_pa = le64_to_cpu(__rsp_pa);
-	rsp_size = crb_fixup_cmd_size(dev, &io_res, rsp_pa,
-				      ioread32(&priv->regs_t->ctrl_rsp_size));
+	rsp_size = ioread32(&priv->regs_t->ctrl_rsp_size);
+
+	iores = NULL;
+	iobase_ptr = NULL;
+	for (i = 0; resource_type(iores_array + i) == IORESOURCE_MEM; ++i) {
+		if (rsp_pa >= iores_array[i].start &&
+		    rsp_pa <= iores_array[i].end) {
+			iores = iores_array + i;
+			iobase_ptr = iobase_array + i;
+			break;
+		}
+	}
+
+	if (iores)
+		rsp_size = crb_fixup_cmd_size(dev, iores, rsp_pa, rsp_size);
 
 	if (cmd_pa != rsp_pa) {
-		priv->rsp = crb_map_res(dev, priv, &io_res, rsp_pa, rsp_size);
+		priv->rsp = crb_map_res(dev, iores, iobase_ptr,
+					rsp_pa, rsp_size);
 		ret = PTR_ERR_OR_ZERO(priv->rsp);
 		goto out;
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v8] tpm_crb: fix fTPM on AMD Zen+ CPUs
  2019-10-16 18:28 [PATCH v8] tpm_crb: fix fTPM on AMD Zen+ CPUs ivan.lazeev
@ 2019-10-21 15:57 ` Jarkko Sakkinen
  2019-10-23 11:51   ` Jarkko Sakkinen
  2019-10-25 14:13   ` Jarkko Sakkinen
  0 siblings, 2 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2019-10-21 15:57 UTC (permalink / raw)
  To: ivan.lazeev
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, linux-kernel

On Wed, Oct 16, 2019 at 09:28:14PM +0300, ivan.lazeev@gmail.com wrote:
> From: Ivan Lazeev <ivan.lazeev@gmail.com>
> 
> Bug link: https://bugzilla.kernel.org/show_bug.cgi?id=195657
> 
> cmd/rsp buffers are expected to be in the same ACPI region.
> For Zen+ CPUs BIOS's might report two different regions, some of
> them also report region sizes inconsistent with values from TPM
> registers.
> 
> Memory configuration on ASRock x470 ITX:
> 
> db0a0000-dc59efff : Reserved
>         dc57e000-dc57efff : MSFT0101:00
>         dc582000-dc582fff : MSFT0101:00
> 
> Work around the issue by storing ACPI regions declared for the
> device in a fixed array and adding an array for pointers to
> corresponding possibly allocated resources in crb_map_io function.
> This data was previously held for a single resource
> in struct crb_priv (iobase field) and local variable io_res in
> crb_map_io function. ACPI resources array is used to find index of
> corresponding region for each buffer and make the buffer size
> consistent with region's length. Array of pointers to allocated
> resources is used to map the region at most once.
> 
> Signed-off-by: Ivan Lazeev <ivan.lazeev@gmail.com>

Almost tested this today. Unfortunately the USB stick at hand was
broken.  I'll retry tomorrow or Wed depending on which day I visit at
the office and which day I WFH.

At least the AMI BIOS had all the TPM stuff in it. The hardware I'll be
using is Udoo Bolt V8 (thanks Jerry for pointing me out this device)
with AMD Ryzen Embedded V1605B [1]

Thanks for the patience with your patch.

[1] https://en.wikichip.org/wiki/amd/ryzen_embedded/v1605b

/Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v8] tpm_crb: fix fTPM on AMD Zen+ CPUs
  2019-10-21 15:57 ` Jarkko Sakkinen
@ 2019-10-23 11:51   ` Jarkko Sakkinen
  2019-10-23 23:20     ` Jerry Snitselaar
  2019-10-25 14:13   ` Jarkko Sakkinen
  1 sibling, 1 reply; 8+ messages in thread
From: Jarkko Sakkinen @ 2019-10-23 11:51 UTC (permalink / raw)
  To: ivan.lazeev, jsnitsel
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, linux-kernel

On Mon, Oct 21, 2019 at 06:57:35PM +0300, Jarkko Sakkinen wrote:
> Almost tested this today. Unfortunately the USB stick at hand was
> broken.  I'll retry tomorrow or Wed depending on which day I visit at
> the office and which day I WFH.
> 
> At least the AMI BIOS had all the TPM stuff in it. The hardware I'll be
> using is Udoo Bolt V8 (thanks Jerry for pointing me out this device)
> with AMD Ryzen Embedded V1605B [1]
> 
> Thanks for the patience with your patch.
> 
> [1] https://en.wikichip.org/wiki/amd/ryzen_embedded/v1605b

Jerry, are you confident to give this tested-by?

I'm still in process of finding what I should put to .config in order
to get USB keyboard working with UDOO BOLT.

/Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v8] tpm_crb: fix fTPM on AMD Zen+ CPUs
  2019-10-23 11:51   ` Jarkko Sakkinen
@ 2019-10-23 23:20     ` Jerry Snitselaar
  2019-10-24 15:57       ` Jerry Snitselaar
  2019-10-24 19:02       ` Jarkko Sakkinen
  0 siblings, 2 replies; 8+ messages in thread
From: Jerry Snitselaar @ 2019-10-23 23:20 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: ivan.lazeev, Peter Huewe, Jason Gunthorpe, Arnd Bergmann,
	Greg Kroah-Hartman, linux-integrity, linux-kernel

On Wed Oct 23 19, Jarkko Sakkinen wrote:
>On Mon, Oct 21, 2019 at 06:57:35PM +0300, Jarkko Sakkinen wrote:
>> Almost tested this today. Unfortunately the USB stick at hand was
>> broken.  I'll retry tomorrow or Wed depending on which day I visit at
>> the office and which day I WFH.
>>
>> At least the AMI BIOS had all the TPM stuff in it. The hardware I'll be
>> using is Udoo Bolt V8 (thanks Jerry for pointing me out this device)
>> with AMD Ryzen Embedded V1605B [1]
>>
>> Thanks for the patience with your patch.
>>
>> [1] https://en.wikichip.org/wiki/amd/ryzen_embedded/v1605b
>
>Jerry, are you confident to give this tested-by?
>
>I'm still in process of finding what I should put to .config in order
>to get USB keyboard working with UDOO BOLT.
>
>/Jarkko

I ran it through the tpm2 kselftests and it passed:

TAP version 13
1..2
# selftests: tpm2: test_smoke.sh
# test_read_partial_overwrite (tpm2_tests.SmokeTest) ... ok
# test_read_partial_resp (tpm2_tests.SmokeTest) ... ok
# test_seal_with_auth (tpm2_tests.SmokeTest) ... ok
# test_seal_with_policy (tpm2_tests.SmokeTest) ... ok
# test_seal_with_too_long_auth (tpm2_tests.SmokeTest) ... ok
# test_send_two_cmds (tpm2_tests.SmokeTest) ... ok
# test_too_short_cmd (tpm2_tests.SmokeTest) ... ok
# test_unseal_with_wrong_auth (tpm2_tests.SmokeTest) ... ok
# test_unseal_with_wrong_policy (tpm2_tests.SmokeTest) ... ok
#
# ----------------------------------------------------------------------
# Ran 9 tests in 12.305s
#
# OK
ok 1 selftests: tpm2: test_smoke.sh
# selftests: tpm2: test_space.sh
# test_flush_context (tpm2_tests.SpaceTest) ... ok
# test_get_handles (tpm2_tests.SpaceTest) ... ok
# test_invalid_cc (tpm2_tests.SpaceTest) ... ok
# test_make_two_spaces (tpm2_tests.SpaceTest) ... ok
#
# ----------------------------------------------------------------------
# Ran 4 tests in 11.355s
#
# OK
ok 2 selftests: tpm2: test_space.sh


I also did some other testing of tpm2-tools commands, creating a
trusted key and encrypted key, and running rngtest against /dev/random
with the current hwrng being tpm-rng-0.

I ran the selftests on an intel nuc as well:

TAP version 13
1..2
# selftests: tpm2: test_smoke.sh
# test_read_partial_overwrite (tpm2_tests.SmokeTest) ... ok
# test_read_partial_resp (tpm2_tests.SmokeTest) ... ok
# test_seal_with_auth (tpm2_tests.SmokeTest) ... ok
# test_seal_with_policy (tpm2_tests.SmokeTest) ... ok
# test_seal_with_too_long_auth (tpm2_tests.SmokeTest) ... ok
# test_send_two_cmds (tpm2_tests.SmokeTest) ... ok
# test_too_short_cmd (tpm2_tests.SmokeTest) ... ok
# test_unseal_with_wrong_auth (tpm2_tests.SmokeTest) ... ok
# test_unseal_with_wrong_policy (tpm2_tests.SmokeTest) ... ok
# 
# ----------------------------------------------------------------------
# Ran 9 tests in 29.620s
# 
# OK
ok 1 selftests: tpm2: test_smoke.sh
# selftests: tpm2: test_space.sh
# test_flush_context (tpm2_tests.SpaceTest) ... ok
# test_get_handles (tpm2_tests.SpaceTest) ... ok
# test_invalid_cc (tpm2_tests.SpaceTest) ... ok
# test_make_two_spaces (tpm2_tests.SpaceTest) ... ok
# 
# ----------------------------------------------------------------------
# Ran 4 tests in 26.337s
# 
# OK
ok 2 selftests: tpm2: test_space.sh


So,

Tested-by: Jerry Snitselaar <jsnitsel@redhat.com>



One thing I've noticed on the bolt and the nuc:

[    0.808935] tpm_tis MSFT0101:00: IRQ index 0 not found

I'm guessing this is Stefan's patches causing this?

1ea32c83c699 | 2019-09-02 | tpm_tis_core: Set TPM_CHIP_FLAG_IRQ before probing for interrupts (Stefan Berger)
5b359c7c4372 | 2019-09-02 | tpm_tis_core: Turn on the TPM before probing IRQ's (Stefan Berger)

I've never noticed tpm_tis messages before on a tpm_crb system, and doublechecked that I don't see it with 5.3.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v8] tpm_crb: fix fTPM on AMD Zen+ CPUs
  2019-10-23 23:20     ` Jerry Snitselaar
@ 2019-10-24 15:57       ` Jerry Snitselaar
  2019-10-24 19:15         ` Jarkko Sakkinen
  2019-10-24 19:02       ` Jarkko Sakkinen
  1 sibling, 1 reply; 8+ messages in thread
From: Jerry Snitselaar @ 2019-10-24 15:57 UTC (permalink / raw)
  To: Jarkko Sakkinen, ivan.lazeev, Peter Huewe, Jason Gunthorpe,
	Arnd Bergmann, Greg Kroah-Hartman, linux-integrity, linux-kernel

On Wed Oct 23 19, Jerry Snitselaar wrote:
>On Wed Oct 23 19, Jarkko Sakkinen wrote:
>>On Mon, Oct 21, 2019 at 06:57:35PM +0300, Jarkko Sakkinen wrote:
>>>Almost tested this today. Unfortunately the USB stick at hand was
>>>broken.  I'll retry tomorrow or Wed depending on which day I visit at
>>>the office and which day I WFH.
>>>
>>>At least the AMI BIOS had all the TPM stuff in it. The hardware I'll be
>>>using is Udoo Bolt V8 (thanks Jerry for pointing me out this device)
>>>with AMD Ryzen Embedded V1605B [1]
>>>
>>>Thanks for the patience with your patch.
>>>
>>>[1] https://en.wikichip.org/wiki/amd/ryzen_embedded/v1605b
>>
>>Jerry, are you confident to give this tested-by?
>>
>>I'm still in process of finding what I should put to .config in order
>>to get USB keyboard working with UDOO BOLT.
>>
>>/Jarkko
>
>I ran it through the tpm2 kselftests and it passed:
>
>TAP version 13
>1..2
># selftests: tpm2: test_smoke.sh
># test_read_partial_overwrite (tpm2_tests.SmokeTest) ... ok
># test_read_partial_resp (tpm2_tests.SmokeTest) ... ok
># test_seal_with_auth (tpm2_tests.SmokeTest) ... ok
># test_seal_with_policy (tpm2_tests.SmokeTest) ... ok
># test_seal_with_too_long_auth (tpm2_tests.SmokeTest) ... ok
># test_send_two_cmds (tpm2_tests.SmokeTest) ... ok
># test_too_short_cmd (tpm2_tests.SmokeTest) ... ok
># test_unseal_with_wrong_auth (tpm2_tests.SmokeTest) ... ok
># test_unseal_with_wrong_policy (tpm2_tests.SmokeTest) ... ok
>#
># ----------------------------------------------------------------------
># Ran 9 tests in 12.305s
>#
># OK
>ok 1 selftests: tpm2: test_smoke.sh
># selftests: tpm2: test_space.sh
># test_flush_context (tpm2_tests.SpaceTest) ... ok
># test_get_handles (tpm2_tests.SpaceTest) ... ok
># test_invalid_cc (tpm2_tests.SpaceTest) ... ok
># test_make_two_spaces (tpm2_tests.SpaceTest) ... ok
>#
># ----------------------------------------------------------------------
># Ran 4 tests in 11.355s
>#
># OK
>ok 2 selftests: tpm2: test_space.sh
>
>
>I also did some other testing of tpm2-tools commands, creating a
>trusted key and encrypted key, and running rngtest against /dev/random
>with the current hwrng being tpm-rng-0.
>
>I ran the selftests on an intel nuc as well:
>
>TAP version 13
>1..2
># selftests: tpm2: test_smoke.sh
># test_read_partial_overwrite (tpm2_tests.SmokeTest) ... ok
># test_read_partial_resp (tpm2_tests.SmokeTest) ... ok
># test_seal_with_auth (tpm2_tests.SmokeTest) ... ok
># test_seal_with_policy (tpm2_tests.SmokeTest) ... ok
># test_seal_with_too_long_auth (tpm2_tests.SmokeTest) ... ok
># test_send_two_cmds (tpm2_tests.SmokeTest) ... ok
># test_too_short_cmd (tpm2_tests.SmokeTest) ... ok
># test_unseal_with_wrong_auth (tpm2_tests.SmokeTest) ... ok
># test_unseal_with_wrong_policy (tpm2_tests.SmokeTest) ... ok
># # 
>----------------------------------------------------------------------
># Ran 9 tests in 29.620s
># # OK
>ok 1 selftests: tpm2: test_smoke.sh
># selftests: tpm2: test_space.sh
># test_flush_context (tpm2_tests.SpaceTest) ... ok
># test_get_handles (tpm2_tests.SpaceTest) ... ok
># test_invalid_cc (tpm2_tests.SpaceTest) ... ok
># test_make_two_spaces (tpm2_tests.SpaceTest) ... ok
># # 
>----------------------------------------------------------------------
># Ran 4 tests in 26.337s
># # OK
>ok 2 selftests: tpm2: test_space.sh
>
>
>So,
>
>Tested-by: Jerry Snitselaar <jsnitsel@redhat.com>
>
>
>
>One thing I've noticed on the bolt and the nuc:
>
>[    0.808935] tpm_tis MSFT0101:00: IRQ index 0 not found
>
>I'm guessing this is Stefan's patches causing this?
>
>1ea32c83c699 | 2019-09-02 | tpm_tis_core: Set TPM_CHIP_FLAG_IRQ before probing for interrupts (Stefan Berger)
>5b359c7c4372 | 2019-09-02 | tpm_tis_core: Turn on the TPM before probing IRQ's (Stefan Berger)
>
>I've never noticed tpm_tis messages before on a tpm_crb system, and doublechecked that I don't see it with 5.3.

I see there is a patch from Hans de Goede already dealing with this: "tpm: Switch to platform_get_irq_optional()".


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v8] tpm_crb: fix fTPM on AMD Zen+ CPUs
  2019-10-23 23:20     ` Jerry Snitselaar
  2019-10-24 15:57       ` Jerry Snitselaar
@ 2019-10-24 19:02       ` Jarkko Sakkinen
  1 sibling, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2019-10-24 19:02 UTC (permalink / raw)
  To: ivan.lazeev, Peter Huewe, Jason Gunthorpe, Arnd Bergmann,
	Greg Kroah-Hartman, linux-integrity, linux-kernel

On Wed, Oct 23, 2019 at 04:20:35PM -0700, Jerry Snitselaar wrote:
> On Wed Oct 23 19, Jarkko Sakkinen wrote:
> > On Mon, Oct 21, 2019 at 06:57:35PM +0300, Jarkko Sakkinen wrote:
> > > Almost tested this today. Unfortunately the USB stick at hand was
> > > broken.  I'll retry tomorrow or Wed depending on which day I visit at
> > > the office and which day I WFH.
> > > 
> > > At least the AMI BIOS had all the TPM stuff in it. The hardware I'll be
> > > using is Udoo Bolt V8 (thanks Jerry for pointing me out this device)
> > > with AMD Ryzen Embedded V1605B [1]
> > > 
> > > Thanks for the patience with your patch.
> > > 
> > > [1] https://en.wikichip.org/wiki/amd/ryzen_embedded/v1605b
> > 
> > Jerry, are you confident to give this tested-by?
> > 
> > I'm still in process of finding what I should put to .config in order
> > to get USB keyboard working with UDOO BOLT.
> > 
> > /Jarkko
> 
> I ran it through the tpm2 kselftests and it passed:
> 
> TAP version 13
> 1..2
> # selftests: tpm2: test_smoke.sh
> # test_read_partial_overwrite (tpm2_tests.SmokeTest) ... ok
> # test_read_partial_resp (tpm2_tests.SmokeTest) ... ok
> # test_seal_with_auth (tpm2_tests.SmokeTest) ... ok
> # test_seal_with_policy (tpm2_tests.SmokeTest) ... ok
> # test_seal_with_too_long_auth (tpm2_tests.SmokeTest) ... ok
> # test_send_two_cmds (tpm2_tests.SmokeTest) ... ok
> # test_too_short_cmd (tpm2_tests.SmokeTest) ... ok
> # test_unseal_with_wrong_auth (tpm2_tests.SmokeTest) ... ok
> # test_unseal_with_wrong_policy (tpm2_tests.SmokeTest) ... ok
> #
> # ----------------------------------------------------------------------
> # Ran 9 tests in 12.305s
> #
> # OK
> ok 1 selftests: tpm2: test_smoke.sh
> # selftests: tpm2: test_space.sh
> # test_flush_context (tpm2_tests.SpaceTest) ... ok
> # test_get_handles (tpm2_tests.SpaceTest) ... ok
> # test_invalid_cc (tpm2_tests.SpaceTest) ... ok
> # test_make_two_spaces (tpm2_tests.SpaceTest) ... ok
> #
> # ----------------------------------------------------------------------
> # Ran 4 tests in 11.355s
> #
> # OK
> ok 2 selftests: tpm2: test_space.sh
> 
> 
> I also did some other testing of tpm2-tools commands, creating a
> trusted key and encrypted key, and running rngtest against /dev/random
> with the current hwrng being tpm-rng-0.
> 
> I ran the selftests on an intel nuc as well:
> 
> TAP version 13
> 1..2
> # selftests: tpm2: test_smoke.sh
> # test_read_partial_overwrite (tpm2_tests.SmokeTest) ... ok
> # test_read_partial_resp (tpm2_tests.SmokeTest) ... ok
> # test_seal_with_auth (tpm2_tests.SmokeTest) ... ok
> # test_seal_with_policy (tpm2_tests.SmokeTest) ... ok
> # test_seal_with_too_long_auth (tpm2_tests.SmokeTest) ... ok
> # test_send_two_cmds (tpm2_tests.SmokeTest) ... ok
> # test_too_short_cmd (tpm2_tests.SmokeTest) ... ok
> # test_unseal_with_wrong_auth (tpm2_tests.SmokeTest) ... ok
> # test_unseal_with_wrong_policy (tpm2_tests.SmokeTest) ... ok
> # # ----------------------------------------------------------------------
> # Ran 9 tests in 29.620s
> # # OK
> ok 1 selftests: tpm2: test_smoke.sh
> # selftests: tpm2: test_space.sh
> # test_flush_context (tpm2_tests.SpaceTest) ... ok
> # test_get_handles (tpm2_tests.SpaceTest) ... ok
> # test_invalid_cc (tpm2_tests.SpaceTest) ... ok
> # test_make_two_spaces (tpm2_tests.SpaceTest) ... ok
> # # ----------------------------------------------------------------------
> # Ran 4 tests in 26.337s
> # # OK
> ok 2 selftests: tpm2: test_space.sh
> 
> 
> So,
> 
> Tested-by: Jerry Snitselaar <jsnitsel@redhat.com>
> 
> 
> 
> One thing I've noticed on the bolt and the nuc:
> 
> [    0.808935] tpm_tis MSFT0101:00: IRQ index 0 not found
> 
> I'm guessing this is Stefan's patches causing this?
> 
> 1ea32c83c699 | 2019-09-02 | tpm_tis_core: Set TPM_CHIP_FLAG_IRQ before probing for interrupts (Stefan Berger)
> 5b359c7c4372 | 2019-09-02 | tpm_tis_core: Turn on the TPM before probing IRQ's (Stefan Berger)
> 
> I've never noticed tpm_tis messages before on a tpm_crb system, and doublechecked that I don't see it with 5.3.

I'd guess it is related to:

https://patchwork.kernel.org/patch/11200049/

Thank you for the tested-by. I pushed this now. I'll try to get also
my tested-by before sending the PR (still fighting to find correct
kernel config to enable USB keyboard with UDOO BOLT).

/Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v8] tpm_crb: fix fTPM on AMD Zen+ CPUs
  2019-10-24 15:57       ` Jerry Snitselaar
@ 2019-10-24 19:15         ` Jarkko Sakkinen
  0 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2019-10-24 19:15 UTC (permalink / raw)
  To: ivan.lazeev, Peter Huewe, Jason Gunthorpe, Arnd Bergmann,
	Greg Kroah-Hartman, linux-integrity, linux-kernel

On Thu, Oct 24, 2019 at 08:57:08AM -0700, Jerry Snitselaar wrote:
> I see there is a patch from Hans de Goede already dealing with this:
> "tpm: Switch to platform_get_irq_optional()".

Sorry did not notice this before sending my response. Yes, Hans' patch
will fix this glitch.

/Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v8] tpm_crb: fix fTPM on AMD Zen+ CPUs
  2019-10-21 15:57 ` Jarkko Sakkinen
  2019-10-23 11:51   ` Jarkko Sakkinen
@ 2019-10-25 14:13   ` Jarkko Sakkinen
  1 sibling, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2019-10-25 14:13 UTC (permalink / raw)
  To: ivan.lazeev
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, linux-kernel

On Mon, Oct 21, 2019 at 06:57:35PM +0300, Jarkko Sakkinen wrote:
> On Wed, Oct 16, 2019 at 09:28:14PM +0300, ivan.lazeev@gmail.com wrote:
> > From: Ivan Lazeev <ivan.lazeev@gmail.com>
> > 
> > Bug link: https://bugzilla.kernel.org/show_bug.cgi?id=195657
> > 
> > cmd/rsp buffers are expected to be in the same ACPI region.
> > For Zen+ CPUs BIOS's might report two different regions, some of
> > them also report region sizes inconsistent with values from TPM
> > registers.
> > 
> > Memory configuration on ASRock x470 ITX:
> > 
> > db0a0000-dc59efff : Reserved
> >         dc57e000-dc57efff : MSFT0101:00
> >         dc582000-dc582fff : MSFT0101:00
> > 
> > Work around the issue by storing ACPI regions declared for the
> > device in a fixed array and adding an array for pointers to
> > corresponding possibly allocated resources in crb_map_io function.
> > This data was previously held for a single resource
> > in struct crb_priv (iobase field) and local variable io_res in
> > crb_map_io function. ACPI resources array is used to find index of
> > corresponding region for each buffer and make the buffer size
> > consistent with region's length. Array of pointers to allocated
> > resources is used to map the region at most once.
> > 
> > Signed-off-by: Ivan Lazeev <ivan.lazeev@gmail.com>
> 
> Almost tested this today. Unfortunately the USB stick at hand was
> broken.  I'll retry tomorrow or Wed depending on which day I visit at
> the office and which day I WFH.
> 
> At least the AMI BIOS had all the TPM stuff in it. The hardware I'll be
> using is Udoo Bolt V8 (thanks Jerry for pointing me out this device)
> with AMD Ryzen Embedded V1605B [1]
> 
> Thanks for the patience with your patch.
> 
> [1] https://en.wikichip.org/wiki/amd/ryzen_embedded/v1605b

Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Heh, the problem with my kernel config was that I didn't have xhci
(USB3) my kernel config :-) Never needed that one before when testing
TPM changes.

/Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-10-25 14:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-16 18:28 [PATCH v8] tpm_crb: fix fTPM on AMD Zen+ CPUs ivan.lazeev
2019-10-21 15:57 ` Jarkko Sakkinen
2019-10-23 11:51   ` Jarkko Sakkinen
2019-10-23 23:20     ` Jerry Snitselaar
2019-10-24 15:57       ` Jerry Snitselaar
2019-10-24 19:15         ` Jarkko Sakkinen
2019-10-24 19:02       ` Jarkko Sakkinen
2019-10-25 14:13   ` Jarkko Sakkinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).