Re: [Qemu-devel] [PATCH v3] tpm: add CRB device

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 29 Jan 2018 11:50:04 -0500
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 01/29/2018 11:24 AM, Igor Mammedov wrote:
> > On Fri, 26 Jan 2018 13:03:06 +0100
> > Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> >  
> >> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB)
> >> Interface as defined in TCG PC Client Platform TPM Profile (PTP)
> >> Specification Family “2.0” Level 00 Revision 01.03 v22.
> >>
> >> The PTP allows device implementation to switch between TIS and CRB
> >> model at run time, but given that CRB is a simpler device to
> >> implement, I chose to implement it as a different device.
> >>
> >> The device doesn't implement other locality than 0 for now (my laptop
> >> TPM doesn't either, so I assume this isn't so bad)
> >>
> >> Tested with some success with Linux upstream and Windows 10, seabios &
> >> modified ovmf. The device is recognized and correctly transmit
> >> command/response with passthrough & emu. However, we are missing PPI
> >> ACPI part atm.
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> ---
> >>
> >> The patch is based on stefanb/tpm-next git branch.
> >>  
> > [...]
> >  
> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >> index dc4b2b9ffe..ed78c4ed9f 100644
> >> --- a/hw/i386/acpi-build.c
> >> +++ b/hw/i386/acpi-build.c
> >> @@ -2224,6 +2224,22 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >>               aml_append(sb_scope, scope);
> >>           }
> >>       }
> >> +
> >> +    if (TPM_IS_CRB(tpm_find())) {
> >> +        dev = aml_device("TPM");
> >> +        aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
> >> +        crs = aml_resource_template();
> >> +        aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE,
> >> +                                           TPM_CRB_ADDR_SIZE, AML_READ_WRITE));
> >> +        aml_append(dev, aml_name_decl("_CRS", crs));
> >> +  
> > [...]  
> >> +        method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> >> +        aml_append(method, aml_return(aml_int(0x0f)));
> >> +        aml_append(dev, method);  
> > this is not needed as 0x0f is assumed default value if _STA is missing  
> 
> Well, it seems to be in good neighborhood with other devices created 
> that do the same thing: build_kbd_device_aml(), 
> build_mouse_device_aml(), part of \_SB.PCI0.ISA...
I'm in process of getting rid of that, so please don't add
new ones. 

PS:
(in case there won't be respin, a fixup patch that
maintainer could squash in would do)

[...]