2010-06-12 13:35:02

by Paolo Giarrusso

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH] x86, hweight: Fix UML boot crash

On Sun, May 30, 2010 at 23:09, H. Peter Anvin <[email protected]> wrote:
> On 05/30/2010 01:17 PM, Borislav Petkov wrote:
>>>> This bothers me, because it really feels like something is fundamentally
>>>> broken in UML tryingto track the upstream architecture, and this is just
>>>> a bandage.
>>>
>>> First of all, scratch that patch. It is indeed dumb idea to sprinkle UML
>>> special cases in x86 just because they include it.
>>>
>>> Which begs the question why _is_ UML sucking in x86 stuff and can anyone
>>> provide us with some sensible reasons? Because if there aren't any, it
>>> is their includes that should be fixed. Let me see what I can do to
>>> redirect hweight stuff properly...
>>
>> Ok, AFAICT UML is sucking in the includes of the sub-architecture the
>> UML "guest" is running on. See below¹ for the whole gcc string make
>> executes. Among the switches is
>>
>> "-I/home/boris/kernel/linux-2.6/arch/x86/include"
>>
>> so there will be no untangling today. Instead, we could do another
>> bandaid which is confined to UML include space only and redirect
>> arch_hweight.h includes to the generic ones. Check this out, it seems to
>> work here:
>>
>
> That looks better to me, although I'm still wondering why UML can't
> stomach the register-saving tricks... it is not at all "obvious" why
> that can't be done.
Hi all, and sorry for the delay, I hope you still care about this.

First, ARCH_HWEIGHT_CFLAGS should IMHO be shared with UML. I.e., moved
to arch/x86/Kconfig.cpu (which was born as Kconfig code shared with
UML), or copied in UML (it's not defined, as far as I can see).
Otherwise it just can't work. And I think that's it.

Second, I've been looking at arch_hweight.h to try answering as well,
and my question is: did somebody ever implement ALTERNATIVE support on
UML? When I worked on it, this thing didn't exist at all. The user
declared the host CPU, and we enabled features based on that. There's
barely code for exception tables, and we never used it to implement
copy_from_user and staff like that (I recall the exception handler was
set at run-time).

Indeed, arch/um/kernel/um_arch.c:apply_alternatives() is empty. And I
mean, implementing it is not so trivial (unlike exception handling),
simply because it requires making the binary mapping writable, and I'm
not sure UML does it already.

A third note is that UML links with glibc, so it can have a different
calling convention from the kernel. Say, on x86 32bit regparm doesn't
work (in fact, -mregparm is set in arch/x86/Makefile and not in
arch/x86/Makefile_32.cpu). And since popcnt is supported on 32bit, it
might in theory make a difference for that case. But maybe those flags
are simply fine, I didn't recheck the possible calling conventions.

Good bye!
--
Paolo Giarrusso - Ph.D. Student
http://www.informatik.uni-marburg.de/~pgiarrusso/


2010-06-12 14:18:58

by Borislav Petkov

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH] x86, hweight: Fix UML boot crash

From: Paolo Giarrusso <[email protected]>
Date: Sat, Jun 12, 2010 at 03:34:38PM +0200

Hi,

> > That looks better to me, although I'm still wondering why UML can't
> > stomach the register-saving tricks... it is not at all "obvious" why
> > that can't be done.
> Hi all, and sorry for the delay, I hope you still care about this.
>
> First, ARCH_HWEIGHT_CFLAGS should IMHO be shared with UML. I.e., moved
> to arch/x86/Kconfig.cpu (which was born as Kconfig code shared with
> UML), or copied in UML (it's not defined, as far as I can see).
> Otherwise it just can't work. And I think that's it.
>
> Second, I've been looking at arch_hweight.h to try answering as well,
> and my question is: did somebody ever implement ALTERNATIVE support on
> UML? When I worked on it, this thing didn't exist at all. The user
> declared the host CPU, and we enabled features based on that. There's
> barely code for exception tables, and we never used it to implement
> copy_from_user and staff like that (I recall the exception handler was
> set at run-time).
>
> Indeed, arch/um/kernel/um_arch.c:apply_alternatives() is empty. And I
> mean, implementing it is not so trivial (unlike exception handling),
> simply because it requires making the binary mapping writable, and I'm
> not sure UML does it already.

Which would mean that UML doesn't use alternatives at all and uses the
instructions which are meant to be replaced instead, no? In that case,
fixing this is either by rerouting the includes (easiest, already in
-tip) or adding alternatives support (harder, needs volunteers :)).

> A third note is that UML links with glibc, so it can have a different
> calling convention from the kernel. Say, on x86 32bit regparm doesn't
> work (in fact, -mregparm is set in arch/x86/Makefile and not in
> arch/x86/Makefile_32.cpu). And since popcnt is supported on 32bit, it
> might in theory make a difference for that case. But maybe those flags
> are simply fine, I didn't recheck the possible calling conventions.

If this is also the case, the -fcall-saved-* stuff won't work on UML and
yet another way of doing "call *func" from within asm("...") and making
sure the callee doesn't clobber caller's regs will be needed for UML.

--
Regards/Gruss,
Boris.

2010-06-12 16:02:10

by Paolo Giarrusso

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH] x86, hweight: Fix UML boot crash

On Sat, Jun 12, 2010 at 16:18, Borislav Petkov <[email protected]> wrote:
> From: Paolo Giarrusso <[email protected]>
> Date: Sat, Jun 12, 2010 at 03:34:38PM +0200
>
> Hi,
>
>> > That looks better to me, although I'm still wondering why UML can't
>> > stomach the register-saving tricks... it is not at all "obvious" why
>> > that can't be done.
>> Hi all, and sorry for the delay, I hope you still care about this.
>>
>> First, ARCH_HWEIGHT_CFLAGS should IMHO be shared with UML. I.e., moved
>> to arch/x86/Kconfig.cpu (which was born as Kconfig code shared with
>> UML), or copied in UML (it's not defined, as far as I can see).
>> Otherwise it just can't work. And I think that's it.

Just to be sure: by "that's it" I meant "this is the problem".
You didn't answer here - did you see it? What do you think? Can you
try the one-line fix at some point?
Just to make it clear: I've not been actively developing UML (or
almost anything in kernel space) for ages (~4 years), so it's unlikely
that I'll try fixing this. It just happens that things on the UML
front stayed mostly the same, so I thought that my knowledge of the
code is still useful.

>> Second, I've been looking at arch_hweight.h to try answering as well,
>> and my question is: did somebody ever implement ALTERNATIVE support on
>> UML? When I worked on it, this thing didn't exist at all. The user
>> declared the host CPU, and we enabled features based on that. There's
>> barely code for exception tables, and we never used it to implement
>> copy_from_user and staff like that (I recall the exception handler was
>> set at run-time).

>> Indeed, arch/um/kernel/um_arch.c:apply_alternatives() is empty. And I
>> mean, implementing it is not so trivial (unlike exception handling),
>> simply because it requires making the binary mapping writable, and I'm
>> not sure UML does it already.

> Which would mean that UML doesn't use alternatives at all and uses the
> instructions which are meant to be replaced instead, no?

Exactly.

> In that case,
> fixing this is either by rerouting the includes (easiest, already in
> -tip) or adding alternatives support (harder, needs volunteers :)).

Well, even doing just nothing should work, if you fix the trivial
thing above (which at least for 64bit should work).

>> A third note is that UML links with glibc, so it can have a different
>> calling convention from the kernel. Say, on x86 32bit regparm doesn't
>> work (in fact, -mregparm is set in arch/x86/Makefile and not in
>> arch/x86/Makefile_32.cpu). And since popcnt is supported on 32bit, it
>> might in theory make a difference for that case. But maybe those flags
>> are simply fine, I didn't recheck the possible calling conventions.

> If this is also the case, the -fcall-saved-* stuff won't work on UML and
> yet another way of doing "call *func" from within asm("...") and making
> sure the callee doesn't clobber caller's regs will be needed for UML.

Hmpf... anyway, 64bit should be fine since there's just one calling
convention, everywhere, and already regparm'ed.

Regards
--
Paolo Giarrusso - Ph.D. Student
http://www.informatik.uni-marburg.de/~pgiarrusso/

2010-06-12 16:35:08

by Borislav Petkov

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH] x86, hweight: Fix UML boot crash

From: Paolo Giarrusso <[email protected]>
Date: Sat, Jun 12, 2010 at 06:01:44PM +0200

> >> First, ARCH_HWEIGHT_CFLAGS should IMHO be shared with UML. I.e., moved
> >> to arch/x86/Kconfig.cpu (which was born as Kconfig code shared with
> >> UML), or copied in UML (it's not defined, as far as I can see).
> >> Otherwise it just can't work. And I think that's it.
>
> Just to be sure: by "that's it" I meant "this is the problem".
> You didn't answer here - did you see it? What do you think? Can you
> try the one-line fix at some point?
> Just to make it clear: I've not been actively developing UML (or
> almost anything in kernel space) for ages (~4 years), so it's unlikely
> that I'll try fixing this. It just happens that things on the UML
> front stayed mostly the same, so I thought that my knowledge of the
> code is still useful.

Cool :). However, according to Geert, this doesn't fix it:

http://marc.info/?l=linux-kernel&m=127522065202435&w=2

It could be related to the -mregparm being broken on 32-bit UML since
Geert's UML "guest" is 32-bit. However, even if we fix this, it won't
be used since, as you said, UML doesn't do alternatives. Which means
that it doesn't make sense fixing it until there are no alternatives -
instead, we should simply fall back to the software hweight* stuff and
be done with it.

> > In that case, fixing this is either by rerouting the includes
> > (easiest, already in -tip) or adding alternatives support (harder,
> > needs volunteers :)).
>
> Well, even doing just nothing should work, if you fix the trivial
> thing above (which at least for 64bit should work).

See above.

> >> A third note is that UML links with glibc, so it can have a different
> >> calling convention from the kernel. Say, on x86 32bit regparm doesn't
> >> work (in fact, -mregparm is set in arch/x86/Makefile and not in
> >> arch/x86/Makefile_32.cpu). And since popcnt is supported on 32bit, it
> >> might in theory make a difference for that case. But maybe those flags
> >> are simply fine, I didn't recheck the possible calling conventions.
>
> > If this is also the case, the -fcall-saved-* stuff won't work on UML and
> > yet another way of doing "call *func" from within asm("...") and making
> > sure the callee doesn't clobber caller's regs will be needed for UML.
>
> Hmpf... anyway, 64bit should be fine since there's just one calling
> convention, everywhere, and already regparm'ed.

Right, as I said, this would leave 32-bit broken which doesn't cut it
either for a subset of people using UML.

Thanks.

--
Regards/Gruss,
Boris.

2010-06-12 18:37:42

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH] x86, hweight: Fix UML boot crash

On Sat, Jun 12, 2010 at 18:34, Borislav Petkov <[email protected]> wrote:
> From: Paolo Giarrusso <[email protected]>
> Date: Sat, Jun 12, 2010 at 06:01:44PM +0200
>
>> >> First, ARCH_HWEIGHT_CFLAGS should IMHO be shared with UML. I.e., moved
>> >> to arch/x86/Kconfig.cpu (which was born as Kconfig code shared with
>> >> UML), or copied in UML (it's not defined, as far as I can see).
>> >> Otherwise it just can't work. And I think that's it.
>>
>> Just to be sure: by "that's it" I meant "this is the problem".
>> You didn't answer here - did you see it? What do you think? Can you
>> try the one-line fix at some point?
>> Just to make it clear: I've not been actively developing UML (or
>> almost anything in kernel space) for ages (~4 years), so it's unlikely
>> that I'll try fixing this. It just happens that things on the UML
>> front stayed mostly the same, so I thought that my knowledge of the
>> code is still useful.
>
> Cool :). However, according to Geert, this doesn't fix it:
>
> http://marc.info/?l=linux-kernel&m=127522065202435&w=2
>
> It could be related to the -mregparm being broken on 32-bit UML since
> Geert's UML "guest" is 32-bit. However, even if we fix this, it won't

No, guest and host are both x86_64.

> be used since, as you said, UML doesn't do alternatives. Which means
> that it doesn't make sense fixing it until there are no alternatives -
> instead, we should simply fall back to the software hweight* stuff and
> be done with it.
>
>> > In that case, fixing this is either by rerouting the includes
>> > (easiest, already in -tip) or adding alternatives support (harder,
>> > needs volunteers :)).
>>
>> Well, even doing just nothing should work, if you fix the trivial
>> thing above (which at least for 64bit should work).
>
> See above.
>
>> >> A third note is that UML links with glibc, so it can have a different
>> >> calling convention from the kernel. Say, on x86 32bit regparm doesn't
>> >> work (in fact, -mregparm is set in arch/x86/Makefile and not in
>> >> arch/x86/Makefile_32.cpu). And since popcnt is supported on 32bit, it
>> >> might in theory make a difference for that case. But maybe those flags
>> >> are simply fine, I didn't recheck the possible calling conventions.
>>
>> > If this is also the case, the -fcall-saved-* stuff won't work on UML and
>> > yet another way of doing "call *func" from within asm("...") and making
>> > sure the callee doesn't clobber caller's regs will be needed for UML.
>>
>> Hmpf... anyway, 64bit should be fine since there's just one calling
>> convention, everywhere, and already regparm'ed.
>
> Right, as I said, this would leave 32-bit broken which doesn't cut it
> either for a subset of people using UML.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2010-06-13 07:08:07

by Borislav Petkov

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH] x86, hweight: Fix UML boot crash

From: Geert Uytterhoeven <[email protected]>
Date: Sat, Jun 12, 2010 at 08:37:39PM +0200

> > Cool :). However, according to Geert, this doesn't fix it:
> >
> > http://marc.info/?l=linux-kernel&m=127522065202435&w=2
> >
> > It could be related to the -mregparm being broken on 32-bit UML since
> > Geert's UML "guest" is 32-bit. However, even if we fix this, it won't
>
> No, guest and host are both x86_64.

Ok, maybe I don't understand UML - it's just that all address values in
the backtrace are 32-bit (e.g. RDX: 00000000ffff8aed, with the upper
8 bytes zeroed out) and I assumed that this is a 32-bit "guest" on a
64-bit host.

--
Regards/Gruss,
Boris.