Re: [PATCH v2] tests/functional/aarch64: add tests for FEAT_RME

[Pierrick Bouvier]

Hi Marcin,

On 12/1/24 05:34, Marcin Juszkiewicz wrote:
> W dniu 28.11.2024 o 22:37, Pierrick Bouvier pisze:
> > This boot an OP-TEE environment, and launch a nested guest VM inside it
> > using the Realms feature. We do it for virt and sbsa-ref platforms.
> >
> > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>
> > diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> > index 5c048cfac6d..b975a1560df 100644
> > --- a/tests/functional/meson.build
> > +++ b/tests/functional/meson.build
> > @@ -13,6 +13,8 @@ endif
> >    test_timeouts = {
> >      'aarch64_aspeed' : 600,
> >      'aarch64_raspi4' : 480,
>
> > +  'aarch64_rme_virt' : 720,
>
> Took 2974.95s on M1 Pro macbook.
>
> > +  'aarch64_rme_sbsaref' : 720,
>
> This one needed 2288.29s.
>
> >      'aarch64_sbsaref_alpine' : 720,
>
> Have to check cause timed out.
>
> >      'aarch64_sbsaref_freebsd' : 720,
>
> 331.65s
>
> So RME tests probably need longer timeouts or would not run at all.

By any chance, are you running those tests in debug mode?
It seems to me that CI is running functional tests with optimized 
builds, so I'm not sure we want to support debug "times" here.

> > +++ b/tests/functional/test_aarch64_rme_sbsaref.py
>
> > +        self.vm.add_args('-accel', 'tcg')
>
> That's default value so can be skipped.

Ok.

> > +        self.vm.add_args('-cpu', 'max,x-rme=on')
>
> > +        self.vm.add_args('-m', '2G')
>
> I sent patch to bump default memsize to 2G recently.

Is that merged already, or will be later?

> > +        self.vm.add_args('-M', 'sbsa-ref')
> > +        self.vm.add_args('-drive', f'file={pflash0},format=raw,if=pflash')
> > +        self.vm.add_args('-drive', f'file={pflash1},format=raw,if=pflash')
> > +        self.vm.add_args('-drive', f'file=fat:rw:{virtual},format=raw')
>
> > +        self.vm.add_args('-drive', f'format=raw,if=none,file={drive},id=hd0')
> > +        self.vm.add_args('-device', 'virtio-blk-pci,drive=hd0')
>
> sbsa-ref is fully emulated target. There is AHCI controller built-in so
> only "-drive" argument should be needed (no "-device" one).

I followed official instructions from Jean Philippe to build RME stack, 
and I think it's better to keep them in sync.

> > +        self.vm.add_args('-device', 'virtio-9p-pci,fsdev=shr0,mount_tag=shr0')
> > +        self.vm.add_args('-fsdev', 
> > f'local,security_model=none,path={rme_stack},id=shr0')
>
> > +        self.vm.add_args('-device', 'virtio-net-pci,netdev=net0')
> > +        self.vm.add_args('-netdev', 'user,id=net0')
>
> e1000e is built-in already

This is needed, because without this, there is an explicit wait for a 
network interface when booting. Adding this device allows to skip it.

> As both virt and sbsa-ref tests do "more or less" the same stuff then it
> would be good to make common file/class and reuse it both tests by
> adding hardware differences.

I was thinking that at the beginning, but most of the config is 
different. The only common part is the nested guest test.

However, I didn't see any other tests that were importing functions from 
other files, and since we want to keep those two tests in separate files 
(to allow parallelism), the most pragmatic solution was to duplicate.
Overall, tests files should be as simple as possible, even if the price 
is to repeat a few lines.
If you have a cleaner solution, I'm open to implement it.

Thanks,
Pierrick