[PATCH 0/4] xen: auto-generate symbols for xen hypercalls

[PATCH 0/4] xen: auto-generate symbols for xen hypercalls

[Juergen Gross]

The Xen hypercalls are defined in include/xen/interface/xen.h. There
are some places where for each hypercall a table element is created.
Instead of manually add each hypercall element to these tables use
an auto generated header built during the make process of the kernel.

Juergen Gross (4):
  xen: build infrastructure for generating hypercall depending symbols
  xen: synchronize include/xen/interface/xen.h with xen
  xen: use generated hypervisor symbols in arch/x86/xen/trace.c
  xen: use generated hypercall symbols in arch/x86/xen/xen-head.S

 arch/x86/syscalls/Makefile  |  9 +++++++
 arch/x86/xen/trace.c        | 50 +++---------------------------------
 arch/x86/xen/xen-head.S     | 62 ++++++++-------------------------------------
 include/xen/interface/xen.h |  6 ++++-
 scripts/xen-hypercalls.sh   | 11 ++++++++
 5 files changed, 39 insertions(+), 99 deletions(-)
 create mode 100644 scripts/xen-hypercalls.sh

-- 
2.1.2

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

[Juergen Gross]

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

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

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

[Boris Ostrovsky]

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

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

[Juergen Gross]

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

Re: [Xen-devel] [PATCH 1/4] xen: build infrastructure for generating hypercall depending symbols

[Jan Beulich]

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

Re: [Xen-devel] [PATCH 1/4] xen: build infrastructure for generating hypercall depending symbols

[David Vrabel]

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

Re: [Xen-devel] [PATCH 1/4] xen: build infrastructure for generating hypercall depending symbols

[Jan Beulich]

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

Re: [Xen-devel] [PATCH 1/4] xen: build infrastructure for generating hypercall depending symbols

[David Vrabel]

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

Re: [Xen-devel] [PATCH 1/4] xen: build infrastructure for generating hypercall depending symbols

[Juergen Gross]

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

[PATCH 2/4] xen: synchronize include/xen/interface/xen.h with xen

[Juergen Gross]

The header include/xen/interface/xen.h doesn't contain all definitions
from Xen's version of that header. Update it accordingly.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/xen/trace.c        | 2 +-
 include/xen/interface/xen.h | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/trace.c b/arch/x86/xen/trace.c
index 520022d..8296cdf 100644
--- a/arch/x86/xen/trace.c
+++ b/arch/x86/xen/trace.c
@@ -29,7 +29,7 @@ static const char *xen_hypercall_names[] = {
 	N(vcpu_op),
 	N(set_segment_base),
 	N(mmuext_op),
-	N(acm_op),
+	N(xsm_op),
 	N(nmi_op),
 	N(sched_op),
 	N(callback_op),
diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index f68719f..a483789 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -67,7 +67,7 @@
 #define __HYPERVISOR_vcpu_op              24
 #define __HYPERVISOR_set_segment_base     25 /* x86/64 only */
 #define __HYPERVISOR_mmuext_op            26
-#define __HYPERVISOR_acm_op               27
+#define __HYPERVISOR_xsm_op               27
 #define __HYPERVISOR_nmi_op               28
 #define __HYPERVISOR_sched_op             29
 #define __HYPERVISOR_callback_op          30
@@ -75,7 +75,11 @@
 #define __HYPERVISOR_event_channel_op     32
 #define __HYPERVISOR_physdev_op           33
 #define __HYPERVISOR_hvm_op               34
+#define __HYPERVISOR_sysctl               35
+#define __HYPERVISOR_domctl               36
+#define __HYPERVISOR_kexec_op             37
 #define __HYPERVISOR_tmem_op              38
+#define __HYPERVISOR_xc_reserved_op       39 /* reserved for XenClient */
 
 /* Architecture-specific hypercall definitions. */
 #define __HYPERVISOR_arch_0               48
-- 
2.1.2

Re: [Xen-devel] [PATCH 2/4] xen: synchronize include/xen/interface/xen.h with xen

[David Vrabel]

On 11/12/14 18:04, Juergen Gross wrote:
> The header include/xen/interface/xen.h doesn't contain all definitions
> from Xen's version of that header. Update it accordingly.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

[PATCH 3/4] xen: use generated hypervisor symbols in arch/x86/xen/trace.c

[Juergen Gross]

Instead of manually list all hypervisor calls in arch/x86/xen/trace.c
use the auto generated list.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/xen/trace.c | 50 ++++----------------------------------------------
 1 file changed, 4 insertions(+), 46 deletions(-)

diff --git a/arch/x86/xen/trace.c b/arch/x86/xen/trace.c
index 8296cdf..a702ec2 100644
--- a/arch/x86/xen/trace.c
+++ b/arch/x86/xen/trace.c
@@ -1,54 +1,12 @@
 #include <linux/ftrace.h>
 #include <xen/interface/xen.h>
+#include <xen/interface/xen-mca.h>
 
-#define N(x)	[__HYPERVISOR_##x] = "("#x")"
+#define HYPERCALL(x)	[__HYPERVISOR_##x] = "("#x")",
 static const char *xen_hypercall_names[] = {
-	N(set_trap_table),
-	N(mmu_update),
-	N(set_gdt),
-	N(stack_switch),
-	N(set_callbacks),
-	N(fpu_taskswitch),
-	N(sched_op_compat),
-	N(dom0_op),
-	N(set_debugreg),
-	N(get_debugreg),
-	N(update_descriptor),
-	N(memory_op),
-	N(multicall),
-	N(update_va_mapping),
-	N(set_timer_op),
-	N(event_channel_op_compat),
-	N(xen_version),
-	N(console_io),
-	N(physdev_op_compat),
-	N(grant_table_op),
-	N(vm_assist),
-	N(update_va_mapping_otherdomain),
-	N(iret),
-	N(vcpu_op),
-	N(set_segment_base),
-	N(mmuext_op),
-	N(xsm_op),
-	N(nmi_op),
-	N(sched_op),
-	N(callback_op),
-	N(xenoprof_op),
-	N(event_channel_op),
-	N(physdev_op),
-	N(hvm_op),
-
-/* Architecture-specific hypercall definitions. */
-	N(arch_0),
-	N(arch_1),
-	N(arch_2),
-	N(arch_3),
-	N(arch_4),
-	N(arch_5),
-	N(arch_6),
-	N(arch_7),
+#include <asm/xen-hypercalls.h>
 };
-#undef N
+#undef HYPERCALL
 
 static const char *xen_hypercall_name(unsigned op)
 {
-- 
2.1.2

Re: [Xen-devel] [PATCH 3/4] xen: use generated hypervisor symbols in arch/x86/xen/trace.c

[David Vrabel]

On 11/12/14 18:04, Juergen Gross wrote:
> Instead of manually list all hypervisor calls in arch/x86/xen/trace.c
> use the auto generated list.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

[PATCH 4/4] xen: use generated hypercall symbols in arch/x86/xen/xen-head.S

[Juergen Gross]

Instead of manually list each hypercall in arch/x86/xen/xen-head.S
use the auto generated symbol list.

This also corrects the wrong address of xen_hypercall_mca which was
located 32 bytes higher than it should.

Symbol addresses have been verified to match the correct ones via
objdump output.

Based-on-patch-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/xen/xen-head.S | 62 ++++++++-----------------------------------------
 1 file changed, 10 insertions(+), 52 deletions(-)

diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
index 674b2225..b2ba341 100644
--- a/arch/x86/xen/xen-head.S
+++ b/arch/x86/xen/xen-head.S
@@ -12,6 +12,8 @@
 
 #include <xen/interface/elfnote.h>
 #include <xen/interface/features.h>
+#include <xen/interface/xen.h>
+#include <xen/interface/xen-mca.h>
 #include <asm/xen/interface.h>
 
 #ifdef CONFIG_XEN_PVH
@@ -85,58 +87,14 @@ ENTRY(xen_pvh_early_cpu_init)
 .pushsection .text
 	.balign PAGE_SIZE
 ENTRY(hypercall_page)
-#define NEXT_HYPERCALL(x) \
-	ENTRY(xen_hypercall_##x) \
-	.skip 32
-
-NEXT_HYPERCALL(set_trap_table)
-NEXT_HYPERCALL(mmu_update)
-NEXT_HYPERCALL(set_gdt)
-NEXT_HYPERCALL(stack_switch)
-NEXT_HYPERCALL(set_callbacks)
-NEXT_HYPERCALL(fpu_taskswitch)
-NEXT_HYPERCALL(sched_op_compat)
-NEXT_HYPERCALL(platform_op)
-NEXT_HYPERCALL(set_debugreg)
-NEXT_HYPERCALL(get_debugreg)
-NEXT_HYPERCALL(update_descriptor)
-NEXT_HYPERCALL(ni)
-NEXT_HYPERCALL(memory_op)
-NEXT_HYPERCALL(multicall)
-NEXT_HYPERCALL(update_va_mapping)
-NEXT_HYPERCALL(set_timer_op)
-NEXT_HYPERCALL(event_channel_op_compat)
-NEXT_HYPERCALL(xen_version)
-NEXT_HYPERCALL(console_io)
-NEXT_HYPERCALL(physdev_op_compat)
-NEXT_HYPERCALL(grant_table_op)
-NEXT_HYPERCALL(vm_assist)
-NEXT_HYPERCALL(update_va_mapping_otherdomain)
-NEXT_HYPERCALL(iret)
-NEXT_HYPERCALL(vcpu_op)
-NEXT_HYPERCALL(set_segment_base)
-NEXT_HYPERCALL(mmuext_op)
-NEXT_HYPERCALL(xsm_op)
-NEXT_HYPERCALL(nmi_op)
-NEXT_HYPERCALL(sched_op)
-NEXT_HYPERCALL(callback_op)
-NEXT_HYPERCALL(xenoprof_op)
-NEXT_HYPERCALL(event_channel_op)
-NEXT_HYPERCALL(physdev_op)
-NEXT_HYPERCALL(hvm_op)
-NEXT_HYPERCALL(sysctl)
-NEXT_HYPERCALL(domctl)
-NEXT_HYPERCALL(kexec_op)
-NEXT_HYPERCALL(tmem_op) /* 38 */
-ENTRY(xen_hypercall_rsvr)
-	.skip 320
-NEXT_HYPERCALL(mca) /* 48 */
-NEXT_HYPERCALL(arch_1)
-NEXT_HYPERCALL(arch_2)
-NEXT_HYPERCALL(arch_3)
-NEXT_HYPERCALL(arch_4)
-NEXT_HYPERCALL(arch_5)
-NEXT_HYPERCALL(arch_6)
+	.skip PAGE_SIZE
+
+#define HYPERCALL(n) \
+	.equ xen_hypercall_##n, hypercall_page + __HYPERVISOR_##n * 32; \
+	.type xen_hypercall_##n, function; .size xen_hypercall_##n, 32
+#include <asm/xen-hypercalls.h>
+#undef HYPERCALL
+
 	.balign PAGE_SIZE
 .popsection
 
-- 
2.1.2

Re: [Xen-devel] [PATCH 4/4] xen: use generated hypercall symbols in arch/x86/xen/xen-head.S

[David Vrabel]

On 11/12/14 18:04, Juergen Gross wrote:
> Instead of manually list each hypercall in arch/x86/xen/xen-head.S
> use the auto generated symbol list.
> 
> This also corrects the wrong address of xen_hypercall_mca which was
> located 32 bytes higher than it should.
> 
> Symbol addresses have been verified to match the correct ones via
> objdump output.
[...]
> +
> +#define HYPERCALL(n) \
> +	.equ xen_hypercall_##n, hypercall_page + __HYPERVISOR_##n * 32; \
> +	.type xen_hypercall_##n, function; .size xen_hypercall_##n, 32
> +#include <asm/xen-hypercalls.h>
> +#undef HYPERCALL

The gas manual[1] suggests the syntax you've used for .type is invalid
and suggest using .type <name>, STT_FUNC

> +
>  	.balign PAGE_SIZE

You can remove this .balign.

David

[1] https://sourceware.org/binutils/docs/as/Type.html#Type

Re: [Xen-devel] [PATCH 4/4] xen: use generated hypercall symbols in arch/x86/xen/xen-head.S

[Juergen Gross]

On 12/15/2014 01:05 PM, David Vrabel wrote:
> On 11/12/14 18:04, Juergen Gross wrote:
>> Instead of manually list each hypercall in arch/x86/xen/xen-head.S
>> use the auto generated symbol list.
>>
>> This also corrects the wrong address of xen_hypercall_mca which was
>> located 32 bytes higher than it should.
>>
>> Symbol addresses have been verified to match the correct ones via
>> objdump output.
> [...]
>> +
>> +#define HYPERCALL(n) \
>> +	.equ xen_hypercall_##n, hypercall_page + __HYPERVISOR_##n * 32; \
>> +	.type xen_hypercall_##n, function; .size xen_hypercall_##n, 32
>> +#include <asm/xen-hypercalls.h>
>> +#undef HYPERCALL
>
> The gas manual[1] suggests the syntax you've used for .type is invalid
> and suggest using .type <name>, STT_FUNC

Really? In the link below I see:

The types supported are:

STT_FUNC
function
     Mark the symbol as being a function name.
...

So "function" seems to be okay.

>
>> +
>>   	.balign PAGE_SIZE
>
> You can remove this .balign.

Indeed.


Juergen

Re: [Xen-devel] [PATCH 4/4] xen: use generated hypercall symbols in arch/x86/xen/xen-head.S

[David Vrabel]

On 16/12/14 05:55, Juergen Gross wrote:
> On 12/15/2014 01:05 PM, David Vrabel wrote:
>> On 11/12/14 18:04, Juergen Gross wrote:
>>> Instead of manually list each hypercall in arch/x86/xen/xen-head.S
>>> use the auto generated symbol list.
>>>
>>> This also corrects the wrong address of xen_hypercall_mca which was
>>> located 32 bytes higher than it should.
>>>
>>> Symbol addresses have been verified to match the correct ones via
>>> objdump output.
>> [...]
>>> +
>>> +#define HYPERCALL(n) \
>>> +    .equ xen_hypercall_##n, hypercall_page + __HYPERVISOR_##n * 32; \
>>> +    .type xen_hypercall_##n, function; .size xen_hypercall_##n, 32
>>> +#include <asm/xen-hypercalls.h>
>>> +#undef HYPERCALL
>>
>> The gas manual[1] suggests the syntax you've used for .type is invalid
>> and suggest using .type <name>, STT_FUNC
> 
> Really? In the link below I see:
> 
> The types supported are:
> 
> STT_FUNC
> function
>     Mark the symbol as being a function name.
> ...
> 
> So "function" seems to be okay.

>From the manual

    The syntaxes supported are:

       .type <name> STT_<TYPE_IN_UPPER_CASE>
       .type <name>,#<type>
       .type <name>,@<type>
       .type <name>,%<type>
       .type <name>,"<type>"

And

    The first variant will be accepted by the GNU assembler on all
    architectures...

David

Re: [Xen-devel] [PATCH 4/4] xen: use generated hypercall symbols in arch/x86/xen/xen-head.S

[Juergen Gross]

On 12/16/2014 11:24 AM, David Vrabel wrote:
> On 16/12/14 05:55, Juergen Gross wrote:
>> On 12/15/2014 01:05 PM, David Vrabel wrote:
>>> On 11/12/14 18:04, Juergen Gross wrote:
>>>> Instead of manually list each hypercall in arch/x86/xen/xen-head.S
>>>> use the auto generated symbol list.
>>>>
>>>> This also corrects the wrong address of xen_hypercall_mca which was
>>>> located 32 bytes higher than it should.
>>>>
>>>> Symbol addresses have been verified to match the correct ones via
>>>> objdump output.
>>> [...]
>>>> +
>>>> +#define HYPERCALL(n) \
>>>> +    .equ xen_hypercall_##n, hypercall_page + __HYPERVISOR_##n * 32; \
>>>> +    .type xen_hypercall_##n, function; .size xen_hypercall_##n, 32
>>>> +#include <asm/xen-hypercalls.h>
>>>> +#undef HYPERCALL
>>>
>>> The gas manual[1] suggests the syntax you've used for .type is invalid
>>> and suggest using .type <name>, STT_FUNC
>>
>> Really? In the link below I see:
>>
>> The types supported are:
>>
>> STT_FUNC
>> function
>>      Mark the symbol as being a function name.
>> ...
>>
>> So "function" seems to be okay.
>
>>From the manual
>
>      The syntaxes supported are:
>
>         .type <name> STT_<TYPE_IN_UPPER_CASE>
>         .type <name>,#<type>
>         .type <name>,@<type>
>         .type <name>,%<type>
>         .type <name>,"<type>"
>
> And
>
>      The first variant will be accepted by the GNU assembler on all
>      architectures...

grepping through the x86 assembler sources

.type <name>,@function

seems to be the preferred syntax (100%). I think I'll switch to that.


Juergen