Re: [PATCH v5 3/3] tests/qtest/tpm: add unit test to tis-spi

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

Hi Stefan,

What's the best way to catch these in my own local testing without 
submitting it to Travis CI?

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


On 2024-11-04 09:20, Stefan Berger wrote:
> On 11/4/24 1:43 AM, dan tan wrote:
>> Add qtest cases to exercise main TPM functionality
>> The TPM device emulation is provided by swtpm, which is TCG
>> TPM 2.0, and TCG TPM TIS compliant. See
>> https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_TPM_Profile_PTP_2.0_r1.03_v22.pdf
>> https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientTPMInterfaceSpecification_TIS__1-3_27_03212013.pdf
>> The SPI registers are specific to the PowerNV platform
>> architecture
>> 
>> Signed-off-by: dan tan <dantan@linux.vnet.ibm.com>
>> ---
>> 
>> v3:
>> - removed the function prototypes declaration
>> - fixed code format to comply with convention
>> - changed function names and variable names to be the same
>>    as the tpm-tis-i2c test.
>> - change hard coded numbers to #define's with meaningful
>>    names that are identifiable with spec documentation
>> 
>> v4:
>> - git commit amend only
>> 
>> v5:
>> - modified tpm_reg_readl() by
>>    - removing the special case for TPM_TIS_REG_DID_VID.
>>      - however, I did not use the more efficient 32bit access due
>>        to the SPI bus master implementation. The 16bit register
>>        still require special treatment with the SPI RWX bits.
>>    - correcting tpm_reg_readb() with uint16_t reg
>> - tpm_set_verify_loc() added checking for 
>> TPM_TIS_CAPABILITIES_SUPPORTED2_0
>> - test_spi_tpm_transmit_test() added
>>    - TPM_TIS_STS_TPM_FAMILY2_0 check in status register
>>    - TPM responses verification
>> - fixed the PowerNV stdout msg from running 
>> qtest-ppc64/tpm-tis-spi-pnv-test
>> ---
>>   tests/qtest/tpm-tis-spi-pnv-test.c | 713 
>> +++++++++++++++++++++++++++++
>>   tests/qtest/meson.build            |   2 +
>>   2 files changed, 715 insertions(+)
>>   create mode 100644 tests/qtest/tpm-tis-spi-pnv-test.c
>> 
>> diff --git a/tests/qtest/tpm-tis-spi-pnv-test.c 
>> b/tests/qtest/tpm-tis-spi-pnv-test.c
>> new file mode 100644
>> index 0000000000..9eeeea41f7
>> --- /dev/null
>> +++ b/tests/qtest/tpm-tis-spi-pnv-test.c
>> @@ -0,0 +1,713 @@
>> +/*
>> + * QTest testcase for a Nuvoton NPCT75x TPM SPI device
>> + *                      running on the PowerNV machine.
>> + *
>> + * Copyright (c) 2024, IBM Corporation.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +#include "qemu/osdep.h"
>> +#include <glib/gstdio.h>
>> +#include "libqtest-single.h"
>> +#include "hw/acpi/tpm.h"
>> +#include "hw/pci/pci_ids.h"
>> +#include "qtest_aspeed.h"
>> +#include "tpm-emu.h"
>> +#include "hw/ssi/pnv_spi_regs.h"
>> +#include "pnv-xscom.h"
>> +
>> +#define SPI_TPM_BASE            0xc0080
>> +#define SPI_SHIFT_COUNTER_N1    0x30000000
>> +#define SPI_SHIFT_COUNTER_N2    0x40000000
>> +#define SPI_RWX_OPCODE_SHIFT    56
>> +#define SPI_RWX_ADDR_SHIFT      32
>> +#define SPI_CMD_DATA_SHIFT      56
>> +
>> +#define CFG_COUNT_COMPARE_1     0x0000000200000000
>> +#define MM_REG_RDR_MATCH        0x00000000ff01ff00
>> +#define SEQ_OP_REG_BASIC        0x1134416200100000
>> +
>> +#define TPM_TIS_8BITS_MASK      0xff
>> +#define SPI_TPM_TIS_ADDR        0xd40000
>> +#define SPI_EXTEND              0x03
>> +#define TPM_WRITE_OP            0x0
>> +#define TPM_READ_OP             0x80
>> +
>> +#define SHORT_MAX_RETRIES       5
>> +#define LONG_MAX_RETRIES        10
>> +
>> +static const uint8_t TPM_CMD[12] =
>> +                     
>> "\x80\x01\x00\x00\x00\x0c\x00\x00\x01\x44\x00\x00";
>> +
>> +#define DPRINTF(fmt, ...) do { \
>> +    if (DEBUG_TIS_TEST) { \
>> +        printf(fmt, ## __VA_ARGS__); \
>> +    } \
>> +} while (0)
>> +
>> +#define DEBUG_TIS_TEST 0
>> +
>> +#define DPRINTF_ACCESS \
>> +    DPRINTF("%s: %d: locty=%d l=%d access=0x%02x 
>> pending_request_flag=0x%x\n", \
>> +            __func__, __LINE__, locty, l, access, 
>> pending_request_flag)
>> +
>> +#define DPRINTF_STS \
>> +    DPRINTF("%s: %d: sts = 0x%08x\n", __func__, __LINE__, sts)
>> +
>> +static uint64_t pnv_spi_tpm_read(const PnvChip *chip, uint32_t reg)
>> +{
>> +    uint32_t pcba = SPI_TPM_BASE + reg;
>> +
>> +    return qtest_readq(global_qtest, pnv_xscom_addr(chip, pcba));
>> +}
>> +
>> +static void pnv_spi_tpm_write(const PnvChip *chip,
>> +                              uint32_t reg,
>> +                              uint64_t val)
>> +{
>> +    uint32_t pcba = SPI_TPM_BASE + reg;
>> +
>> +    qtest_writeq(global_qtest, pnv_xscom_addr(chip, pcba), val);
>> +}
>> +
>> +static void spi_op_complete(const PnvChip *chip)
>> +{
>> +    uint64_t cfg_reg;
>> +
>> +    cfg_reg = pnv_spi_tpm_read(chip, SPI_CLK_CFG_REG);
>> +    g_assert_cmpuint(CFG_COUNT_COMPARE_1, ==, cfg_reg);
>> +    pnv_spi_tpm_write(chip, SPI_CLK_CFG_REG, 0);
>> +}
>> +
>> +static void spi_write_reg(const PnvChip *chip, uint64_t val)
>> +{
>> +    int i;
>> +    uint64_t spi_sts;
>> +
>> +    for (i = 0; i < LONG_MAX_RETRIES; i++) {
>> +        spi_sts = pnv_spi_tpm_read(chip, SPI_STS_REG);
>> +        if (GETFIELD(SPI_STS_TDR_FULL, spi_sts) == 1) {
>> +            sleep(0.5);
> 
> 
> One of the Travis builds complains about this:
> 
> ../tests/qtest/tpm-tis-spi-pnv-test.c:90:19: error: implicit
> conversion from 'double' to 'unsigned int' changes value from 0.5 to 0
> [-Werror,-Wliteral-conversion]
> 
>             sleep(0.5);
> 
>             ~~~~~ ^~~
> 
> ../tests/qtest/tpm-tis-spi-pnv-test.c:104:19: error: implicit
> conversion from 'double' to 'unsigned int' changes value from 0.1 to 0
> [-Werror,-Wliteral-conversion]
> 
>             sleep(0.1);
> 
>             ~~~~~ ^~~
> 
> ../tests/qtest/tpm-tis-spi-pnv-test.c:123:15: error: implicit
> conversion from 'double' to 'unsigned int' changes value from 0.5 to 0
> [-Werror,-Wliteral-conversion]
> 
>         sleep(0.5);
> 
>         ~~~~~ ^~~
> 
> ../tests/qtest/tpm-tis-spi-pnv-test.c:128:19: error: implicit
> conversion from 'double' to 'unsigned int' changes value from 0.1 to 0
> [-Werror,-Wliteral-conversion]
> 
>             sleep(0.1);
> 
>             ~~~~~ ^~~
> 
> You need to use g_usleep() here.
> 
> 
> 
>> +        } else {
>> +            break;
>> +        }
>> +    }
>> +    /* cannot write if SPI_STS_TDR_FULL bit is still set */
>> +    g_assert_cmpuint(0, ==, GETFIELD(SPI_STS_TDR_FULL, spi_sts));
>> +    pnv_spi_tpm_write(chip, SPI_XMIT_DATA_REG, val);
>> +
>> +    for (i = 0; i < SHORT_MAX_RETRIES; i++) {
>> +        spi_sts = pnv_spi_tpm_read(chip, SPI_STS_REG);
>> +        if (GETFIELD(SPI_STS_SHIFTER_FSM, spi_sts) & FSM_DONE) {
>> +            break;
>> +        } else {
>> +            sleep(0.1);
> 
> Also here.