XTF: tests SPEC_CTRL added bits

[PATCH] XTF: tests SPEC_CTRL added bits

Posted by Roger Pau Monne 2 months, 3 weeks ago

Dummy set/clear tests for additional spec_ctrl bits.
---
 docs/all-tests.dox  |   2 +
 tests/test/Makefile |   9 ++++
 tests/test/main.c   | 100 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+)
 create mode 100644 tests/test/Makefile
 create mode 100644 tests/test/main.c

diff --git a/docs/all-tests.dox b/docs/all-tests.dox
index 892a9e474743..5a66ac252ea5 100644
--- a/docs/all-tests.dox
+++ b/docs/all-tests.dox
@@ -187,3 +187,5 @@ states.
 
 @subpage test-nested-vmx - Nested VT-x tests.
 */
+# Placeholder: Merge into the appropriate location above
+@subpage test-test - @todo title
diff --git a/tests/test/Makefile b/tests/test/Makefile
new file mode 100644
index 000000000000..19bc4b6a4639
--- /dev/null
+++ b/tests/test/Makefile
@@ -0,0 +1,9 @@
+include $(ROOT)/build/common.mk
+
+NAME      := test
+CATEGORY  := utility
+TEST-ENVS := hvm32 pv64
+
+obj-perenv += main.o
+
+include $(ROOT)/build/gen.mk
diff --git a/tests/test/main.c b/tests/test/main.c
new file mode 100644
index 000000000000..9a25e95d91b7
--- /dev/null
+++ b/tests/test/main.c
@@ -0,0 +1,100 @@
+/**
+ * @file tests/test/main.c
+ * @ref test-test
+ *
+ * @page test-test test
+ *
+ * @todo Docs for test-test
+ *
+ * @see tests/test/main.c
+ */
+#include <xtf.h>
+
+#define MSR_SPEC_CTRL                       0x00000048
+#define  SPEC_CTRL_IPRED_DIS_U              (_AC(1, ULL) <<  3)
+#define  SPEC_CTRL_IPRED_DIS_S              (_AC(1, ULL) <<  4)
+#define  SPEC_CTRL_RRSBA_DIS_U              (_AC(1, ULL) <<  5)
+#define  SPEC_CTRL_RRSBA_DIS_S              (_AC(1, ULL) <<  6)
+#define  SPEC_CTRL_DDP_DIS_U                (_AC(1, ULL) <<  8)
+#define  SPEC_CTRL_BHI_DIS_S                (_AC(1, ULL) << 10)
+
+const char test_title[] = "SPEC_CTRL";
+
+static void update_spec_ctrl(uint64_t mask, bool set)
+{
+    uint64_t spec_ctrl = rdmsr(MSR_SPEC_CTRL);
+
+    if ( set )
+        spec_ctrl |= mask;
+    else
+        spec_ctrl &= ~mask;
+
+    wrmsr(MSR_SPEC_CTRL, spec_ctrl);
+}
+
+static void assert_spec_ctrl(uint64_t mask, bool set)
+{
+    uint64_t spec_ctrl = rdmsr(MSR_SPEC_CTRL);
+
+    if ( (spec_ctrl & mask) != (set ? mask : 0) )
+    {
+        xtf_failure("SPEC_CTRL expected: %#" PRIx64 " got: %#" PRIx64 "\n",
+                    set ? (spec_ctrl | mask) : (spec_ctrl & ~mask),
+                    spec_ctrl);
+        xtf_exit();
+    }
+}
+
+static void test_loop(uint64_t mask)
+{
+    update_spec_ctrl(mask, true);
+    assert_spec_ctrl(mask, true);
+    /* Ensure context switch to Xen. */
+    hypercall_yield();
+    assert_spec_ctrl(mask, true);
+
+    update_spec_ctrl(mask, false);
+    assert_spec_ctrl(mask, false);
+    /* Ensure context switch to Xen. */
+    hypercall_yield();
+    assert_spec_ctrl(mask, false);
+}
+
+void test_main(void)
+{
+    static const struct {
+        const char *name;
+        unsigned int feat;
+        uint64_t mask;
+    } tests[] = {
+        { "IPRED CTRL", 1, SPEC_CTRL_IPRED_DIS_U | SPEC_CTRL_IPRED_DIS_S },
+        { "RRSBA CTRL", 2, SPEC_CTRL_RRSBA_DIS_U | SPEC_CTRL_RRSBA_DIS_S },
+        { "DDP DIS", 3, SPEC_CTRL_DDP_DIS_U },
+        { "BHI DIS", 4, SPEC_CTRL_BHI_DIS_S },
+    };
+    unsigned int i;
+    uint32_t regs[4];
+
+    cpuid_count(7, 2, &regs[0], &regs[1], &regs[2], &regs[3]);
+
+    for ( i = 0; i < ARRAY_SIZE(tests); i++ )
+    {
+        if ( !test_bit(tests[i].feat, &regs[3]) )
+            continue;
+
+        printk("Testing %s\n", tests[i].name);
+        test_loop(tests[i].mask);
+    }
+
+    xtf_success(NULL);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.43.0

Re: [PATCH] XTF: tests SPEC_CTRL added bits

Posted by Jan Beulich 2 months, 3 weeks ago

On 30.01.2024 11:27, Roger Pau Monne wrote:
> Dummy set/clear tests for additional spec_ctrl bits.
> ---
>  docs/all-tests.dox  |   2 +
>  tests/test/Makefile |   9 ++++
>  tests/test/main.c   | 100 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 111 insertions(+)
>  create mode 100644 tests/test/Makefile
>  create mode 100644 tests/test/main.c

I'm puzzled: Why "test"? That doesn't describe in any way what this test
is about.

> --- /dev/null
> +++ b/tests/test/Makefile
> @@ -0,0 +1,9 @@
> +include $(ROOT)/build/common.mk
> +
> +NAME      := test
> +CATEGORY  := utility
> +TEST-ENVS := hvm32 pv64

Any reason for this limitation?

> --- /dev/null
> +++ b/tests/test/main.c
> @@ -0,0 +1,100 @@
> +/**
> + * @file tests/test/main.c
> + * @ref test-test
> + *
> + * @page test-test test
> + *
> + * @todo Docs for test-test
> + *
> + * @see tests/test/main.c
> + */
> +#include <xtf.h>
> +
> +#define MSR_SPEC_CTRL                       0x00000048
> +#define  SPEC_CTRL_IPRED_DIS_U              (_AC(1, ULL) <<  3)
> +#define  SPEC_CTRL_IPRED_DIS_S              (_AC(1, ULL) <<  4)
> +#define  SPEC_CTRL_RRSBA_DIS_U              (_AC(1, ULL) <<  5)
> +#define  SPEC_CTRL_RRSBA_DIS_S              (_AC(1, ULL) <<  6)
> +#define  SPEC_CTRL_DDP_DIS_U                (_AC(1, ULL) <<  8)
> +#define  SPEC_CTRL_BHI_DIS_S                (_AC(1, ULL) << 10)
> +
> +const char test_title[] = "SPEC_CTRL";
> +
> +static void update_spec_ctrl(uint64_t mask, bool set)
> +{
> +    uint64_t spec_ctrl = rdmsr(MSR_SPEC_CTRL);
> +
> +    if ( set )
> +        spec_ctrl |= mask;
> +    else
> +        spec_ctrl &= ~mask;
> +
> +    wrmsr(MSR_SPEC_CTRL, spec_ctrl);
> +}
> +
> +static void assert_spec_ctrl(uint64_t mask, bool set)
> +{
> +    uint64_t spec_ctrl = rdmsr(MSR_SPEC_CTRL);
> +
> +    if ( (spec_ctrl & mask) != (set ? mask : 0) )
> +    {
> +        xtf_failure("SPEC_CTRL expected: %#" PRIx64 " got: %#" PRIx64 "\n",
> +                    set ? (spec_ctrl | mask) : (spec_ctrl & ~mask),
> +                    spec_ctrl);
> +        xtf_exit();
> +    }
> +}
> +
> +static void test_loop(uint64_t mask)
> +{
> +    update_spec_ctrl(mask, true);
> +    assert_spec_ctrl(mask, true);
> +    /* Ensure context switch to Xen. */
> +    hypercall_yield();

I'm afraid yielding doesn't guarantee context switching in Xen, if the
system (or even just the one CPU) is otherwise idle. Hence at the very
least please don't say "ensure" in the comment. But perhaps more
reliable to e.g. use "poll" with a timeout. While I didn't post that
addition, I've used such for testing my vCPU-area-registration work:

        struct sched_poll poll = { .timeout = s + SECONDS(1) };
        rc = hypercall_sched_op(SCHEDOP_poll, &poll);
        if ( rc )
            xtf_error("Could not poll (%d)\n", rc);

(there also to ensure enough time passes for the time area to be
updated).

I actually found this to have another neat side effect: The guest then
can't go away so quickly that "xl console" doesn't manage to attach to
the guest (which otherwise I observe to work only about every other
time).

Jan

Re: [PATCH] XTF: tests SPEC_CTRL added bits

Posted by Roger Pau Monné 2 months, 3 weeks ago

On Tue, Jan 30, 2024 at 11:42:43AM +0100, Jan Beulich wrote:
> On 30.01.2024 11:27, Roger Pau Monne wrote:
> > Dummy set/clear tests for additional spec_ctrl bits.
> > ---
> >  docs/all-tests.dox  |   2 +
> >  tests/test/Makefile |   9 ++++
> >  tests/test/main.c   | 100 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 111 insertions(+)
> >  create mode 100644 tests/test/Makefile
> >  create mode 100644 tests/test/main.c
> 
> I'm puzzled: Why "test"? That doesn't describe in any way what this test
> is about.

That's just my place holder for random XTF stuff.  I don't intend this
to be committed.

> > --- /dev/null
> > +++ b/tests/test/Makefile
> > @@ -0,0 +1,9 @@
> > +include $(ROOT)/build/common.mk
> > +
> > +NAME      := test
> > +CATEGORY  := utility
> > +TEST-ENVS := hvm32 pv64
> 
> Any reason for this limitation?

Just wanted a PV and an HVM context.

> > --- /dev/null
> > +++ b/tests/test/main.c
> > @@ -0,0 +1,100 @@
> > +/**
> > + * @file tests/test/main.c
> > + * @ref test-test
> > + *
> > + * @page test-test test
> > + *
> > + * @todo Docs for test-test
> > + *
> > + * @see tests/test/main.c
> > + */
> > +#include <xtf.h>
> > +
> > +#define MSR_SPEC_CTRL                       0x00000048
> > +#define  SPEC_CTRL_IPRED_DIS_U              (_AC(1, ULL) <<  3)
> > +#define  SPEC_CTRL_IPRED_DIS_S              (_AC(1, ULL) <<  4)
> > +#define  SPEC_CTRL_RRSBA_DIS_U              (_AC(1, ULL) <<  5)
> > +#define  SPEC_CTRL_RRSBA_DIS_S              (_AC(1, ULL) <<  6)
> > +#define  SPEC_CTRL_DDP_DIS_U                (_AC(1, ULL) <<  8)
> > +#define  SPEC_CTRL_BHI_DIS_S                (_AC(1, ULL) << 10)
> > +
> > +const char test_title[] = "SPEC_CTRL";
> > +
> > +static void update_spec_ctrl(uint64_t mask, bool set)
> > +{
> > +    uint64_t spec_ctrl = rdmsr(MSR_SPEC_CTRL);
> > +
> > +    if ( set )
> > +        spec_ctrl |= mask;
> > +    else
> > +        spec_ctrl &= ~mask;
> > +
> > +    wrmsr(MSR_SPEC_CTRL, spec_ctrl);
> > +}
> > +
> > +static void assert_spec_ctrl(uint64_t mask, bool set)
> > +{
> > +    uint64_t spec_ctrl = rdmsr(MSR_SPEC_CTRL);
> > +
> > +    if ( (spec_ctrl & mask) != (set ? mask : 0) )
> > +    {
> > +        xtf_failure("SPEC_CTRL expected: %#" PRIx64 " got: %#" PRIx64 "\n",
> > +                    set ? (spec_ctrl | mask) : (spec_ctrl & ~mask),
> > +                    spec_ctrl);
> > +        xtf_exit();
> > +    }
> > +}
> > +
> > +static void test_loop(uint64_t mask)
> > +{
> > +    update_spec_ctrl(mask, true);
> > +    assert_spec_ctrl(mask, true);
> > +    /* Ensure context switch to Xen. */
> > +    hypercall_yield();
> 
> I'm afraid yielding doesn't guarantee context switching in Xen,

It will ensure a vmexit/trap, which is what I was after here.  Maybe the
comment should be "Trap into Xen." or some such.  It wasn't about
ensuring VM context switching.

Thanks, Roger.

Re: [PATCH] XTF: tests SPEC_CTRL added bits

Posted by Jan Beulich 2 months, 3 weeks ago

On 30.01.2024 12:46, Roger Pau Monné wrote:
> On Tue, Jan 30, 2024 at 11:42:43AM +0100, Jan Beulich wrote:
>> On 30.01.2024 11:27, Roger Pau Monne wrote:
>>> Dummy set/clear tests for additional spec_ctrl bits.
>>> ---
>>>  docs/all-tests.dox  |   2 +
>>>  tests/test/Makefile |   9 ++++
>>>  tests/test/main.c   | 100 ++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 111 insertions(+)
>>>  create mode 100644 tests/test/Makefile
>>>  create mode 100644 tests/test/main.c
>>
>> I'm puzzled: Why "test"? That doesn't describe in any way what this test
>> is about.
> 
> That's just my place holder for random XTF stuff.  I don't intend this
> to be committed.

Could have been said then one way or another.

Jan

Re: [PATCH] XTF: tests SPEC_CTRL added bits

Posted by Roger Pau Monné 2 months, 3 weeks ago

On Tue, Jan 30, 2024 at 01:55:56PM +0100, Jan Beulich wrote:
> On 30.01.2024 12:46, Roger Pau Monné wrote:
> > On Tue, Jan 30, 2024 at 11:42:43AM +0100, Jan Beulich wrote:
> >> On 30.01.2024 11:27, Roger Pau Monne wrote:
> >>> Dummy set/clear tests for additional spec_ctrl bits.
> >>> ---
> >>>  docs/all-tests.dox  |   2 +
> >>>  tests/test/Makefile |   9 ++++
> >>>  tests/test/main.c   | 100 ++++++++++++++++++++++++++++++++++++++++++++
> >>>  3 files changed, 111 insertions(+)
> >>>  create mode 100644 tests/test/Makefile
> >>>  create mode 100644 tests/test/main.c
> >>
> >> I'm puzzled: Why "test"? That doesn't describe in any way what this test
> >> is about.
> > 
> > That's just my place holder for random XTF stuff.  I don't intend this
> > to be committed.
> 
> Could have been said then one way or another.

Yes, realized later when speaking with Andrew that I had forgot to send
the test I've used, and then didn't adjust the message when sending to
note this wasn't supposed to be applied.

Regards, Roger.

Re: [PATCH] XTF: tests SPEC_CTRL added bits

Posted by Andrew Cooper 2 months, 3 weeks ago

On 30/01/2024 3:02 pm, Roger Pau Monné wrote:
> On Tue, Jan 30, 2024 at 01:55:56PM +0100, Jan Beulich wrote:
>> On 30.01.2024 12:46, Roger Pau Monné wrote:
>>> On Tue, Jan 30, 2024 at 11:42:43AM +0100, Jan Beulich wrote:
>>>> On 30.01.2024 11:27, Roger Pau Monne wrote:
>>>>> Dummy set/clear tests for additional spec_ctrl bits.
>>>>> ---
>>>>>  docs/all-tests.dox  |   2 +
>>>>>  tests/test/Makefile |   9 ++++
>>>>>  tests/test/main.c   | 100 ++++++++++++++++++++++++++++++++++++++++++++
>>>>>  3 files changed, 111 insertions(+)
>>>>>  create mode 100644 tests/test/Makefile
>>>>>  create mode 100644 tests/test/main.c
>>>> I'm puzzled: Why "test"? That doesn't describe in any way what this test
>>>> is about.
>>> That's just my place holder for random XTF stuff.  I don't intend this
>>> to be committed.
>> Could have been said then one way or another.
> Yes, realized later when speaking with Andrew that I had forgot to send
> the test I've used, and then didn't adjust the message when sending to
> note this wasn't supposed to be applied.

I've got a local test with some of this in, which I'll extend.

But as with many other things, it's waiting on fixing the test-revision
build infrastructure so the OSSTest Bisector doesn't break when adding
new content to an existing test.

~Andrew