| Message ID | 20230425174733.795961-2-jennifer.herbert@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | acpi: Make TPM version configurable. | expand |
Hi, Jennifer, On Tue, Apr 25, 2023 at 1:48 PM Jennifer Herbert <jennifer.herbert@citrix.com> 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@citrix.com> ... > --- 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? Looks good though. Reviewed-by: Jason Andryuk <jandryuk@gmail.com> Thanks, Jason
On 26/04/2023 21:27, Jason Andryuk wrote: > Hi, Jennifer, > > On Tue, Apr 25, 2023 at 1:48 PM Jennifer Herbert > <jennifer.herbert@citrix.com> 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@citrix.com> > ... >> --- 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@gmail.com> > > Thanks, > Jason
On Thu, Apr 27, 2023 at 8:49 AM Jennifer Herbert <jennifer.herbert@citrix.com> wrote: > > > On 26/04/2023 21:27, Jason Andryuk wrote: > > Hi, Jennifer, > > > > On Tue, Apr 25, 2023 at 1:48 PM Jennifer Herbert > > <jennifer.herbert@citrix.com> 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@citrix.com> > > ... > >> --- 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. Ok, sounds good to leave it as you wrote it here. The split of hvmloader and libacpi requires the backwards compatible approach inside libacpi. Thanks, Jason
On 25.04.2023 19:47, Jennifer Herbert 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@citrix.com> > --- > docs/misc/xenstore-paths.pandoc | 9 +++++ > tools/firmware/hvmloader/util.c | 19 ++++++--- > tools/libacpi/build.c | 69 +++++++++++++++++++-------------- > tools/libacpi/libacpi.h | 3 +- > 4 files changed, 64 insertions(+), 36 deletions(-) Please can you get used to providing a brief rev log somewhere here? > --- a/tools/firmware/hvmloader/util.c > +++ b/tools/firmware/hvmloader/util.c > @@ -994,13 +994,22 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, > if ( !strncmp(xenstore_read("platform/acpi_laptop_slate", "0"), "1", 1) ) > config->table_flags |= ACPI_HAS_SSDT_LAPTOP_SLATE; > > - config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | > - ACPI_HAS_WAET | ACPI_HAS_PMTIMER | > - ACPI_HAS_BUTTONS | ACPI_HAS_VGA | > - ACPI_HAS_8042 | ACPI_HAS_CMOS_RTC); > + config->table_flags |= (ACPI_HAS_IOAPIC | ACPI_HAS_WAET | > + ACPI_HAS_PMTIMER | ACPI_HAS_BUTTONS | > + ACPI_HAS_VGA | ACPI_HAS_8042 | > + ACPI_HAS_CMOS_RTC); > config->acpi_revision = 4; > > - config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; > + s = xenstore_read("platform/tpm_version", "1"); > + config->tpm_version = strtoll(s, NULL, 0); Due to field width, someone specifying 257 will also get a 1.2 TPM, if I'm not mistaken. > + switch( config->tpm_version ) Nit: Style (missing blank). > --- a/tools/libacpi/build.c > +++ b/tools/libacpi/build.c > @@ -409,38 +409,47 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt, > memcpy(ssdt, ssdt_laptop_slate, sizeof(ssdt_laptop_slate)); > table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); > } > - > - /* TPM TCPA and SSDT. */ > - if ( (config->table_flags & ACPI_HAS_TCPA) && > - (config->tis_hdr[0] != 0 && config->tis_hdr[0] != 0xffff) && > - (config->tis_hdr[1] != 0 && config->tis_hdr[1] != 0xffff) ) > + /* TPM and its SSDT. */ > + if ( config->table_flags & ACPI_HAS_TPM ) > { > - ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16); > - if (!ssdt) return -1; > - memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm)); > - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); > - > - tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16); > - if (!tcpa) return -1; > - memset(tcpa, 0, sizeof(*tcpa)); > - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa); > - > - tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE; > - tcpa->header.length = sizeof(*tcpa); > - tcpa->header.revision = ACPI_2_0_TCPA_REVISION; > - fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID); > - fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID); > - tcpa->header.oem_revision = ACPI_OEM_REVISION; > - tcpa->header.creator_id = ACPI_CREATOR_ID; > - tcpa->header.creator_revision = ACPI_CREATOR_REVISION; > - if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL ) > + 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 */ Along the lines of what Jason said: Unless this is known to be needed for anything, I'd prefer if it was omitted. Jan
On 02/05/2023 12:54, Jan Beulich wrote: > On 25.04.2023 19:47, Jennifer Herbert 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@citrix.com> >> --- >> docs/misc/xenstore-paths.pandoc | 9 +++++ >> tools/firmware/hvmloader/util.c | 19 ++++++--- >> tools/libacpi/build.c | 69 +++++++++++++++++++-------------- >> tools/libacpi/libacpi.h | 3 +- >> 4 files changed, 64 insertions(+), 36 deletions(-) > Please can you get used to providing a brief rev log somewhere here? Yes, ok. >> --- a/tools/firmware/hvmloader/util.c >> +++ b/tools/firmware/hvmloader/util.c >> @@ -994,13 +994,22 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, >> if ( !strncmp(xenstore_read("platform/acpi_laptop_slate", "0"), "1", 1) ) >> config->table_flags |= ACPI_HAS_SSDT_LAPTOP_SLATE; >> >> - config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | >> - ACPI_HAS_WAET | ACPI_HAS_PMTIMER | >> - ACPI_HAS_BUTTONS | ACPI_HAS_VGA | >> - ACPI_HAS_8042 | ACPI_HAS_CMOS_RTC); >> + config->table_flags |= (ACPI_HAS_IOAPIC | ACPI_HAS_WAET | >> + ACPI_HAS_PMTIMER | ACPI_HAS_BUTTONS | >> + ACPI_HAS_VGA | ACPI_HAS_8042 | >> + ACPI_HAS_CMOS_RTC); >> config->acpi_revision = 4; >> >> - config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; >> + s = xenstore_read("platform/tpm_version", "1"); >> + config->tpm_version = strtoll(s, NULL, 0); > Due to field width, someone specifying 257 will also get a 1.2 TPM, > if I'm not mistaken. Seems likely. And i few other wacky values would give you 1.2 as well I'd think. There could also be trailing junk on the version number. I was a bit phased by the lack of any real error cases in hvmloader_acpi_build_tables. It seemed the approch was if you put in junk, you'll get something, but possibly not what your expecting. Do I take it you'd prefer it to only accept a strict '1' for 1.2 and any other value would result in no TPM being probed? Or is it only the overflow cases your concerned about? >> + switch( config->tpm_version ) > Nit: Style (missing blank). yup >> --- a/tools/libacpi/build.c >> +++ b/tools/libacpi/build.c >> @@ -409,38 +409,47 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt, >> memcpy(ssdt, ssdt_laptop_slate, sizeof(ssdt_laptop_slate)); >> table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); >> } >> - >> - /* TPM TCPA and SSDT. */ >> - if ( (config->table_flags & ACPI_HAS_TCPA) && >> - (config->tis_hdr[0] != 0 && config->tis_hdr[0] != 0xffff) && >> - (config->tis_hdr[1] != 0 && config->tis_hdr[1] != 0xffff) ) >> + /* TPM and its SSDT. */ >> + if ( config->table_flags & ACPI_HAS_TPM ) >> { >> - ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16); >> - if (!ssdt) return -1; >> - memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm)); >> - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); >> - >> - tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16); >> - if (!tcpa) return -1; >> - memset(tcpa, 0, sizeof(*tcpa)); >> - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa); >> - >> - tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE; >> - tcpa->header.length = sizeof(*tcpa); >> - tcpa->header.revision = ACPI_2_0_TCPA_REVISION; >> - fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID); >> - fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID); >> - tcpa->header.oem_revision = ACPI_OEM_REVISION; >> - tcpa->header.creator_id = ACPI_CREATOR_ID; >> - tcpa->header.creator_revision = ACPI_CREATOR_REVISION; >> - if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL ) >> + 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 */ > Along the lines of what Jason said: Unless this is known to be needed for > anything, I'd prefer if it was omitted. I'm not awair of anything, but your comment 2 lines down from version 2 made me think you knew of some. So if your happy with me removing this line, I am! > Jan
On 03.05.2023 00:40, Jennifer Herbert wrote: > On 02/05/2023 12:54, Jan Beulich wrote: >> On 25.04.2023 19:47, Jennifer Herbert wrote: >>> --- a/tools/firmware/hvmloader/util.c >>> +++ b/tools/firmware/hvmloader/util.c >>> @@ -994,13 +994,22 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, >>> if ( !strncmp(xenstore_read("platform/acpi_laptop_slate", "0"), "1", 1) ) >>> config->table_flags |= ACPI_HAS_SSDT_LAPTOP_SLATE; >>> >>> - config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | >>> - ACPI_HAS_WAET | ACPI_HAS_PMTIMER | >>> - ACPI_HAS_BUTTONS | ACPI_HAS_VGA | >>> - ACPI_HAS_8042 | ACPI_HAS_CMOS_RTC); >>> + config->table_flags |= (ACPI_HAS_IOAPIC | ACPI_HAS_WAET | >>> + ACPI_HAS_PMTIMER | ACPI_HAS_BUTTONS | >>> + ACPI_HAS_VGA | ACPI_HAS_8042 | >>> + ACPI_HAS_CMOS_RTC); >>> config->acpi_revision = 4; >>> >>> - config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; >>> + s = xenstore_read("platform/tpm_version", "1"); >>> + config->tpm_version = strtoll(s, NULL, 0); >> Due to field width, someone specifying 257 will also get a 1.2 TPM, >> if I'm not mistaken. > > Seems likely. And i few other wacky values would give you 1.2 as well > I'd think. There could also be trailing junk on the version number. > > I was a bit phased by the lack of any real error cases in > hvmloader_acpi_build_tables. It seemed the approch was if you put in > junk, you'll get something, but possibly not what your expecting. > > Do I take it you'd prefer it to only accept a strict '1' for 1.2 and any > other value would result in no TPM being probed? Or is it only the > overflow cases your concerned about? Iirc xs convention is that numeric values can be spelled out in arbitrary ways. So limiting to strictly "1" may be too restrictive. Anything that evaluates to 1 without overflow nor trailing junk ought to be taken to mean 1, I think. >>> --- a/tools/libacpi/build.c >>> +++ b/tools/libacpi/build.c >>> @@ -409,38 +409,47 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt, >>> memcpy(ssdt, ssdt_laptop_slate, sizeof(ssdt_laptop_slate)); >>> table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); >>> } >>> - >>> - /* TPM TCPA and SSDT. */ >>> - if ( (config->table_flags & ACPI_HAS_TCPA) && >>> - (config->tis_hdr[0] != 0 && config->tis_hdr[0] != 0xffff) && >>> - (config->tis_hdr[1] != 0 && config->tis_hdr[1] != 0xffff) ) >>> + /* TPM and its SSDT. */ >>> + if ( config->table_flags & ACPI_HAS_TPM ) >>> { >>> - ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16); >>> - if (!ssdt) return -1; >>> - memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm)); >>> - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); >>> - >>> - tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16); >>> - if (!tcpa) return -1; >>> - memset(tcpa, 0, sizeof(*tcpa)); >>> - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa); >>> - >>> - tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE; >>> - tcpa->header.length = sizeof(*tcpa); >>> - tcpa->header.revision = ACPI_2_0_TCPA_REVISION; >>> - fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID); >>> - fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID); >>> - tcpa->header.oem_revision = ACPI_OEM_REVISION; >>> - tcpa->header.creator_id = ACPI_CREATOR_ID; >>> - tcpa->header.creator_revision = ACPI_CREATOR_REVISION; >>> - if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL ) >>> + 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 */ >> Along the lines of what Jason said: Unless this is known to be needed for >> anything, I'd prefer if it was omitted. > > I'm not awair of anything, but your comment 2 lines down from version 2 > made me think you knew of some. So if your happy with me removing this > line, I am! I'm afraid I can't make the connection to that comment (assuming I fished out the right one). In any event, especially with Jason's observation that ACPI_HAS_TPM won't be set alongside ->tpm_version being zero, I see no use for keeping the "case 0:". Jan
diff --git a/docs/misc/xenstore-paths.pandoc b/docs/misc/xenstore-paths.pandoc index 5cd5c8a3b9..e67e164855 100644 --- a/docs/misc/xenstore-paths.pandoc +++ b/docs/misc/xenstore-paths.pandoc @@ -269,6 +269,15 @@ at the guest physical address in HVM_PARAM_VM_GENERATION_ID_ADDR. See Microsoft's "Virtual Machine Generation ID" specification for the circumstances where the generation ID needs to be changed. + +#### ~/platform/tpm_version = INTEGER [HVM,INTERNAL] + +The TPM version to be probed for. + +A value of 1 indicates to probe for TPM 1.2. +A value of 0 or an invalid value will result in no TPM being probed. +If unset, a default of 1 is assumed. + ### Frontend device paths Paravirtual device frontends are generally specified by their own diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c index 581b35e5cf..f39a8e584f 100644 --- a/tools/firmware/hvmloader/util.c +++ b/tools/firmware/hvmloader/util.c @@ -994,13 +994,22 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, if ( !strncmp(xenstore_read("platform/acpi_laptop_slate", "0"), "1", 1) ) config->table_flags |= ACPI_HAS_SSDT_LAPTOP_SLATE; - config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | - ACPI_HAS_WAET | ACPI_HAS_PMTIMER | - ACPI_HAS_BUTTONS | ACPI_HAS_VGA | - ACPI_HAS_8042 | ACPI_HAS_CMOS_RTC); + config->table_flags |= (ACPI_HAS_IOAPIC | ACPI_HAS_WAET | + ACPI_HAS_PMTIMER | ACPI_HAS_BUTTONS | + ACPI_HAS_VGA | ACPI_HAS_8042 | + ACPI_HAS_CMOS_RTC); config->acpi_revision = 4; - config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; + s = xenstore_read("platform/tpm_version", "1"); + config->tpm_version = strtoll(s, NULL, 0); + + switch( config->tpm_version ) + { + case 1: + config->table_flags |= ACPI_HAS_TPM; + config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; + break; + } config->numa.nr_vmemranges = nr_vmemranges; config->numa.nr_vnodes = nr_vnodes; diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c index fe2db66a62..716cb49624 100644 --- a/tools/libacpi/build.c +++ b/tools/libacpi/build.c @@ -409,38 +409,47 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt, memcpy(ssdt, ssdt_laptop_slate, sizeof(ssdt_laptop_slate)); table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); } - - /* TPM TCPA and SSDT. */ - if ( (config->table_flags & ACPI_HAS_TCPA) && - (config->tis_hdr[0] != 0 && config->tis_hdr[0] != 0xffff) && - (config->tis_hdr[1] != 0 && config->tis_hdr[1] != 0xffff) ) + /* TPM and its SSDT. */ + if ( config->table_flags & ACPI_HAS_TPM ) { - ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16); - if (!ssdt) return -1; - memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm)); - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); - - tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16); - if (!tcpa) return -1; - memset(tcpa, 0, sizeof(*tcpa)); - table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa); - - tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE; - tcpa->header.length = sizeof(*tcpa); - tcpa->header.revision = ACPI_2_0_TCPA_REVISION; - fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID); - fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID); - tcpa->header.oem_revision = ACPI_OEM_REVISION; - tcpa->header.creator_id = ACPI_CREATOR_ID; - tcpa->header.creator_revision = ACPI_CREATOR_REVISION; - if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL ) + 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 */ + case 1: + if ( config->tis_hdr[0] == 0 || config->tis_hdr[0] == 0xffff || + config->tis_hdr[1] == 0 || config->tis_hdr[1] == 0xffff ) + break; + + ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16); + if (!ssdt) return -1; + memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm)); + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt); + + tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16); + if (!tcpa) return -1; + memset(tcpa, 0, sizeof(*tcpa)); + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, tcpa); + + tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE; + tcpa->header.length = sizeof(*tcpa); + tcpa->header.revision = ACPI_2_0_TCPA_REVISION; + fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID); + fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID); + tcpa->header.oem_revision = ACPI_OEM_REVISION; + tcpa->header.creator_id = ACPI_CREATOR_ID; + tcpa->header.creator_revision = ACPI_CREATOR_REVISION; + + lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16); + if ( lasa ) + { + 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); + } + break; } } diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h index a2efd23b0b..f69452401f 100644 --- a/tools/libacpi/libacpi.h +++ b/tools/libacpi/libacpi.h @@ -27,7 +27,7 @@ #define ACPI_HAS_SSDT_PM (1<<4) #define ACPI_HAS_SSDT_S3 (1<<5) #define ACPI_HAS_SSDT_S4 (1<<6) -#define ACPI_HAS_TCPA (1<<7) +#define ACPI_HAS_TPM (1<<7) #define ACPI_HAS_IOAPIC (1<<8) #define ACPI_HAS_WAET (1<<9) #define ACPI_HAS_PMTIMER (1<<10) @@ -66,6 +66,7 @@ struct acpi_config { uint32_t table_flags; uint8_t acpi_revision; + uint8_t tpm_version; uint64_t vm_gid[2]; unsigned long vm_gid_addr; /* OUT parameter */
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@citrix.com> --- docs/misc/xenstore-paths.pandoc | 9 +++++ tools/firmware/hvmloader/util.c | 19 ++++++--- tools/libacpi/build.c | 69 +++++++++++++++++++-------------- tools/libacpi/libacpi.h | 3 +- 4 files changed, 64 insertions(+), 36 deletions(-)