| Message ID | 20230728181730.3065977-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/amd: Fix DE_CFG truncation in amd_check_zenbleed() | expand |
On 28.07.2023 20:17, Andrew Cooper wrote: > This line: > > val &= ~chickenbit; > > ends up truncating val to 32 bits, and turning off various errata workarounds > in Zen2 systems. > > Fixes: f91c5ea97067 ("x86/amd: Mitigations for Zenbleed") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Wei Liu <wl@xen.org> > > The choice is between int or uint64_t. This is one case where the insistence > on using unsigned int as a default data type is genuinely unsafe. It is not. The (unsigned) type should have been wide enough. From a Misra perspective I'm pretty sure we would be better off using uint64_t. But in the interest of getting this in without a lot of discussion I'll leave the decision up to you; either way Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
On 31/07/2023 10:02 am, Jan Beulich wrote: > On 28.07.2023 20:17, Andrew Cooper wrote: >> This line: >> >> val &= ~chickenbit; >> >> ends up truncating val to 32 bits, and turning off various errata workarounds >> in Zen2 systems. >> >> Fixes: f91c5ea97067 ("x86/amd: Mitigations for Zenbleed") >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> CC: Wei Liu <wl@xen.org> >> >> The choice is between int or uint64_t. This is one case where the insistence >> on using unsigned int as a default data type is genuinely unsafe. > It is not. The (unsigned) type should have been wide enough. From a Misra > perspective I'm pretty sure we would be better off using uint64_t. But in > the interest of getting this in without a lot of discussion I'll leave the > decision up to you; either way > Reviewed-by: Jan Beulich <jbeulich@suse.com> Roger asked for uint64_t too so I'll go with that. Thanks. ~Andrew
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c index 3ed06f670491..089038bf62c5 100644 --- a/xen/arch/x86/cpu/amd.c +++ b/xen/arch/x86/cpu/amd.c @@ -909,8 +909,9 @@ void __init detect_zen2_null_seg_behaviour(void) void amd_check_zenbleed(void) { const struct cpu_signature *sig = &this_cpu(cpu_sig); - unsigned int good_rev, chickenbit = (1 << 9); + unsigned int good_rev; uint64_t val, old_val; + int chickenbit = (1 << 9); /* * If we're virtualised, we can't do family/model checks safely, and
This line: val &= ~chickenbit; ends up truncating val to 32 bits, and turning off various errata workarounds in Zen2 systems. Fixes: f91c5ea97067 ("x86/amd: Mitigations for Zenbleed") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> The choice is between int or uint64_t. This is one case where the insistence on using unsigned int as a default data type is genuinely unsafe. --- xen/arch/x86/cpu/amd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)