Besides being able to execute PowerPC instructions in cavatools, we also need to support the PowerPC ABI to execute the real code interacting with the OS.
I've published the cavatools code: https://git.libre-soc.org/?p=cavatools.git;a=summary. I also pushed a couple of minor style commits (removal of trailing lines in Makefiles, addition of git ignore files). I checked the code. In addition to changes I provided in scope of bug #982, we'll also need syscalls generation for RISC-V architecture; this one does not have dedicated system calls table and uses generic syscalls (at least someone is mostly free from legacy nonsense). https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=387d059a1e3128a7aba238c34626246dee1820d4 There were also several commits intended to provide foundations for code generation and fix some aspects: https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=5b92251469be60e1d5548db78a1a99639b0e3f61 https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=24dacd1a1b285b1d22079011649cbedb17097e14 https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=b3d426282cafabd414ded35742b932d508f06782 https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=69b1ac09c26238cfdf615d1689413b7e22ad254b https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=f1db0ff17ec91d2f5cc3c620a032cff9c3ecb8db
The idea is to replace the code in cavatools with the unified generation mechanism we provided, and extend cavatools with ability to support the PPC guest architecture syscall translation. We'll need to generate the different tables depending on the guest architecture.
This was quite a step-by-step development. First, I had to fix some things in cavatools, like inability to reuse headers. https://git.libre-soc.org/?p=cavatools.git;a=commitdiff;h=9bd7bb427e195aa29656f33acb758f3e2abced1c https://git.libre-soc.org/?p=cavatools.git;a=commitdiff;h=11adcb0291d2267f14b375b1f57b6a50e90b51f1 https://git.libre-soc.org/?p=cavatools.git;a=commitdiff;h=5394b37dc8825ce618f6fc117318ae5724ac6162 Second, since cavatools is quite architecture-specific, I had no other choice but refactor the code to make it more generic: https://git.libre-soc.org/?p=cavatools.git;a=commitdiff;h=3e8ff210980e83f4ec0627196b8cc72e5a63aaa0 https://git.libre-soc.org/?p=cavatools.git;a=commitdiff;h=005252f3e2a64902079d92ac031e2a536919deb2 Then I implemented the code generation for riscv64 and ppc/ppc64 in cavatools-friendly format: https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=5172b93982f042f5d7a273cb0e92562305c03ccc https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=c7510177d5cd00b2db7cc97a4d60ac405f56f35d https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=34572b57ea72fb22e38af830565b5ad7f50a0cc0 https://git.libre-soc.org/?p=dev-env-setup.git;a=commitdiff;h=f79e0ceb28de39380bf1f27debfdc527910d0d23 https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=7a3fe407a232baaa535a86efab95a85a90552fa7 https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=b2839bc6b13965acb95a3b3792fdf9f0042755d2 https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=f3dada849e14b33a8a10f31826c73342a605c193 Third, I dropped the original system calls mechanism and replaced it with the one capable of generating riscv64, ppc and ppc64: https://git.libre-soc.org/?p=cavatools.git;a=commitdiff;h=97e66b71006a8e1ea9957c7cb80078abbc01d9e3 Then I introduced the G and H variables (cavatools tend to use short variables, they already had B and R variables for build and prefix directories): https://git.libre-soc.org/?p=cavatools.git;a=commitdiff;h=5f03e28394fcf2963eb17b6c6b3153b93fa25d4f Obviously, since cavatools cannot yet execute PPC instructions, I only had an option to check whether the original RISC-V code works; however, I checked whether the table is generated correctly for PPC as well. I also adjusted the calling convention for PPC (that is, the register identifiers to be used as arguments and result). Once we support ecall analogue for PPC (sc/scv), the code can be enabled.
Folks, do you consider this to be sufficient for closing the task?
(In reply to Dmitry Selyutin from comment #3) > https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=b2839bc6b13965acb95a3b3792fdf9f0042755d2 I find it unnecessarily confusing to have a function `ecall_entry` with the same name as a struct, can you please rename one or the other? The rest of it looks fine, though I didn't think about it too hard. (In reply to Dmitry Selyutin from comment #4) > Folks, do you consider this to be sufficient for closing the task? The task seems complete to me, though, EUR 4500 is a lot of money for what looks like a few days at most, maybe we should add a subtask to implement more system calls in ISACaller? we can justify that as necessary research. The work you did so far could be paid in a separate subtask for that work, so you don't have to wait to get paid for what you already did. Dmitry, Luke, Andrey, what do you think?
(In reply to Jacob Lifshay from comment #5) > (In reply to Dmitry Selyutin from comment #3) > > https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=b2839bc6b13965acb95a3b3792fdf9f0042755d2 > > I find it unnecessarily confusing to have a function `ecall_entry` with the > same name as a struct, can you please rename one or the other? I contrarily find it quite logical, it follows the same convention as stat and is not expected to be populated. But OK, I'll rename it. :-) > The rest of it looks fine, though I didn't think about it too hard. > > (In reply to Dmitry Selyutin from comment #4) > > Folks, do you consider this to be sufficient for closing the task? > > The task seems complete to me, though, EUR 4500 is a lot of money for what > looks like a few days at most https://bugs.libre-soc.org/show_bug.cgi?id=979#c53 I never commented on the budgets I considered too big for tasks handled by somebody else. Only cases I consider are when a) my task is underrated, b) my task is overrated, c) my task can be dropped at all as it's non-critical and its budget can be moved for others. I suggest to follow this approach as it's easier for the task owner to rate the task. Deal? > maybe we should add a subtask to implement > more system calls in ISACaller? we can justify that as necessary research. > The work you did so far could be paid in a separate subtask for that work, > so you don't have to wait to get paid for what you already did. > > Dmitry, Luke, Andrey, what do you think? I'd say it fits bug #982. I don't expect much to be done there; also parts of it fit ELF support which I, frankly speaking, find to be underrated.
(In reply to Dmitry Selyutin from comment #6) > Deal? ok with me > I'd say it fits bug #982. i was thinking #982 would only end up with a few syscalls implemented since luke was advocating for just open/openat, close, exit, read, and write (or something), so the additional budget here would allow implementing a lot more. > I don't expect much to be done there; also parts > of it fit ELF support which I, frankly speaking, find to be underrated. what do you mean by underrated? that the 1900 EUR estimate is too big? that it's too small? something else? keep in mind there is 6000 eur available in the ELF task's budget parent and since the statically-linked ELF task is too small I want to add dynamically-linked executable support as a separate task and implement more syscalls ... if there's not enough work to take up the 2mo of funds then I'll add more.
(In reply to Jacob Lifshay from comment #7) > (In reply to Dmitry Selyutin from comment #6) > > Deal? > > ok with me > > > I'd say it fits bug #982. > > i was thinking #982 would only end up with a few syscalls implemented since > luke was advocating for just open/openat, close, exit, read, and write (or > something), so the additional budget here would allow implementing a lot > more. mmap. This is a blocker. It needs to be done. Plus free pages list, basically a trivial allocator. > > I don't expect much to be done there; also parts > > of it fit ELF support which I, frankly speaking, find to be underrated. > > what do you mean by underrated? that the 1900 EUR estimate is too big? that > it's too small? something else? Too small, rationale below. > keep in mind there is 6000 eur available in the ELF task's budget parent and > since the statically-linked ELF task is too small I want to add > dynamically-linked executable support as a separate task and implement more > syscalls ... if there's not enough work to take up the 2mo of funds then > I'll add more. I expect several problems there. First of all, you'll have to implement mmap support, and this includes introducing something like a free list for bookkeeping the memory allocations, even if these are backed just by the big mapping. Second, you'll have to support different sections differently, not just "parse and map at fixed offset". Third, I expect some challenges in ISACaller and a learning curve with pyelf (if we take it; if not, this is even worse). I'd say with all the stuff I mentioned it's already no less than 4-5K. But, as I said, it's up to task owner to rate it budget-wise, I'm just expressing my considerations. And, since you mentioned dynamic linkage. I expect it to be damn hard and time-nsuming based on my experience with relocations. Most of the task will be reading ELF and architecture specs. The code size won't be that huge; but the amount of time will... Again, your estimations might be different.
(In reply to Dmitry Selyutin from comment #8) responded here, sorry, i forgot to add you to the cc list: https://bugs.libre-soc.org/show_bug.cgi?id=1169#c12
(In reply to Dmitry Selyutin from comment #8) > mmap. This is a blocker. It needs to be done. Plus free pages list, > basically a trivial allocator. reminder that this is why cavatools has its own memory-allocator override and iirc free() is a stub that does not actually de-allocate. you'll need to look at the source code closely to confirm, then copy-cat that in ISACaller.
(In reply to Jacob Lifshay from comment #5) > The task seems complete to me, though, EUR 4500 is a lot of money for what > looks like a few days at most, maybe we should add a subtask to implement > more system calls in ISACaller? we can justify that as necessary research. > The work you did so far could be paid in a separate subtask for that work, > so you don't have to wait to get paid for what you already did. > > Dmitry, Luke, Andrey, what do you think? Sorry for late response, been very busy recently. Thanks again for the work Dmitry, Jacob, brilliant progress! I'm happy to extend the scope of the task, so long as it pertains to ISACaller, and doesn't take longer than **two weeks** or so (about how much budget you'll have left). Don't forget that we don't need all the syscalls, so I prefer to do the absolute minimum. We can have a final confirmation with Luke when outline the proposal, and *follow the sub-task procedure* outlined in https://libre-soc.org/libresoc_bug_process (except contacting NLnet, I hope that job will eventually be delegated to me to limit number of people sending stuff to Michiel).
(In reply to Andrey Miroshnikov from comment #11) > with Luke when outline the proposal, and *follow the sub-task procedure* > outlined in https://libre-soc.org/libresoc_bug_process (except https://bugs.libre-soc.org/show_bug.cgi?id=1126#c19 https://libre-soc.org/HDL_workflow/libresoc_bug_process/
Hi Andrey, some questions from my side. 1. Am I right that works on cavatools itself are frozen and the commits will not be used? 2. Am I right that we need to continue to work on 982 to allow a thin set of syscalls (I suggest the same limited set as in cavatools, not more)? 3. What'd be the budget? The only reason I set budget in 982 so low is that I counted on 981 being used. And, in fact, considering amount of work and progress, even at this point I expected this task budget plus the amount from 982bassigned to me at this point already. Could you please clarify your vision on this?
I must confess that it is a strange news. During our meetings I mentioned several times that I'm working on this, discussed the budget and whether this task should be moved or not, did all works and then found it's effectively cancelled. I'd prefer not to waste my energy and do something more practical if I knew that in advance. Please provide some rationale.
(In reply to Dmitry Selyutin from comment #13) > Hi Andrey, some questions from my side. > > 1. Am I right that works on cavatools itself are frozen and the commits will > not be used? We're missing most of the code required to actually use cavatools (no support for PowerISA). So at present these commits cannot be used (*they will be in the future*, so this task will be funded of course). Luke mentioned in #c10: (In reply to Luke Kenneth Casson Leighton from comment #10) > reminder that this is why cavatools has its own memory-allocator > override and iirc free() is a stub that does not actually de-allocate. > you'll need to look at the source code closely to confirm, then > copy-cat that in ISACaller. So perhaps you could do an investigation (doesn't require new code, just document your findings) on what it would take to re-use the cavatools code in ISACaller. This can be funded as moving towards cavatools support. If it's not a lot of work to actually modify ISACaller code, then could be considered as a sub-task, but prioritise research. > 2. Am I right that we need to continue to work on 982 to allow a thin set of > syscalls (I suggest the same limited set as in cavatools, not more)? Yes, that's correct. > 3. What'd be the budget? The only reason I set budget in 982 so low is that > I counted on 981 being used. And, in fact, considering amount of work and > progress, even at this point I expected this task budget plus the amount > from 982bassigned to me at this point already. > > Could you please clarify your vision on this? Dmitry, I'm happy for you to get most of the budget for bug #982, as well as the entirety of bug #981, with the included research sub-task I've suggested above. (In reply to Dmitry Selyutin from comment #14) > I must confess that it is a strange news. During our meetings I mentioned > several times that I'm working on this, discussed the budget and whether > this task should be moved or not, did all works and then found it's > effectively cancelled. I'd prefer not to waste my energy and do something > more practical if I knew that in advance. Please provide some rationale. I apologise Dmitry, there's been much confusion and I'm catching up myself. The reason I said to focus on ISACaller, is because we can use it right now, to aid both general LibreSOC development, as well plan for cavatools work (the original scope of this grant was simply too ambitious, so we'll need more time and resources to get what intially planned). That said, given your rapid progress, I'm not against you studying cavatools a little more, even just to learn about some of its tricks which we can carry over to ISACaller (until we have sufficient funding and people to actually port PowerISA over to cavatools). Luke, I hope this is satisfactory. My reasoning to those answers is primarily to get the grant over and done with, while at the same time giving Dmitry the opportunity to expand our knowledge and potentially improve ISACaller.
(In reply to Andrey Miroshnikov from comment #15) > Luke, I hope this is satisfactory. My reasoning to those answers is > primarily to get the grant over and done with, while at the same time giving > Dmitry the opportunity to expand our knowledge and potentially improve > ISACaller. as long as it can actually be used. the top priority as you know is to get ISACaller able to run relocatable *simple* static-compiled ELF binaries, for customers to be able to use, and that's why we announced at the wednesday meeting that this bug's budget was to be re-targetted "towards" that goal, so that we have all the information needed to make a more accurate better-informed grant request, next year.
quick thought. one way to utilise the work already done here is to also write a python-based c module. this is actually not that hard, i have done it many times. the parameters would not even need to be that large, just pass the GPR table (or if that is awkward just pass 10 parameters - the relevant GPRs as PyObject_Ints) and the mmap memory address. there will be literally dozens of examples online.
(In reply to Luke Kenneth Casson Leighton from comment #17) > there will be literally dozens of examples online. *snort*. how hard is that. https://realpython.com/build-python-c-extension-module/ #include <Python.h> static PyObject *method_fputs(PyObject *self, PyObject *args) { char *str, *filename = NULL; int bytes_copied = -1; /* Parse arguments */ if(!PyArg_ParseTuple(args, "ss", &str, &filename)) { return NULL; } FILE *fp = fopen(filename, "w"); bytes_copied = fputs(str, fp); fclose(fp); return PyLong_FromLong(bytes_copied); } modify that PyArgParseTuple to be a bunch of ints (from the GPR) put the syscall in place of fopen/fputs/fc!ose, blat, done if you really want to get fancy pass a list of the GPRs and use the example here to do it, remember to correct the example by using PyLong_AsLong as advised in the answers https://stackoverflow.com/questions/48328309/pass-python-list-of-ints-to-c-function-using-python-c-api PyTuple is faster but i could not immediately find a suitable example to cut/paste. getting the GPRs as a list is just of course list(self.sim.values())
Folks, this is not how it should work. The task was quite specific and exact: support PPC ABI. It was a complement on cavatools side for task #982: do the same in ISACaller. I did everything which was common between #981 and #982, then proceeded to #981 to make it complete, and completed it. Now we talk about researching cavatools as a whole. Generating the code for the Python module. malloc and free overrides. Don't you yourselves feel the contradiction?
On Python generation part. No idea why we'd want to generate the C Python module, because we already have what we need in plain Python. No, calling C method is not going to save us that many cycles, because we are already going call C bindings (ctypes, check the committed code). Next, on allocation. I don't know why you keep referring to wrapping malloc or free in scope of system calls. These are _not_ system calls, and no sane person ever implements these as system calls. I already told what should be wrapped: mmap/munmap. For those who lived before the historical materialism, there's sbrk (which almost nobody uses these days other than for kegacy reasons, preferring to use MAP_ANON instead). The last but not the least, on the investigation part. I wouldn't have been able to complete this work without investigation and research. Obviously I had to investigate cavatools code before commiting anything there; that's simply how the code gets written. My conclusions for now are as follows: 1. cavatools would be a challenging choice for us. It is RISCV-centric in the bad sense: it does not provide an opportunity to introduce other architectures there other than by extensively modifying the code. 2. The code quality leaves much to be desired. The way the code is written makes it look like a pet project, probably which it indeed is. Most of the code is written ad-hoc. Even syscall numbers were hardcoded as plain numbers. I'm not even speaking about the API part, lack of header guards, naming and style. 3. What can be taken are concepts and ideas. These are not invented by cavatools, sorry, nothing new.
Also, more on allocation: the only thing that was wrapped regarding allocation was sbrk (which, as I mentioned, almost nobody uses these days other than for legacy reasons). And guess what? Even this was as a syscall number, but completely commented out under #if 0 section.
(In reply to Dmitry Selyutin from comment #20) > On Python generation part. No idea why we'd want to generate the C Python > module, because we already have what we need in plain Python. No, calling C > method is not going to save us that many cycles, because we are already > going call C bindings (ctypes, check the committed code). You're right, thanks for reminding me. As you have already written the Python code needed for syscall access (see bug #982, comments 28-46), no more Python work is required for this task. https://git.libre-soc.org/?p=openpower-isa.git;a=tree;f=src/openpower/syscalls;hb=180638ac02637f38301eb97d15a8590293b3a3a4 > > The last but not the least, on the investigation part. I wouldn't have been > able to complete this work without investigation and research. Obviously I > had to investigate cavatools code before commiting anything there; that's > simply how the code gets written. My conclusions for now are as follows: > The best part Dmitry, is this is *exactly* the kind of research we find very useful! :-) > 1. cavatools would be a challenging choice for us. It is RISCV-centric in > the bad sense: it does not provide an opportunity to introduce other > architectures there other than by extensively modifying the code. > 2. The code quality leaves much to be desired. The way the code is written > makes it look like a pet project, probably which it indeed is. Most of the > code is written ad-hoc. Even syscall numbers were hardcoded as plain > numbers. I'm not even speaking about the API part, lack of header guards, > naming and style. > 3. What can be taken are concepts and ideas. These are not invented by > cavatools, sorry, nothing new. Although I knew it was a RISCV-only simulator, I had no idea about such shortcomings. This is incredibly valuable, and worth the full budget of this task in research alone. As a result we will need to rethink our future strategy (and any future grants related to simulation). I'm happy to declare this task complete, given the produced C source code, and the findings about cavatools itself. Luke, are you in agreement?
(In reply to Dmitry Selyutin from comment #20) > 1. cavatools would be a challenging choice for us. It is RISCV-centric in > the bad sense: it does not provide an opportunity to introduce other > architectures there other than by extensively modifying the code. > 2. The code quality leaves much to be desired. these are not in any way fair statements to make, given the context, which nobody knows about (as work should not have started at all on cavatools) Peter Hsu, the author of this work, is the designer of the MIPS R8000 from 1994. the R8000 was extremely significant: it was able to significantly outperform Cray Supercomputers at Sparse Matrix multiplication. Peter retired only two years ago and cavatools was his last main work during his employment at the Barcelona Supercomputing Group where they had only money for RISC-V from the EU, despite BSC wishing that they could develop Power ISA. Peter very much wanted to add additional architectures but under the time and budgetary pressure he could not do so. He was therefore delighted to hear from me when I saw his ISC 2021 talk as I realised the potential of cavatools as a high performance simulator. Most of the planned platform-independent design work is therefore *in his head*, and he also was pushed to add RISC-V RVV, before the basic design was ready to have it added. This left him in the awkward position, close to retirement, of having to short-cut much of the plans he had for cavatools, which were that it be designed fully to support multiple architectures. Peter agreed to port to Power ISA if we got him the pieces that are too complex for him to do: the Decoder and the Compiler. 50 "Forms" in Power ISA instead of only 8. 214 instructions instead of 96. He used parts of the riscv-spike simulator as a short-cut because it was necessary, but it interfered both with the performance and with the overall design. the budget of bug #981 was *supposed* to be 100% exclusively for him to do the Power ABI work, conditional on us providing him with the Power Decoder and Compiler. nobody - none of us - were supposed to touch not even one single line of cavatools. that work was for Peter Hsu to carry out, as he is its author and he is the one that knows it best and how it fits together. I was not able to tell anyone any of this, because work started without me being consulted on the context and background of how to go about completing this complex task within the very small budget and time available to us. this role of finding creative solutions to complex problems is what I am good at. i know *instinctively* how to cut corners by finding the right people and the right pieces, but if any of you attempt to rush ahead without consulting me first, or do work without listening to what I am saying (because you think i am "ordering you about") then the project will simply fail. fortunately enough of the bug #982 task has been completed such that with a little more work it will be possible to justify putting the budget of bug #981 onto ISACaller, because it works TOWARDS cavatools. but i cannot in any way justify signing off a rate of EUR 4,500 per day when the agreed rate as approved by the EU Independent External Reviewers is EUR 3,000 a month. the difference of 50x higher is just too great. if however work on ISACaller ABI support continues *here* for a few more days, such that more information is available to make a better new cavatools grant, that is perfect.
After having a private conversation with Dmitry, I'm happy to confirm that he has done a sufficient amount of work, and that the budget has been justified. Closing this bug, Dmitry, feel free to submit the RFP.
(In reply to Andrey Miroshnikov from comment #24) > After having a private conversation with Dmitry, I'm happy to confirm that > he has done a sufficient amount of work, and that the budget has been > justified. > > Closing this bug, Dmitry, feel free to submit the RFP. Let me write some conclusions in this task before RFP can be submitted so that the task details and implementation details are obvious for everybody around and don't require anybody of us to dig through the whole discussion in task #981 and task #982. The major part of this task actually consisted of developing the flexible system for dynamic system call tables generation; all these commits can be found in master branch, all prefixed with "syscall:" in commit description[0]. Once this mechanism has been implemented, the rest of the task was simpler: since cavatools already possessed a similar mechanism for RISC-V, I only had prepare cavatools for other system call table and generate the cavatools-compatible code based on the previous works mentioned above. All commits are enlisted in comment #2 and comment #3. Obviously these changes are reserved for later: running the userspace does not consist exclusively of the system calls, but also needs a working ISA, which is outside of the scope of this task. In our realities, we could reuse libopid, which I developed before. Once I checked cavatools, I implemented the same concepts in ISACaller; the code resides in a separate branch, scemu[1]. In scope of this particular task, cavatools was mostly a way for us to understand how to achieve the same goal for ISACaller, which, unlike cavatools, is ready to be used with PowerPC architecture. The statements on that part can be found in task #982. [0] https://git.libre-soc.org/?p=openpower-isa.git;a=shortlog;h=refs/heads/master [1] https://git.libre-soc.org/?p=openpower-isa.git;a=shortlog;h=refs/heads/scemu