Re: [kvm-unit-tests PATCH v5 12/29] powerpc/sprs: Avoid taking async interrupts caused by register fuzzing

["Nicholas Piggin" <npiggin@gmail.com>]

On Tue Dec 19, 2023 at 9:47 PM AEST, Thomas Huth wrote:
> On 16/12/2023 14.42, Nicholas Piggin wrote:
> > Storing certain values in some registers can cause asynchronous
> > interrupts that can crash the test case, for example decrementer
> > or PMU interrupts.
> > 
> > Change the msleep to mdelay which does not enable MSR[EE] and so
> > avoids the problem. This allows removing some of the SPR special
> > casing.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   powerpc/sprs.c | 14 ++------------
> >   1 file changed, 2 insertions(+), 12 deletions(-)
> > 
> > diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> > index 01041912..313698e0 100644
> > --- a/powerpc/sprs.c
> > +++ b/powerpc/sprs.c
> > @@ -481,12 +481,7 @@ static void set_sprs(uint64_t val)
> >   			continue;
> >   		if (sprs[i].type & SPR_HARNESS)
> >   			continue;
> > -		if (!strcmp(sprs[i].name, "MMCR0")) {
> > -			/* XXX: could use a comment or better abstraction! */
> > -			__mtspr(i, (val & 0xfffffffffbab3fffULL) | 0xfa0b2070);
> > -		} else {
> > -			__mtspr(i, val);
> > -		}
> > +		__mtspr(i, val);
> >   	}
> >   }
> >   
> > @@ -536,12 +531,7 @@ int main(int argc, char **argv)
> >   	if (pause) {
> >   		migrate_once();
> >   	} else {
> > -		msleep(2000);
> > -
> > -		/* Taking a dec updates SRR0, SRR1, SPRG1, so don't fail. */
> > -		sprs[26].type |= SPR_ASYNC;
> > -		sprs[27].type |= SPR_ASYNC;
> > -		sprs[273].type |= SPR_ASYNC;
> > +		mdelay(2000);
> >   	}
>
> IIRC I used the H_CEDE stuff here on purpose to increase the possibility 
> that the guest gets rescheduled onto another CPU core on the host, and thus 
> that it uncovers sprs that are not saved and restored on the host more 
> easily. So I'd rather keep the msleep() here.

Ah. Not a bad idea. I'll see about making it deal with H_CEDE
instead.

I wonder if there would be a way to stress host CPU migration
in the test harness. Instead of waiting for QEMU to exit, the
script could run taskset in a loop to bounce it around.
Conceptually easy, probably fiddly to wire things up nicely.

I may try to take a look at adding that... but after this
series so it doesn't get any bigger :/

Thanks,
Nick