On Tue Jun 4, 2024 at 4:12 PM AEST, Thomas Huth wrote: > On 04/05/2024 14.28, Nicholas Piggin wrote: > > This has a known failure on QEMU TCG machines where the decrementer > > interrupt is not lowered when the DEC wraps from -ve to +ve. > > Would it then make sense to mark the test with accel = kvm to avoid the test > failure when running with TCG? Still want to test it with TCG though, which had quite a few bugs I used these tests to fix. It is marked as known fail for TCG (once the later host accel patch is merged). > > +/* Check amount of CPUs nodes that have the TM flag */ > > +static int find_dec_bits(void) > > +{ > > + int ret; > > + > > + ret = dt_for_each_cpu_node(cpu_dec_bits, NULL); > > What sense does it make to run this for each CPU node if the cpu_dec_bits > function always overwrites the global dec_bits variable? > Wouldn't it be sufficient to run this for the first node only? Or should the > cpu_dec_bits function maybe check that all nodes have the same value? Yeah it could use first subnode. > > + if (ret < 0) > > + return ret; > > + > > + return dec_bits; > > +} > > + > > + > > +static bool do_migrate = false; > > +static volatile bool got_interrupt; > > +static volatile struct pt_regs recorded_regs; > > + > > +static uint64_t dec_max; > > +static uint64_t dec_min; > > + > > +static void test_tb(int argc, char **argv) > > +{ > > + uint64_t tb; > > + > > + tb = get_tb(); > > + if (do_migrate) > > + migrate(); > > + report(get_tb() >= tb, "timebase is incrementing"); > > If you use >= for testing, it could also mean that the TB stays at the same > value, so "timebase is incrementing" sounds misleading. Maybe rather > "timebase is not decreasing" ? Or wait a little bit, then check with ">" only ? Yeah, maybe first immediate test could ensure it doesn't go backward, then wait a bit and check it increments. > > +} > > + > > +static void dec_stop_handler(struct pt_regs *regs, void *data) > > +{ > > + mtspr(SPR_DEC, dec_max); > > +} > > + > > +static void dec_handler(struct pt_regs *regs, void *data) > > +{ > > + got_interrupt = true; > > + memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs)); > > + regs->msr &= ~MSR_EE; > > +} > > + > > +static void test_dec(int argc, char **argv) > > +{ > > + uint64_t tb1, tb2, dec; > > + int i; > > + > > + handle_exception(0x900, &dec_handler, NULL); > > + > > + for (i = 0; i < 100; i++) { > > + tb1 = get_tb(); > > + mtspr(SPR_DEC, dec_max); > > + dec = mfspr(SPR_DEC); > > + tb2 = get_tb(); > > + if (tb2 - tb1 < dec_max - dec) > > + break; > > + } > > + /* POWER CPUs can have a slight (few ticks) variation here */ > > + report_kfail(true, tb2 - tb1 >= dec_max - dec, "decrementer remains within TB after mtDEC"); > > + > > + tb1 = get_tb(); > > + mtspr(SPR_DEC, dec_max); > > + mdelay(1000); > > + dec = mfspr(SPR_DEC); > > + tb2 = get_tb(); > > + report(tb2 - tb1 >= dec_max - dec, "decrementer remains within TB after 1s"); > > + > > + mtspr(SPR_DEC, dec_max); > > + local_irq_enable(); > > + local_irq_disable(); > > + if (mfspr(SPR_DEC) <= dec_max) { > > + report(!got_interrupt, "no interrupt on decrementer positive"); > > + } > > + got_interrupt = false; > > + > > + mtspr(SPR_DEC, 1); > > + mdelay(100); /* Give the timer a chance to run */ > > + if (do_migrate) > > + migrate(); > > + local_irq_enable(); > > + local_irq_disable(); > > + report(got_interrupt, "interrupt on decrementer underflow"); > > + got_interrupt = false; > > + > > + if (do_migrate) > > + migrate(); > > + local_irq_enable(); > > + local_irq_disable(); > > + report(got_interrupt, "interrupt on decrementer still underflown"); > > + got_interrupt = false; > > + > > + mtspr(SPR_DEC, 0); > > + mdelay(100); /* Give the timer a chance to run */ > > + if (do_migrate) > > + migrate(); > > + local_irq_enable(); > > + local_irq_disable(); > > + report(got_interrupt, "DEC deal with set to 0"); > > + got_interrupt = false; > > + > > + /* Test for level-triggered decrementer */ > > + mtspr(SPR_DEC, -1ULL); > > + if (do_migrate) > > + migrate(); > > + local_irq_enable(); > > + local_irq_disable(); > > + report(got_interrupt, "interrupt on decrementer write MSB"); > > + got_interrupt = false; > > + > > + mtspr(SPR_DEC, dec_max); > > + local_irq_enable(); > > + if (do_migrate) > > + migrate(); > > + mtspr(SPR_DEC, -1); > > + local_irq_disable(); > > + report(got_interrupt, "interrupt on decrementer write MSB with irqs on"); > > + got_interrupt = false; > > + > > + mtspr(SPR_DEC, dec_min + 1); > > + mdelay(100); > > + local_irq_enable(); > > + local_irq_disable(); > > + /* TCG does not model this correctly */ > > + report_kfail(true, !got_interrupt, "no interrupt after wrap to positive"); > > + got_interrupt = false; > > + > > + handle_exception(0x900, NULL, NULL); > > +} > > + > > +static void test_hdec(int argc, char **argv) > > +{ > > + uint64_t tb1, tb2, hdec; > > + > > + if (!machine_is_powernv()) { > > + report_skip("skipping on !powernv machine"); > > I'd rather say "not running on powernv machine" Okay. Thanks, Nick