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

[Stefan Berger]

On 01/29/2018 11:24 AM, Igor Mammedov wrote:
> On Fri, 26 Jan 2018 13:03:06 +0100
> Marc-André Lureau <address@hidden> 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 <address@hidden>
> > ---
> >
> > 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...

> [...]
>
> > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > new file mode 100644
> > index 0000000000..687d2557b7
> > --- /dev/null
> > +++ b/hw/tpm/tpm_crb.c
> [...]
>
> > +static void tpm_crb_realize(DeviceState *dev, Error **errp)
> > +{
> > +    CRBState *s = CRB(dev);
> > +
> > +    if (!tpm_find()) {
> > +        error_setg(errp, "at most one TPM device is permitted");
> > +        return;
> > +    }
> > +    if (!s->tpmbe) {
> > +        error_setg(errp, "'tpmdev' property is required");
> > +        return;
> > +    }
> > +
> > +    memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s,
> > +        "tpm-crb-mmio", sizeof(s->regs));
> > +    memory_region_init_ram(&s->cmdmem, OBJECT(s),
> > +        "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp);
> > +
> > +    memory_region_add_subregion(get_system_memory(),
> > +        TPM_CRB_ADDR_BASE, &s->mmio);
> > +    memory_region_add_subregion(get_system_memory(),
> > +        TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
> just curious, what if there is something else mapped at this addresses,
> will it fail or just ignore error?

All these memory APIs return void...



> > +
> > +    tpm_backend_reset(s->tpmbe);
> > +
> > +    ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
> > +                     InterfaceType, CRB_INTF_TYPE_CRB_ACTIVE);
> > +    ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
> > +                     InterfaceVersion, CRB_INTF_VERSION_CRB);
> > +    ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
> > +                     CapLocality, CRB_INTF_CAP_LOCALITY_0_ONLY);
> > +    ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
> > +                     CapCRBIdleBypass, CRB_INTF_CAP_IDLE_FAST);
> > +    ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
> > +                     CapDataXferSizeSupport, CRB_INTF_CAP_XFER_SIZE_64);
> > +    ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
> > +                     CapFIFO, CRB_INTF_CAP_FIFO_NOT_SUPPORTED);
> > +    ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
> > +                     CapCRB, CRB_INTF_CAP_CRB_SUPPORTED);
> > +    ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
> > +                     InterfaceSelector, CRB_INTF_IF_SELECTOR_CRB);
> > +    ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
> > +                     RID, 0b0000);
> > +    ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID2,
> > +                     VID, PCI_VENDOR_ID_IBM);
> > +
> > +    s->regs[R_CRB_CTRL_CMD_SIZE] = CRB_CTRL_CMD_SIZE;
> > +    s->regs[R_CRB_CTRL_CMD_LADDR] = TPM_CRB_ADDR_BASE + A_CRB_DATA_BUFFER;
> > +    s->regs[R_CRB_CTRL_RSP_SIZE] = CRB_CTRL_CMD_SIZE;
> > +    s->regs[R_CRB_CTRL_RSP_ADDR] = TPM_CRB_ADDR_BASE + A_CRB_DATA_BUFFER;
> > +
> > +    s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->tpmbe),
> > +                            CRB_CTRL_CMD_SIZE);
> > +
> > +    tpm_backend_startup_tpm(s->tpmbe, s->be_buffer_size);
> > +}
> [...]