On Tue, Dec 05, 2023 at 12:49:38PM +0100, Jan Beulich wrote: > On 28.11.2023 11:03, Roger Pau Monne wrote: > > Introduce a basic livepatch test using the interface to run self modifying > > tests. The introduced test relies on changing a function from returning > > false > > to returning true. > > > > To simplify the burden of keeping a patch that can be provided to > > livepatch-build-tools, introduce two new files: one containing the unpatched > > test functions, and another one that contains the patched forms of such > > functions. Note that only the former is linked into the Xen image, the > > latter > > is built but the object file is not consumed afterwards. Do this to assert > > that the file containing the patched functions continues to build. > > > > Since livepatch testing will ensure that the functions are not patched > > previous > > the applying the livepatch, allow the livepatch related tests to fail > > without > > tainting the hypervisor. > > > > Note the livepatch tests are not run as part of the self modifying checks > > executed during boot, as they would obviously fail. > > > > Signed-off-by: Roger Pau Monné <roger....@citrix.com> > > --- > > Changes since v1: > > - New interface & test. > > --- > > tools/misc/xen-livepatch.c | 29 +++++++++++++++++++++++++++++ > > xen/arch/x86/Makefile | 2 ++ > > xen/arch/x86/include/asm/test-smc.h | 2 ++ > > xen/arch/x86/setup.c | 2 +- > > xen/arch/x86/test-smc-lp-alt.c | 23 +++++++++++++++++++++++ > > xen/arch/x86/test-smc-lp.c | 23 +++++++++++++++++++++++ > > xen/arch/x86/test-smc.c | 11 ++++++++++- > > xen/include/public/sysctl.h | 6 +++++- > > 8 files changed, 95 insertions(+), 3 deletions(-) > > create mode 100644 xen/arch/x86/test-smc-lp-alt.c > > create mode 100644 xen/arch/x86/test-smc-lp.c > > Can these (and perhaps also the one file introduced earlier in the series) > perhaps become xen/arch/x86/test/smc*.c?
Yes, sure, I don't see why not. > > --- /dev/null > > +++ b/xen/arch/x86/test-smc-lp-alt.c > > @@ -0,0 +1,23 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > + > > +#include <asm/test-smc.h> > > + > > +/* > > + * Interesting case because `return false` can be encoded as an xor > > + * instruction, which is shorter than `return true` which is a mov > > instruction, > > + * and also shorter than a jmp instruction. > > + */ > > I'm a little wary of this comment: "mov $1, %al" is two bytes only, just like Don't we need to zero the high part of the register also? Or since the return type is a bool the compiler could assume it's truncated by the caller? > "xor %eax, %eax" is. GCC 13.2 (from godbolt) generates at -O2: mov $0x1,%eax ret Which is 5 bytes long mov insn. The return false case is: xor %eax,%eax ret I can adjust to mention this specific behavior. > > +bool cf_check test_lp_insn_replacement(void) > > What's the purpose of the cf_check here? Because it's added to the array of test functions to call in test_smc(). Doesn't it need cf_check in that case? Thanks, Roger.
