[v2,2/7] x86/mce: add compat struct checking for XEN_MC_inject_v2
diff mbox series

Message ID 007679c8-84d5-2e91-e48e-68746741fb45@suse.com
State Superseded
Headers show
Series
  • x86: compat header generation and checking adjustments
Related show

Commit Message

Jan Beulich July 1, 2020, 10:25 a.m. UTC
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>

Comments

Roger Pau Monne July 14, 2020, 10:24 a.m. UTC | #1
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.
Jan Beulich July 14, 2020, 11:44 a.m. UTC | #2
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
Roger Pau Monne July 14, 2020, 2:30 p.m. UTC | #3
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.

Patch
diff mbox series

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