On Wed, 2019-05-15 at 18:22 +0200, Laurent Vivier wrote:
> On 15/05/2019 02:28, Suraj Jitindar Singh wrote:
> > Currently the handler for a decrementer exception will simply
> > reload the
> > maximum value (0x7FFFFFFF), which will take ~4 seconds to expire
> > again.
> > This means that if a vcpu cedes, it will be ~4 seconds between
> > wakeups.
> >
> > The h_cede_tm test is testing a known breakage when a guest cedes
> > while
> > suspended. To be sure we cede 500 times to check for the bug.
> > However
> > since it takes ~4 seconds to be woken up once we've ceded, we only
> > get
> > through ~20 iterations before we reach the 90 seconds timeout and
> > the
> > test appears to fail.
> >
> > Add an option when registering the decrementer handler to specify
> > the
> > value which should be reloaded by the handler, allowing the timeout
> > to be
> > chosen.
> >
> > Modify the spr test to use the max timeout to preserve existing
> > behaviour.
> > Modify the h_cede_tm test to use a 10ms timeout to ensure we can
> > perform
> > 500 iterations before hitting the 90 second time limit for a test.
> >
> > This means the h_cede_tm test now succeeds rather than timing out.
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@xxxxxxxxx>
> >
> > ---
> >
> > V1 -> V2:
> > - Make decr variables static
> > - Load intial decr value in tm test to ensure known value present
> > ---
> > lib/powerpc/handlers.c | 7 ++++---
> > powerpc/sprs.c | 5 +++--
> > powerpc/tm.c | 4 +++-
> > 3 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/powerpc/handlers.c b/lib/powerpc/handlers.c
> > index be8226a..c8721e0 100644
> > --- a/lib/powerpc/handlers.c
> > +++ b/lib/powerpc/handlers.c
> > @@ -12,11 +12,12 @@
> >
> > /*
> > * Generic handler for decrementer exceptions (0x900)
> > - * Just reset the decrementer back to its maximum value
> > (0x7FFFFFFF)
> > + * Just reset the decrementer back to the value specified when
> > registering the
> > + * handler
> > */
> > -void dec_except_handler(struct pt_regs *regs __unused, void *data
> > __unused)
> > +void dec_except_handler(struct pt_regs *regs __unused, void *data)
> > {
> > - uint32_t dec = 0x7FFFFFFF;
> > + uint64_t dec = *((uint64_t *) data);
> >
> > asm volatile ("mtdec %0" : : "r" (dec));
> > }
> > diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> > index 6744bd8..0e2e1c9 100644
> > --- a/powerpc/sprs.c
> > +++ b/powerpc/sprs.c
> > @@ -253,6 +253,7 @@ int main(int argc, char **argv)
> > 0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL,
> > -1ULL,
> > };
> > + static uint64_t decr = 0x7FFFFFFF; /* Max value */
> >
> > for (i = 1; i < argc; i++) {
> > if (!strcmp(argv[i], "-w")) {
> > @@ -288,8 +289,8 @@ int main(int argc, char **argv)
> > (void) getchar();
> > } else {
> > puts("Sleeping...\n");
> > - handle_exception(0x900, &dec_except_handler,
> > NULL);
> > - asm volatile ("mtdec %0" : : "r" (0x3FFFFFFF));
> > + handle_exception(0x900, &dec_except_handler,
> > &decr);
> > + asm volatile ("mtdec %0" : : "r" (decr));
>
> why do you replace the 0x3FFFFFFF by decr which is 0x7FFFFFFF?
Oh yeah, my mistake. I mis-read and thought they were the same. Is
there any reason it was 0x3FFFFFFF? Can this be fixed up when applying
or should I resend?
>
> > hcall(H_CEDE);
> > }
> >
> > diff --git a/powerpc/tm.c b/powerpc/tm.c
> > index bd56baa..c588985 100644
> > --- a/powerpc/tm.c
> > +++ b/powerpc/tm.c
> > @@ -95,11 +95,13 @@ static bool enable_tm(void)
> > static void test_h_cede_tm(int argc, char **argv)
> > {
> > int i;
> > + static uint64_t decr = 0x3FFFFF; /* ~10ms */
> >
> > if (argc > 2)
> > report_abort("Unsupported argument: '%s'",
> > argv[2]);
> >
> > - handle_exception(0x900, &dec_except_handler, NULL);
> > + handle_exception(0x900, &dec_except_handler, &decr);
> > + asm volatile ("mtdec %0" : : "r" (decr));
> >
> > if (!start_all_cpus(halt, 0))
> > report_abort("Failed to start secondary cpus");
> >
>
> Thanks,
> Laurent
