Ian Campbell
2013-Mar-13 12:43 UTC
[PATCH V3 00/03] xen: public interface and foreign struct check changes for arm
This is v3 of the series. The bits about evtchn xen_ulong_t have already gone in so what remains is about extending the foreign checks to cover more structs on ARM, so as to get better coverage for ABI compatibility between arm32 and arm64. The main change since last time is to only ifdef struct start_info instead of moving it as requested/suggested by Jan. This required teaching mkheader.py about such things. This also means that equivalent Linux side change is no longer really worth it, so I won''t be resending that bit (CCing Konrad here just FYI). I also added an extra patch which causes mkchecker to actually confirm that arm32/arm64 are compatible. Ian.
Ian Campbell
2013-Mar-13 12:45 UTC
[PATCH 1/3] xen: only expose start_info on architectures which have a PV boot path
Most of this struct is PV MMU specific and it is not used on ARM at all. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Jan Beulich <JBeulich@suse.com> Cc: Keir (Xen.org) <keir@xen.org> Cc: Tim Deegan <tim@xen.org> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- v3: Renamed from "xen: make start_info x86 specific." and use an ifdef instead of moving the definition. --- tools/libxc/xenctrl.h | 5 ++--- xen/include/public/arch-x86/xen.h | 2 ++ xen/include/public/xen.h | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h index 32122fd..9e4a741 100644 --- a/tools/libxc/xenctrl.h +++ b/tools/libxc/xenctrl.h @@ -395,15 +395,14 @@ typedef union shared_info_t s; } shared_info_any_t; +#if defined(__i386__) || defined(__x86_64__) typedef union { -#if defined(__i386__) || defined(__x86_64__) start_info_x86_64_t x64; start_info_x86_32_t x32; -#endif start_info_t s; } start_info_any_t; - +#endif int xc_domain_create(xc_interface *xch, uint32_t ssidref, diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h index b7f6a51..7ae8c90 100644 --- a/xen/include/public/arch-x86/xen.h +++ b/xen/include/public/arch-x86/xen.h @@ -70,6 +70,8 @@ typedef unsigned long xen_pfn_t; #define PRI_xen_pfn "lx" #endif +#define XEN_HAVE_PV_GUEST_ENTRY 1 + /* * `incontents 200 segdesc Segment Descriptor Tables */ diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index e9431e2..0d92b1f 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -705,7 +705,7 @@ typedef struct shared_info shared_info_t; * bootstrap element. If necessary, the bootstrap virtual region is * extended by an extra 4MB to ensure this. */ - +#ifdef XEN_HAVE_PV_GUEST_ENTRY #define MAX_GUEST_CMDLINE 1024 struct start_info { /* THE FOLLOWING ARE FILLED IN BOTH ON INITIAL BOOT AND ON RESUME. */ @@ -745,6 +745,7 @@ typedef struct start_info start_info_t; #define console_mfn console.domU.mfn #define console_evtchn console.domU.evtchn #endif +#endif /* These flags are passed in the ''flags'' field of start_info_t. */ #define SIF_PRIVILEGED (1<<0) /* Is the domain privileged? */ -- 1.7.2.5
Ian Campbell
2013-Mar-13 12:45 UTC
[PATCH 2/3] xen: arm: include public/xen.h in foreign interface checking
mkheader.py doesn''t cope with structr foo { }; so add a newline. Define unsigned long and long to a non-existent type on ARM so as to catch their use. Teach mkheader.py to cope with structs which are ifdef''d. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Jan Beulich <JBeulich@suse.com> Cc: Keir (Xen.org) <keir@xen.org> Cc: Tim Deegan <tim@xen.org> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- v3: rework for ifdeffed instead of moved start_info. --- tools/include/xen-foreign/Makefile | 4 ++-- tools/include/xen-foreign/mkheader.py | 14 +++++++++----- tools/include/xen-foreign/reference.size | 10 +++++----- tools/include/xen-foreign/structs.py | 2 ++ xen/include/public/arch-arm.h | 8 +++++--- xen/include/public/xen.h | 2 +- 6 files changed, 24 insertions(+), 16 deletions(-) diff --git a/tools/include/xen-foreign/Makefile b/tools/include/xen-foreign/Makefile index 8e0be83..06b844c 100644 --- a/tools/include/xen-foreign/Makefile +++ b/tools/include/xen-foreign/Makefile @@ -22,10 +22,10 @@ check-headers: checker diff -u reference.size tmp.size rm tmp.size -arm32.h: mkheader.py structs.py $(ROOT)/arch-arm.h +arm32.h: mkheader.py structs.py $(ROOT)/arch-arm.h $(ROOT)/xen.h $(PYTHON) $< $* $@ $(filter %.h,$^) -arm64.h: mkheader.py structs.py $(ROOT)/arch-arm.h +arm64.h: mkheader.py structs.py $(ROOT)/arch-arm.h $(ROOT)/xen.h $(PYTHON) $< $* $@ $(filter %.h,$^) x86_32.h: mkheader.py structs.py $(ROOT)/arch-x86/xen-x86_32.h $(ROOT)/arch-x86/xen.h $(ROOT)/xen.h diff --git a/tools/include/xen-foreign/mkheader.py b/tools/include/xen-foreign/mkheader.py index b19292f..0504cb8 100644 --- a/tools/include/xen-foreign/mkheader.py +++ b/tools/include/xen-foreign/mkheader.py @@ -18,8 +18,8 @@ footer = {}; #arm inttypes["arm32"] = { - "unsigned long" : "uint32_t", - "long" : "uint32_t", + "unsigned long" : "__danger_unsigned_long_on_arm32", + "long" : "__danger_long_on_arm32", "xen_pfn_t" : "__align8__ uint64_t", "xen_ulong_t" : "__align8__ uint64_t", "uint64_t" : "__align8__ uint64_t", @@ -124,6 +124,8 @@ if arch in header: output += header[arch]; output += "\n"; +defined = {} + # add defines to output for line in re.findall("#define[^\n]+", input): for define in defines: @@ -131,6 +133,7 @@ for line in re.findall("#define[^\n]+", input): match = re.search(regex, line); if None == match: continue; + defined[define] = 1 if define.upper()[0] == define[0]: replace = define + "_" + arch.upper(); else: @@ -156,12 +159,13 @@ for union in unions: # add structs to output for struct in structs: - regex = "struct\s+%s\s*\{(.*?)\n\};" % struct; + regex = "(?:#ifdef ([A-Z_]+))?\nstruct\s+%s\s*\{(.*?)\n\};" % struct; match = re.search(regex, input, re.S) - if None == match: + if None == match or \ + (match.group(1) is not None and match.group(1) not in defined): output += "#define %s_has_no_%s 1\n" % (arch, struct); else: - output += "struct %s_%s {%s\n};\n" % (struct, arch, match.group(1)); + output += "struct %s_%s {%s\n};\n" % (struct, arch, match.group(2)); output += "typedef struct %s_%s %s_%s_t;\n" % (struct, arch, struct, arch); output += "\n"; diff --git a/tools/include/xen-foreign/reference.size b/tools/include/xen-foreign/reference.size index de36455..b3347b4 100644 --- a/tools/include/xen-foreign/reference.size +++ b/tools/include/xen-foreign/reference.size @@ -6,9 +6,9 @@ trap_info | - - 8 16 cpu_user_regs | - - 68 200 vcpu_guest_core_regs | 304 304 - - vcpu_guest_context | 336 336 2800 5168 -arch_vcpu_info | - - 24 16 -vcpu_time_info | - - 32 32 -vcpu_info | - - 64 64 -arch_shared_info | - - 268 280 -shared_info | - - 2584 3368 +arch_vcpu_info | 0 0 24 16 +vcpu_time_info | 32 32 32 32 +vcpu_info | 48 48 64 64 +arch_shared_info | 0 0 268 280 +shared_info | 1088 1088 2584 3368 diff --git a/tools/include/xen-foreign/structs.py b/tools/include/xen-foreign/structs.py index 0b33a77..476eb85 100644 --- a/tools/include/xen-foreign/structs.py +++ b/tools/include/xen-foreign/structs.py @@ -19,6 +19,8 @@ defines = [ "__arm__", "__i386__", "__x86_64__", + "XEN_HAVE_PV_GUEST_ENTRY", + # arm # None diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h index 3333399..8c63d4a 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -184,14 +184,16 @@ struct vcpu_guest_context { typedef struct vcpu_guest_context vcpu_guest_context_t; DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t); -struct arch_vcpu_info { }; +struct arch_vcpu_info { +}; typedef struct arch_vcpu_info arch_vcpu_info_t; -struct arch_shared_info { }; +struct arch_shared_info { +}; typedef struct arch_shared_info arch_shared_info_t; typedef uint64_t xen_callback_t; -#endif /* ifndef __ASSEMBLY __ */ +#endif /* PSR bits (CPSR, SPSR)*/ diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index 0d92b1f..ef78846 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -706,7 +706,6 @@ typedef struct shared_info shared_info_t; * extended by an extra 4MB to ensure this. */ #ifdef XEN_HAVE_PV_GUEST_ENTRY -#define MAX_GUEST_CMDLINE 1024 struct start_info { /* THE FOLLOWING ARE FILLED IN BOTH ON INITIAL BOOT AND ON RESUME. */ char magic[32]; /* "xen-<version>-<platform>". */ @@ -733,6 +732,7 @@ struct start_info { /* (PFN of pre-loaded module if */ /* SIF_MOD_START_PFN set in flags). */ unsigned long mod_len; /* Size (bytes) of pre-loaded module. */ +#define MAX_GUEST_CMDLINE 1024 int8_t cmd_line[MAX_GUEST_CMDLINE]; /* The pfn range here covers both page table and p->m table frames. */ unsigned long first_p2m_pfn;/* 1st pfn forming initial P->M table. */ -- 1.7.2.5
Ian Campbell
2013-Mar-13 12:45 UTC
[PATCH 3/3] tools: foreign: add checks for compatible architectures
That is architectures whose struct layout must be identical. Use this for arm32 and arm64. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Jan Beulich <JBeulich@suse.com> Cc: Keir (Xen.org) <keir@xen.org> Cc: Tim Deegan <tim@xen.org> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- tools/include/xen-foreign/mkchecker.py | 21 +++++++++++++++++++-- tools/include/xen-foreign/structs.py | 5 +++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/tools/include/xen-foreign/mkchecker.py b/tools/include/xen-foreign/mkchecker.py index 66c17b1..fdad869 100644 --- a/tools/include/xen-foreign/mkchecker.py +++ b/tools/include/xen-foreign/mkchecker.py @@ -1,7 +1,7 @@ #!/usr/bin/python import sys; -from structs import structs; +from structs import structs, compat_arches; # command line arguments outfile = sys.argv[1]; @@ -37,10 +37,27 @@ for struct in structs: f.write(''\tprintf("%%-25s |", "%s");\n'' % struct); for a in archs: s = struct + "_" + a; + if compat_arches.has_key(a): + compat = compat_arches[a] + c = struct + "_" + compat; + else: + compat = None f.write(''#ifdef %s_has_no_%s\n'' % (a, struct)); - f.write(''\tprintf("%8s", "-");\n''); + f.write(''\tprintf("%8s",\n''); + if compat: + f.write(''# ifndef %s_has_no_%s\n'' % (compat, struct)); + f.write(''\t\t"!"\n''); + f.write(''# else\n'') + f.write(''\t\t"-"\n''); + f.write(''# endif\n'') + else: + f.write(''\t\t"-"\n''); + f.write(''\t);\n'') f.write("#else\n"); f.write(''\tprintf("%%8zd", sizeof(struct %s));\n'' % s); + if compat: + f.write(''\tif (sizeof(struct %s) != sizeof(struct %s))\n'' % (s, c)) + f.write(''\t\tprintf("!");\n'') f.write("#endif\n"); f.write(''\tprintf("\\n");\n\n''); diff --git a/tools/include/xen-foreign/structs.py b/tools/include/xen-foreign/structs.py index 476eb85..3d9f2fe 100644 --- a/tools/include/xen-foreign/structs.py +++ b/tools/include/xen-foreign/structs.py @@ -58,3 +58,8 @@ defines = [ "__arm__", "XEN_LEGACY_MAX_VCPUS", "MAX_GUEST_CMDLINE" ]; +# Architectures which must be compatible, i.e. identical +compat_arches = { + ''arm32'': ''arm64'', + ''arm64'': ''arm32'', +} -- 1.7.2.5
Jan Beulich
2013-Mar-13 13:00 UTC
Re: [PATCH 1/3] xen: only expose start_info on architectures which have a PV boot path
>>> On 13.03.13 at 13:45, Ian Campbell <ian.campbell@citrix.com> wrote: > Most of this struct is PV MMU specific and it is not used on ARM at all. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Looks a lot nicer now. Acked-by: Jan Beulich <jbeulich@suse.com>> Cc: Jan Beulich <JBeulich@suse.com> > Cc: Keir (Xen.org) <keir@xen.org> > Cc: Tim Deegan <tim@xen.org> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > v3: Renamed from "xen: make start_info x86 specific." and use an ifdef > instead > of moving the definition. > --- > tools/libxc/xenctrl.h | 5 ++--- > xen/include/public/arch-x86/xen.h | 2 ++ > xen/include/public/xen.h | 3 ++- > 3 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h > index 32122fd..9e4a741 100644 > --- a/tools/libxc/xenctrl.h > +++ b/tools/libxc/xenctrl.h > @@ -395,15 +395,14 @@ typedef union > shared_info_t s; > } shared_info_any_t; > > +#if defined(__i386__) || defined(__x86_64__) > typedef union > { > -#if defined(__i386__) || defined(__x86_64__) > start_info_x86_64_t x64; > start_info_x86_32_t x32; > -#endif > start_info_t s; > } start_info_any_t; > - > +#endif > > int xc_domain_create(xc_interface *xch, > uint32_t ssidref, > diff --git a/xen/include/public/arch-x86/xen.h > b/xen/include/public/arch-x86/xen.h > index b7f6a51..7ae8c90 100644 > --- a/xen/include/public/arch-x86/xen.h > +++ b/xen/include/public/arch-x86/xen.h > @@ -70,6 +70,8 @@ typedef unsigned long xen_pfn_t; > #define PRI_xen_pfn "lx" > #endif > > +#define XEN_HAVE_PV_GUEST_ENTRY 1 > + > /* > * `incontents 200 segdesc Segment Descriptor Tables > */ > diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h > index e9431e2..0d92b1f 100644 > --- a/xen/include/public/xen.h > +++ b/xen/include/public/xen.h > @@ -705,7 +705,7 @@ typedef struct shared_info shared_info_t; > * bootstrap element. If necessary, the bootstrap virtual region is > * extended by an extra 4MB to ensure this. > */ > - > +#ifdef XEN_HAVE_PV_GUEST_ENTRY > #define MAX_GUEST_CMDLINE 1024 > struct start_info { > /* THE FOLLOWING ARE FILLED IN BOTH ON INITIAL BOOT AND ON RESUME. > */ > @@ -745,6 +745,7 @@ typedef struct start_info start_info_t; > #define console_mfn console.domU.mfn > #define console_evtchn console.domU.evtchn > #endif > +#endif > > /* These flags are passed in the ''flags'' field of start_info_t. */ > #define SIF_PRIVILEGED (1<<0) /* Is the domain privileged? */ > -- > 1.7.2.5
Ian Campbell
2013-Apr-10 15:25 UTC
Re: [PATCH 2/3] xen: arm: include public/xen.h in foreign interface checking
ping? Also for the 3/3 patch. On Wed, 2013-03-13 at 12:45 +0000, Ian Campbell wrote:> mkheader.py doesn''t cope with > structr foo { }; > so add a newline. > > Define unsigned long and long to a non-existent type on ARM so as to catch > their use. > > Teach mkheader.py to cope with structs which are ifdef''d. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Jan Beulich <JBeulich@suse.com> > Cc: Keir (Xen.org) <keir@xen.org> > Cc: Tim Deegan <tim@xen.org> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > v3: rework for ifdeffed instead of moved start_info. > --- > tools/include/xen-foreign/Makefile | 4 ++-- > tools/include/xen-foreign/mkheader.py | 14 +++++++++----- > tools/include/xen-foreign/reference.size | 10 +++++----- > tools/include/xen-foreign/structs.py | 2 ++ > xen/include/public/arch-arm.h | 8 +++++--- > xen/include/public/xen.h | 2 +- > 6 files changed, 24 insertions(+), 16 deletions(-) > > diff --git a/tools/include/xen-foreign/Makefile b/tools/include/xen-foreign/Makefile > index 8e0be83..06b844c 100644 > --- a/tools/include/xen-foreign/Makefile > +++ b/tools/include/xen-foreign/Makefile > @@ -22,10 +22,10 @@ check-headers: checker > diff -u reference.size tmp.size > rm tmp.size > > -arm32.h: mkheader.py structs.py $(ROOT)/arch-arm.h > +arm32.h: mkheader.py structs.py $(ROOT)/arch-arm.h $(ROOT)/xen.h > $(PYTHON) $< $* $@ $(filter %.h,$^) > > -arm64.h: mkheader.py structs.py $(ROOT)/arch-arm.h > +arm64.h: mkheader.py structs.py $(ROOT)/arch-arm.h $(ROOT)/xen.h > $(PYTHON) $< $* $@ $(filter %.h,$^) > > x86_32.h: mkheader.py structs.py $(ROOT)/arch-x86/xen-x86_32.h $(ROOT)/arch-x86/xen.h $(ROOT)/xen.h > diff --git a/tools/include/xen-foreign/mkheader.py b/tools/include/xen-foreign/mkheader.py > index b19292f..0504cb8 100644 > --- a/tools/include/xen-foreign/mkheader.py > +++ b/tools/include/xen-foreign/mkheader.py > @@ -18,8 +18,8 @@ footer = {}; > > #arm > inttypes["arm32"] = { > - "unsigned long" : "uint32_t", > - "long" : "uint32_t", > + "unsigned long" : "__danger_unsigned_long_on_arm32", > + "long" : "__danger_long_on_arm32", > "xen_pfn_t" : "__align8__ uint64_t", > "xen_ulong_t" : "__align8__ uint64_t", > "uint64_t" : "__align8__ uint64_t", > @@ -124,6 +124,8 @@ if arch in header: > output += header[arch]; > output += "\n"; > > +defined = {} > + > # add defines to output > for line in re.findall("#define[^\n]+", input): > for define in defines: > @@ -131,6 +133,7 @@ for line in re.findall("#define[^\n]+", input): > match = re.search(regex, line); > if None == match: > continue; > + defined[define] = 1 > if define.upper()[0] == define[0]: > replace = define + "_" + arch.upper(); > else: > @@ -156,12 +159,13 @@ for union in unions: > > # add structs to output > for struct in structs: > - regex = "struct\s+%s\s*\{(.*?)\n\};" % struct; > + regex = "(?:#ifdef ([A-Z_]+))?\nstruct\s+%s\s*\{(.*?)\n\};" % struct; > match = re.search(regex, input, re.S) > - if None == match: > + if None == match or \ > + (match.group(1) is not None and match.group(1) not in defined): > output += "#define %s_has_no_%s 1\n" % (arch, struct); > else: > - output += "struct %s_%s {%s\n};\n" % (struct, arch, match.group(1)); > + output += "struct %s_%s {%s\n};\n" % (struct, arch, match.group(2)); > output += "typedef struct %s_%s %s_%s_t;\n" % (struct, arch, struct, arch); > output += "\n"; > > diff --git a/tools/include/xen-foreign/reference.size b/tools/include/xen-foreign/reference.size > index de36455..b3347b4 100644 > --- a/tools/include/xen-foreign/reference.size > +++ b/tools/include/xen-foreign/reference.size > @@ -6,9 +6,9 @@ trap_info | - - 8 16 > cpu_user_regs | - - 68 200 > vcpu_guest_core_regs | 304 304 - - > vcpu_guest_context | 336 336 2800 5168 > -arch_vcpu_info | - - 24 16 > -vcpu_time_info | - - 32 32 > -vcpu_info | - - 64 64 > -arch_shared_info | - - 268 280 > -shared_info | - - 2584 3368 > +arch_vcpu_info | 0 0 24 16 > +vcpu_time_info | 32 32 32 32 > +vcpu_info | 48 48 64 64 > +arch_shared_info | 0 0 268 280 > +shared_info | 1088 1088 2584 3368 > > diff --git a/tools/include/xen-foreign/structs.py b/tools/include/xen-foreign/structs.py > index 0b33a77..476eb85 100644 > --- a/tools/include/xen-foreign/structs.py > +++ b/tools/include/xen-foreign/structs.py > @@ -19,6 +19,8 @@ defines = [ "__arm__", > "__i386__", > "__x86_64__", > > + "XEN_HAVE_PV_GUEST_ENTRY", > + > # arm > # None > > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > index 3333399..8c63d4a 100644 > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -184,14 +184,16 @@ struct vcpu_guest_context { > typedef struct vcpu_guest_context vcpu_guest_context_t; > DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t); > > -struct arch_vcpu_info { }; > +struct arch_vcpu_info { > +}; > typedef struct arch_vcpu_info arch_vcpu_info_t; > > -struct arch_shared_info { }; > +struct arch_shared_info { > +}; > typedef struct arch_shared_info arch_shared_info_t; > typedef uint64_t xen_callback_t; > > -#endif /* ifndef __ASSEMBLY __ */ > +#endif > > /* PSR bits (CPSR, SPSR)*/ > > diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h > index 0d92b1f..ef78846 100644 > --- a/xen/include/public/xen.h > +++ b/xen/include/public/xen.h > @@ -706,7 +706,6 @@ typedef struct shared_info shared_info_t; > * extended by an extra 4MB to ensure this. > */ > #ifdef XEN_HAVE_PV_GUEST_ENTRY > -#define MAX_GUEST_CMDLINE 1024 > struct start_info { > /* THE FOLLOWING ARE FILLED IN BOTH ON INITIAL BOOT AND ON RESUME. */ > char magic[32]; /* "xen-<version>-<platform>". */ > @@ -733,6 +732,7 @@ struct start_info { > /* (PFN of pre-loaded module if */ > /* SIF_MOD_START_PFN set in flags). */ > unsigned long mod_len; /* Size (bytes) of pre-loaded module. */ > +#define MAX_GUEST_CMDLINE 1024 > int8_t cmd_line[MAX_GUEST_CMDLINE]; > /* The pfn range here covers both page table and p->m table frames. */ > unsigned long first_p2m_pfn;/* 1st pfn forming initial P->M table. */