I'm Seunghun Han and work at the Affiliated Institute of ETRI. I got an AMD
system which had a Ryzen Threadripper 1950X and MSI mainboard, and I had
a problem with AMD's fTPM. My machine showed an error message below, and
the fTPM didn't work because of it.

[    5.732084] tpm_crb MSFT0101:00: can't request region for resource
               [mem 0x79b4f000-0x79b4ffff]
[    5.732089] tpm_crb: probe of MSFT0101:00 failed with error -16

When I saw the iomem areas and found two TPM CRB regions were in the ACPI
NVS area.  The iomem regions are below.

79a39000-79b6afff : ACPI Non-volatile Storage
  79b4b000-79b4bfff : MSFT0101:00
  79b4f000-79b4ffff : MSFT0101:00

After analyzing this issue, I found out that a busy bit was set to the ACPI
NVS area, and the current Linux kernel allowed nothing to be assigned in
it. I also found that the kernel couldn't calculate the sizes of command
and response buffers correctly when the TPM regions were two or more.

To support AMD's fTPM, I removed the busy bit from the ACPI NVS area
so that AMD's fTPM regions could be assigned in it. I also fixed the bug
that did not calculate the sizes of command and response buffer correctly.

Signed-off-by: Seunghun Han <kkama...@gmail.com>
---
 arch/x86/kernel/e820.c     |  2 +-
 drivers/char/tpm/tpm_crb.c | 50 ++++++++++++++++++++++++++------------
 2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 7da2bcd2b8eb..79a99249b46f 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1085,11 +1085,11 @@ static bool __init do_mark_busy(enum e820_type type, 
struct resource *res)
        case E820_TYPE_RESERVED:
        case E820_TYPE_PRAM:
        case E820_TYPE_PMEM:
+       case E820_TYPE_NVS:
                return false;
        case E820_TYPE_RESERVED_KERN:
        case E820_TYPE_RAM:
        case E820_TYPE_ACPI:
-       case E820_TYPE_NVS:
        case E820_TYPE_UNUSABLE:
        default:
                return true;
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index e59f1f91d7f3..d970a2289def 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -442,6 +442,9 @@ static int crb_check_resource(struct acpi_resource *ares, 
void *data)
            acpi_dev_resource_address_space(ares, &win)) {
                *io_res = *res;
                io_res->name = NULL;
+
+               /* Add this TPM CRB resource to the list. */
+               return 0;
        }
 
        return 1;
@@ -471,20 +474,30 @@ static void __iomem *crb_map_res(struct device *dev, 
struct crb_priv *priv,
  * region vs the registers. Trust the ACPI region. Such broken systems
  * probably cannot send large TPM commands since the buffer will be truncated.
  */
-static u64 crb_fixup_cmd_size(struct device *dev, struct resource *io_res,
+static u64 crb_fixup_cmd_size(struct device *dev, struct list_head *resources,
                              u64 start, u64 size)
 {
-       if (io_res->start > start || io_res->end < start)
-               return size;
+       struct resource_entry *pos;
+       struct resource *cur_res;
+
+       /* Check all TPM CRB resources with the start and size values. */
+       resource_list_for_each_entry(pos, resources) {
+               cur_res = pos->res;
+
+               if (cur_res->start > start || cur_res->end < start)
+                       continue;
 
-       if (start + size - 1 <= io_res->end)
-               return size;
+               if (start + size - 1 <= cur_res->end)
+                       return size;
 
-       dev_err(dev,
-               FW_BUG "ACPI region does not cover the entire command/response 
buffer. %pr vs %llx %llx\n",
-               io_res, start, size);
+               dev_err(dev,
+                       FW_BUG "ACPI region does not cover the entire 
command/response buffer. %pr vs %llx %llx\n",
+                       cur_res, start, size);
+
+               return cur_res->end - start + 1;
+       }
 
-       return io_res->end - start + 1;
+       return size;
 }
 
 static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
@@ -506,16 +519,18 @@ static int crb_map_io(struct acpi_device *device, struct 
crb_priv *priv,
                                     &io_res);
        if (ret < 0)
                return ret;
-       acpi_dev_free_resource_list(&resources);
 
        if (resource_type(&io_res) != IORESOURCE_MEM) {
                dev_err(dev, FW_BUG "TPM2 ACPI table does not define a memory 
resource\n");
-               return -EINVAL;
+               ret = -EINVAL;
+               goto out_early;
        }
 
        priv->iobase = devm_ioremap_resource(dev, &io_res);
-       if (IS_ERR(priv->iobase))
-               return PTR_ERR(priv->iobase);
+       if (IS_ERR(priv->iobase)) {
+               ret = PTR_ERR(priv->iobase);
+               goto out_early;
+       }
 
        /* 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
@@ -532,7 +547,7 @@ static int crb_map_io(struct acpi_device *device, struct 
crb_priv *priv,
 
        ret = __crb_request_locality(dev, priv, 0);
        if (ret)
-               return ret;
+               goto out_early;
 
        priv->regs_t = crb_map_res(dev, priv, &io_res, buf->control_address,
                                   sizeof(struct crb_regs_tail));
@@ -552,7 +567,7 @@ 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,
+       cmd_size = crb_fixup_cmd_size(dev, &resources, cmd_pa,
                                      ioread32(&priv->regs_t->ctrl_cmd_size));
 
        dev_dbg(dev, "cmd_hi = %X cmd_low = %X cmd_size %X\n",
@@ -566,7 +581,7 @@ 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,
+       rsp_size = crb_fixup_cmd_size(dev, &resources, rsp_pa,
                                      ioread32(&priv->regs_t->ctrl_rsp_size));
 
        if (cmd_pa != rsp_pa) {
@@ -596,6 +611,9 @@ static int crb_map_io(struct acpi_device *device, struct 
crb_priv *priv,
 
        __crb_relinquish_locality(dev, priv, 0);
 
+out_early:
+       acpi_dev_free_resource_list(&resources);
+
        return ret;
 }
 
-- 
2.21.0

Reply via email to