This is needed so there's a contiguous chunk of memory that can be passed to host syscalls. * DONE: add MemMMap class * DONE: ensure it works with UTF-8 validation unit test * DONE: test MemMMap.get_ctypes method
(In reply to Jacob Lifshay from comment #0) > This is needed so there's a contiguous chunk of memory that can be passed to > host syscalls. no: do not replace the existing Mem class. too low level at this point. a cascade of errors may result in blowing the budget fixing bugs caused by massive disruption for many weeks. write a *new* one and provide an option to ISACaller to use it (another bool flag). you'll need to propagate that up all the way to pypowersim (and through all unit test infrastructure). make ABSOLUTELY certain to add it as the LAST parameter to all functions (and to ISACaller itself). run_tst, class Program, the whole lot. bit of a pain but it is what it is.
I added a new MemMMap class. I also added a method that the syscalls should use when they need direct access to the memory (method not yet tested, I'll add that next): class MemMMap: def get_ctypes(self, start_addr, size, is_write): """ returns a ctypes ubyte array referring to the memory at `start_addr` with size `size` """ https://git.libre-soc.org/?p=openpower-isa.git;a=shortlog;h=9437aa56d550b9a66772068cf685b06e2d7d263f commit 9437aa56d550b9a66772068cf685b06e2d7d263f Author: Jacob Lifshay <programmerjake@gmail.com> Date: Fri Sep 22 18:25:21 2023 -0700 switch UTF-8 validation tests to use MemMMap so it gets some testing commit abdd57ce2d872a88465dc2d4c4726f29bc8e6bdd Author: Jacob Lifshay <programmerjake@gmail.com> Date: Fri Sep 22 18:23:22 2023 -0700 add MemMMap class https://bugs.libre-soc.org/show_bug.cgi?id=1173 commit a8d7f5322d57395c14e42136f20caf3c78f5dc71 Author: Jacob Lifshay <programmerjake@gmail.com> Date: Fri Sep 22 15:40:30 2023 -0700 split out most Mem methods into MemCommon base class commit 2301b3d8a6d278ae9af7fb61c332e33d6f12d9bf Author: Jacob Lifshay <programmerjake@gmail.com> Date: Fri Sep 22 15:10:14 2023 -0700 format mem.py
(In reply to Jacob Lifshay from comment #2) > I added a new MemMMap class. > > I also added a method that the syscalls should use when they need direct > access to the memory (method not yet tested, I'll add that next): > class MemMMap: > def get_ctypes(self, start_addr, size, is_write): > """ returns a ctypes ubyte array referring to the memory at > `start_addr` with size `size` > """ awesome, makes sense > commit 9437aa56d550b9a66772068cf685b06e2d7d263f > Author: Jacob Lifshay <programmerjake@gmail.com> > Date: Fri Sep 22 18:25:21 2023 -0700 > > switch UTF-8 validation tests to use MemMMap so it gets some testing hmm hmmm about that, really it should be possible to set things like MSR in order to flip LE/BE, and MSR.SF, on most unit tests. needs a rethink as part of doing elwidth overrides to be honest as the tests are the same but results are different. hmm hmmmm (edit: bug #1174)
I added tests for MemMMap.get_ctypes testing both reading and writing. I also changed MemMMap so memory dumps skip all the lines with just zero bytes, since MemMMap manages stuff in 64kiB chunks (the minimum granularity prescribed by ppc64le ELF), that's a *lot* of zeros to skip printing. https://git.libre-soc.org/?p=openpower-isa.git;a=shortlog;h=50cb080ba742df15e33422000b82a015c211aa5b commit 50cb080ba742df15e33422000b82a015c211aa5b Author: Jacob Lifshay <programmerjake@gmail.com> Date: Mon Sep 25 15:57:23 2023 -0700 add MemMMap tests commit 06578592de791992c1bcad37eacbac759aac5500 Author: Jacob Lifshay <programmerjake@gmail.com> Date: Mon Sep 25 15:29:25 2023 -0700 skip zero words when iterating words in MemMMap commit 07eff25a9d8adb5087c8e70ccd9e4d9d77d6e935 Author: Jacob Lifshay <programmerjake@gmail.com> Date: Mon Sep 25 14:41:49 2023 -0700 format src/openpower/decoder/isa/test_mem.py
self.mem_blocks = { File "/home/libresoc/src/openpower-isa/src/openpower/decoder/isa/mem.py", line 318, in <dictcomp> addr: mmap.mmap(-1, BLOCK_SIZE) for addr in block_addrs} OverflowError: Python int too large to convert to C ssize_t on an armv8l system
(In reply to Luke Kenneth Casson Leighton from comment #5) > self.mem_blocks = { > File "/home/libresoc/src/openpower-isa/src/openpower/decoder/isa/mem.py", > line 318, in <dictcomp> > addr: mmap.mmap(-1, BLOCK_SIZE) for addr in block_addrs} > OverflowError: Python int too large to convert to C ssize_t > > on an armv8l system you need a 64-bit version of python. if you have a 32-bit python there isn't enough address space to easily emulate 64-bit powerpc, since it creates 2x 4GiB mmap-blocks (one at 0-4GiB and one where the powerpc64le stack gets mapped at iirc 2**47 - 4GiB to 2**47). (it doesn't use that much ram since linux allocates memory when it's accessed, not when it's mmapped, so your system won't run out of ram.)
(In reply to Jacob Lifshay from comment #6) > (In reply to Luke Kenneth Casson Leighton from comment #5) > > self.mem_blocks = { > > File "/home/libresoc/src/openpower-isa/src/openpower/decoder/isa/mem.py", > > line 318, in <dictcomp> > > addr: mmap.mmap(-1, BLOCK_SIZE) for addr in block_addrs} > > OverflowError: Python int too large to convert to C ssize_t > > > > on an armv8l system > > you need a 64-bit version of python. if you have a 32-bit python there isn't > enough address space to easily emulate 64-bit powerpc this is not the only reason you need 64-bit python, the main pytest process often needs >4GiB ram in my experience when running the openpower-isa tests
https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=49cd5029f5aa997d80bfcdd5dbaaa873913f8b93 Author: Jacob Lifshay <programmerjake@gmail.com> Date: Mon Oct 23 16:09:51 2023 -0700 reduce mmap BLOCK_SIZE to 1 << 28 so it works on armv7a
https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/decoder/isa/test_caller_alu.py;hb=HEAD 16 class TestALU(TestRunnerBase): 17 def __init__(self, test): 18 assert test == 'test' 19 super().__init__(ALUTestCase().test_data) 20 21 def test(self): 22 # dummy function to make unittest try to test this class 23 pass 16 class TestALU(TestRunnerBase): 17 def __init__(self, test): 18 assert test == 'test' 19 super().__init__(ALUTestCase(use_mmap=True).test_data) 20 21 def test(self): 22 # dummy function to make unittest try to test this class 23 pass
174 tc = TestCase(prog, test_name, 175 regs=initial_regs, sprs=initial_sprs, cr=initial_cr, 176 msr=initial_msr, becomes: kwargs = copy(self.underlay_args) kwargs.update( { regs=initial_regs, sprs=initial_sprs, cr=initial_cr, msr=initial_msr, )} 174 tc = TestCase(prog, test_name, **kwargs
128 class TestAccumulatorBase: 129 __test__ = False # pytest should ignore this class 130 131 def __init__(self, flags=()): 132 self.__subtest_args = {} 133 self.flags = frozenset(flags) => 128 class TestAccumulatorBase: 129 __test__ = False # pytest should ignore this class 130 131 def __init__(self, flags=(), **kwargs): 132 self.__subtest_args = {} 133 self.flags = frozenset(flags) self.underlay_args = kwargs
(In reply to Luke Kenneth Casson Leighton from comment #10) > kwargs = copy(self.underlay_args) if initial_regs is not None: kwargs['regs'] = initial_regs
I just remembered use_mmap_mem is a parameter of TestRunnerBase.__init__, not TestAccumulatorBase.add_case, because it's can be thought of as configuring the cpu features like svp64=True does
(In reply to Jacob Lifshay from comment #13) > I just remembered use_mmap_mem is a parameter of TestRunnerBase.__init__, > not TestAccumulatorBase.add_case, because it's can be thought of as > configuring the cpu features like svp64=True does I'll post this anyway, since i already wrote it all out: i'm proposing: class TestAccumulatorBase: __test__ = False # pytest should ignore this class def __init__(self, flags=(), defaults=None): self.__subtest_args = {} self.flags = frozenset(flags) if defaults is None: defaults = {} else: defaults = dict(defaults) # copy and/or convert self.defaults = defaults ... ... def add_case(self, all, the, current, args, *, src_loc_at=0, **kwargs): args = { "all": all, "the": the, "current": current, "args": args, **kwargs } for k in args.keys() | self.defaults.keys(): v = args.get(k) if v is None: v = self.defaults.get(k) if v is None: args.pop(k) else: args[k] = v c = sys._getframe(1 + src_loc_at).f_code # name of caller of this function test_name = c.co_name # name of file containing test case test_file = os.path.splitext(os.path.basename(c.co_filename))[0] tc = TestCase(name=test_name, test_file=test_file, subtest_args=self.__subtest_args.copy(), **args) self.test_data.append(tc) class TestCase: __test__ = False # pytest should ignore this class def __init__(self, program, name, regs=None, sprs=None, cr=0, mem=None, msr=0, do_sim=True, extra_break_addr=None, svstate=0, expected=None, stop_at_pc=None, test_file=None, subtest_args=None, fpregs=None, initial_fpscr=None, **kwargs): self.program = program self.name = name if regs is None: regs = [0] * 32 if sprs is None: sprs = {} if mem is None: mem = {} if fpregs is None: fpregs = [0] * 32 self.regs = regs self.fpregs = fpregs self.sprs = sprs self.cr = cr self.mem = mem self.msr = msr self.do_sim = do_sim self.extra_break_addr = extra_break_addr self.svstate = svstate self.expected = expected # expected results from the test self.stop_at_pc = stop_at_pc # hard-stop address (do not attempt to run) self.test_file = test_file self.subtest_args = {} if subtest_args is None else dict(subtest_args) if initial_fpscr is None: initial_fpscr = 0 self.initial_fpscr = initial_fpscr self.rest = kwargs
(In reply to Jacob Lifshay from comment #14) > > def add_case(self, all, the, current, args, *, src_loc_at=0, **kwargs): > args = { > "all": all, > "the": the, > "current": current, > "args": args, > **kwargs > } > for k in args.keys() | self.defaults.keys(): needs to be the other way round for k in self.defaults.keys() | args.keys(): so that if the *test case* argument is supplied, it actually gets supplied. the way you wrote it, there is no way to "override" a default.
(In reply to Luke Kenneth Casson Leighton from comment #15) > (In reply to Jacob Lifshay from comment #14) > > for k in args.keys() | self.defaults.keys(): > > needs to be the other way round those are sets of keys, not dicts, so are order-independent.
(In reply to Jacob Lifshay from comment #16) > those are sets of keys, not dicts, so are order-independent. aw doh :)
(In reply to Jacob Lifshay from comment #14) > class TestCase: > __test__ = False # pytest should ignore this class > > def __init__(self, program, name, regs=None, sprs=None, cr=0, mem=None, > msr=0, None. > do_sim=True, > extra_break_addr=None, > svstate=0, None. then back in ISACaller, "if msr is None: msr = DEFAULT_MSR" same with svstate.
(In reply to Luke Kenneth Casson Leighton from comment #18) done: https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=505a72ebccd9b23109006aec22b5e23d915c06f0 commit 505a72ebccd9b23109006aec22b5e23d915c06f0 Author: Jacob Lifshay <programmerjake@gmail.com> Date: Wed Nov 15 19:31:32 2023 -0800 msr and svstate default to None in TestCase, they're replaced with actual values in ISACaller > > then back in ISACaller, "if msr is None: msr = DEFAULT_MSR" same with > svstate. ISACaller already does that.