2014-12-11 18:06:53

by Juergen Gross

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

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


2014-12-11 18:06:54

by Juergen Gross

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

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 <[email protected]>
Signed-off-by: Juergen Gross <[email protected]>
---
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

2014-12-11 18:07:36

by Juergen Gross

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

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 <[email protected]>
---
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

2014-12-11 18:07:35

by Juergen Gross

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

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

Signed-off-by: Juergen Gross <[email protected]>
---
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

2014-12-11 18:07:34

by Juergen Gross

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

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 <[email protected]>
Signed-off-by: Juergen Gross <[email protected]>
---
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

2014-12-12 22:46:51

by Boris Ostrovsky

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

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

2014-12-15 05:20:47

by Juergen Gross

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

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

2014-12-15 10:00:48

by Jan Beulich

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

>>> On 12.12.14 at 23:48, <[email protected]> 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

2014-12-15 11:38:48

by David Vrabel

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

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

2014-12-15 11:45:31

by David Vrabel

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

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 <[email protected]>

David

2014-12-15 11:46:34

by David Vrabel

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

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 <[email protected]>

David

2014-12-15 11:47:18

by Jan Beulich

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

>>> On 15.12.14 at 12:38, <[email protected]> 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

2014-12-15 12:06:10

by David Vrabel

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

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

2014-12-16 05:55:56

by Juergen Gross

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

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

2014-12-16 10:24:50

by David Vrabel

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

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

2014-12-16 11:20:10

by Juergen Gross

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

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

2014-12-16 14:40:09

by Juergen Gross

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

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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2014-12-16 14:52:08

by David Vrabel

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

On 15/12/14 11:47, Jan Beulich wrote:
>>>> On 15.12.14 at 12:38, <[email protected]> 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