Re: [PATCH v4 1/5] acpi: Convert build_tpm2() to build_append* API

[Igor Mammedov]

On Mon, 22 Jun 2020 11:47:26 +0200
Auger Eric <eric.auger@redhat.com> wrote:

> Hi Igor,
> 
> On 6/22/20 11:39 AM, Igor Mammedov wrote:
> > On Fri, 19 Jun 2020 07:19:51 -0400
> > Stefan Berger <stefanb@linux.ibm.com> wrote:
> >   
> >> On 6/19/20 5:43 AM, Auger Eric wrote:  
> >>> Hi Laszlo,
> >>>
> >>> On 6/19/20 11:38 AM, Laszlo Ersek wrote:    
> >>>> On 06/18/20 09:50, Auger Eric wrote:    
> >>>>> Hi Stefan, Igor,
> >>>>>
> >>>>> On 6/16/20 4:11 PM, Stefan Berger wrote:    
> >>>>>> On 6/16/20 8:33 AM, Igor Mammedov wrote:    
> >>>>>>> nevertheless looks like faithfull conversion,
> >>>>>>> btw why you didn't drop Acpi20TPM2 structure definition?
> >>>>>>>    
> >>>>>> If we get rid of the table we should keep a reference to this document,
> >>>>>> table 7: "TCG ACPI Specification; Family 1.2 and 2.0; Level 00 Revision
> >>>>>> 00.37, December 19, 2014"
> >>>>>>
> >>>>>> https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_1-10_0-37-Published.pdf
> >>>>>>
> >>>>>>
> >>>>>>    
> >>>>> Further looking at this spec, the log_area_minimum_length and
> >>>>> log_area_start_address only are described in
> >>>>> - Table 2 (TCG Hardware InterfaceDescription Table Format for TPM 1.2
> >>>>> Clients)
> >>>>> - Table 4 (TCG Hardware Interface Description Table Format for TPM 1.2
> >>>>> Servers)
> >>>>> but not in Table 7, ie. not for TPM 2.0.
> >>>>>
> >>>>> Are they really needed for TPM2 or what do I miss?    
> >>>> (side comment:
> >>>>
> >>>> LASA and LAML are optional with TPM-2.0. From the discussion at
> >>>> <https://bugzilla.tianocore.org/show_bug.cgi?id=978>.    
> >>
> >>
> >> They are needed for (x86) BIOS, such as SeaBIOS, not for UEFI, though. I 
> >> do not know about ARM.
> >>
> >>  
> >>> Thank you for the pointer and info. I failed to find this info in the
> >>> spec. Given the risk of confusion, I would personally keep struct
> >>> Acpi20TPM2 and maybe add a comment. Stefan?    
> >>
> >> Either way is fine with me for as long as we know where to find the 
> >> layout of the structure.  
> > I'd remove Acpi20TPM2 as it hardly documents anything, and add a comment
> > pointing to the concrete spec that has these fields.
> > 
> > TCGTCG ACPI SpecificationFamily “1.2” and “2.0”Version 1.2,Revision 8  
> 
> [PATCH v6 0/3] vTPM/aarch64 ACPI support was posted.
> 
> As documented in the cover letter (history log), the presence of the
> LAML and LASA fields in the TPM2 table is not clearly documented in the
> spec (at least I failed to find it). It is for TPM 1.2. On the other
> hand, Stefan said it is mandated for some x86 BIOS to work. Given this
> weirdness I think keeping the  Acpi20TPM2 struct is not too bad. See v6 ...

Laszlo pointed to spec version where LAML/LASA in TPM2 are documented,
so I'd just use that as a spec this code is based on.

PS:
Acpi20TPM2 struct doesn't document anything, it's just another way to do
the same thing as build_appen_* calls do. Having it just adds to confusion. 

> 
> Thanks
> 
> Eric
> >   
> >>
> >>    Stefan
> >>  
> >>>
> >>> Thanks
> >>>
> >>> Eric    
> >>>> )
> >>>>
> >>>> Thanks
> >>>> Laszlo
> >>>>    
> >>  
> > 
> >