Patchwork [v2] Added uapi directory into linux-header

login
register
mail settings
Submitter Alexander Graf
Date Dec. 17, 2012, 5:48 p.m.
Message ID <0C2DB64D-B8EB-492B-876F-111BD818101C@suse.de>
Download mbox | patch
Permalink /patch/206939/
State New
Headers show

Comments

Alexander Graf - Dec. 17, 2012, 5:48 p.m.
On 17.12.2012, at 17:01, Bharat Bhushan wrote:

> Linux ARCH specific header files are now in uapi/ directory also.
> These header files (epapr_hcalls.h on powerpc) are needed in qemu
> in linux-headers/uapi/asm/ directory as these are referenced by
> other header files in linux-headers/asm/.
> 
> This patch is about changing the scripts for same.
> 
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> ---
> v2:
> - remove symlink on distclean
> - review comments on v1
> 
> Makefile                        |    1 +
> configure                       |    4 ++++
> scripts/update-linux-headers.sh |   12 ++++++++++++
> 3 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 9ecbcbb..ed320ea 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -282,6 +282,7 @@ distclean: clean
> 	rm -f qemu-doc.vr
> 	rm -f config.log
> 	rm -f linux-headers/asm
> +	rm -f linux-headers/uapi/asm
> 	rm -f qemu-tech.info qemu-tech.aux qemu-tech.cp qemu-tech.dvi qemu-tech.fn qemu-tech.info qemu-tech.ky qemu-tech.log qemu-tech.pdf qemu-tech.pg qemu-tech.toc qemu-tech.tp qemu-tech.vr
> 	for d in $(TARGET_DIRS) $(QEMULIBS); do \
> 	rm -rf $$d || exit 1 ; \
> diff --git a/configure b/configure
> index 38b1cc6..4c11e3d 100755
> --- a/configure
> +++ b/configure
> @@ -3725,6 +3725,10 @@ if test "$linux" = "yes" ; then
>     if [ -e "$source_path/linux-headers/asm-$linux_arch" ]; then
>       symlink "$source_path/linux-headers/asm-$linux_arch" linux-headers/asm
>     fi
> +    if [ -e "$source_path/linux-headers/uapi/asm-$linux_arch" ]; then
> +      symlink "$source_path/linux-headers/uapi/asm-$linux_arch" linux-headers/uapi/asm
> +    fi
> +
> fi
> 
> for target in $target_list; do
> diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
> index 4c7b566..44e6cad 100755
> --- a/scripts/update-linux-headers.sh
> +++ b/scripts/update-linux-headers.sh
> @@ -46,14 +46,26 @@ for arch in $ARCHLIST; do
> 
>     make -C "$linux" INSTALL_HDR_PATH="$tmpdir" SRCARCH=$arch headers_install
> 
> +    if [ -e "$linux/arch/$arch/include/uapi" ] &&
> +        ! [ -e "$output/linux-headers/uapi" ] ; then
> +        mkdir "$output/linux-headers/uapi"

mkdir -p

But looking through this whole thing, it seems like the root cause is actually different. We don't want any uapi directories exposed to user space. So let's go back a step:

Why do we need the uapi include dir? Because some header is using it.

linux-headers/asm-powerpc/kvm_para.h:

    #include <uapi/asm/epapr_hcalls.h>

This is the root cause of the problem. We must never manually include any uapi header paths. We only ever include their normal asm-counterparts which then may include uapi (in kernel) or actually are uapi (in user space). David, please correct me if I'm wrong.

Could you please try and see if this kernel side patch makes things work for you too?


Alex
Bharat Bhushan - Dec. 18, 2012, 1:14 a.m.
> > +++ b/scripts/update-linux-headers.sh
> > @@ -46,14 +46,26 @@ for arch in $ARCHLIST; do
> >
> >     make -C "$linux" INSTALL_HDR_PATH="$tmpdir" SRCARCH=$arch
> > headers_install
> >
> > +    if [ -e "$linux/arch/$arch/include/uapi" ] &&
> > +        ! [ -e "$output/linux-headers/uapi" ] ; then
> > +        mkdir "$output/linux-headers/uapi"
> 
> mkdir -p
> 
> But looking through this whole thing, it seems like the root cause is actually
> different. We don't want any uapi directories exposed to user space. So let's go
> back a step:
> 
> Why do we need the uapi include dir? Because some header is using it.
> 
> linux-headers/asm-powerpc/kvm_para.h:

The kvm_para.h (also kvm.h) are now defined in include/uapi/asm/

Is not this the correct thing that any header file in include/uapi/asm/ (in this case kvm_para.h) includes another header file (epapr_hcalls.h) in same directory?

Also I think now only the uapi/asm/*.h files should be exposed to userspace (QEMU here).

Waiting for David's comment for better clarity ... 

-Bharat

> 
>     #include <uapi/asm/epapr_hcalls.h>
> 
> This is the root cause of the problem. We must never manually include any uapi
> header paths. We only ever include their normal asm-counterparts which then may
> include uapi (in kernel) or actually are uapi (in user space). David, please
> correct me if I'm wrong.
> 
> Could you please try and see if this kernel side patch makes things work for you
> too?
> 
> 
> Alex
> 
> 
> diff --git a/arch/powerpc/include/uapi/asm/kvm_para.h
> b/arch/powerpc/include/uapi/asm/kvm_para.h
> index ed0e025..e3af328 100644
> --- a/arch/powerpc/include/uapi/asm/kvm_para.h
> +++ b/arch/powerpc/include/uapi/asm/kvm_para.h
> @@ -78,7 +78,7 @@ struct kvm_vcpu_arch_shared {
> 
>  #define KVM_HCALL_TOKEN(num)     _EV_HCALL_TOKEN(EV_KVM_VENDOR_ID, num)
> 
> -#include <uapi/asm/epapr_hcalls.h>
> +#include <asm/epapr_hcalls.h>
> 
>  #define KVM_FEATURE_MAGIC_PAGE	1
>
Alexander Graf - Dec. 18, 2012, 1:20 a.m.
On 18.12.2012, at 02:14, Bhushan Bharat-R65777 wrote:

>>> +++ b/scripts/update-linux-headers.sh
>>> @@ -46,14 +46,26 @@ for arch in $ARCHLIST; do
>>> 
>>>    make -C "$linux" INSTALL_HDR_PATH="$tmpdir" SRCARCH=$arch
>>> headers_install
>>> 
>>> +    if [ -e "$linux/arch/$arch/include/uapi" ] &&
>>> +        ! [ -e "$output/linux-headers/uapi" ] ; then
>>> +        mkdir "$output/linux-headers/uapi"
>> 
>> mkdir -p
>> 
>> But looking through this whole thing, it seems like the root cause is actually
>> different. We don't want any uapi directories exposed to user space. So let's go
>> back a step:
>> 
>> Why do we need the uapi include dir? Because some header is using it.
>> 
>> linux-headers/asm-powerpc/kvm_para.h:
> 
> The kvm_para.h (also kvm.h) are now defined in include/uapi/asm/
> 
> Is not this the correct thing that any header file in include/uapi/asm/ (in this case kvm_para.h) includes another header file (epapr_hcalls.h) in same directory?
> 
> Also I think now only the uapi/asm/*.h files should be exposed to userspace (QEMU here).

make headers_install should basically remove all the uapi magic and give us normal backwards-compatible asm trees :).


Alex

> 
> Waiting for David's comment for better clarity ... 
> 
> -Bharat
> 
>> 
>>    #include <uapi/asm/epapr_hcalls.h>
>> 
>> This is the root cause of the problem. We must never manually include any uapi
>> header paths. We only ever include their normal asm-counterparts which then may
>> include uapi (in kernel) or actually are uapi (in user space). David, please
>> correct me if I'm wrong.
>> 
>> Could you please try and see if this kernel side patch makes things work for you
>> too?
>> 
>> 
>> Alex
>> 
>> 
>> diff --git a/arch/powerpc/include/uapi/asm/kvm_para.h
>> b/arch/powerpc/include/uapi/asm/kvm_para.h
>> index ed0e025..e3af328 100644
>> --- a/arch/powerpc/include/uapi/asm/kvm_para.h
>> +++ b/arch/powerpc/include/uapi/asm/kvm_para.h
>> @@ -78,7 +78,7 @@ struct kvm_vcpu_arch_shared {
>> 
>> #define KVM_HCALL_TOKEN(num)     _EV_HCALL_TOKEN(EV_KVM_VENDOR_ID, num)
>> 
>> -#include <uapi/asm/epapr_hcalls.h>
>> +#include <asm/epapr_hcalls.h>
>> 
>> #define KVM_FEATURE_MAGIC_PAGE	1
>> 
> 
> 
>
Bharat Bhushan - Dec. 18, 2012, 1:27 a.m.
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Tuesday, December 18, 2012 6:51 AM
> To: Bhushan Bharat-R65777
> Cc: qemu-devel qemu-devel; Peter Maydell; Jan Kiszka; qemu-ppc@nongnu.org List;
> Marcelo Tosatti; David Howells
> Subject: Re: [PATCH v2] Added uapi directory into linux-header
> 
> 
> On 18.12.2012, at 02:14, Bhushan Bharat-R65777 wrote:
> 
> >>> +++ b/scripts/update-linux-headers.sh
> >>> @@ -46,14 +46,26 @@ for arch in $ARCHLIST; do
> >>>
> >>>    make -C "$linux" INSTALL_HDR_PATH="$tmpdir" SRCARCH=$arch
> >>> headers_install
> >>>
> >>> +    if [ -e "$linux/arch/$arch/include/uapi" ] &&
> >>> +        ! [ -e "$output/linux-headers/uapi" ] ; then
> >>> +        mkdir "$output/linux-headers/uapi"
> >>
> >> mkdir -p
> >>
> >> But looking through this whole thing, it seems like the root cause is
> >> actually different. We don't want any uapi directories exposed to
> >> user space. So let's go back a step:
> >>
> >> Why do we need the uapi include dir? Because some header is using it.
> >>
> >> linux-headers/asm-powerpc/kvm_para.h:
> >
> > The kvm_para.h (also kvm.h) are now defined in include/uapi/asm/
> >
> > Is not this the correct thing that any header file in include/uapi/asm/ (in
> this case kvm_para.h) includes another header file (epapr_hcalls.h) in same
> directory?
> >
> > Also I think now only the uapi/asm/*.h files should be exposed to userspace
> (QEMU here).
> 
> make headers_install should basically remove all the uapi magic and give us
> normal backwards-compatible asm trees :).

I am perfectly fine, How we can do this now :)

-Bharat

> 
> 
> Alex
> 
> >
> > Waiting for David's comment for better clarity ...
> >
> > -Bharat
> >
> >>
> >>    #include <uapi/asm/epapr_hcalls.h>
> >>
> >> This is the root cause of the problem. We must never manually include
> >> any uapi header paths. We only ever include their normal
> >> asm-counterparts which then may include uapi (in kernel) or actually
> >> are uapi (in user space). David, please correct me if I'm wrong.
> >>
> >> Could you please try and see if this kernel side patch makes things
> >> work for you too?
> >>
> >>
> >> Alex
> >>
> >>
> >> diff --git a/arch/powerpc/include/uapi/asm/kvm_para.h
> >> b/arch/powerpc/include/uapi/asm/kvm_para.h
> >> index ed0e025..e3af328 100644
> >> --- a/arch/powerpc/include/uapi/asm/kvm_para.h
> >> +++ b/arch/powerpc/include/uapi/asm/kvm_para.h
> >> @@ -78,7 +78,7 @@ struct kvm_vcpu_arch_shared {
> >>
> >> #define KVM_HCALL_TOKEN(num)     _EV_HCALL_TOKEN(EV_KVM_VENDOR_ID, num)
> >>
> >> -#include <uapi/asm/epapr_hcalls.h>
> >> +#include <asm/epapr_hcalls.h>
> >>
> >> #define KVM_FEATURE_MAGIC_PAGE	1
> >>
> >
> >
> >
>
Alexander Graf - Dec. 18, 2012, 1:29 a.m.
On 18.12.2012, at 02:27, Bhushan Bharat-R65777 wrote:

> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Tuesday, December 18, 2012 6:51 AM
>> To: Bhushan Bharat-R65777
>> Cc: qemu-devel qemu-devel; Peter Maydell; Jan Kiszka; qemu-ppc@nongnu.org List;
>> Marcelo Tosatti; David Howells
>> Subject: Re: [PATCH v2] Added uapi directory into linux-header
>> 
>> 
>> On 18.12.2012, at 02:14, Bhushan Bharat-R65777 wrote:
>> 
>>>>> +++ b/scripts/update-linux-headers.sh
>>>>> @@ -46,14 +46,26 @@ for arch in $ARCHLIST; do
>>>>> 
>>>>>   make -C "$linux" INSTALL_HDR_PATH="$tmpdir" SRCARCH=$arch
>>>>> headers_install
>>>>> 
>>>>> +    if [ -e "$linux/arch/$arch/include/uapi" ] &&
>>>>> +        ! [ -e "$output/linux-headers/uapi" ] ; then
>>>>> +        mkdir "$output/linux-headers/uapi"
>>>> 
>>>> mkdir -p
>>>> 
>>>> But looking through this whole thing, it seems like the root cause is
>>>> actually different. We don't want any uapi directories exposed to
>>>> user space. So let's go back a step:
>>>> 
>>>> Why do we need the uapi include dir? Because some header is using it.
>>>> 
>>>> linux-headers/asm-powerpc/kvm_para.h:
>>> 
>>> The kvm_para.h (also kvm.h) are now defined in include/uapi/asm/
>>> 
>>> Is not this the correct thing that any header file in include/uapi/asm/ (in
>> this case kvm_para.h) includes another header file (epapr_hcalls.h) in same
>> directory?
>>> 
>>> Also I think now only the uapi/asm/*.h files should be exposed to userspace
>> (QEMU here).
>> 
>> make headers_install should basically remove all the uapi magic and give us
>> normal backwards-compatible asm trees :).
> 
> I am perfectly fine, How we can do this now :)

Well, for starters, do the headers work if you apply the patch I sent in a previous mail plus the epapr_hcall.h copy? If so, then that's the way to go :)


Alex
Bharat Bhushan - Dec. 18, 2012, 2:07 a.m.
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Tuesday, December 18, 2012 7:00 AM
> To: Bhushan Bharat-R65777
> Cc: qemu-devel qemu-devel; Peter Maydell; Jan Kiszka; qemu-ppc@nongnu.org List;
> Marcelo Tosatti; David Howells
> Subject: Re: [PATCH v2] Added uapi directory into linux-header
> 
> 
> On 18.12.2012, at 02:27, Bhushan Bharat-R65777 wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:agraf@suse.de]
> >> Sent: Tuesday, December 18, 2012 6:51 AM
> >> To: Bhushan Bharat-R65777
> >> Cc: qemu-devel qemu-devel; Peter Maydell; Jan Kiszka;
> >> qemu-ppc@nongnu.org List; Marcelo Tosatti; David Howells
> >> Subject: Re: [PATCH v2] Added uapi directory into linux-header
> >>
> >>
> >> On 18.12.2012, at 02:14, Bhushan Bharat-R65777 wrote:
> >>
> >>>>> +++ b/scripts/update-linux-headers.sh
> >>>>> @@ -46,14 +46,26 @@ for arch in $ARCHLIST; do
> >>>>>
> >>>>>   make -C "$linux" INSTALL_HDR_PATH="$tmpdir" SRCARCH=$arch
> >>>>> headers_install
> >>>>>
> >>>>> +    if [ -e "$linux/arch/$arch/include/uapi" ] &&
> >>>>> +        ! [ -e "$output/linux-headers/uapi" ] ; then
> >>>>> +        mkdir "$output/linux-headers/uapi"
> >>>>
> >>>> mkdir -p
> >>>>
> >>>> But looking through this whole thing, it seems like the root cause
> >>>> is actually different. We don't want any uapi directories exposed
> >>>> to user space. So let's go back a step:
> >>>>
> >>>> Why do we need the uapi include dir? Because some header is using it.
> >>>>
> >>>> linux-headers/asm-powerpc/kvm_para.h:
> >>>
> >>> The kvm_para.h (also kvm.h) are now defined in include/uapi/asm/
> >>>
> >>> Is not this the correct thing that any header file in
> >>> include/uapi/asm/ (in
> >> this case kvm_para.h) includes another header file (epapr_hcalls.h)
> >> in same directory?
> >>>
> >>> Also I think now only the uapi/asm/*.h files should be exposed to
> >>> userspace
> >> (QEMU here).
> >>
> >> make headers_install should basically remove all the uapi magic and
> >> give us normal backwards-compatible asm trees :).
> >
> > I am perfectly fine, How we can do this now :)
> 
> Well, for starters, do the headers work if you apply the patch I sent in a
> previous mail plus the epapr_hcall.h copy? If so, then that's the way to go :)

Are you really sure that applying a patch and then syncing (or other way round)  is the way you want to go ?

To me it does not look good, I think we can go with the script changes to make install_header is updated to do the work.

-Bharat

> 
> 
> Alex
>
Alexander Graf - Dec. 18, 2012, 10:08 a.m.
On 18.12.2012, at 03:07, Bhushan Bharat-R65777 <R65777@freescale.com> wrote:

> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Tuesday, December 18, 2012 7:00 AM
>> To: Bhushan Bharat-R65777
>> Cc: qemu-devel qemu-devel; Peter Maydell; Jan Kiszka; qemu-ppc@nongnu.org List;
>> Marcelo Tosatti; David Howells
>> Subject: Re: [PATCH v2] Added uapi directory into linux-header
>> 
>> 
>> On 18.12.2012, at 02:27, Bhushan Bharat-R65777 wrote:
>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>> Sent: Tuesday, December 18, 2012 6:51 AM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: qemu-devel qemu-devel; Peter Maydell; Jan Kiszka;
>>>> qemu-ppc@nongnu.org List; Marcelo Tosatti; David Howells
>>>> Subject: Re: [PATCH v2] Added uapi directory into linux-header
>>>> 
>>>> 
>>>> On 18.12.2012, at 02:14, Bhushan Bharat-R65777 wrote:
>>>> 
>>>>>>> +++ b/scripts/update-linux-headers.sh
>>>>>>> @@ -46,14 +46,26 @@ for arch in $ARCHLIST; do
>>>>>>> 
>>>>>>>  make -C "$linux" INSTALL_HDR_PATH="$tmpdir" SRCARCH=$arch
>>>>>>> headers_install
>>>>>>> 
>>>>>>> +    if [ -e "$linux/arch/$arch/include/uapi" ] &&
>>>>>>> +        ! [ -e "$output/linux-headers/uapi" ] ; then
>>>>>>> +        mkdir "$output/linux-headers/uapi"
>>>>>> 
>>>>>> mkdir -p
>>>>>> 
>>>>>> But looking through this whole thing, it seems like the root cause
>>>>>> is actually different. We don't want any uapi directories exposed
>>>>>> to user space. So let's go back a step:
>>>>>> 
>>>>>> Why do we need the uapi include dir? Because some header is using it.
>>>>>> 
>>>>>> linux-headers/asm-powerpc/kvm_para.h:
>>>>> 
>>>>> The kvm_para.h (also kvm.h) are now defined in include/uapi/asm/
>>>>> 
>>>>> Is not this the correct thing that any header file in
>>>>> include/uapi/asm/ (in
>>>> this case kvm_para.h) includes another header file (epapr_hcalls.h)
>>>> in same directory?
>>>>> 
>>>>> Also I think now only the uapi/asm/*.h files should be exposed to
>>>>> userspace
>>>> (QEMU here).
>>>> 
>>>> make headers_install should basically remove all the uapi magic and
>>>> give us normal backwards-compatible asm trees :).
>>> 
>>> I am perfectly fine, How we can do this now :)
>> 
>> Well, for starters, do the headers work if you apply the patch I sent in a
>> previous mail plus the epapr_hcall.h copy? If so, then that's the way to go :)
> 
> Are you really sure that applying a patch and then syncing (or other way round)  is the way you want to go ?

Yes, because I'm quite confident we're generating broken headers right now.

Alex

> 
> To me it does not look good, I think we can go with the script changes to make install_header is updated to do the work.
> 
> -Bharat
> 
>> 
>> 
>> Alex
>> 
> 
>
Bharat Bhushan - Dec. 18, 2012, 10:19 a.m.
> >>>>>>> +++ b/scripts/update-linux-headers.sh
> >>>>>>> @@ -46,14 +46,26 @@ for arch in $ARCHLIST; do
> >>>>>>>
> >>>>>>>  make -C "$linux" INSTALL_HDR_PATH="$tmpdir" SRCARCH=$arch
> >>>>>>> headers_install
> >>>>>>>
> >>>>>>> +    if [ -e "$linux/arch/$arch/include/uapi" ] &&
> >>>>>>> +        ! [ -e "$output/linux-headers/uapi" ] ; then
> >>>>>>> +        mkdir "$output/linux-headers/uapi"
> >>>>>>
> >>>>>> mkdir -p
> >>>>>>
> >>>>>> But looking through this whole thing, it seems like the root
> >>>>>> cause is actually different. We don't want any uapi directories
> >>>>>> exposed to user space. So let's go back a step:
> >>>>>>
> >>>>>> Why do we need the uapi include dir? Because some header is using it.
> >>>>>>
> >>>>>> linux-headers/asm-powerpc/kvm_para.h:
> >>>>>
> >>>>> The kvm_para.h (also kvm.h) are now defined in include/uapi/asm/
> >>>>>
> >>>>> Is not this the correct thing that any header file in
> >>>>> include/uapi/asm/ (in
> >>>> this case kvm_para.h) includes another header file (epapr_hcalls.h)
> >>>> in same directory?
> >>>>>
> >>>>> Also I think now only the uapi/asm/*.h files should be exposed to
> >>>>> userspace
> >>>> (QEMU here).
> >>>>
> >>>> make headers_install should basically remove all the uapi magic and
> >>>> give us normal backwards-compatible asm trees :).
> >>>
> >>> I am perfectly fine, How we can do this now :)
> >>
> >> Well, for starters, do the headers work if you apply the patch I sent
> >> in a previous mail plus the epapr_hcall.h copy? If so, then that's
> >> the way to go :)
> >
> > Are you really sure that applying a patch and then syncing (or other way
> round)  is the way you want to go ?
> 
> Yes, because I'm quite confident we're generating broken headers right now.

Ok, so every time someone does the sync he/she has to do this? Also do we think that sometime in future this will be taken care by make header_install?

Thanks
-Bharat

> 
> Alex
> 
> >
> > To me it does not look good, I think we can go with the script changes to make
> install_header is updated to do the work.
> >
> > -Bharat
> >
> >>
> >>
> >> Alex
> >>
> >
> >
Alexander Graf - Dec. 18, 2012, 10:38 a.m.
On 18.12.2012, at 11:19, Bhushan Bharat-R65777 <R65777@freescale.com> wrote:

>>>>>>>>> +++ b/scripts/update-linux-headers.sh
>>>>>>>>> @@ -46,14 +46,26 @@ for arch in $ARCHLIST; do
>>>>>>>>> 
>>>>>>>>> make -C "$linux" INSTALL_HDR_PATH="$tmpdir" SRCARCH=$arch
>>>>>>>>> headers_install
>>>>>>>>> 
>>>>>>>>> +    if [ -e "$linux/arch/$arch/include/uapi" ] &&
>>>>>>>>> +        ! [ -e "$output/linux-headers/uapi" ] ; then
>>>>>>>>> +        mkdir "$output/linux-headers/uapi"
>>>>>>>> 
>>>>>>>> mkdir -p
>>>>>>>> 
>>>>>>>> But looking through this whole thing, it seems like the root
>>>>>>>> cause is actually different. We don't want any uapi directories
>>>>>>>> exposed to user space. So let's go back a step:
>>>>>>>> 
>>>>>>>> Why do we need the uapi include dir? Because some header is using it.
>>>>>>>> 
>>>>>>>> linux-headers/asm-powerpc/kvm_para.h:
>>>>>>> 
>>>>>>> The kvm_para.h (also kvm.h) are now defined in include/uapi/asm/
>>>>>>> 
>>>>>>> Is not this the correct thing that any header file in
>>>>>>> include/uapi/asm/ (in
>>>>>> this case kvm_para.h) includes another header file (epapr_hcalls.h)
>>>>>> in same directory?
>>>>>>> 
>>>>>>> Also I think now only the uapi/asm/*.h files should be exposed to
>>>>>>> userspace
>>>>>> (QEMU here).
>>>>>> 
>>>>>> make headers_install should basically remove all the uapi magic and
>>>>>> give us normal backwards-compatible asm trees :).
>>>>> 
>>>>> I am perfectly fine, How we can do this now :)
>>>> 
>>>> Well, for starters, do the headers work if you apply the patch I sent
>>>> in a previous mail plus the epapr_hcall.h copy? If so, then that's
>>>> the way to go :)
>>> 
>>> Are you really sure that applying a patch and then syncing (or other way
>> round)  is the way you want to go ?
>> 
>> Yes, because I'm quite confident we're generating broken headers right now.
> 
> Ok, so every time someone does the sync he/she has to do this? Also do we think that sometime in future this will be taken care by make header_install?

That's the point.

The QEMU header sync is a hack that allows us to not use system headers. System headers could be outdated wrt features we want to support.

However, system headers get generated using make headers_install. And there we generate a header that refers to a uapi directory that doesn't exist after headers_install.

So the next time a distro updates their system headers, they get broken ones. That's what the patch against Linux is trying to fix.


Alex
David Howells - Dec. 18, 2012, 2:10 p.m.
Alexander Graf <agraf@suse.de> wrote:

> But looking through this whole thing, it seems like the root cause is
> actually different. We don't want any uapi directories exposed to user
> space. So let's go back a step:
> 
> Why do we need the uapi include dir? Because some header is using it.
> 
> linux-headers/asm-powerpc/kvm_para.h:
> 
>     #include <uapi/asm/epapr_hcalls.h>
> 
> This is the root cause of the problem. We must never manually include any
> uapi header paths. We only ever include their normal asm-counterparts which
> then may include uapi (in kernel) or actually are uapi (in user
> space). David, please correct me if I'm wrong.

I think you're correct, if I understand what you're saying.

Within the kernel sources, the userspace facing headers are in uapi/
directories whilst the stuff userspace shouldn't see is outside of that.

uapi/ headers should _not_ #include headers with uapi/ prefixes; rather they
should rely on the -I flags to pull in the UAPI header if the KAPI header of
the same name does not exist.

However, KAPI headers that shadow UAPI headers (linux/fs.h for example) _must_
manually #include the UAPI header with the uapi/ prefix as there's no other
way to reach it.  I originally used #include_next for this, but some people
refused to countenance it because it's a gcc-ism, but no matter.

After doing make headers_install in the kernel, you should find the contents
of the appropriate uapi/ directories installed in the target directory without
any uapi/ directories present.  Further, the _UAPI prefixes on the guards are
removed if present.  There should be no #includes with "uapi/" in their
filenames.

David
David Howells - Dec. 18, 2012, 2:11 p.m.
Alexander Graf <agraf@suse.de> wrote:

> Could you please try and see if this kernel side patch makes things work for you too?
> 
> 
> Alex
> 
> 
> diff --git a/arch/powerpc/include/uapi/asm/kvm_para.h b/arch/powerpc/include/uapi/asm/kvm_para.h
> index ed0e025..e3af328 100644
> --- a/arch/powerpc/include/uapi/asm/kvm_para.h
> +++ b/arch/powerpc/include/uapi/asm/kvm_para.h
> @@ -78,7 +78,7 @@ struct kvm_vcpu_arch_shared {
>  
>  #define KVM_HCALL_TOKEN(num)     _EV_HCALL_TOKEN(EV_KVM_VENDOR_ID, num)
>  
> -#include <uapi/asm/epapr_hcalls.h>
> +#include <asm/epapr_hcalls.h>
>  
>  #define KVM_FEATURE_MAGIC_PAGE	1

This appears to be correct.

Acked-by: David Howells <dhowells@redhat.com>

Patch

diff --git a/arch/powerpc/include/uapi/asm/kvm_para.h b/arch/powerpc/include/uapi/asm/kvm_para.h
index ed0e025..e3af328 100644
--- a/arch/powerpc/include/uapi/asm/kvm_para.h
+++ b/arch/powerpc/include/uapi/asm/kvm_para.h
@@ -78,7 +78,7 @@  struct kvm_vcpu_arch_shared {
 
 #define KVM_HCALL_TOKEN(num)     _EV_HCALL_TOKEN(EV_KVM_VENDOR_ID, num)
 
-#include <uapi/asm/epapr_hcalls.h>
+#include <asm/epapr_hcalls.h>
 
 #define KVM_FEATURE_MAGIC_PAGE	1