| Message ID | 007679c8-84d5-2e91-e48e-68746741fb45@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: compat header generation and checking adjustments | expand |
On Wed, Jul 01, 2020 at 12:25:48PM +0200, Jan Beulich wrote: > 84e364f2eda2 ("x86: add CMCI software injection interface") merely made > sure things would build, without any concern about things actually > working: > - despite the addition of xenctl_bitmap to xlat.lst, the resulting macro > wasn't invoked anywhere (which would have lead to recognizing that the > structure appeared to have no fully compatible layout, despite the use > of a 64-bit handle), > - the interface struct itself was neither added to xlat.lst (and the > resulting macro then invoked) nor was any manual checking of > individual fields added. > > Adjust compat header generation logic to retain XEN_GUEST_HANDLE_64(), > which is intentionally layed out to be compatible between different size > guests. Invoke the missing checking (implicitly through CHECK_mc). > > No change in the resulting generated code. > > Fixes: 84e364f2eda2 ("x86: add CMCI software injection interface") > Signed-off-by: Jan Beulich <jbeulich@suse.com> LGTM, just one question below. > --- a/xen/tools/compat-build-header.py > +++ b/xen/tools/compat-build-header.py > @@ -19,6 +19,7 @@ pats = [ > [ r"(^|[^\w])xen_?(\w*)_compat_t([^\w]|$$)", r"\1compat_\2_t\3" ], > [ r"(^|[^\w])XEN_?", r"\1COMPAT_" ], > [ r"(^|[^\w])Xen_?", r"\1Compat_" ], > + [ r"(^|[^\w])COMPAT_HANDLE_64\(", r"\1XEN_GUEST_HANDLE_64(" ], Why do you need to match with the opening parenthesis? Is this for the #ifndef XEN_GUEST_HANDLE_64 instances? Don't they need to also be replaced with the compat types? Thanks, Roger.
On 14.07.2020 12:24, Roger Pau Monné wrote: > On Wed, Jul 01, 2020 at 12:25:48PM +0200, Jan Beulich wrote: >> --- a/xen/tools/compat-build-header.py >> +++ b/xen/tools/compat-build-header.py >> @@ -19,6 +19,7 @@ pats = [ >> [ r"(^|[^\w])xen_?(\w*)_compat_t([^\w]|$$)", r"\1compat_\2_t\3" ], >> [ r"(^|[^\w])XEN_?", r"\1COMPAT_" ], >> [ r"(^|[^\w])Xen_?", r"\1Compat_" ], >> + [ r"(^|[^\w])COMPAT_HANDLE_64\(", r"\1XEN_GUEST_HANDLE_64(" ], > > Why do you need to match with the opening parenthesis? > > Is this for the #ifndef XEN_GUEST_HANDLE_64 instances? Don't they need > to also be replaced with the compat types? Well, I wanted to be as strict as possible, i.e. along with the matching of a non-identifier char at the beginning I also wanted the other side to be delimited. Jan
On Wed, Jul 01, 2020 at 12:25:48PM +0200, Jan Beulich wrote: > 84e364f2eda2 ("x86: add CMCI software injection interface") merely made > sure things would build, without any concern about things actually > working: > - despite the addition of xenctl_bitmap to xlat.lst, the resulting macro > wasn't invoked anywhere (which would have lead to recognizing that the > structure appeared to have no fully compatible layout, despite the use > of a 64-bit handle), > - the interface struct itself was neither added to xlat.lst (and the > resulting macro then invoked) nor was any manual checking of > individual fields added. > > Adjust compat header generation logic to retain XEN_GUEST_HANDLE_64(), > which is intentionally layed out to be compatible between different size > guests. Invoke the missing checking (implicitly through CHECK_mc). > > No change in the resulting generated code. > > Fixes: 84e364f2eda2 ("x86: add CMCI software injection interface") > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks.
--- a/xen/arch/x86/cpu/mcheck/mce.c +++ b/xen/arch/x86/cpu/mcheck/mce.c @@ -1312,10 +1312,12 @@ CHECK_FIELD_(struct, mc_fetch, fetch_id) CHECK_FIELD_(struct, mc_physcpuinfo, ncpus); # define CHECK_compat_mc_physcpuinfo struct mc_physcpuinfo -#define CHECK_compat_mc_inject_v2 struct mc_inject_v2 +# define xen_ctl_bitmap xenctl_bitmap + CHECK_mc; # undef CHECK_compat_mc_fetch # undef CHECK_compat_mc_physcpuinfo +# undef xen_ctl_bitmap # define xen_mc_info mc_info CHECK_mc_info; --- a/xen/include/public/arch-x86/xen-mca.h +++ b/xen/include/public/arch-x86/xen-mca.h @@ -429,6 +429,7 @@ struct xen_mc_inject_v2 { uint32_t flags; xenctl_bitmap_t cpumap; }; +typedef struct xen_mc_inject_v2 xen_mc_inject_v2_t; #endif struct xen_mc { @@ -441,7 +442,7 @@ struct xen_mc { xen_mc_msrinject_t mc_msrinject; xen_mc_mceinject_t mc_mceinject; #if defined(__XEN__) || defined(__XEN_TOOLS__) - struct xen_mc_inject_v2 mc_inject_v2; + xen_mc_inject_v2_t mc_inject_v2; #endif } u; }; --- a/xen/include/xlat.lst +++ b/xen/include/xlat.lst @@ -25,6 +25,7 @@ ? mcinfo_recovery arch-x86/xen-mca.h ! mc_fetch arch-x86/xen-mca.h ? mc_info arch-x86/xen-mca.h +? mc_inject_v2 arch-x86/xen-mca.h ? mc_mceinject arch-x86/xen-mca.h ? mc_msrinject arch-x86/xen-mca.h ? mc_notifydomain arch-x86/xen-mca.h --- a/xen/tools/compat-build-header.py +++ b/xen/tools/compat-build-header.py @@ -19,6 +19,7 @@ pats = [ [ r"(^|[^\w])xen_?(\w*)_compat_t([^\w]|$$)", r"\1compat_\2_t\3" ], [ r"(^|[^\w])XEN_?", r"\1COMPAT_" ], [ r"(^|[^\w])Xen_?", r"\1Compat_" ], + [ r"(^|[^\w])COMPAT_HANDLE_64\(", r"\1XEN_GUEST_HANDLE_64(" ], [ r"(^|[^\w])long([^\w]|$$)", r"\1int\2" ] ]; --- a/xen/tools/compat-build-source.py +++ b/xen/tools/compat-build-source.py @@ -10,7 +10,7 @@ pats = [ [ r"^\s*#\s*define\s+([A-Z_]*_GUEST_HANDLE)", r"#define HIDE_\1" ], [ r"^\s*#\s*define\s+([a-z_]*_guest_handle)", r"#define hide_\1" ], [ r"^\s*#\s*define\s+(u?int64)_aligned_t\s.*", r"typedef \1_T __attribute__((aligned(4))) \1_compat_T;" ], - [ r"XEN_GUEST_HANDLE(_[0-9A-Fa-f]+)?", r"COMPAT_HANDLE" ], + [ r"XEN_GUEST_HANDLE", r"COMPAT_HANDLE" ], ]; xlatf = open('xlat.lst', 'r')
84e364f2eda2 ("x86: add CMCI software injection interface") merely made sure things would build, without any concern about things actually working: - despite the addition of xenctl_bitmap to xlat.lst, the resulting macro wasn't invoked anywhere (which would have lead to recognizing that the structure appeared to have no fully compatible layout, despite the use of a 64-bit handle), - the interface struct itself was neither added to xlat.lst (and the resulting macro then invoked) nor was any manual checking of individual fields added. Adjust compat header generation logic to retain XEN_GUEST_HANDLE_64(), which is intentionally layed out to be compatible between different size guests. Invoke the missing checking (implicitly through CHECK_mc). No change in the resulting generated code. Fixes: 84e364f2eda2 ("x86: add CMCI software injection interface") Signed-off-by: Jan Beulich <jbeulich@suse.com>