Re: [RFC 4/6] tests: tpm-emu: Remove assert on TPM2_ST_NO_SESSIONS

[Stefan Berger]

On 6/5/20 5:35 AM, Auger Eric wrote:
> Hi Stefan,
>
> On 6/2/20 6:17 PM, Stefan Berger wrote:
> > On 6/2/20 12:13 PM, Auger Eric wrote:
> > > Hi Stefan,
> > >
> > > On 6/2/20 3:39 PM, Stefan Berger wrote:
> > > > On 6/1/20 6:21 AM, Eric Auger wrote:
> > > > > While writing tests for checking the content of TPM2 and DSDT
> > > > > along with TPM-TIS instantiation I attempted to reuse the
> > > > > framework used for TPM-TIS tests. However While dumping the
> > > > > ACPI tables I get an assert on TPM2_ST_NO_SESSIONS. My assumption
> > > > > is maybe the other tests did not execute long enough to encounter
> > > > > this. So I tentatively propose to remove the assert as it
> > > > > does not seem to break other tests and enable the new ones.
> > > > >
> > > > > Signed-off-by: Eric Auger <eric.auger@redhat.com>
> > > > > ---
> > > > >     tests/qtest/tpm-emu.c | 1 -
> > > > >     1 file changed, 1 deletion(-)
> > > > >
> > > > > diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
> > > > > index c43ac4aef8..298d0eec74 100644
> > > > > --- a/tests/qtest/tpm-emu.c
> > > > > +++ b/tests/qtest/tpm-emu.c
> > > > > @@ -49,7 +49,6 @@ static void *tpm_emu_tpm_thread(void *data)
> > > > >             s->tpm_msg->tag = be16_to_cpu(s->tpm_msg->tag);
> > > > >             s->tpm_msg->len = be32_to_cpu(s->tpm_msg->len);
> > > > >             g_assert_cmpint(s->tpm_msg->len, >=, minhlen);
> > > > > -        g_assert_cmpint(s->tpm_msg->tag, ==, TPM2_ST_NO_SESSIONS);
> > > > You should not have to remove this. The tests are skipped if swtpm does
> > > > not support TPM 2 via --tpm2 option. This would be a very old swtpm
> > > > version, though. So, all tests are run with --tpm2 option and any
> > > > response received from the TPM would be a TPM 2 response that should
> > > > have TPM2_ST_NO_SESSIONS as the tag. I'd be curious what other value you
> > > > are seeing there.
> > > If I revert this patch I am getting TPM2_ST_SESSIONS on my end.
> > Is firmware/BIOS active? There's no TPM2_ST_SESSIONS coming out of QEMU.
> So it looks SeaBIOS is in use (bios-256k.bin loaded).
>
> I can see MMIO accesses to the TPM and the following commands are
> observable:
> tpm_emu_tpm_thread code=0x181 tag=0x8001 len=0xa
> tpm_emu_tpm_thread code=0x144 tag=0x8001 len=0xc
> tpm_emu_tpm_thread code=0x121 tag=0x8002 len=0x20
> This last one causes the assert (TPM2_CC_HierarchyControl)
>
> I checked in Seabios and effectively tpm20_hierarchycontrol() tags the
> TPM2_CC_HierarchyControl command with TPM2_ST_SESSIONS
>
> Due to our emulation, maybe tpm_set_failure() gets called, inducing
> tpm20_hierarchycontrol() call.
>
> That being said, what do you recommend? Remove the assert, improve the
> emulation, other?

So this is an ACPI test. What role does the firmware play for success of 
the test? If the test relies on the firmware showing some sort of 
expected result, then I would recommend only running this test with an 
attached swtpm, like we run some other tests. If we don't need the 
firmware to succeed then I would just get rid of the assert. Probably no 
other test we have implemented so far was running the firmware...


   Stefan


> Thank you in advance
>
> Best Regards
>
> Eric
>
> >     Stefan