-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of Li, Fei1
Sent: Wednesday, September 29, 2021 3:42 am
To: Liu, Yifan1 <yifan1.liu@...>
Cc: acrn-dev@...; VanCutsem, Geoffroy
<geoffroy.vancutsem@...>
Subject: Re: [acrn-dev] [PATCH] dm: TPM2 passthrough for post-launched
VM with eventlog support

On Wed, Sep 29, 2021 at 09:17:33AM +0800, Liu, Yifan1 wrote:

-----Original Message-----
From: VanCutsem, Geoffroy <geoffroy.vancutsem@...>
Sent: Tuesday, September 28, 2021 5:15 PM
To: Liu, Yifan1 <yifan1.liu@...>;
acrn-dev@...
Subject: RE: [acrn-dev] [PATCH] dm: TPM2 passthrough for
post-launched VM with eventlog support

-----Original Message-----
From: Liu, Yifan1 <yifan1.liu@...>
Sent: Tuesday, September 28, 2021 4:01 am
To: acrn-dev@...
Cc: VanCutsem, Geoffroy <geoffroy.vancutsem@...>
Subject: RE: [acrn-dev] [PATCH] dm: TPM2 passthrough for
post-launched VM with eventlog support

-----Original Message-----
From: VanCutsem, Geoffroy <geoffroy.vancutsem@...>
Sent: Monday, September 27, 2021 11:22 PM
To: acrn-dev@...
Cc: Liu, Yifan1 <yifan1.liu@...>
Subject: RE: [acrn-dev] [PATCH] dm: TPM2 passthrough for
post-launched VM with eventlog support

-----Original Message-----
From: acrn-dev@...
<acrn-dev@...> On Behalf Of Liu, Yifan1
Sent: Monday, September 27, 2021 8:34 am
To: acrn-dev@...
Cc: Liu, Yifan1 <yifan1.liu@...>
Subject: [acrn-dev] [PATCH] dm: TPM2 passthrough for
post-launched VM with eventlog support

From: Yifan Liu <yifan1.liu@...>

This patch enables TPM2 passthrough to post-launched VM with
eventlog support.
User starts by providing command line "--acpidev_pt
tpm2:<TPM2_HID>", of

Does this introduce a new format for this command-line argument?
Our

documentation indicates this as an example for it:

--acpidev_pt <HID>

The leading "tpm2:" seems new. Why is it introduced?

[Liu, Yifan1] The original <HID> implicitly means TPM2 device HID
as we don't support any other kind of ACPI devices.
I can go with <HID> and add "tpm2:" later when we actually
supports any other kind of ACPI devices, or I can add it now.

Ah, I did not know we only supported TPM2 devices at the moment.
Will we need to add a prefix when we start supporting more ACPI device

types? Or could the HID alone be sufficient?

My take on this is the following:
* If the HID alone is sufficient, let's not add a prefix ever
* If a prefix is *absolutely* necessary when we add support for more
ACPI device types, then let's add it now (and update our documentation

accordingly).

[Liu, Yifan1] I think it's necessary as different types of ACPI devices requires

different pass-through logic. Li Fei, Could you please also comment?

IMHO, the ACPI device type would simplfy our code a lot. Take TPM2 for
example, if we only add the HID, we need to maintain an array to transfer
HID to its ACPI device type.
Because from the HID, we can't know its ACPI device type.

Oh, I did not know that! Are we able to catch a mismatch between the device type and HID if the user mixed them up?

It sounds like adding the prefix is the best approach then.

If we need to
support a new HID TPM2 device, we need to update the array now and then.
However, if we specify the ACPI device type, we could support any TPM2
devices whose start method meets our requirements without any code
change.

From the user side, they know the ACPI device type first before they know
the HID.

So I suggest we should add the ACPI device type now.

thanks.

Thanks,
Geoffroy

which the <TPM2_HID> will be searched from /proc/iomem for
TPM2 buffer start address and size. Furthermore, If TPM2
eventlog is supported,
TPM2 eventlog information will be retrieved from sysfs TPM2
table and passed-through as well.

Signed-off-by: Yifan Liu <yifan1.liu@...>
---
devicemodel/hw/mmio/core.c | 171

++++++++++++++++++++++++----

devicemodel/hw/platform/acpi/acpi.c | 6 +-
devicemodel/hw/platform/tpm/tpm.c | 20 +++-
devicemodel/include/acpi.h | 11 ++
devicemodel/include/mmio_dev.h | 2 +-
devicemodel/include/tpm.h | 3 +
6 files changed, 189 insertions(+), 24 deletions(-)

diff --git a/devicemodel/hw/mmio/core.c

b/devicemodel/hw/mmio/core.c

index 0fa4b50f1..d30f8e13b 100644
--- a/devicemodel/hw/mmio/core.c
+++ b/devicemodel/hw/mmio/core.c
@@ -29,6 +29,7 @@ struct mmio_dev { };

#define MAX_MMIO_DEV_NUM 2
+#define TPM2_TABLE_NATIVE_FILE_PATH_IN_SOS
"/sys/firmware/acpi/tables/TPM2"

static struct mmio_dev mmio_devs[MAX_MMIO_DEV_NUM];

static

uint32_t mmio_dev_idx = 0U; @@ -62,6 +63,150 @@ int
mmio_dev_alloc_gpa_resource32(uint32_t *addr, uint32_t size_in)
}
}

+static inline char *ltrim(char *s) {
+ while (*s && *s == ' ')
+ s++;
+ return s;
+}
+
+static inline int checksum(uint8_t *buf, size_t size) {
+ size_t i;
+ uint8_t sum = 0;
+ for (i = 0; i < size; i++)
+ sum += buf[i];
+ return sum == 0;
+}
+
+/**
+ * Parse /proc/iomem to see if there's an entry that contains

name.

+ * Returns false if not found,
+ * Returns true if an entry is found, and res_start and
+res_size will be

filled

+ * to the start and (end - start + 1) of the first found entry.
+ *
+ * @pre (name != NULL) && (strlen(name) > 0) */ static int
+search_proc_iomem(char *name, uint64_t *res_start, uint64_t
+*res_size) {
+ FILE *fp;
+ uint64_t start, end;
+ int found = false;
+ char line[128];
+ char *cp;
+
+ fp = fopen("/proc/iomem", "r");
+ if (!fp) {
+ pr_err("Error opening /proc/iomem\n");
+ return found;
+ }
+
+ while (fgets(line, sizeof(line), fp)) {
+ if (strstr(line, name)) {
+ if ((!dm_strtoul(ltrim(line), &cp, 16,
+ &start) && *cp
== '-') &&
+ (!dm_strtoul(cp + 1, &cp, 16, &end))) {
+ if ((start == 0) && (end == 0)) {
+ pr_err("Please run
+ acrn-dm with
super user privilege\n");
+ break;
+ }
+ }
+
+ *res_start = start;
+ /* proc/iomem displays regions like:
+ 000-fff so we
add 1 as size */
+ *res_size = end - start + 1;
+ found = true;
+ break;
+ }
+ }
+
+ fclose(fp);
+ return found;
+}
+
+static int read_sysfs_tpm2_table(struct acpi_table_tpm2

*tpm2_out) {

+ FILE *fp;
+ size_t ch_read;
+ struct acpi_table_tpm2 tpm2 = { 0 };
+
+ fp = fopen(TPM2_TABLE_NATIVE_FILE_PATH_IN_SOS, "r");
+ if (!fp)
+ return -errno;
+
+ ch_read = fread(&tpm2, 1, sizeof(tpm2), fp);
+ fclose(fp);
+
+ /* we may read less than sizeof(tpm2) as laml and lasa
+are optional
*/
+ if (!ch_read)
+ return -errno;
+
+ if (strncmp(tpm2.header.signature, "TPM2", 4) ||
+ !checksum((uint8_t *)&tpm2, tpm2.header.length))
+ return -EINVAL;
+
+ memcpy(tpm2_out, &tpm2, ch_read);
+
+ return 0;
+}
+
+static int is_tpm2_eventlog_supported(struct acpi_table_tpm2

*tpm2) {

+ /* Per TCG ACPI spec ver 1.2 rev 8, if LAML and LASA
+field are
present, length is 76 */
+ return ((tpm2->header.length == 76) && tpm2->lasa &&
+ tpm2-

laml); }

+
+/**
+ * Add TPM2 device with HID hid to mmio_devs to be passed-

through.

+If eventlog
+ * is also supported, it will be added to the second resource
+of the TPM2
mmio_dev.
+ *
+ * @pre: hid should be a valid HID of the TPM2 device being
+ passed-
through.
+ */
+static int prepare_tpm2_pt(char *hid) {
+ int err = 0;
+ uint64_t tpm2_buffer_hpa, tpm2_buffer_size;
+ uint32_t base = 0;
+ struct acpi_table_tpm2 tpm2 = { 0 };
+
+ /* TODO: Currently we did not validate if the hid is a valid one.
+ * We trust it to be valid as specifying --acpidev_pt is regarded
+ * as root user operation.
+ */
+ if (!hid || !*hid)
+ return -EINVAL;
+
+ /* parse /proc/iomem to find the address and size of tpm

buffer */

+ if (!search_proc_iomem(hid, &tpm2_buffer_hpa,
&tpm2_buffer_size))
+ return -ENODEV;
+
+ err = read_sysfs_tpm2_table(&tpm2);
+ if (err)
+ return err;
+
+ if ((tpm2.start_method != 6) && (tpm2.start_method != 7)) {
+ pr_err("TPM2 start method %d not
+ supported.\n",
tpm2.start_method);
+ return -EINVAL;
+ }
+
+ strncpy(mmio_devs[mmio_dev_idx].name, "MSFT0101", 8);
+ strncpy(mmio_devs[mmio_dev_idx].dev.name, "tpm2", 4);
+ mmio_devs[mmio_dev_idx].dev.res[0].host_pa =
tpm2_buffer_hpa;
+ mmio_devs[mmio_dev_idx].dev.res[0].user_vm_pa =
tpm2_buffer_hpa;
+ mmio_devs[mmio_dev_idx].dev.res[0].size =
+ tpm2_buffer_size;
+
+ /* Search for eventlog */
+ if (is_tpm2_eventlog_supported(&tpm2) &&
+ !mmio_dev_alloc_gpa_resource32(&base, tpm2.laml)) {
+ mmio_devs[mmio_dev_idx].dev.res[1].host_pa =

tpm2.lasa;

+ mmio_devs[mmio_dev_idx].dev.res[1].user_vm_pa =

base;

+ mmio_devs[mmio_dev_idx].dev.res[1].size = tpm2.laml;
+ }
+
+ mmio_dev_idx++;
+
+ return 0;
+}
+
int parse_pt_acpidev(char *opt) {
int err = 0;
@@ -70,21 +215,11 @@ int parse_pt_acpidev(char *opt)
pr_err("MMIO dev number exceed
MAX_MMIO_DEV_NUM!!!\n");
return -EINVAL;
}
- /* TODO: support acpi dev framework, remove these TPM hard

code

*/
- if (strncmp(opt, "MSFT0101", 8) == 0) {
- strncpy(mmio_devs[mmio_dev_idx].name, "MSFT0101",

8);

-
- strncpy(mmio_devs[mmio_dev_idx].dev.name, "tpm2", 4);
- /* TODO: We would parse the /proc/iomem to get the
corresponding resources */
- mmio_devs[mmio_dev_idx].dev.res[0].host_pa =
0xFED40000UL;
- /* FIXME: The 0xFED40000 doesn't conflict with other mmio
or system memory so far.
- * This need to be fixed by redesign the
mmio_dev_alloc_gpa_resource32().
- */
- mmio_devs[mmio_dev_idx].dev.res[0].user_vm_pa =
0xFED40000UL;
- mmio_devs[mmio_dev_idx].dev.res[0].size =

0x00005000UL;

- mmio_dev_idx++;
+
+ opt = strstr(opt, "tpm2:");
+ /* Format opt: --acpidev_pt tpm2:<HID> */
+ if (opt && ((err = prepare_tpm2_pt(opt + 5)) == 0))
pt_tpm2 = true;
- }

return err;
}
@@ -215,19 +350,17 @@ struct mmio_dev_ops tpm2 = {
DEFINE_MMIO_DEV(tpm2);

/* @pre (pt_tpm2 == true) */
-uint64_t get_mmio_dev_tpm2_base_gpa(void)
+struct acrn_mmiodev *get_tpm2_mmio_dev(void)
{
int i;
- uint64_t base_gpa = 0UL;

for (i = 0; i < mmio_dev_idx; i++) {
if (!strcmp(mmio_devs[i].name, "MSFT0101")) {
- base_gpa = mmio_devs[i].dev.res[0].user_vm_pa;
- break;
+ return &mmio_devs[i].dev;
}
}

- return base_gpa;
+ return NULL;
}

struct mmio_dev_ops pt_mmiodev = { diff --git
a/devicemodel/hw/platform/acpi/acpi.c
b/devicemodel/hw/platform/acpi/acpi.c
index 12214a850..39f1386a2 100644
--- a/devicemodel/hw/platform/acpi/acpi.c
+++ b/devicemodel/hw/platform/acpi/acpi.c
@@ -807,8 +807,8 @@ basl_fwrite_tpm2(FILE *fp, struct vmctx

*ctx)

EFPRINTF(fp, "[0004]\t\tStart Method : 00000007\n");

EFPRINTF(fp, "[0012]\t\tMethod Parameters : 00 00 00
00 00 00 00
00
00 00 00 00\n");
- EFPRINTF(fp, "[0004]\t\tMinimum Log Length : 00000000\n");
- EFPRINTF(fp, "[0008]\t\tLog Address : 0000000000000000\n");
+ EFPRINTF(fp, "[0004]\t\tMinimum Log Length : %08X\n",
get_tpm2_table_minimal_log_length());
+ EFPRINTF(fp, "[0008]\t\tLog Address : %016lX\n",
+get_tpm2_table_log_address());

EFFLUSH(fp);

@@ -895,7 +895,7 @@ static void tpm2_crb_fwrite_dsdt(void)
dsdt_line(" {");
dsdt_indent(4);
/* TODO: consider a better framework for mmio likes
pci's vdev_write_dsdt. */
- dsdt_fixed_mem32(get_tpm_crb_mmio_addr(),
TPM_CRB_MMIO_SIZE);
+ dsdt_fixed_mem32(get_tpm_crb_mmio_addr(),
get_tpm_crb_mmio_size());
dsdt_unindent(4);
dsdt_line(" })");
dsdt_line(" Method (_STA, 0, NotSerialized) // _STA:

Status");

diff --git a/devicemodel/hw/platform/tpm/tpm.c
b/devicemodel/hw/platform/tpm/tpm.c
index 6a656c0f5..5afdc78e5 100644
--- a/devicemodel/hw/platform/tpm/tpm.c
+++ b/devicemodel/hw/platform/tpm/tpm.c
@@ -37,7 +37,8 @@ uint32_t get_tpm_crb_mmio_addr(void)
uint32_t base;

if (pt_tpm2) {
- base = (uint32_t)get_mmio_dev_tpm2_base_gpa();
+ struct acrn_mmiodev *d = get_tpm2_mmio_dev();
+ base = d ? (uint32_t)d->res[0].host_pa : 0U;
} else {
base = get_vtpm_crb_mmio_addr();
}
@@ -45,6 +46,23 @@ uint32_t get_tpm_crb_mmio_addr(void)
return base;
}

+uint32_t get_tpm_crb_mmio_size(void) {
+ struct acrn_mmiodev *d = get_tpm2_mmio_dev();
+ return (pt_tpm2 && d) ? d->res[0].size :
+TPM_CRB_MMIO_SIZE; }
+
+uint32_t get_tpm2_table_minimal_log_length(void)
+{
+ struct acrn_mmiodev *d = get_tpm2_mmio_dev();
+ return (pt_tpm2 && d && d->res[1].host_pa) ?
+d->res[1].size : 0U; }
+
+uint64_t get_tpm2_table_log_address(void) {
+ struct acrn_mmiodev *d = get_tpm2_mmio_dev();
+ return (pt_tpm2 && d && d->res[1].host_pa) ? d-

res[1].user_vm_pa :

+0UL; }

enum {
SOCK_PATH_OPT = 0
diff --git a/devicemodel/include/acpi.h
b/devicemodel/include/acpi.h index 4bf14b77e..7a11bc2da 100644
--- a/devicemodel/include/acpi.h
+++ b/devicemodel/include/acpi.h
@@ -62,6 +62,17 @@ struct acpi_table_hdr {
uint32_t asl_compiler_revision;
} __attribute__((packed));

+struct acpi_table_tpm2 {
+ struct acpi_table_hdr header;
+ uint16_t platform_class;
+ uint16_t reserved;
+ uint64_t control_address;
+ uint32_t start_method;
+ uint8_t start_method_spec_para[12];
+ uint32_t laml;
+ uint64_t lasa;
+} __attribute__((packed));
+
/* All dynamic table entry no. */
#define NHLT_ENTRY_NO 8

diff --git a/devicemodel/include/mmio_dev.h
b/devicemodel/include/mmio_dev.h index 51dc58660..417b5f8ce
100644
--- a/devicemodel/include/mmio_dev.h
+++ b/devicemodel/include/mmio_dev.h
@@ -15,7 +15,7 @@ int init_mmio_devs(struct vmctx *ctx); void
deinit_mmio_devs(struct vmctx *ctx);

int mmio_dev_alloc_gpa_resource32(uint32_t *addr, uint32_t
size_in); - uint64_t get_mmio_dev_tpm2_base_gpa(void);
+struct acrn_mmiodev *get_tpm2_mmio_dev(void);

#define MMIO_DEV_BASE 0xF0000000U #define

MMIO_DEV_LIMIT

0xFE000000U diff --git a/devicemodel/include/tpm.h
b/devicemodel/include/tpm.h index
82b948141..f06f6ca14 100644
--- a/devicemodel/include/tpm.h
+++ b/devicemodel/include/tpm.h
@@ -13,6 +13,9 @@

uint32_t get_vtpm_crb_mmio_addr(void); uint32_t
get_tpm_crb_mmio_addr(void);
+uint32_t get_tpm_crb_mmio_size(void); uint32_t
+get_tpm2_table_minimal_log_length(void);
+uint64_t get_tpm2_table_log_address(void);

/* TPM CRB registers */
enum {
--
2.25.1