On 28.11.2023 11:03, Roger Pau Monne wrote:
> --- /dev/null
> +++ b/xen/arch/x86/test-smc.c
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <xen/errno.h>
> +
> +#include <asm/alternative.h>
> +#include <asm/cpufeature.h>
> +#include <asm/test-smc.h>
> +
> +static bool cf_check test_insn_replacement(void)
> +{
> +#define EXPECTED_VALUE 2
> + unsigned int r = ~EXPECTED_VALUE;
The compiler is permitted to elide the initializer unless ...
> + alternative_io("", "mov $" STR(EXPECTED_VALUE) ", %0",
> + X86_FEATURE_ALWAYS, "=r"(r));
... you use "+r" here. Also (nit) there's a blank missing between that
string and the opening parethesis. Also what's wrong with passing
EXPECTED_VALUE in as an "i" constraint input operand?
> + return r == EXPECTED_VALUE;
> +#undef EXPECTED_VALUE
> +}
> +
> +int test_smc(uint32_t selection, uint32_t *results)
> +{
> + struct {
> + unsigned int mask;
> + bool (*test)(void);
> + const char *name;
> + } static const tests[] = {
> + { XEN_SYSCTL_TEST_SMC_INSN_REPL, &test_insn_replacement,
> + "alternative instruction replacement" },
> + };
> + unsigned int i;
> +
> + if ( selection & ~XEN_SYSCTL_TEST_SMC_ALL )
> + return -EINVAL;
> +
> + if ( results )
> + *results = 0;
> +
> + printk(XENLOG_INFO "Checking Self Modify Code\n");
> +
> + for ( i = 0; i < ARRAY_SIZE(tests); i++ )
> + {
> + if ( !(selection & tests[i].mask) )
> + continue;
> +
> + if ( tests[i].test() )
> + {
> + if ( results )
> + *results |= tests[i].mask;
> + continue;
> + }
> +
> + add_taint(TAINT_ERROR_SMC);
> + printk(XENLOG_ERR "%s test failed\n", tests[i].name);
> + }
> +
> + return 0;
> +}
I think I need to look at the next patch to make sense of this.
> @@ -1261,6 +1269,7 @@ struct xen_sysctl {
> struct xen_sysctl_livepatch_op livepatch;
> #if defined(__i386__) || defined(__x86_64__)
> struct xen_sysctl_cpu_policy cpu_policy;
> + struct xen_sysctl_test_smc smc;
Imo the field name would better be test_smc (leaving aside Stefano's comment).
Jan
