Re: [Qemu-devel] [PATCH] tpm: Set tpmRegValidSts flag to '1' in device reset

[Marc-André Lureau]

Hi

On Mon, Mar 19, 2018 at 5:21 PM, Stefan Berger
<address@hidden> wrote:
> Fix the initialization of the tpmRegValidSts flag and set it to '1'
> during device reset without expecting a write to another register.
> This seems to also be the default behavior of real hardware.
>
> Signed-off-by: Stefan Berger <address@hidden>
> ---
>  hw/tpm/tpm_crb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index d8917cb..114b66e 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -145,8 +145,6 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr,
>                               beenSeized, 0);
>              ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE,
>                               locAssigned, 1);
> -            ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE,
> -                             tpmRegValidSts, 1);
>              break;
>          }
>          break;
> @@ -210,6 +208,8 @@ static void tpm_crb_reset(void *dev)
>
>      tpm_backend_reset(s->tpmbe);
>
> +    ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE,
> +                     tpmRegValidSts, 1);
>      ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
>                       InterfaceType, CRB_INTF_TYPE_CRB_ACTIVE);
>      ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,

Hmm, the specification says they default to 0. Except tpmEstablished
to 1, which we should probably manipulate somehow ("The TPM clears
this bit to 0 upon receipt of _TPM_Hash_End The TPM sets this bit to a
1 when the TPM_LOC_CTRL_x.resetEstablishment field is set to 1.").
Help welcome!

Shouldn't it also set locAssigned to 1 in this case ?

I am not very familiar with the locality support, since I didn't
implement it for CRB. You may have more clues what is expected here.

-- 
Marc-André Lureau