All of lore.kernel.org
 help / color / mirror / Atom feed
From: ivan.lazeev@gmail.com
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Peter Huewe <peterhuewe@gmx.de>, Jason Gunthorpe <jgg@ziepe.ca>,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
	Ivan Lazeev <ivan.lazeev@gmail.com>
Subject: [PATCH v5] tpm_crb: fix fTPM on AMD Zen+ CPUs
Date: Thu, 26 Sep 2019 00:56:46 +0300	[thread overview]
Message-ID: <20190925215646.24844-1-ivan.lazeev@gmail.com> (raw)

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 struct crb_resources. It contains fixed arrays of
copies of ACPI resourses (iores field) and pointers
to a possibly allocated with devm_ioremap_resource memory region
(iobase field), along with number of entries (num field).
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 with crb_containing_res_idx,
make the buffer size consistent with region's length and map it at
most once, storing pointer to the allocated resource in iobase
array at the same index.

Signed-off-by: Ivan Lazeev <ivan.lazeev@gmail.com>
---

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

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

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index e59f1f91d7f3..3c2485c71260 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;
@@ -108,6 +108,12 @@ struct tpm2_crb_smc {
 	u32 smc_func_id;
 };
 
+struct tpm_crb_resources {
+	struct resource iores[TPM_CRB_MAX_RESOURCES];
+	void __iomem *iobase[TPM_CRB_MAX_RESOURCES];
+	int num;
+};
+
 static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
 				unsigned long timeout)
 {
@@ -432,38 +438,75 @@ static const struct tpm_class_ops tpm_crb = {
 	.req_complete_val = CRB_DRV_STS_COMPLETE,
 };
 
+static bool tpm_crb_resource_contains(struct resource *iores,
+				      u64 address)
+{
+	return address >= iores->start && address <= iores->end;
+}
+
+static int tpm_crb_containing_res_idx(struct tpm_crb_resources *resources,
+				      u64 address)
+{
+	int res_idx;
+
+	for (res_idx = 0; res_idx < resources->num; ++res_idx) {
+		if (tpm_crb_resource_contains(&resources->iores[res_idx],
+					      address))
+			return res_idx;
+	}
+
+	return -1;
+}
+
 static int crb_check_resource(struct acpi_resource *ares, void *data)
 {
-	struct resource *io_res = data;
+	struct tpm_crb_resources *resources = data;
 	struct resource_win win;
 	struct resource *res = &(win.res);
 
 	if (acpi_dev_resource_memory(ares, res) ||
 	    acpi_dev_resource_address_space(ares, &win)) {
-		*io_res = *res;
-		io_res->name = NULL;
+		if (resources->num < TPM_CRB_MAX_RESOURCES) {
+			resources->iores[resources->num] = *res;
+			resources->iores[resources->num].name = NULL;
+		}
+		resources->num += 1;
 	}
 
 	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 tpm_crb_resources *resources,
+				 int res_idx,
+				 u64 start, u32 size)
 {
 	struct resource new_res = {
 		.start	= start,
 		.end	= start + size - 1,
 		.flags	= IORESOURCE_MEM,
 	};
+	struct resource *iores;
+	void __iomem *iobase;
 
 	/* Detect a 64 bit address on a 32 bit system */
 	if (start != new_res.start)
 		return (void __iomem *) ERR_PTR(-EINVAL);
 
-	if (!resource_contains(io_res, &new_res))
+	if (res_idx < 0)
 		return devm_ioremap_resource(dev, &new_res);
 
-	return priv->iobase + (new_res.start - io_res->start);
+	iores = &resources->iores[res_idx];
+	iobase = resources->iobase[res_idx];
+	if (!iobase) {
+		iobase = devm_ioremap_resource(dev, iores);
+		if (IS_ERR(iobase))
+			return iobase;
+
+		resources->iobase[res_idx] = iobase;
+	}
+
+	return iobase + (new_res.start - iores->start);
 }
 
 /*
@@ -490,9 +533,10 @@ 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_resources;
 	struct device *dev = &device->dev;
+	struct tpm_crb_resources resources;
+	int res_idx;
 	u32 pa_high, pa_low;
 	u64 cmd_pa;
 	u32 cmd_size;
@@ -501,21 +545,36 @@ 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_resources);
+	memset(&resources, 0, sizeof(resources));
+	ret = acpi_dev_get_resources(device, &acpi_resources,
+				     crb_check_resource, &resources);
 	if (ret < 0)
 		return ret;
-	acpi_dev_free_resource_list(&resources);
+	acpi_dev_free_resource_list(&acpi_resources);
 
-	if (resource_type(&io_res) != IORESOURCE_MEM) {
+	if (resources.num == 0) {
 		dev_err(dev, FW_BUG "TPM2 ACPI table does not define a memory resource\n");
 		return -EINVAL;
+	} else if (resources.num > TPM_CRB_MAX_RESOURCES) {
+		dev_warn(dev, FW_BUG "TPM2 ACPI table defines too many memory resources\n");
+		resources.num = TPM_CRB_MAX_RESOURCES;
 	}
 
-	priv->iobase = devm_ioremap_resource(dev, &io_res);
-	if (IS_ERR(priv->iobase))
-		return PTR_ERR(priv->iobase);
+	res_idx = tpm_crb_containing_res_idx(&resources, buf->control_address);
+
+	if (res_idx >= 0 &&
+	    !tpm_crb_resource_contains(&resources.iores[res_idx],
+				       buf->control_address +
+				       sizeof(struct crb_regs_tail) - 1))
+		res_idx = -1;
+
+	priv->regs_t = crb_map_res(dev, &resources, res_idx,
+				   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 +582,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 (res_idx >= 0 &&
+		    buf->control_address == resources.iores[res_idx].start +
 		    sizeof(*priv->regs_h))
-			priv->regs_h = priv->iobase;
+			priv->regs_h = resources.iobase[res_idx];
 		else
 			dev_warn(dev, FW_BUG "Bad ACPI memory layout");
 	}
@@ -534,13 +594,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 +605,17 @@ 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);
+
+	res_idx = tpm_crb_containing_res_idx(&resources, cmd_pa);
+	if (res_idx >= 0)
+		cmd_size = crb_fixup_cmd_size(dev, &resources.iores[res_idx],
+					      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, &resources, res_idx, cmd_pa, cmd_size);
 	if (IS_ERR(priv->cmd)) {
 		ret = PTR_ERR(priv->cmd);
 		goto out;
@@ -566,11 +623,17 @@ 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);
+
+	res_idx = tpm_crb_containing_res_idx(&resources, rsp_pa);
+
+	if (res_idx >= 0)
+		rsp_size = crb_fixup_cmd_size(dev, &resources.iores[res_idx],
+					      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, &resources, res_idx,
+					rsp_pa, rsp_size);
 		ret = PTR_ERR_OR_ZERO(priv->rsp);
 		goto out;
 	}
-- 
2.20.1


             reply	other threads:[~2019-09-25 22:01 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-25 21:56 ivan.lazeev [this message]
2019-09-27 14:56 ` [PATCH v5] tpm_crb: fix fTPM on AMD Zen+ CPUs Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190925215646.24844-1-ivan.lazeev@gmail.com \
    --to=ivan.lazeev@gmail.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jgg@ziepe.ca \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterhuewe@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.