On 02/05/2023 14:41, Jan Beulich wrote:
On 25.04.2023 19:47, Jennifer Herbert wrote:
--- a/tools/libacpi/acpi2_0.h
+++ b/tools/libacpi/acpi2_0.h
@@ -121,6 +121,36 @@ struct acpi_20_tcpa {
  };
  #define ACPI_2_0_TCPA_LAML_SIZE (64*1024)
+/*
+ * TPM2
+ */
Nit: While I'm willing to accept the comment style violation here as
(apparently) intentional, ...

Well, I was trying to keep the file consistant.   As far as I can tell, this styling is used thoughout the file - unless I'm misunderstanding your 'Nit'. (You object to a multi-line coment used for a single line? )
But I'm codes style blind, so just say how you want it.


+struct acpi_20_tpm2 {
+    struct acpi_header header;
+    uint16_t platform_class;
+    uint16_t reserved;
+    uint64_t control_area_address;
+    uint32_t start_method;
+    uint8_t start_method_params[12];
+    uint32_t log_area_minimum_length;
+    uint64_t log_area_start_address;
+};
+#define TPM2_ACPI_CLASS_CLIENT      0
+#define TPM2_START_METHOD_CRB       7
+
+/* TPM register I/O Mapped region, location of which defined in the
+ * TCG PC Client Platform TPM Profile Specification for TPM 2.0.
+ * See table 9 - Only Locality 0 is used here. This is emulated by QEMU.
+ * Definition of Register space is found in table 12.
+ */
... this comment wants adjusting to hypervisor style (/* on its own line),
as that looks to be the aimed-at style in this file.

Will do.


@@ -352,6 +353,7 @@ static int construct_secondary_tables(struct acpi_ctxt 
*ctxt,
      struct acpi_20_tcpa *tcpa;
      unsigned char *ssdt;
      void *lasa;
+    struct acpi_20_tpm2 *tpm2;
Could I talk you into moving this up by two lines, such that it'll be
adjacent to "tcpa"?


No problem.


@@ -450,6 +452,43 @@ static int construct_secondary_tables(struct acpi_ctxt 
*ctxt,
                               tcpa->header.length);
              }
              break;
+
+        case 2:
+            /* Check VID stored in bits 37:32 (3rd 16 bit word) of CRB
+             * identifier register.  See table 16 of TCG PC client platform
+             * TPM profile specification for TPM 2.0.
+             */
Nit: This comment again wants a style adjustment.

ok


--- /dev/null
+++ b/tools/libacpi/ssdt_tpm2.asl
@@ -0,0 +1,36 @@
+/*
+ * ssdt_tpm2.asl
+ *
+ * Copyright (c) 2018-2022, Citrix Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
While the full conversion to SPDX was done in the hypervisor only so far,
I think new tool stack source files would better use the much shorter
SPDX equivalent, too.

OK, this is where I get a bit confused.  I belive I copied the licence from ssdt_tpm.asl, for consistancy.

So I think i need to use a 'LGPL-2.1-only' but then it says its using exceptions on linking as discribed in LICENSE, but um, which LICENSE file?  So i'm not sure what exception I should be adding. Do you know?


Then on top of Jason's R-b,
Acked-by: Jan Beulich <jbeul...@suse.com>

Jan


Thanks,

-jenny


Reply via email to