https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/test/trap/trap_cases.py;h=32b20125f8f54fd67174c8545a0ea3392b365053;hb=30f2d8a8e92ad2939775f19e6a0f387499e9842b#l77 def case_2_rfid(self): lst = ["rfid"] initial_regs = [0] * 32 initial_regs[1] = 1 initial_sprs = {'SRR0': 0x12345678, 'SRR1': 0xb000000000001033} e = ExpectedState(pc=0x700) e.intregs[1] = 1 e.msr = 0xb000000000001033 # TODO, not actually checked self.add_case(Program(lst, bigendian), initial_regs, initial_sprs, initial_msr=0xa000000000000003, expected=e) pc=0x700 looks just plain wrong, since we are in privileged mode...I don't think the fix is obvious since it looks like it was trying to test that rfid causes a privileged instruction trap when running in problem mode... The ExpectedState I get if expected=None (pc changed to hex): e = ExpectedState(pc=0x12345678) e.intregs[1] = 0x1 e.sprs['SRR0'] = 0x12345678 e.sprs['SRR1'] = 0xb000000000001033 e.msr = 0xa000000000000033 msr looks wrong, the MSB end should be 0xb... not 0xa... since: MSR[0:2] <- SRR1[0:2] Since I need to ensure all the trap opcodes work correctly when fixing bug #1066 after it was reverted, I'm marking TrapTestCase.case_2_rfid as skipped with a FIXME for now.
(In reply to Jacob Lifshay from comment #0) > Since I need to ensure all the trap opcodes work i did not specify that. please re-read what i said, twice. i said *sc*. > #1066 after it was reverted, I'm marking TrapTestCase.case_2_rfid as skipped > with a FIXME for now. please remove that. it works as compatible with microwatt. if you remove tat test it could damage TestIssuer.
(In reply to Luke Kenneth Casson Leighton from comment #1) > (In reply to Jacob Lifshay from comment #0) > > > Since I need to ensure all the trap opcodes work > > i did not specify that. please re-read what i said, twice. i said > *sc*. yes, but in order for pytest/CI to properly test sc, the other cases in the same class need to pass, otherwise they stop the test early and later cases don't get run. This is because pytest treats the whole class as a unit. > > > #1066 after it was reverted, I'm marking TrapTestCase.case_2_rfid as skipped > > with a FIXME for now. > > please remove that. it works as compatible with microwatt. > if you remove tat test it could damage TestIssuer. ok, well in that case, microwatt is just wrong, since it's in privileged mode, not problem mode (unless reserved MSR bits 1:2 are causing the trap). I'll revert that skip_case commit.
(In reply to Jacob Lifshay from comment #2) > (In reply to Luke Kenneth Casson Leighton from comment #1) > > please remove that. it works as compatible with microwatt. > > if you remove tat test it could damage TestIssuer. > > ok, well in that case, microwatt is just wrong, since it's in privileged > mode, not problem mode (unless reserved MSR bits 1:2 are causing the trap). > I'll revert that skip_case commit. reverted: https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=fb9d0848913af3389f572497992e37883ea7b99c the commit message where the expected state for case_2_rfid was added makes me think you added whatever made the soc tests happy at the time, rather than what matched microwatt: https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=cf8eaed82a022f2de46e8a4a358b211aefddb0d7 this is when you were working on https://bugs.libre-soc.org/show_bug.cgi?id=859 microwatt's privileged tests do give pc=0x700, but only because they set Problem state and try to run rfid, which traps because it is privileged. https://github.com/antonblanchard/microwatt/blob/63d4553faeea897926f89eb7486dd991cd9e83c8/tests/privileged/privileged.c#L215 https://github.com/antonblanchard/microwatt/blob/63d4553faeea897926f89eb7486dd991cd9e83c8/tests/privileged/privileged.c#L174
(In reply to Jacob Lifshay from comment #2) > yes, but in order for pytest/CI to properly test sc, why are you using pytest in a mode that does not run all tests? what is the point of that? > the other cases in the > same class need to pass, otherwise they stop the test early and later cases > don't get run. This is because pytest treats the whole class as a unit. then fix that in the CI so that they are all run! whenever i run python3 test_caller_trap.py it runs everything. therefore there is something wrong with how you have set up CI, which needs fixing, not skipping the test case. > ok, well in that case, microwatt is just wrong, since it's in privileged > mode, not problem mode (unless reserved MSR bits 1:2 are causing the trap). > I'll revert that skip_case commit. thank you. (In reply to Jacob Lifshay from comment #3) > > the commit message where the expected state for case_2_rfid was added makes > me think you added whatever made the soc tests happy at the time, rather > than what matched microwatt: > > https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff; > h=cf8eaed82a022f2de46e8a4a358b211aefddb0d7 > > this is when you were working on > https://bugs.libre-soc.org/show_bug.cgi?id=859 > > microwatt's privileged tests do give pc=0x700, but only because they set > Problem state and try to run rfid, which traps because it is privileged. sigh this sounds right. it was much too long ago to remember. okaaay it needs to go on the increasingly-long list of additional future work to get better unit tests.
(In reply to Luke Kenneth Casson Leighton from comment #4) > (In reply to Jacob Lifshay from comment #2) > > > yes, but in order for pytest/CI to properly test sc, > > why are you using pytest in a mode that does not run all tests? it does run all tests, but only when they all pass. this is because pytest runs what it sees as a single test: running test_caller_trap.TrapTest.test (trapped by __init__ so it runs TestRunnerBase.run_test instead) this is worked around by the case_* machinery running everything under self.subTest() ... > what is the point of that? ... but I didn't install the subtests plugin for pytest (iirc something ran out of ram, maybe vscode's pytest integration). It didn't occur to me to add that plugin to CI since I expected we'd be proactive about fixing broken tests (sadness...) > > > > the other cases in the > > same class need to pass, otherwise they stop the test early and later cases > > don't get run. This is because pytest treats the whole class as a unit. > > then fix that in the CI so that they are all run! ok, I'll work on that tomorrow. creating a bug...
(In reply to Jacob Lifshay from comment #5) > it does run all tests, but only when they all pass. this is because pytest > runs what it sees as a single test: running test_caller_trap.TrapTest.test > (trapped by __init__ so it runs TestRunnerBase.run_test instead) actually, it's not trapped by __init__, instead pytest attempts to run each test method by invoking run_test, which is overridden by TestRunnerBase.run_test