qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
@ 2012-12-17 16:01 Bharat Bhushan
  2012-12-17 17:48 ` Alexander Graf
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Bharat Bhushan @ 2012-12-17 16:01 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, jan.kiszka, agraf, qemu-ppc; +Cc: Bharat Bhushan

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"
+    fi
+
     rm -rf "$output/linux-headers/asm-$arch"
     mkdir -p "$output/linux-headers/asm-$arch"
+    if [ -e "$linux/arch/$arch/include/uapi/asm" ] ; then
+        rm -rf "$output/linux-headers/uapi/asm-$arch"
+        mkdir -p "$output/linux-headers/uapi/asm-$arch"
+    fi
     for header in kvm.h kvm_para.h; do
         cp "$tmpdir/include/asm/$header" "$output/linux-headers/asm-$arch"
     done
     if [ $arch = x86 ]; then
         cp "$tmpdir/include/asm/hyperv.h" "$output/linux-headers/asm-x86"
     fi
+    if [ $arch = powerpc ]; then
+        cp "$tmpdir/include/asm/epapr_hcalls.h" "$output/linux-headers/uapi/asm-powerpc/"
+    fi
 done
 
 rm -rf "$output/linux-headers/linux"
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
  2012-12-17 16:01 [Qemu-devel] [PATCH v2] Added uapi directory into linux-header Bharat Bhushan
@ 2012-12-17 17:48 ` Alexander Graf
  2012-12-18  1:14   ` Bhushan Bharat-R65777
  2012-12-18 14:10 ` David Howells
  2012-12-18 14:11 ` David Howells
  2 siblings, 1 reply; 12+ messages in thread
From: Alexander Graf @ 2012-12-17 17:48 UTC (permalink / raw)
  To: Bharat Bhushan
  Cc: Peter Maydell, David Howells, Jan Kiszka, Marcelo Tosatti,
	qemu-devel qemu-devel, Bharat Bhushan, qemu-ppc@nongnu.org List


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


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
 

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
  2012-12-17 17:48 ` Alexander Graf
@ 2012-12-18  1:14   ` Bhushan Bharat-R65777
  2012-12-18  1:20     ` Alexander Graf
  0 siblings, 1 reply; 12+ messages in thread
From: Bhushan Bharat-R65777 @ 2012-12-18  1:14 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Peter Maydell, Jan Kiszka, Marcelo Tosatti, qemu-devel qemu-devel,
	David Howells, qemu-ppc@nongnu.org List

> > +++ 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
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
  2012-12-18  1:14   ` Bhushan Bharat-R65777
@ 2012-12-18  1:20     ` Alexander Graf
  2012-12-18  1:27       ` Bhushan Bharat-R65777
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Graf @ 2012-12-18  1:20 UTC (permalink / raw)
  To: Bhushan Bharat-R65777
  Cc: Peter Maydell, Jan Kiszka, Marcelo Tosatti, qemu-devel qemu-devel,
	David Howells, qemu-ppc@nongnu.org List


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
>> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
  2012-12-18  1:20     ` Alexander Graf
@ 2012-12-18  1:27       ` Bhushan Bharat-R65777
  2012-12-18  1:29         ` Alexander Graf
  0 siblings, 1 reply; 12+ messages in thread
From: Bhushan Bharat-R65777 @ 2012-12-18  1:27 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Peter Maydell, Jan Kiszka, Marcelo Tosatti, qemu-devel qemu-devel,
	David Howells, qemu-ppc@nongnu.org List



> -----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
> >>
> >
> >
> >
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
  2012-12-18  1:27       ` Bhushan Bharat-R65777
@ 2012-12-18  1:29         ` Alexander Graf
  2012-12-18  2:07           ` Bhushan Bharat-R65777
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Graf @ 2012-12-18  1:29 UTC (permalink / raw)
  To: Bhushan Bharat-R65777
  Cc: Peter Maydell, Jan Kiszka, Marcelo Tosatti, qemu-devel qemu-devel,
	David Howells, qemu-ppc@nongnu.org List


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
  2012-12-18  1:29         ` Alexander Graf
@ 2012-12-18  2:07           ` Bhushan Bharat-R65777
  2012-12-18 10:08             ` Alexander Graf
  0 siblings, 1 reply; 12+ messages in thread
From: Bhushan Bharat-R65777 @ 2012-12-18  2:07 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Peter Maydell, Jan Kiszka, Marcelo Tosatti, qemu-devel qemu-devel,
	David Howells, qemu-ppc@nongnu.org List



> -----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
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
  2012-12-18  2:07           ` Bhushan Bharat-R65777
@ 2012-12-18 10:08             ` Alexander Graf
  2012-12-18 10:19               ` Bhushan Bharat-R65777
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Graf @ 2012-12-18 10:08 UTC (permalink / raw)
  To: Bhushan Bharat-R65777
  Cc: Peter Maydell, Jan Kiszka, Marcelo Tosatti, qemu-devel qemu-devel,
	David Howells, qemu-ppc@nongnu.org List



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
>> 
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
  2012-12-18 10:08             ` Alexander Graf
@ 2012-12-18 10:19               ` Bhushan Bharat-R65777
  2012-12-18 10:38                 ` Alexander Graf
  0 siblings, 1 reply; 12+ messages in thread
From: Bhushan Bharat-R65777 @ 2012-12-18 10:19 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Peter Maydell, Jan Kiszka, Marcelo Tosatti, qemu-devel qemu-devel,
	David Howells, qemu-ppc@nongnu.org List

> >>>>>>> +++ 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
> >>
> >
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
  2012-12-18 10:19               ` Bhushan Bharat-R65777
@ 2012-12-18 10:38                 ` Alexander Graf
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Graf @ 2012-12-18 10:38 UTC (permalink / raw)
  To: Bhushan Bharat-R65777
  Cc: Peter Maydell, Jan Kiszka, Marcelo Tosatti, qemu-devel qemu-devel,
	David Howells, qemu-ppc@nongnu.org List



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
  2012-12-17 16:01 [Qemu-devel] [PATCH v2] Added uapi directory into linux-header Bharat Bhushan
  2012-12-17 17:48 ` Alexander Graf
@ 2012-12-18 14:10 ` David Howells
  2012-12-18 14:11 ` David Howells
  2 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2012-12-18 14:10 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Peter Maydell, Jan Kiszka, Marcelo Tosatti, qemu-devel qemu-devel,
	dhowells, qemu-ppc@nongnu.org List, Bharat Bhushan,
	Bharat Bhushan

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2] Added uapi directory into linux-header
  2012-12-17 16:01 [Qemu-devel] [PATCH v2] Added uapi directory into linux-header Bharat Bhushan
  2012-12-17 17:48 ` Alexander Graf
  2012-12-18 14:10 ` David Howells
@ 2012-12-18 14:11 ` David Howells
  2 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2012-12-18 14:11 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Peter Maydell, Jan Kiszka, Marcelo Tosatti, qemu-devel qemu-devel,
	dhowells, qemu-ppc@nongnu.org List, Bharat Bhushan,
	Bharat Bhushan

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>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-12-18 14:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-17 16:01 [Qemu-devel] [PATCH v2] Added uapi directory into linux-header Bharat Bhushan
2012-12-17 17:48 ` Alexander Graf
2012-12-18  1:14   ` Bhushan Bharat-R65777
2012-12-18  1:20     ` Alexander Graf
2012-12-18  1:27       ` Bhushan Bharat-R65777
2012-12-18  1:29         ` Alexander Graf
2012-12-18  2:07           ` Bhushan Bharat-R65777
2012-12-18 10:08             ` Alexander Graf
2012-12-18 10:19               ` Bhushan Bharat-R65777
2012-12-18 10:38                 ` Alexander Graf
2012-12-18 14:10 ` David Howells
2012-12-18 14:11 ` David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).