Message ID | 20210702190334.31271-1-andrew.cooper3@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | tools/libxenguest: Fix max_extd_leaf calculation for legacy restore | expand |
Am Fri, 2 Jul 2021 20:03:34 +0100
schrieb Andrew Cooper <andrew.cooper3@citrix.com>:
> Fixes: 34990446ca91 ("libxl: don't ignore the return value from xc_cpuid_apply_policy")
I think it fixes 111c8c33a8a18588f3da3c5dbb7f5c63ddb98ce5 ("x86/cpuid: do not expand max leaves on restore"), 34990446ca91 just revealed the bug?
Either way, this new variant is what I had tested last week. But with a trailing "u" for the constant. This detail may not make a difference in practice.
Olaf
On 02.07.2021 21:03, Andrew Cooper wrote: > 0x1c is lower than any value which will actually be observed in > p->extd.max_leaf, but higher than the logical 9 leaves worth of extended data > on Intel systems, causing x86_cpuid_copy_to_buffer() to fail with -ENOBUFS. > > Correct the calculation. > > The problem was first noticed in c/s 34990446ca9 "libxl: don't ignore the > return value from xc_cpuid_apply_policy" but introduced earlier. > > Fixes: 34990446ca91 ("libxl: don't ignore the return value from xc_cpuid_apply_policy") > Reported-by: Olaf Hering <olaf@aepfle.de> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> perhaps with, as suggested by Olaf, the Fixes: line changed. Jan
On 05/07/2021 08:35, Olaf Hering wrote: > Am Fri, 2 Jul 2021 20:03:34 +0100 > schrieb Andrew Cooper <andrew.cooper3@citrix.com>: > >> Fixes: 34990446ca91 ("libxl: don't ignore the return value from xc_cpuid_apply_policy") > I think it fixes 111c8c33a8a18588f3da3c5dbb7f5c63ddb98ce5 ("x86/cpuid: do not expand max leaves on restore"), 34990446ca91 just revealed the bug? Urgh... That's what I intended to write here. I'll fix up. > Either way, this new variant is what I had tested last week. But with a trailing "u" for the constant. This detail may not make a difference in practice. The trailing u doesn't matter in this case. Furthermore, the way min() is written, the compiler will object if it were to be wrong. ~Andrew
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c index e01d657e0394..0c9c4fefc1ef 100644 --- a/tools/libs/guest/xg_cpuid_x86.c +++ b/tools/libs/guest/xg_cpuid_x86.c @@ -513,7 +513,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore, /* Clamp maximum leaves to the ones supported on 4.12. */ p->basic.max_leaf = min(p->basic.max_leaf, 0xdu); p->feat.max_subleaf = 0; - p->extd.max_leaf = min(p->extd.max_leaf, 0x1cu); + p->extd.max_leaf = min(p->extd.max_leaf, 0x8000001c); } if ( featureset )
0x1c is lower than any value which will actually be observed in p->extd.max_leaf, but higher than the logical 9 leaves worth of extended data on Intel systems, causing x86_cpuid_copy_to_buffer() to fail with -ENOBUFS. Correct the calculation. The problem was first noticed in c/s 34990446ca9 "libxl: don't ignore the return value from xc_cpuid_apply_policy" but introduced earlier. Fixes: 34990446ca91 ("libxl: don't ignore the return value from xc_cpuid_apply_policy") Reported-by: Olaf Hering <olaf@aepfle.de> 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> CC: Ian Jackson <iwj@xenproject.org> CC: Olaf Hering <olaf@aepfle.de> Olaf - as I've changed the fix, I haven't included your T-by tag, but I'm confident that this will suitably address the issue. --- tools/libs/guest/xg_cpuid_x86.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)