diff mbox

[1/4] xen: build infrastructure for generating hypercall depending symbols

Message ID 1418321065-10212-2-git-send-email-jgross@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jürgen Groß Dec. 11, 2014, 6:04 p.m. UTC 
Today there are several places in the kernel which build tables
containing one entry for each possible Xen hypercall. Create an
infrastructure to be able to generate these tables at build time.

Based-on-patch-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/syscalls/Makefile |  9 +++++++++
 scripts/xen-hypercalls.sh  | 11 +++++++++++
 2 files changed, 20 insertions(+)
 create mode 100644 scripts/xen-hypercalls.sh

Comments

Boris Ostrovsky Dec. 12, 2014, 10:48 p.m. UTC | #1 
On 12/11/2014 01:04 PM, Juergen Gross wrote:
> diff --git a/scripts/xen-hypercalls.sh b/scripts/xen-hypercalls.sh
> new file mode 100644
> index 0000000..e6447b7
> --- /dev/null
> +++ b/scripts/xen-hypercalls.sh
> @@ -0,0 +1,11 @@
> +#!/bin/sh
> +out="$1"
> +shift
> +in="$@"
> +
> +for i in $in; do
> +	eval $CPP $LINUXINCLUDE -dD -imacros "$i" -x c /dev/null
> +done | \
> +awk '$1 == "#define" && $2 ~ /__HYPERVISOR_[a-z][a-z_0-9]*/ { v[$3] = $2 }
> +	END {for (i in v) if (!(v[i] in v))
> +		print "HYPERCALL("substr(v[i], 14)")"}' | sort -u >$out

Why do you 'sort -u'? Do you expect multiple definitions of the same 
hypercall?

-boris
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jürgen Groß Dec. 15, 2014, 5:20 a.m. UTC | #2 
On 12/12/2014 11:48 PM, Boris Ostrovsky wrote:
> On 12/11/2014 01:04 PM, Juergen Gross wrote:
>> diff --git a/scripts/xen-hypercalls.sh b/scripts/xen-hypercalls.sh
>> new file mode 100644
>> index 0000000..e6447b7
>> --- /dev/null
>> +++ b/scripts/xen-hypercalls.sh
>> @@ -0,0 +1,11 @@
>> +#!/bin/sh
>> +out="$1"
>> +shift
>> +in="$@"
>> +
>> +for i in $in; do
>> +    eval $CPP $LINUXINCLUDE -dD -imacros "$i" -x c /dev/null
>> +done | \
>> +awk '$1 == "#define" && $2 ~ /__HYPERVISOR_[a-z][a-z_0-9]*/ { v[$3] =
>> $2 }
>> +    END {for (i in v) if (!(v[i] in v))
>> +        print "HYPERCALL("substr(v[i], 14)")"}' | sort -u >$out
>
> Why do you 'sort -u'? Do you expect multiple definitions of the same
> hypercall?

Paranoia related to the use of wildcards for files scanned:

$(srctree)/include/xen/interface/xen*.h


Juergen
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Beulich Dec. 15, 2014, 10 a.m. UTC | #3 
>>> On 12.12.14 at 23:48, <boris.ostrovsky@oracle.com> wrote:
> On 12/11/2014 01:04 PM, Juergen Gross wrote:
>> diff --git a/scripts/xen-hypercalls.sh b/scripts/xen-hypercalls.sh
>> new file mode 100644
>> index 0000000..e6447b7
>> --- /dev/null
>> +++ b/scripts/xen-hypercalls.sh
>> @@ -0,0 +1,11 @@
>> +#!/bin/sh
>> +out="$1"
>> +shift
>> +in="$@"
>> +
>> +for i in $in; do
>> +	eval $CPP $LINUXINCLUDE -dD -imacros "$i" -x c /dev/null
>> +done | \
>> +awk '$1 == "#define" && $2 ~ /__HYPERVISOR_[a-z][a-z_0-9]*/ { v[$3] = $2 }
>> +	END {for (i in v) if (!(v[i] in v))
>> +		print "HYPERCALL("substr(v[i], 14)")"}' | sort -u >$out
> 
> Why do you 'sort -u'? Do you expect multiple definitions of the same 
> hypercall?

For upstream I think this could be dropped; the original version needs
this as the classic tree sticks more closely to the original xen.h (which
has a couple of backward compatibility defines which would get in the
way). Otoh it does no harm afaict...

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Vrabel Dec. 15, 2014, 11:38 a.m. UTC | #4 
On 11/12/14 18:04, Juergen Gross wrote:
> Today there are several places in the kernel which build tables
> containing one entry for each possible Xen hypercall. Create an
> infrastructure to be able to generate these tables at build time.

Does arm and arm64 need something similar?  If so are the tools here
suitable for them?

> --- a/arch/x86/syscalls/Makefile
> +++ b/arch/x86/syscalls/Makefile

Why are these changes here and not in arch/x86/xen/Makefile?

> @@ -19,6 +19,9 @@ quiet_cmd_syshdr = SYSHDR  $@
>  quiet_cmd_systbl = SYSTBL  $@
>        cmd_systbl = $(CONFIG_SHELL) '$(systbl)' $< $@
>  
> +quiet_cmd_hypercalls = HYPERCALLS $@
> +      cmd_hypercalls = $(CONFIG_SHELL) '$<' $@ $(filter-out $<,$^)
> +
>  syshdr_abi_unistd_32 := i386
>  $(uapi)/unistd_32.h: $(syscall32) $(syshdr)
>  	$(call if_changed,syshdr)
> @@ -47,10 +50,16 @@ $(out)/syscalls_32.h: $(syscall32) $(systbl)
>  $(out)/syscalls_64.h: $(syscall64) $(systbl)
>  	$(call if_changed,systbl)
>  
> +$(out)/xen-hypercalls.h: $(srctree)/scripts/xen-hypercalls.sh
> +	$(call if_changed,hypercalls)
> +
> +$(out)/xen-hypercalls.h: $(srctree)/include/xen/interface/xen*.h

The generated header should end up in asm/xen/

> --- /dev/null
> +++ b/scripts/xen-hypercalls.sh
> @@ -0,0 +1,11 @@
> +#!/bin/sh
> +out="$1"
> +shift
> +in="$@"
> +
> +for i in $in; do
> +	eval $CPP $LINUXINCLUDE -dD -imacros "$i" -x c /dev/null
> +done | \
> +awk '$1 == "#define" && $2 ~ /__HYPERVISOR_[a-z][a-z_0-9]*/ { v[$3] = $2 }
> +	END {for (i in v) if (!(v[i] in v))
> +		print "HYPERCALL("substr(v[i], 14)")"}' | sort -u >$out

Include a comment in the generated output saying what generated it. e.g.,

/* auto-generated by scripts/xen-hypercall.sh */

David

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Beulich Dec. 15, 2014, 11:47 a.m. UTC | #5 
>>> On 15.12.14 at 12:38, <david.vrabel@citrix.com> wrote:
> On 11/12/14 18:04, Juergen Gross wrote:
>> --- a/arch/x86/syscalls/Makefile
>> +++ b/arch/x86/syscalls/Makefile
> 
> Why are these changes here and not in arch/x86/xen/Makefile?

Because this needs to be done in a step that (afaict) has no hook
in the Xen-specific Makefile.

>> @@ -47,10 +50,16 @@ $(out)/syscalls_32.h: $(syscall32) $(systbl)
>>  $(out)/syscalls_64.h: $(syscall64) $(systbl)
>>  	$(call if_changed,systbl)
>>  
>> +$(out)/xen-hypercalls.h: $(srctree)/scripts/xen-hypercalls.sh
>> +	$(call if_changed,hypercalls)
>> +
>> +$(out)/xen-hypercalls.h: $(srctree)/include/xen/interface/xen*.h
> 
> The generated header should end up in asm/xen/

Why is generated/asm/ not good enough?

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jürgen Groß Dec. 16, 2014, 2:40 p.m. UTC | #6 
On 12/15/2014 12:38 PM, David Vrabel wrote:
> On 11/12/14 18:04, Juergen Gross wrote:
>> Today there are several places in the kernel which build tables
>> containing one entry for each possible Xen hypercall. Create an
>> infrastructure to be able to generate these tables at build time.
>
> Does arm and arm64 need something similar?  If so are the tools here
> suitable for them?

I don't think arm* needs this. But in case it does, the tools would
support arm as well.


Juergen

>
>> --- a/arch/x86/syscalls/Makefile
>> +++ b/arch/x86/syscalls/Makefile
>
> Why are these changes here and not in arch/x86/xen/Makefile?
>
>> @@ -19,6 +19,9 @@ quiet_cmd_syshdr = SYSHDR  $@
>>   quiet_cmd_systbl = SYSTBL  $@
>>         cmd_systbl = $(CONFIG_SHELL) '$(systbl)' $< $@
>>
>> +quiet_cmd_hypercalls = HYPERCALLS $@
>> +      cmd_hypercalls = $(CONFIG_SHELL) '$<' $@ $(filter-out $<,$^)
>> +
>>   syshdr_abi_unistd_32 := i386
>>   $(uapi)/unistd_32.h: $(syscall32) $(syshdr)
>>   	$(call if_changed,syshdr)
>> @@ -47,10 +50,16 @@ $(out)/syscalls_32.h: $(syscall32) $(systbl)
>>   $(out)/syscalls_64.h: $(syscall64) $(systbl)
>>   	$(call if_changed,systbl)
>>
>> +$(out)/xen-hypercalls.h: $(srctree)/scripts/xen-hypercalls.sh
>> +	$(call if_changed,hypercalls)
>> +
>> +$(out)/xen-hypercalls.h: $(srctree)/include/xen/interface/xen*.h
>
> The generated header should end up in asm/xen/
>
>> --- /dev/null
>> +++ b/scripts/xen-hypercalls.sh
>> @@ -0,0 +1,11 @@
>> +#!/bin/sh
>> +out="$1"
>> +shift
>> +in="$@"
>> +
>> +for i in $in; do
>> +	eval $CPP $LINUXINCLUDE -dD -imacros "$i" -x c /dev/null
>> +done | \
>> +awk '$1 == "#define" && $2 ~ /__HYPERVISOR_[a-z][a-z_0-9]*/ { v[$3] = $2 }
>> +	END {for (i in v) if (!(v[i] in v))
>> +		print "HYPERCALL("substr(v[i], 14)")"}' | sort -u >$out
>
> Include a comment in the generated output saying what generated it. e.g.,
>
> /* auto-generated by scripts/xen-hypercall.sh */
>
> David
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Vrabel Dec. 16, 2014, 2:51 p.m. UTC | #7 
On 15/12/14 11:47, Jan Beulich wrote:
>>>> On 15.12.14 at 12:38, <david.vrabel@citrix.com> wrote:
>> On 11/12/14 18:04, Juergen Gross wrote:
>>> --- a/arch/x86/syscalls/Makefile
>>> +++ b/arch/x86/syscalls/Makefile
>>
>> Why are these changes here and not in arch/x86/xen/Makefile?
> 
> Because this needs to be done in a step that (afaict) has no hook
> in the Xen-specific Makefile.
> 
>>> @@ -47,10 +50,16 @@ $(out)/syscalls_32.h: $(syscall32) $(systbl)
>>>  $(out)/syscalls_64.h: $(syscall64) $(systbl)
>>>  	$(call if_changed,systbl)
>>>  
>>> +$(out)/xen-hypercalls.h: $(srctree)/scripts/xen-hypercalls.sh
>>> +	$(call if_changed,hypercalls)
>>> +
>>> +$(out)/xen-hypercalls.h: $(srctree)/include/xen/interface/xen*.h
>>
>> The generated header should end up in asm/xen/
> 
> Why is generated/asm/ not good enough?

I meant something like generated/asm/xen/hypercall-macros.h

So a "xen/" prefix which is consistent with all the other xen specific
headers.

But if this is non-trivial it's not a major issue.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/syscalls/Makefile b/arch/x86/syscalls/Makefile
index 3323c27..a55abb9 100644
--- a/arch/x86/syscalls/Makefile
+++ b/arch/x86/syscalls/Makefile
@@ -19,6 +19,9 @@  quiet_cmd_syshdr = SYSHDR  $@
 quiet_cmd_systbl = SYSTBL  $@
       cmd_systbl = $(CONFIG_SHELL) '$(systbl)' $< $@
 
+quiet_cmd_hypercalls = HYPERCALLS $@
+      cmd_hypercalls = $(CONFIG_SHELL) '$<' $@ $(filter-out $<,$^)
+
 syshdr_abi_unistd_32 := i386
 $(uapi)/unistd_32.h: $(syscall32) $(syshdr)
 	$(call if_changed,syshdr)
@@ -47,10 +50,16 @@  $(out)/syscalls_32.h: $(syscall32) $(systbl)
 $(out)/syscalls_64.h: $(syscall64) $(systbl)
 	$(call if_changed,systbl)
 
+$(out)/xen-hypercalls.h: $(srctree)/scripts/xen-hypercalls.sh
+	$(call if_changed,hypercalls)
+
+$(out)/xen-hypercalls.h: $(srctree)/include/xen/interface/xen*.h
+
 uapisyshdr-y			+= unistd_32.h unistd_64.h unistd_x32.h
 syshdr-y			+= syscalls_32.h
 syshdr-$(CONFIG_X86_64)		+= unistd_32_ia32.h unistd_64_x32.h
 syshdr-$(CONFIG_X86_64)		+= syscalls_64.h
+syshdr-$(CONFIG_XEN)		+= xen-hypercalls.h
 
 targets	+= $(uapisyshdr-y) $(syshdr-y)
 
diff --git a/scripts/xen-hypercalls.sh b/scripts/xen-hypercalls.sh
new file mode 100644
index 0000000..e6447b7
--- /dev/null
+++ b/scripts/xen-hypercalls.sh
@@ -0,0 +1,11 @@ 
+#!/bin/sh
+out="$1"
+shift
+in="$@"
+
+for i in $in; do
+	eval $CPP $LINUXINCLUDE -dD -imacros "$i" -x c /dev/null
+done | \
+awk '$1 == "#define" && $2 ~ /__HYPERVISOR_[a-z][a-z_0-9]*/ { v[$3] = $2 }
+	END {for (i in v) if (!(v[i] in v))
+		print "HYPERCALL("substr(v[i], 14)")"}' | sort -u >$out