Re: [Fastboot] [PATCH 00/03] kexec: Avoid overwriting the current pgd (V2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux