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 <[email protected]>
---
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
On Mon, Aug 26, 2019 at 02:40:19AM +0900, Seunghun Han wrote:
> 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 <[email protected]>
You need to split this into multiple patches e.g. if you think you've
fixed a bug, please write a patch with just the bug fix and nothing
else.
For further information, read the section three of
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
I'd also recommend to check out the earlier discussion on ACPI NVS:
https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035EF7BC7@ALPMBAPA12.e2k.ad.ge.com/
/Jarkko
>
> On Mon, Aug 26, 2019 at 02:40:19AM +0900, Seunghun Han wrote:
> > 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 <[email protected]>
>
> You need to split this into multiple patches e.g. if you think you've
> fixed a bug, please write a patch with just the bug fix and nothing
> else.
>
> For further information, read the section three of
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
> I'd also recommend to check out the earlier discussion on ACPI NVS:
>
> https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035EF7BC7@ALPMBAPA12.e2k.ad.ge.com/
>
> /Jarkko
Thank you for your advice. I have made two separated patches on your advice.
Please check these patches, https://lkml.org/lkml/2019/8/26/125 and
https://lkml.org/lkml/2019/8/26/163.
In my opinion, the last link you gave me had two problems with AMD's
fTPM. One problem was the ACPI NVS area was set to the busy area, and
TPM regions of the ACPI table were in it. Therefore, TPM CRB driver
couldn't allocate command and response buffers in it because ACPI NVS
area was busy. The other problem was buffer size calculation bugs.
Because of it, TPM CRB driver requested larger than the size ACPI
table described. So, TPM CRB driver also couldn't map command and
response buffers even though the reserved area was not busy.
It seems that my separated patches could handle those two problems and
enable AMD's fTPM in any case.
Seunghun
> From: [email protected] <linux-integrity-
> [email protected]> On Behalf Of Jarkko Sakkinen
> Sent: Monday, August 26, 2019 1:59 AM
> To: Seunghun Han <[email protected]>
> Cc: Peter Huewe <[email protected]>; Thomas Gleixner
> <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: EXT: Re: [PATCH] tpm: tpm_crb: Add an AMD fTPM support feature
>
> On Mon, Aug 26, 2019 at 02:40:19AM +0900, Seunghun Han wrote:
> > 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.
The problem is that the acpi tables are _wrong_ in this and other cases.
They not only incorrectly report the area as reserved, but also report
the command and response buffer sizes incorrectly. If you look at
the addresses for the buffers listed in the crb control area, the sizes
are correct (4Kbytes). My patch uses the control area values, and
everything works. The remaining problem is that if acpi reports the
area as NVS, then the linux nvs driver will try to use the space.
I'm looking at how to fix that. I'm not sure, if simply removing
the busy bit is sufficient.
Dave
> >
> > Signed-off-by: Seunghun Han <[email protected]>
>
> You need to split this into multiple patches e.g. if you think you've fixed a
> bug, please write a patch with just the bug fix and nothing else.
>
> For further information, read the section three of
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
> I'd also recommend to check out the earlier discussion on ACPI NVS:
>
> https://lore.kernel.org/linux-
> integrity/[email protected]
> 2k.ad.ge.com/
>
> /Jarkko
> > From: [email protected] <linux-integrity-
> > [email protected]> On Behalf Of Jarkko Sakkinen
> > Sent: Monday, August 26, 2019 1:59 AM
> > To: Seunghun Han <[email protected]>
> > Cc: Peter Huewe <[email protected]>; Thomas Gleixner
> > <[email protected]>; [email protected]; linux-
> > [email protected]
> > Subject: EXT: Re: [PATCH] tpm: tpm_crb: Add an AMD fTPM support feature
> >
> > On Mon, Aug 26, 2019 at 02:40:19AM +0900, Seunghun Han wrote:
> > > 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.
>
> The problem is that the acpi tables are _wrong_ in this and other cases.
> They not only incorrectly report the area as reserved, but also report
> the command and response buffer sizes incorrectly. If you look at
> the addresses for the buffers listed in the crb control area, the sizes
> are correct (4Kbytes). My patch uses the control area values, and
> everything works. The remaining problem is that if acpi reports the
> area as NVS, then the linux nvs driver will try to use the space.
> I'm looking at how to fix that. I'm not sure, if simply removing
> the busy bit is sufficient.
> Dave
Thank you for your reply. I have read your patch. However, I would
like to solve this problem without a kernel parameter. In my view, I'd
better change the crb_fixup_cmd_size() function because the TPM CRB
driver wants to get the command buffer and response buffer sizes by
using the function.
I agree on that removing the busy bit is sufficient.
Seunghun
>
> > >
> > > Signed-off-by: Seunghun Han <[email protected]>
> >
> > You need to split this into multiple patches e.g. if you think you've fixed a
> > bug, please write a patch with just the bug fix and nothing else.
> >
> > For further information, read the section three of
> >
> > https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> >
> > I'd also recommend to check out the earlier discussion on ACPI NVS:
> >
> > https://lore.kernel.org/linux-
> > integrity/[email protected]
> > 2k.ad.ge.com/
> >
> > /Jarkko