Re: [PATCH v6 1/3] tpm/tpm_tis_spi: Support TPM for SPI (Serial Peripheral Interface)

[dan tan <dantan@linux.ibm.com>]

Good point, Stefan!

Let me put it through our CI tests, which, in addition to unit tests, 
also does both Linux (rhel-9) and AIX (ver7) boot exercises

thank you,
---
dan tan
power simulation
phone:+1.7373.099.138
email:dantan@linux.ibm.com


On 2024-11-08 09:38, Stefan Berger wrote:
> On 11/4/24 12:18 PM, dan tan wrote:
>> Implement support for TPM via SPI interface. The SPI bus master
>> is provided by PowerNV SPI device which is an SSI peripheral.
>> It can uses the tpm_emulator driver backend with the external
>> swtpm.
>> 
>> Signed-off-by: dan tan <dantan@linux.ibm.com>
>> ---
>> 
> 
>> diff --git a/hw/tpm/tpm_tis_spi.c b/hw/tpm/tpm_tis_spi.c
>> new file mode 100644
>> index 0000000000..079972de03
>> --- /dev/null
>> +++ b/hw/tpm/tpm_tis_spi.c
>> @@ -0,0 +1,357 @@
>> +/*
>> + * QEMU SPI TPM 2.0 model
>> + *
>> + * Copyright (c) 2024, IBM Corporation.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/log.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/acpi/tpm.h"
>> +#include "tpm_prop.h"
>> +#include "qemu/log.h"
>> +#include "trace.h"
>> +#include "tpm_tis.h"
>> +#include "hw/ssi/ssi.h"
>> +#include "migration/vmstate.h"
>> +
>> +typedef struct TPMStateSPI {
>> +    /*< private >*/
>> +    SSIPeripheral parent_object;
>> +
>> +    uint8_t     byte_offset;     /* byte offset in transfer */
>> +    uint8_t     wait_state_cnt;  /* wait state counter */
> 
> #define NUM_WAIT_STATES 1
> 
> Are 4 wait states actually needed? I ran the test with 1. More wait
> states just have impact on performance and I don't think we need to be
> emulating the real hardware entirely.
> 
> @@ -236,9 +237,9 @@ static uint32_t tpm_tis_spi_transfer(SSIPeripheral
> *ss, uint32_t tx)
>              trace_tpm_tis_spi_transfer_addr("reg_addr", 
> spist->reg_addr);
>              break;
>          default:    /* data bytes */
> -            if (spist->wait_state_cnt < 4) {
> +            if (spist->wait_state_cnt < NUM_WAIT_STATES) {
>                  spist->wait_state_cnt++;
> -                if (spist->wait_state_cnt == 4) {
> +                if (spist->wait_state_cnt == NUM_WAIT_STATES) {
>                      trace_tpm_tis_spi_transfer_data("wait complete, 
> count",
>  spist->wait_state_cnt);
>                      rx = rx | (0x01 << (24 - offset * 8));
> @@ -274,7 +275,8 @@ static uint32_t tpm_tis_spi_transfer(SSIPeripheral
> *ss, uint32_t tx)
>              }
>              break;
>          }
> -        if ((spist->wait_state_cnt == 0) || (spist->wait_state_cnt == 
> 4)) {
> +        if ((spist->wait_state_cnt == 0) ||
> +            (spist->wait_state_cnt == NUM_WAIT_STATES)) {
>              offset++;
>              spist->byte_offset++;
>          } else {