On Thu, Nov 30, 2023 at 06:02:55PM +0100, Jan Beulich wrote:
> 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.
I see, '=' assumes the operand is always written to, which is not the
case if alternative is not applied.
> 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?
Me not knowing enough inline assembly I think, that's what's wrong.
> > @@ -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).
Right, will see what Stefano thinks about using test_smoc.
Thanks, Roger.
