Re: [RFC v2 5/7] test/qtest: Add test for tpm crb chunking

[marcandre.lureau@redhat.com]

On Thu, 19 Mar 2026 19:23:14 +0530, Arun Menon <armenon@redhat.com> wrote:

Hi

>
> diff --git a/tests/qtest/tpm-util.c b/tests/qtest/tpm-util.c
> index 2cb2dd47961..dfc1fd70b7f 100644
> --- a/tests/qtest/tpm-util.c
> +++ b/tests/qtest/tpm-util.c
> @@ -14,16 +14,42 @@
>  
>  #include "qemu/osdep.h"
>  #include <glib/gstdio.h>
> +#include "system/memory.h"

Use qemu/bswap.h instead

>  
>  #include "hw/acpi/tpm.h"
>  #include "libqtest.h"
>  #include "tpm-util.h"
>  #include "qobject/qdict.h"
>  
> +#define CRB_ADDR_START (TPM_CRB_ADDR_BASE + A_CRB_CTRL_START)
> +#define CRB_CTRL_STS (TPM_CRB_ADDR_BASE + A_CRB_CTRL_STS)
> +#define CRB_CTRL_CMD_SIZE (TPM_CRB_ADDR_BASE + A_CRB_CTRL_CMD_SIZE)

Shadows tpm.h macro. Use CRB_ADDR_CTRL_STS ?

> +
> +#define BIT_START_INVOKE  (1 << 0)

Similar (although in tpm_crb.c unit)

> [ ... skip 11 lines ... ]
> +            break;
> +        }
> +        if (g_get_monotonic_time() >= end_time) {
> +            break;
> +        }
> +    }

Consider adding a g_assert() here

> @@ -31,24 +57,74 @@ void tpm_util_crb_transfer(QTestState *s,
> [ ... skip 30 lines ... ]
> +
> +    qtest_writeb(s, TPM_CRB_ADDR_BASE + A_CRB_LOC_CTRL, 1);
> +
> +    for (size_t i = 0 ; i < req_size; i += chunk_size) {
> +        bool last_chunk = false;
> +        size_t current_chunk_size = chunk_size ;

Trailing whitespace before the semicolon.

> +
> +        if (i + chunk_size > req_size) {
> +            last_chunk = true;
> +            current_chunk_size = req_size - i;
>          }
> -        if (g_get_monotonic_time() >= end_time) {
> -            break;
> +
> +        qtest_memwrite(s, caddr, req + i, current_chunk_size);
> +
> +        if (last_chunk) {
> +            qtest_writel(s, CRB_ADDR_START , BIT_START_INVOKE);

Trailing whitespace before the comma.

Style: should be tpm_sts per QEMU naming conventions.
Style: declarations should be at the top of the function.
Same: declaration mixed with statements.

Style: should be tpm_sts per QEMU naming conventions.
Style: declarations should be at the top of the function.
Same: declaration mixed with statements.

-- 
Marc-André Lureau <marcandre.lureau@redhat.com>