| Message ID | 5133a01d-346d-4779-b319-a156ef944669@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/shadow: don't leave trace record field uninitialized | expand |
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 <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com> Thanks, Roger.
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 <jbeulich@suse.com> 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
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 <jbeulich@suse.com> Release-acked-by: Oleksii Kurochko <oleksii.kurochko.com> ~ 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), &d); > + trace(TRC_SHADOW_EMULATE | ((GUEST_PAGING_LEVELS - 2) << 8), > + sizeof(d), &d); > } > } > #endif /* CONFIG_HVM */
--- 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), &d); + trace(TRC_SHADOW_EMULATE | ((GUEST_PAGING_LEVELS - 2) << 8), + sizeof(d), &d); } } #endif /* CONFIG_HVM */
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 <jbeulich@suse.com>