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

Re: [PATCH v3 1/2] acpi: Make TPM version configurable.


  • To: Jason Andryuk <jandryuk@xxxxxxxxx>
  • From: Jennifer Herbert <jennifer.herbert@xxxxxxxxxx>
  • Date: Thu, 27 Apr 2023 13:48:55 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.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=hWKRVpndLVHinVcMqV1UFuUnAHGkarNRH1V6BXiQHHY=; b=Lf0oG7BpSaxXsSSaRCdQABq+VpHZgT6H6Aud6lHrFc6vflGKQhZWXgbDlI84s5lOGVhqD9cXWYisqe8z8aWVPdhi3aZZqMyKaCSqfX5X+ak3Wl8z0Cr1YJLJpXQuYi0v1kZTQn+s2oAP3L4xZb3E0/safIzfKUQF44H1uGBULvcmtTEZ/n/YtJDjK+0mukBVrpU5G3ONyVWUQuirQxxq95NfuvSRCr0AqB0SOC1ka1UUvdW1IJfbsSPS7qaZosUJjBsCXbEfdClsChpwNAfqMRq/rl9TqQME5/dMwDvCLnHSvIy87UN7oC1txkyovKGjp5U3jYpd484784u6F/5qhg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=oIWum3x6Mp+b/9/kZCaK9Kccn7Eyx/p7DC8O5hOx9rdhSuLMvdqJz79pgb31+qm8ec5efNBlfOq3isNfGXdri9j3B33CkhHBMGj8m93wEZaJFV+nocXQvT+GDNSlZF0tk7wzbYBtdhnGUi/4tOW+aR/bPmtW9odnWeDGbyBt+rJ4qVoQ+FoA8CaiCfDAn81vzwmUdXyneBdFdPqPlBwpNcCQxKzlE+cEYFmCQf8gnVpR/bnCudOPgM72V+WkUZr9qeMkQG0b8F0Z1N6BWYhZbZgv09IrUoUtsNtHFFZMUyv3q7N1leGNWlIQgvAxN0u7euk3alRBusHsa6RXNpadgA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: jennifer.herbert@xxxxxxxxx, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Thu, 27 Apr 2023 12:49:29 +0000
  • Ironport-data: A9a23:R0fmqK1vc0KKiLH4R/bD5Rhwkn2cJEfYwER7XKvMYLTBsI5bpzYCy zYaUGzQPvqKamT1fY8iOdyxo0MGv5WGmN83HQNqpC1hF35El5HIVI+TRqvS04F+DeWYFR46s J9OAjXkBJppJpMJjk71atANlVEliefTAOK6ULWeUsxIbVcMYD87jh5+kPIOjIdtgNyoayuAo tq3qMDEULOf82cc3lk8tuTS+XuDgNyo4GlD5gBkOqgR1LPjvyJ94Kw3dPnZw0TQGuG4LsbiL 87fwbew+H/u/htFIrtJRZ6iLyXm6paLVeS/oiI+t5qK23CulQRrukoPD9IOaF8/ttm8t4sZJ OOhF3CHYVxB0qXkwIzxWvTDes10FfUuFLTveRBTvSEPpqFvnrSFL/hGVSkL0YMkFulfUDsW0 9EHMCg0QxWOi/jpmb60EPFRmZF2RCXrFNt3VnBI6xj8VK9ja7aTBqLA6JlfwSs6gd1IEbDGf c0FZDFzbRPGJRpSJlMQD5F4l+Ct7pX9W2QA9BTJ+uxqsy6Kkl0ZPLvFabI5fvSISMNTn0iVv CTG8n7zDwsGHNee1SCE4jSngeqncSbTAdpNSO3gp6c76LGV7jQsSzkGaGG5nfWorxODXd1Gd HwL/yV7+MDe82TuFLERRSaQoneCsgQNRtl4HOgz6QXLwa3Riy6bC24CTzBMcpomudU8SCY2/ lSIg8n5QzdotdW9WX+bs7uZsz62ESwUNnMZIz8JSxMf5Nvuq511iQjAJv5hGqOoitz+GRnr3 iuH6iM5gt0uYdUj0qy6+RXNhWKqr52QFwotvFyJDySi8x9zY5Oja8qw81/H4P1cLYGfCF6co HwDnMvY5+cLZX2QqBGwrCw2NOnBz5643Pf02DaDw7FJG+yRxkOe
  • Ironport-hdrordr: A9a23:LXssXqCp9hm80YXlHem955DYdb4zR+YMi2TDtnoddfUxSKfzqy nApoV56faKskdyZJhNo7690cq7LU80l6QU3WB5B97LYOCMggSVxe9ZjLcKygeQfhHDyg==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


On 26/04/2023 21:27, Jason Andryuk wrote:
Hi, Jennifer,

On Tue, Apr 25, 2023 at 1:48 PM Jennifer Herbert
<jennifer.herbert@xxxxxxxxxx> wrote:
This patch makes the TPM version, for which the ACPI libary probes, 
configurable.
If acpi_config.tpm_verison is set to 1, it indicates that 1.2 (TCPA) should be 
probed.
I have also added to hvmloader an option to allow setting this new config, 
which can
be triggered by setting the platform/tpm_version xenstore key.

Signed-off-by: Jennifer Herbert <jennifer.herbert@xxxxxxxxxx>
...
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -409,38 +409,47 @@ static int construct_secondary_tables(struct acpi_ctxt 
*ctxt,
...
+        switch ( config->tpm_version )
          {
-            tcpa->lasa = ctxt->mem_ops.v2p(ctxt, lasa);
-            tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE;
-            memset(lasa, 0, tcpa->laml);
-            set_checksum(tcpa,
-                         offsetof(struct acpi_header, checksum),
-                         tcpa->header.length);
+        case 0: /* Assume legacy code wanted tpm 1.2 */
This shouldn't be reached, since tpm_version == 0 won't have
ACPI_HAS_TPM set.  Still, do you want to make it a break or drop the
case to avoid falling through to the TPM1.2 code?

So there was concerns in v2 about backward compatilibity before of this area of code.  The exact nature of the concern was vauge, so I made this very conservative.   This was intended to mitigate agaist this code being run, but with the structure being set up with something other then  the code in tools/firmware/hvmloader/util.c.  Any such alaternate code would set the tpm flag, but not know about the version field, and so leave it at zero.  In this case, dropping into 1.2 probing would be the correct solution.

As you say, in the use cases I'm farmiliar with, this would not get reached, so shoudn't affect anything. Haveing a break or dropping the case would result in it silently ignoring the 'invalid' tpm configuration, so if not compatibilty mode, I'd personally prefer some sort of assert, although i'm not sure how best to do that in this code.


-jenny


Looks good though.

Reviewed-by: Jason Andryuk <jandryuk@xxxxxxxxx>

Thanks,
Jason



 


Rackspace

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