2012-06-12 21:07:10

by Geert Uytterhoeven

[permalink] [raw]
Subject: asm/kvm_para.h from asm-generic (was: Re: linux-next: triage for April 19, 2012)

On Wed, May 30, 2012 at 10:52 AM, Geert Uytterhoeven
<[email protected]> wrote:
> On Mon, May 28, 2012 at 5:04 PM, Geert Uytterhoeven
> <[email protected]> wrote:
>> On Mon, May 28, 2012 at 4:37 PM, Avi Kivity <[email protected]> wrote:
>>> On 05/25/2012 11:59 PM, Geert Uytterhoeven wrote:
>>>> On Fri, Apr 20, 2012 at 4:00 AM, Paul Gortmaker
>>>> <[email protected]> wrote:
>>>>> The parisc got borked by some kvm header shuffle it seems?
>>>>> Now complaining about "file 'asm-generic/kvm_para.h' is not exported"
>>>>> [ http://kisskb.ellerman.id.au/kisskb/buildresult/6137786/ ]
>>>>
>>>> Not only parisc.
>>>>
>>>> This breakage has now entered mainline:
>>>>
>>>> parisc deconfig http://kisskb.ellerman.id.au/kisskb/buildresult/6365677/
>>>> m68k allmodconfig: http://kisskb.ellerman.id.au/kisskb/buildresult/6365681/
>>>
>>>
>>> Does the following patch help?
>>
>> Thanks, that fixes it!
>>
>> Tested-by: Geert Uytterhoeven <[email protected]>
>>
>>> From: Avi Kivity <[email protected]>
>>> Date: Mon, 28 May 2012 17:35:22 +0300
>>> Subject: [PATCH] KVM: Export asm-generic/kvm_para.h
>>>
>>> Prevents build failures on non-KVM archs.
>>>
>>> Signed-off-by: Avi Kivity <[email protected]>
>>> ---
>>>  include/asm-generic/Kbuild |    1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
>>> index 53f91b1..2c85a0f 100644
>>> --- a/include/asm-generic/Kbuild
>>> +++ b/include/asm-generic/Kbuild
>>> @@ -8,6 +8,7 @@ header-y += int-ll64.h
>>>  header-y += ioctl.h
>>>  header-y += ioctls.h
>>>  header-y += ipcbuf.h
>>> +header-y += kvm_para.h
>>>  header-y += mman-common.h
>>>  header-y += mman.h
>>>  header-y += msgbuf.h
>
> I just noticed include/asm-generic/Kbuild.asm already had
>
> ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h \
>                  $(srctree)/include/asm-$(SRCARCH)/kvm_para.h),)
> header-y  += kvm_para.h
> endif
>
> but this doesn't seem to work.
>
> Kbuild people: which one is correct?

Oops...

After linux-next commit 2bbc89a8e9c652ee71c6c3b2e0679b7ecedb1a09
("m68k: Use Kbuild logic to import asm-generic headers"), which does:
- remove arch/m68k/include/asm/kvm_para.h, which just included the
asm-generic version,
- add "generic-y += kvm_para.h" to arch/m68k/include/asm/Kbuild,
it fails again:

$ make ARCH=m68k headers_check
CHK include/linux/version.h
INSTALL include/asm-generic (35 files)
INSTALL include/linux (372 files)
CHECK include/asm-generic (35 files)
CHECK include/linux (372 files)
usr/include/linux/kexec.h:49: userspace cannot reference function or
variable defined in the kernel
usr/include/linux/kvm_para.h:26: included file 'asm-m68k/kvm_para.h'
is not exported
usr/include/linux/soundcard.h:1054: userspace cannot reference
function or variable defined in the kernel
make[2]: *** [usr/include/linux/.check] Error 123
make[1]: *** [linux] Error 2
make: *** [headers_check] Error 2
$

After reverting Avi's fix (commit
7beb8e723c8d7da7decbbe217b79525aef73fccb. ("KVM:
Export asm-generic/kvm_para.h")) it works again. Note that it now _removes_
kvm_para.h:

$ make ARCH=m68k headers_check
CHK include/linux/version.h
REMOVE kvm_para.h
INSTALL include/asm-generic (34 files)
REMOVE kvm_para.h
INSTALL include/linux (371 files)
CHECK include/asm-generic (34 files)
CHECK include/linux (371 files)
usr/include/linux/kexec.h:49: userspace cannot reference function or
variable defined in the kernel
usr/include/linux/soundcard.h:1054: userspace cannot reference
function or variable defined in the kernel
$

What's the proper way to get this working in both cases??
1. arch has it's own asm/kvm_para.h (cfr. m68k and all other arches
in mainline)
2. arch includes asm-generic/kvm_para.h via Kbuild logic (cfr.
(only) m68k in -next)
Or is this not possible, and should I create arch/m68k/include/asm/kvm_para.h
again, like all other arches seem to do?

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


2012-06-13 12:46:37

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: asm/kvm_para.h from asm-generic (was: Re: linux-next: triage for April 19, 2012)

On Tue, Jun 12, 2012 at 11:07 PM, Geert Uytterhoeven
<[email protected]> wrote:
> On Wed, May 30, 2012 at 10:52 AM, Geert Uytterhoeven
> <[email protected]> wrote:
>> On Mon, May 28, 2012 at 5:04 PM, Geert Uytterhoeven
>> <[email protected]> wrote:
>>> On Mon, May 28, 2012 at 4:37 PM, Avi Kivity <[email protected]> wrote:
>>>> On 05/25/2012 11:59 PM, Geert Uytterhoeven wrote:
>>>>> On Fri, Apr 20, 2012 at 4:00 AM, Paul Gortmaker
>>>>> <[email protected]> wrote:
>>>>>> The parisc got borked by some kvm header shuffle it seems?
>>>>>> Now complaining about "file 'asm-generic/kvm_para.h' is not exported"
>>>>>> [ http://kisskb.ellerman.id.au/kisskb/buildresult/6137786/ ]
>>>>>
>>>>> Not only parisc.
>>>>>
>>>>> This breakage has now entered mainline:
>>>>>
>>>>> parisc deconfig http://kisskb.ellerman.id.au/kisskb/buildresult/6365677/
>>>>> m68k allmodconfig: http://kisskb.ellerman.id.au/kisskb/buildresult/6365681/
>>>>
>>>>
>>>> Does the following patch help?
>>>
>>> Thanks, that fixes it!
>>>
>>> Tested-by: Geert Uytterhoeven <[email protected]>
>>>
>>>> From: Avi Kivity <[email protected]>
>>>> Date: Mon, 28 May 2012 17:35:22 +0300
>>>> Subject: [PATCH] KVM: Export asm-generic/kvm_para.h
>>>>
>>>> Prevents build failures on non-KVM archs.
>>>>
>>>> Signed-off-by: Avi Kivity <[email protected]>
>>>> ---
>>>>  include/asm-generic/Kbuild |    1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
>>>> index 53f91b1..2c85a0f 100644
>>>> --- a/include/asm-generic/Kbuild
>>>> +++ b/include/asm-generic/Kbuild
>>>> @@ -8,6 +8,7 @@ header-y += int-ll64.h
>>>>  header-y += ioctl.h
>>>>  header-y += ioctls.h
>>>>  header-y += ipcbuf.h
>>>> +header-y += kvm_para.h
>>>>  header-y += mman-common.h
>>>>  header-y += mman.h
>>>>  header-y += msgbuf.h
>>
>> I just noticed include/asm-generic/Kbuild.asm already had
>>
>> ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h \
>>                  $(srctree)/include/asm-$(SRCARCH)/kvm_para.h),)
>> header-y  += kvm_para.h
>> endif
>>
>> but this doesn't seem to work.
>>
>> Kbuild people: which one is correct?
>
> Oops...
>
> After linux-next commit 2bbc89a8e9c652ee71c6c3b2e0679b7ecedb1a09
> ("m68k: Use Kbuild logic to import asm-generic headers"), which does:
>  - remove arch/m68k/include/asm/kvm_para.h, which just included the
>    asm-generic version,
>  - add "generic-y += kvm_para.h" to arch/m68k/include/asm/Kbuild,
> it fails again:
>
> $ make ARCH=m68k headers_check
>  CHK     include/linux/version.h
>  INSTALL include/asm-generic (35 files)
>  INSTALL include/linux (372 files)
>  CHECK   include/asm-generic (35 files)
>  CHECK   include/linux (372 files)
> usr/include/linux/kexec.h:49: userspace cannot reference function or
> variable defined in the kernel
> usr/include/linux/kvm_para.h:26: included file 'asm-m68k/kvm_para.h'
> is not exported
> usr/include/linux/soundcard.h:1054: userspace cannot reference
> function or variable defined in the kernel
> make[2]: *** [usr/include/linux/.check] Error 123
> make[1]: *** [linux] Error 2
> make: *** [headers_check] Error 2
> $
>
> After reverting Avi's fix (commit
> 7beb8e723c8d7da7decbbe217b79525aef73fccb. ("KVM:
> Export asm-generic/kvm_para.h")) it works again. Note that it now _removes_
> kvm_para.h:
>
> $ make ARCH=m68k headers_check
>  CHK     include/linux/version.h
>  REMOVE  kvm_para.h
>  INSTALL include/asm-generic (34 files)
>  REMOVE  kvm_para.h
>  INSTALL include/linux (371 files)
>  CHECK   include/asm-generic (34 files)
>  CHECK   include/linux (371 files)
> usr/include/linux/kexec.h:49: userspace cannot reference function or
> variable defined in the kernel
> usr/include/linux/soundcard.h:1054: userspace cannot reference
> function or variable defined in the kernel
> $
>
> What's the proper way to get this working in both cases??
>  1. arch has it's own asm/kvm_para.h (cfr. m68k and all other arches
> in mainline)
>  2. arch includes asm-generic/kvm_para.h via Kbuild logic (cfr.
> (only) m68k in -next)
> Or is this not possible, and should I create arch/m68k/include/asm/kvm_para.h
> again, like all other arches seem to do?

I dived into this, and spent a little more time on it than is healthy for me :-(

"make headers_check" is also broken on m32r since Avi's fix, as m32r
doesn't even
have <asm/kvm_param.h>:

| usr/include/linux/kvm_para.h:26: included file 'asm-m32r/kvm_para.h'
is not exported

commit 3b5d56b9317fa7b5407dff1aa7b115bf6cdbd494 ("kvmclock: Add functions to
check if the host has stopped the vm") created <asm/kvm_para.h> for
all architectures,
but forgot cris and m32r.

BTW, do we really want all these <asm/kvm_para.h> pointing to (empty)
<asm-generic/kvm_para.h> after exporting?

commit 7dcf2a9fced59e58e4694cdcf15850c01fdba89b ("remove dummy asm/kvm.h files")
removed the similar dummy <asm/kvm.h> files, and added a rule to
include/asm-generic/Kbuild.asm instead:

ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm.h \
$(srctree)/include/asm-$(SRCARCH)/kvm.h),)
header-y += kvm.h
endif

There's a similar rule for kvm_para.h:

ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h \
$(srctree)/include/asm-$(SRCARCH)/kvm_para.h),)
header-y += kvm_para.h
endif

It's this rule that's now biting m68k, as it no longer works when using
arch/*/include/asm/Kbuild to import <asm-generic/kvm_para.h>.
(I guess it'll bite SH soon, too).

I found two ways to fix it:
1. Remove the "ifneq" above from include/asm-generic/Kbuild.asm,
so kvm_para.h is exported unconditionally.
Note that this breaks m32r and cris, as they don't have
asm/kvm_para.h yet),
2. Add a similar "ifneq" to include/asm-generic/Kbuild, to make
the export of <linux/kvm_para.h> conditional.
I guess we want to remove all "dummy" kvm_para.h again, too?

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

2012-06-13 12:49:26

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: asm/kvm_para.h from asm-generic (was: Re: linux-next: triage for April 19, 2012)

On Wed, Jun 13, 2012 at 2:46 PM, Geert Uytterhoeven
<[email protected]> wrote:
> "make headers_check" is also broken on m32r since Avi's fix, as m32r
> doesn't even
> have <asm/kvm_param.h>:

BTW, you don't need to install any cross-compilers for headers_check, just do

make ARCH=xxx headers_check

if you want to give it a try...

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

2012-06-14 03:30:00

by Paul Mundt

[permalink] [raw]
Subject: Re: asm/kvm_para.h from asm-generic (was: Re: linux-next: triage for April 19, 2012)

On Wed, Jun 13, 2012 at 02:46:32PM +0200, Geert Uytterhoeven wrote:
> There's a similar rule for kvm_para.h:
>
> ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h \
> $(srctree)/include/asm-$(SRCARCH)/kvm_para.h),)
> header-y += kvm_para.h
> endif
>
> It's this rule that's now biting m68k, as it no longer works when using
> arch/*/include/asm/Kbuild to import <asm-generic/kvm_para.h>.
> (I guess it'll bite SH soon, too).
>
So it did, thanks for catching it, today looks to be chock full of header
excitement.

> I found two ways to fix it:
> 1. Remove the "ifneq" above from include/asm-generic/Kbuild.asm,
> so kvm_para.h is exported unconditionally.
> Note that this breaks m32r and cris, as they don't have
> asm/kvm_para.h yet),
> 2. Add a similar "ifneq" to include/asm-generic/Kbuild, to make
> the export of <linux/kvm_para.h> conditional.
> I guess we want to remove all "dummy" kvm_para.h again, too?
>
m32r and cris are broken at the moment anyways because they were
overlooked to begin with, so stubbing in generic-y += kvm_para.h rules
there should be sufficient. Presumably with those in place it should be
possible to kill off the wildcarding and just have it exported
unconditionally, as per your initial fix.

As to the merit of exporting a header with nothing in it to userspace,
Avi will have to figure that one out.

2012-06-19 18:48:07

by Sam Ravnborg

[permalink] [raw]
Subject: Re: asm/kvm_para.h from asm-generic (was: Re: linux-next: triage for April 19, 2012)

> >>
> >> I just noticed include/asm-generic/Kbuild.asm already had
> >>
> >> ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h \
> >> ? ? ? ? ? ? ? ? ?$(srctree)/include/asm-$(SRCARCH)/kvm_para.h),)
> >> header-y ?+= kvm_para.h
> >> endif
> >>
> >> but this doesn't seem to work.
> >>
> >> Kbuild people: which one is correct?

Any use of wildcards in include/asm-generic/Kbuild.asm is bogus.
We should:
1) add explicit header-y for kvm.h + a.out.h to the relevant architectures.
2) add explicit header-y for kvm_para.h for the archictectures that needs this file
3) and drop header-y / dummy file fro the others.

Anything else is just papering over the initial wrong doings.

I actually coded up "header-n" support to explicitly say:
do not include this file for this arch - but looking at the
actual problem being solved this is just bogus so I killed
it off again.

Sam

2012-06-19 19:00:14

by Sam Ravnborg

[permalink] [raw]
Subject: Re: asm/kvm_para.h from asm-generic (was: Re: linux-next: triage for April 19, 2012)

On Tue, Jun 19, 2012 at 08:48:03PM +0200, Sam Ravnborg wrote:
> > >>
> > >> I just noticed include/asm-generic/Kbuild.asm already had
> > >>
> > >> ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h \
> > >> ? ? ? ? ? ? ? ? ?$(srctree)/include/asm-$(SRCARCH)/kvm_para.h),)
> > >> header-y ?+= kvm_para.h
> > >> endif
> > >>
> > >> but this doesn't seem to work.
> > >>
> > >> Kbuild people: which one is correct?
>
> Any use of wildcards in include/asm-generic/Kbuild.asm is bogus.
> We should:
> 1) add explicit header-y for kvm.h + a.out.h to the relevant architectures.
> 2) add explicit header-y for kvm_para.h for the archictectures that needs this file
> 3) and drop header-y / dummy file fro the others.
>
> Anything else is just papering over the initial wrong doings.
>
> I actually coded up "header-n" support to explicitly say:
> do not include this file for this arch - but looking at the
> actual problem being solved this is just bogus so I killed
> it off again.

Just to be clear - I know this will break headers_check_all.
I will try to take a look soon for a better generic solution.

Sam