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

[ebiederm@xmission.com (Eric W. Biederman)]

"Magnus Damm" <magnus.damm@gmail.com> writes:

> On 5/25/06, Eric W. Biederman <ebiederm@xmission.com> wrote:
>> Magnus Damm <magnus@valinux.co.jp> writes:
>> > On Wed, 2006-05-24 at 20:41 -0600, Eric W. Biederman wrote:
>> >> Magnus Damm <magnus@valinux.co.jp> writes:
>>
>> I believe I gave a complete explanation the first round.
>>
>> By having an extra extern variable you can export the offset of
>> a variable on the control code page, or what you need to compute
>> the offset.
>
> You explained some things last round, but there were still some
> questions left open. The main question was regarding "additional
> protection".

Memory that the kernel never sets up for DMA ever.

> I'd be happy to change to code into something that you would feel
> comfortable with, I just like to understand why. Having a
> per-architecture data area in struct kimage is by far the simplest way
> IMO.

But the problem is fundamentally hard.  I do not want to encourage
people to make changes without thinking through all of the consequences.

So far my impression is that an additional per-architecture data area
is struct kimage encourages people to be sloppy, and it moves key structures
farther from where they are used.  I am coming to suspect it is as bad
as ioctl.

I could probably be convinced with a good use of a per-architecture
area and a well reasoned argument.  But at the moment changing the
infrastructure is unnecessary noise, so please drop that for now.

>> Part of the reason is you do more than one thing at a time, which makes
>> review much more difficult than it needs to be.
>
> Yeah, I know. I'm sorry about that. I took some time to split the
> patches in the most logical way I could think of. The reason behind
> not separating the segment code and the page_table_a code was that
> they both touched more or less the same lines.

Dependent patches are not a problem.

> And by global structure you mean the dynamically allocated struct
> kimage? If you find that abusive then I think that _not_ using an
> already existing structure is abusive. =)
>
> I just want to keep things as simple as possible.

Simplicity is good.  

Doing unnecessary things is confusing and the issue is not good,
and at least until the Xen support is merged you were doing
unnecessary things.

Please just take it carefully.  This is possibly the hardest
to debug code path in the kernel and currently it works.  I
don't want to break that.

Eric