On 5/25/06, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > Magnus Damm <[EMAIL PROTECTED]> writes: > > > Hi Eric, > > > > On Wed, 2006-05-24 at 20:56 -0600, Eric W. Biederman wrote: > >> > >> C code is much more accessible to other programmers than arch specific > >> assembly. The code on the control page was almost written in C, and > >> I'm still not quite convinced that it would be wrong to do that. > > > > I agree with you that it is of course better to implement something in C > > if possible compared to writing it in architecture-specific assembly. > > > > But I do not agree that wrapping architecture-specific assembly code in > > C functions makes the code more understandable. I'd really like to meet > > the kernel hacker that is aware of how x86 segmentation works but is > > unable to read x86 assembly. > > For some young programmers it may be a matter of reading ability. > For older programmers it is more likely to be a matter of reading > speed. > > Regardless that is how the code is now, and how it came out of the series > of code reviews I had to go through when I wrote it. I had requests > to do more in C and I never had a request to do more in assembly. > Proving there was no sane way to write the control code page in > C was actually difficult.
Consider this a "more assembly" request then. What about these reasons: - C code requires a stack You must access one page only and therefore you need to setup the stackpointer somewhere. Requires assembly. - We switch to a new page table, twice on x86_64 Requires assembly. - Need to setup segment registeres and cr* Requires assembly. - You need to setup/clear registers before passing control Requires assembly. > If there is a legitimate reason to change the code that is fine. But > as it looked as simply a change without a good reason that is not > fine. We would have to duplicate the segment handling code in our xen port otherwise. It's no biggie, it's just a matter of a few lines of code. Also, I feel that my approach with a valid idt and gdt is more robust. > The big problem was you did several things with a single patch, > and that made the review much more difficult than it had to be. > > Having to check if you correctly modified the page tables, while also > having to check for segmentation, and the interrupt descriptor > transformations was distracting. Let me know which parts you think are good and we will implement and review them bit by bit instead then. Thanks, / magnus _______________________________________________ fastboot mailing list [email protected] https://lists.osdl.org/mailman/listinfo/fastboot
