[PATCH v8] tpm_crb: fix fTPM on AMD Zen+ CPUs
From: Ivan Lazeev
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:
db0a-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
---
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, )) {
- *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, _res))
+ if (!iores)
return devm_ioremap_resource(dev, _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 = >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
[PATCH v7] tpm_crb: fix fTPM on AMD Zen+ CPUs
From: Ivan Lazeev
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:
db0a-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
---
Changes in v7:
- use terminator entry in iores_array
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..49d142721b11 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, )) {
- *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, _res))
+ if (!iores)
return devm_ioremap_resource(dev, _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] = {0};
struct device *dev = >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();
- ret = acpi_dev_get_resources(device, , crb_check_resource,
-_res);
+ INIT_LIST_HEAD(_resource_list);
+ ret = acpi_dev_get_resources(device, _resource_list,
+crb_check_resource, iores_array);
if (ret < 0)
return ret;
- acpi_dev_free_resource_list();
+ acpi_dev_free_resource_list
[PATCH v6] tpm_crb: fix fTPM on AMD Zen+ CPUs
From: Vanya Lazeev
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:
db0a-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
---
Changes in v6:
- got rid of new structures
- open coded helper functions
- removed incorrect FW_BUG
drivers/char/tpm/tpm_crb.c | 126 +++--
1 file changed, 93 insertions(+), 33 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index e59f1f91d7f3..8177406aecd6 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_range = data;
struct resource_win win;
struct resource *res = &(win.res);
if (acpi_dev_resource_memory(ares, res) ||
acpi_dev_resource_address_space(ares, )) {
- *io_res = *res;
- io_res->name = NULL;
+ if (iores_range[0] == iores_range[1])
+ iores_range[0] = NULL;
+
+ if (iores_range[0]) {
+ *iores_range[0] = *res;
+ iores_range[0]->name = NULL;
+ iores_range[0] += 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 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, _res))
+ if (!iores)
return devm_ioremap_resource(dev, _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,17 @@ 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];
+ void __iomem *iobase_array[TPM_CRB_MAX_RESOURCES] = {0};
+ struct resource *iores_range[] = {
+ iores_array,
+ iores_array + TPM_CRB_MAX_RESOURCES
+ };
struct device *dev = >dev;
+ struct resource *iores;
+ void __iomem **iobase_ptr;
+ int num_resources, i;
u32 pa_high, pa_low;
u64 cmd_pa;
u32 cmd_size;
@@ -501,21 +521,40 @@ static int crb_map_io(struct acpi_device *device, struct
crb_priv *priv,
u32 rsp_size;
int ret;
- INIT_LIST_HEAD();
- ret = acpi_dev_get_resources(device, , crb_check_resource,
-_res);
+ INIT_LIST_HEAD(_resource_list);
+ ret = acpi_dev_get_resources(device, _resource_list,
+crb_check_r
[PATCH v5] tpm_crb: fix fTPM on AMD Zen+ CPUs
From: Ivan Lazeev
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:
db0a-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
---
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(>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, )) {
- *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, _res))
+ if (res_idx < 0)
return devm_ioremap_resource(dev, _res);
- return priv->iobase + (new_res.start - io_res->start);
+ iores = >iores[res_idx];
+ iobase = resources->iobase[res_idx];
+ if (!iobase) {
+ iobase = devm_iore
[PATCH v4] tpm_crb: fix fTPM on AMD Zen+ CPUs
From: Ivan Lazeev
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:
db0a-dc59efff : Reserved
dc57e000-dc57efff : MSFT0101:00
dc582000-dc582fff : MSFT0101:00
Work around the issue by storing ACPI regions declared for the
device in a list of struct crb_resource. Each entry holds a
copy of the correcponding ACPI resourse (iores field) and a pointer
to a possibly allocated with devm_ioremap_resource memory region
(iobase 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. The list is used to find corresponding
region for each buffer with crb_containing_resource, make
the buffer size consistent with it's length and map it at most
once, storing the pointer to allocated resource in iobase field
of the entry.
Signed-off-by: Ivan Lazeev
---
Changes in v3:
- make crb_containing_resource search for address only,
because buffer sizes aren't trusted anyway
- improve commit message
Changes in v4:
- rename struct crb_resource fields (style change)
- improve commit message
I believe that storing the data in a in list of
struct crb_resource makes tracking of the resource allocation
state explicit, aiding clarity.
Whilst everything that worked before seems not to be broken,
there is a possibility of allocating with crb_map_resource a resource
that is not from ACPI table, and state of such resource is not
tracked in the current solution. It might be good to track allocation
of all resources, not just ones declared by ACPI, for complete
correctness. However, as I see it now, it will complicate the
code a bit more. Do you think the change should be made, or
such situation is completely hypothetical?
drivers/char/tpm/tpm_crb.c | 137 +++--
1 file changed, 101 insertions(+), 36 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index e59f1f91d7f3..b301f7fc4a73 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -91,7 +91,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 +107,12 @@ struct tpm2_crb_smc {
u32 smc_func_id;
};
+struct crb_resource {
+ struct resource iores;
+ void __iomem *iobase;
+ struct list_head list;
+};
+
static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
unsigned long timeout)
{
@@ -432,23 +437,57 @@ static const struct tpm_class_ops tpm_crb = {
.req_complete_val = CRB_DRV_STS_COMPLETE,
};
+static void crb_free_resource_list(struct list_head *resources)
+{
+ struct crb_resource *cres, *tmp;
+
+ list_for_each_entry_safe(cres, tmp, resources, list)
+ kfree(cres);
+}
+
+static inline bool crb_resource_contains(const struct resource *io_res,
+u64 address)
+{
+ return address >= io_res->start && address <= io_res->end;
+}
+
+static struct crb_resource *crb_containing_resource(
+ const struct list_head *resources, u64 start)
+{
+ struct crb_resource *cres;
+
+ list_for_each_entry(cres, resources, list) {
+ if (crb_resource_contains(>iores, start))
+ return cres;
+ }
+
+ return NULL;
+}
+
static int crb_check_resource(struct acpi_resource *ares, void *data)
{
- struct resource *io_res = data;
+ struct list_head *list = data;
+ struct crb_resource *cres;
struct resource_win win;
struct resource *res = &(win.res);
if (acpi_dev_resource_memory(ares, res) ||
acpi_dev_resource_address_space(ares, )) {
- *io_res = *res;
- io_res->name = NULL;
+ cres = kzalloc(sizeof(*cres), GFP_KERNEL);
+ if (!cres)
+ return -ENOMEM;
+
+ cres->iores = *res;
+ cres->iores.name = NULL;
+
+ list_add_tail(>list, list);
}
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 crb_resource *cres,
+u64 start, u32 size)
{
struct resource new_res = {
.start = start,
@@ -460,10 +499,16 @@ static void __iomem *crb_map_res(struct device *dev,
struct crb_priv *p
[PATCH] tpm_crb: fix fTPM on AMD Zen+ CPUs
From: Ivan Lazeev
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.
Work around the issue by storing ACPI regions declared for the
device in a list, then using it to find entry corresponding
to each buffer. Use information from the entry to map each resource
at most once and make buffer size consistent with the length of the
region.
Signed-off-by: Ivan Lazeev
---
Tested on ASRock x470 ITX motherboard with Ryzen 2600X CPU.
However, I don't have any other hardware to test the changes on and no
expertise to be sure that other TPMs won't break as a result.
drivers/char/tpm/tpm_crb.c | 137 +++--
1 file changed, 101 insertions(+), 36 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index e59f1f91d7f3..0bcfe52db5d6 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -91,7 +91,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 +107,12 @@ struct tpm2_crb_smc {
u32 smc_func_id;
};
+struct crb_resource {
+ struct resource io_res;
+ void __iomem *iobase;
+ struct list_head node;
+};
+
static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
unsigned long timeout)
{
@@ -432,23 +437,57 @@ static const struct tpm_class_ops tpm_crb = {
.req_complete_val = CRB_DRV_STS_COMPLETE,
};
+static void crb_free_resource_list(struct list_head *resources)
+{
+ struct crb_resource *cres, *tmp;
+
+ list_for_each_entry_safe(cres, tmp, resources, node)
+ kfree(cres);
+}
+
+static inline bool crb_resource_contains(const struct resource *io_res,
+u64 address)
+{
+ return address >= io_res->start && address <= io_res->end;
+}
+
+static struct crb_resource *crb_containing_resource(
+ const struct list_head *resources, u64 start)
+{
+ struct crb_resource *cres;
+
+ list_for_each_entry(cres, resources, node) {
+ if (crb_resource_contains(>io_res, start))
+ return cres;
+ }
+
+ return NULL;
+}
+
static int crb_check_resource(struct acpi_resource *ares, void *data)
{
- struct resource *io_res = data;
+ struct list_head *list = data;
+ struct crb_resource *cres;
struct resource_win win;
struct resource *res = &(win.res);
if (acpi_dev_resource_memory(ares, res) ||
acpi_dev_resource_address_space(ares, )) {
- *io_res = *res;
- io_res->name = NULL;
+ cres = kzalloc(sizeof(*cres), GFP_KERNEL);
+ if (!cres)
+ return -ENOMEM;
+
+ cres->io_res = *res;
+ cres->io_res.name = NULL;
+
+ list_add_tail(>node, list);
}
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 crb_resource *cres,
+u64 start, u32 size)
{
struct resource new_res = {
.start = start,
@@ -460,10 +499,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, _res))
+ if (!cres)
return devm_ioremap_resource(dev, _res);
- return priv->iobase + (new_res.start - io_res->start);
+ if (!cres->iobase) {
+ cres->iobase = devm_ioremap_resource(dev, >io_res);
+ if (IS_ERR(cres->iobase))
+ return cres->iobase;
+ }
+
+ return cres->iobase + (new_res.start - cres->io_res.start);
}
/*
@@ -490,9 +535,9 @@ 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, crb_resources;
struct device *dev = >dev;
+ struct crb_resource *cres;
u32 pa_high, pa_low;
u64 cmd_pa;
u32 cmd_size;
@@ -501,21 +546,34 @@ static int crb_map_io(struct acpi_device *device, struct
crb_priv *priv,
u32 rsp_size;
int ret;
- INIT_LIST_HEAD
[PATCH v2] Fix fTPM on AMD Zen+ CPUs
From: Vanya Lazeev
The patch is an attempt to make fTPM on AMD Zen CPUs work.
Bug link: https://bugzilla.kernel.org/show_bug.cgi?id=195657
The problem seems to be that tpm_crb driver doesn't expect tpm command
and response memory regions to belong to different ACPI resources.
Tested on Asrock ITX motherboard with Ryzen 2600X CPU.
However, I don't have any other hardware to test the changes on and no
expertise to be sure that other TPMs won't break as a result.
Hopefully, the patch will be useful.
Changes from v1:
- use list_for_each_safe
Signed-off-by: Vanya Lazeev
---
drivers/char/tpm/tpm_crb.c | 146 -
1 file changed, 110 insertions(+), 36 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index e59f1f91d..b0e797464 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -91,7 +91,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 +107,13 @@ struct tpm2_crb_smc {
u32 smc_func_id;
};
+struct crb_resource {
+ struct resource io_res;
+ void __iomem *iobase;
+
+ struct list_head link;
+};
+
static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
unsigned long timeout)
{
@@ -432,23 +438,69 @@ static const struct tpm_class_ops tpm_crb = {
.req_complete_val = CRB_DRV_STS_COMPLETE,
};
+static void crb_free_resource_list(struct list_head *resources)
+{
+ struct list_head *position, *tmp;
+
+ list_for_each_safe(position, tmp, resources)
+ kfree(list_entry(position, struct crb_resource, link));
+}
+
+/**
+ * Checks if resource @io_res contains range with the specified @start and
@size
+ * completely or, when @strict is false, at least it's beginning.
+ * Non-strict match is needed to work around broken BIOSes that return
+ * inconsistent values from ACPI regions vs registers.
+ */
+static inline bool crb_resource_contains(const struct resource *io_res,
+u64 start, u32 size, bool strict)
+{
+ return start >= io_res->start &&
+ (start + size - 1 <= io_res->end ||
+(!strict && start <= io_res->end));
+}
+
+static struct crb_resource *crb_containing_resource(
+ const struct list_head *resources,
+ u64 start, u32 size, bool strict)
+{
+ struct list_head *pos;
+
+ list_for_each(pos, resources) {
+ struct crb_resource *cres;
+
+ cres = list_entry(pos, struct crb_resource, link);
+ if (crb_resource_contains(>io_res, start, size, strict))
+ return cres;
+ }
+
+ return NULL;
+}
+
static int crb_check_resource(struct acpi_resource *ares, void *data)
{
- struct resource *io_res = data;
+ struct list_head *list = data;
+ struct crb_resource *cres;
struct resource_win win;
struct resource *res = &(win.res);
if (acpi_dev_resource_memory(ares, res) ||
acpi_dev_resource_address_space(ares, )) {
- *io_res = *res;
- io_res->name = NULL;
+ cres = kzalloc(sizeof(*cres), GFP_KERNEL);
+ if (!cres)
+ return -ENOMEM;
+
+ cres->io_res = *res;
+ cres->io_res.name = NULL;
+
+ list_add_tail(>link, list);
}
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 crb_resource *cres,
+u64 start, u32 size)
{
struct resource new_res = {
.start = start,
@@ -460,10 +512,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, _res))
+ if (!cres)
return devm_ioremap_resource(dev, _res);
- return priv->iobase + (new_res.start - io_res->start);
+ if (!cres->iobase) {
+ cres->iobase = devm_ioremap_resource(dev, >io_res);
+ if (IS_ERR(cres->iobase))
+ return cres->iobase;
+ }
+
+ return cres->iobase + (new_res.start - cres->io_res.start);
}
/*
@@ -490,9 +548,9 @@ 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, crb_resources;
[PATCH] Fix fTPM on AMD Zen+ CPUs
From: Vanya Lazeev
The patch is an attempt to make fTPM on AMD Zen CPUs work.
Bug link: https://bugzilla.kernel.org/show_bug.cgi?id=195657
The problem seems to be that tpm_crb driver doesn't expect tpm command
and response memory regions to belong to different ACPI resources.
Tested on Asrock ITX motherboard with Ryzen 2600X CPU.
However, I don't have any other hardware to test the changes on and no
expertise to be sure that other TPMs won't break as a result.
Hopefully, the patch will be useful.
Signed-off-by: Vanya Lazeev
---
drivers/char/tpm/tpm_crb.c | 146 -
1 file changed, 110 insertions(+), 36 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index e59f1f91d..73e4b9105 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -91,7 +91,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 +107,13 @@ struct tpm2_crb_smc {
u32 smc_func_id;
};
+struct crb_resource {
+ struct resource io_res;
+ void __iomem *iobase;
+
+ struct list_head link;
+};
+
static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
unsigned long timeout)
{
@@ -432,23 +438,69 @@ static const struct tpm_class_ops tpm_crb = {
.req_complete_val = CRB_DRV_STS_COMPLETE,
};
+static void crb_free_resource_list(struct list_head *resources)
+{
+ struct list_head *position;
+
+ list_for_each(position, resources)
+ kfree(list_entry(position, struct crb_resource, link));
+}
+
+/**
+ * Checks if resource @io_res contains range with the specified @start and
@size
+ * completely or, when @strict is false, at least it's beginning.
+ * Non-strict match is needed to work around broken BIOSes that return
+ * inconsistent values from ACPI regions vs registers.
+ */
+static inline bool crb_resource_contains(const struct resource *io_res,
+u64 start, u32 size, bool strict)
+{
+ return start >= io_res->start &&
+ (start + size - 1 <= io_res->end ||
+(!strict && start <= io_res->end));
+}
+
+static struct crb_resource *crb_containing_resource(
+ const struct list_head *resources,
+ u64 start, u32 size, bool strict)
+{
+ struct list_head *pos;
+
+ list_for_each(pos, resources) {
+ struct crb_resource *cres;
+
+ cres = list_entry(pos, struct crb_resource, link);
+ if (crb_resource_contains(>io_res, start, size, strict))
+ return cres;
+ }
+
+ return NULL;
+}
+
static int crb_check_resource(struct acpi_resource *ares, void *data)
{
- struct resource *io_res = data;
+ struct list_head *list = data;
+ struct crb_resource *cres;
struct resource_win win;
struct resource *res = &(win.res);
if (acpi_dev_resource_memory(ares, res) ||
acpi_dev_resource_address_space(ares, )) {
- *io_res = *res;
- io_res->name = NULL;
+ cres = kzalloc(sizeof(*cres), GFP_KERNEL);
+ if (!cres)
+ return -ENOMEM;
+
+ cres->io_res = *res;
+ cres->io_res.name = NULL;
+
+ list_add_tail(>link, list);
}
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 crb_resource *cres,
+u64 start, u32 size)
{
struct resource new_res = {
.start = start,
@@ -460,10 +512,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, _res))
+ if (!cres)
return devm_ioremap_resource(dev, _res);
- return priv->iobase + (new_res.start - io_res->start);
+ if (!cres->iobase) {
+ cres->iobase = devm_ioremap_resource(dev, >io_res);
+ if (IS_ERR(cres->iobase))
+ return cres->iobase;
+ }
+
+ return cres->iobase + (new_res.start - cres->io_res.start);
}
/*
@@ -490,9 +548,9 @@ 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, crb_resources;
struct device *dev = >dev;
+ struct crb_resource
