Re: [PATCH] x86/shadow: don't leave trace record field uninitialized
On Wed, 2024-05-22 at 12:17 +0200, Jan Beulich wrote:
> The emulation_count field is set only conditionally right now.
> Convert
> all field setting to an initializer, thus guaranteeing that field to
> be
> set to 0 (default initialized) when GUEST_PAGING_LEVELS != 3.
>
> While there also drop the "event" local variable, thus eliminating an
> instance of the being phased out u32 type.
>
> Coverity ID: 1598430
> Fixes: 9a86ac1aa3d2 ("xentrace 5/7: Additional tracing for the shadow
> code")
> Signed-off-by: Jan Beulich
Release-acked-by: Oleksii Kurochko
~ Oleksii
>
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -2093,20 +2093,18 @@ static inline void trace_shadow_emulate(
> guest_l1e_t gl1e, write_val;
> guest_va_t va;
> uint32_t flags:29, emulation_count:3;
> - } d;
> - u32 event;
> -
> - event = TRC_SHADOW_EMULATE | ((GUEST_PAGING_LEVELS-2)<<8);
> -
> - d.gl1e = gl1e;
> - d.write_val.l1 = this_cpu(trace_emulate_write_val);
> - d.va = va;
> + } d = {
> + .gl1e = gl1e,
> + .write_val.l1 = this_cpu(trace_emulate_write_val),
> + .va = va,
> #if GUEST_PAGING_LEVELS == 3
> - d.emulation_count = this_cpu(trace_extra_emulation_count);
> + .emulation_count =
> this_cpu(trace_extra_emulation_count),
> #endif
> - d.flags = this_cpu(trace_shadow_path_flags);
> + .flags = this_cpu(trace_shadow_path_flags),
> + };
>
> - trace(event, sizeof(d), );
> + trace(TRC_SHADOW_EMULATE | ((GUEST_PAGING_LEVELS - 2) << 8),
> + sizeof(d), );
> }
> }
> #endif /* CONFIG_HVM */
Re: [PATCH] x86/shadow: don't leave trace record field uninitialized
On 22/05/2024 11:17 am, Jan Beulich wrote:
> The emulation_count field is set only conditionally right now. Convert
> all field setting to an initializer, thus guaranteeing that field to be
> set to 0 (default initialized) when GUEST_PAGING_LEVELS != 3.
>
> While there also drop the "event" local variable, thus eliminating an
> instance of the being phased out u32 type.
>
> Coverity ID: 1598430
> Fixes: 9a86ac1aa3d2 ("xentrace 5/7: Additional tracing for the shadow code")
> Signed-off-by: Jan Beulich
This is an improvement, but there's a related mess right next to it.
I think this would be a whole lot better with a couple of tweaks, if
you're willing to wait a little for me to try.
~Andrew
Re: [PATCH] x86/shadow: don't leave trace record field uninitialized
On Wed, May 22, 2024 at 12:17:30PM +0200, Jan Beulich wrote:
> The emulation_count field is set only conditionally right now. Convert
> all field setting to an initializer, thus guaranteeing that field to be
> set to 0 (default initialized) when GUEST_PAGING_LEVELS != 3.
>
> While there also drop the "event" local variable, thus eliminating an
> instance of the being phased out u32 type.
>
> Coverity ID: 1598430
> Fixes: 9a86ac1aa3d2 ("xentrace 5/7: Additional tracing for the shadow code")
> Signed-off-by: Jan Beulich
Acked-by: Roger Pau Monné
Thanks, Roger.
[PATCH] x86/shadow: don't leave trace record field uninitialized
The emulation_count field is set only conditionally right now. Convert
all field setting to an initializer, thus guaranteeing that field to be
set to 0 (default initialized) when GUEST_PAGING_LEVELS != 3.
While there also drop the "event" local variable, thus eliminating an
instance of the being phased out u32 type.
Coverity ID: 1598430
Fixes: 9a86ac1aa3d2 ("xentrace 5/7: Additional tracing for the shadow code")
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -2093,20 +2093,18 @@ static inline void trace_shadow_emulate(
guest_l1e_t gl1e, write_val;
guest_va_t va;
uint32_t flags:29, emulation_count:3;
-} d;
-u32 event;
-
-event = TRC_SHADOW_EMULATE | ((GUEST_PAGING_LEVELS-2)<<8);
-
-d.gl1e = gl1e;
-d.write_val.l1 = this_cpu(trace_emulate_write_val);
-d.va = va;
+} d = {
+.gl1e = gl1e,
+.write_val.l1 = this_cpu(trace_emulate_write_val),
+.va = va,
#if GUEST_PAGING_LEVELS == 3
-d.emulation_count = this_cpu(trace_extra_emulation_count);
+.emulation_count = this_cpu(trace_extra_emulation_count),
#endif
-d.flags = this_cpu(trace_shadow_path_flags);
+.flags = this_cpu(trace_shadow_path_flags),
+};
-trace(event, sizeof(d), );
+trace(TRC_SHADOW_EMULATE | ((GUEST_PAGING_LEVELS - 2) << 8),
+ sizeof(d), );
}
}
#endif /* CONFIG_HVM */
