[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 2/2] acpi: Add TPM2 interface definition.


  • To: Jennifer Herbert <jennifer.herbert@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 2 May 2023 15:41:15 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=X1EDzMdMtmgMtrxFZhOwm06NyGinNaKMmyaW0YO/3FU=; b=J5viAt7NWkC8Syr5cuYg36snwqYtH+X2zwFtjEyY06ay6TQjLkbXhL+4ybMvw5GQ2xKM8WHAeM00cmgLX+O4j7GszJgAtBFlWERWDKeQUeHyILgDthFrgPW8/YXP5DdLnddI4UOFBI1lfO+o+F7phh3yCei8oFlOU0xig1zM0VGFgajjsNvCln309494znxEoraT1gMdKbuaELNpoEWvyju6KKwrHJgPh61jm2mq5/A9x1gr+U6s6ich1o9D5uMx4CM46gpN+dubw7/mPiyRH799IQGLIskYBeqeJWDaSU/C15rRaLjTjY6peAlM0uxTZeIRC0jU1FUbOoZZ/oga/w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TSycrKs8lQBKsfgyb991ekFsr2SrL15ZgbVkZ3jHHTdDClpyiKPhuyjagJB1kRquYw1WldgMrojryzDz8yrhJsZE1SyaKiqVHaaZ1OVU2WJkrNwE2H3XKEUFbIiA5zMOIenTKhlearc/b0CJTPq4XAHCHh1NHEQETom91+0WgQKM/yY1UlGKG/Jo7sROucHsEu+I4gaDQir0VocYthp02QB3iD7zw12UsvjYEa9tkraVYs5ikqe0Z81qdPKDWqBVyYsx61gsHqe58SQ7t0smHtFL5KtbwAN9UcDv5ZM6Q6VL9HXo6dBhAYZnzLBNMxaD0oMOsIyiWqu92JO0D1PWew==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 02 May 2023 13:41:21 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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, ...

> +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.

> @@ -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"?

> @@ -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.

> --- /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.

Then on top of Jason's R-b,
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.