On Fri, 2023-02-10 at 10:07 +0800, Robert Hoo wrote:
> On Thu, 2023-02-09 at 17:27 +0000, Sean Christopherson wrote:
> > On Thu, Feb 09, 2023, Robert Hoo wrote:
> > > On Thu, 2023-02-09 at 14:15 +0800, Chao Gao wrote:
> > > > On Thu, Feb 09, 2023 at 10:40:13AM +0800, Robert Hoo wrote:
> > > > Please add a kvm-unit-test or kselftest for LAM, particularly
> > > > for
> > > > operations (e.g., canonical check for supervisor pointers,
> > > > toggle
> > > > CR4.LAM_SUP) which aren't covered by the test in Kirill's
> > > > series.
> > >
> > > OK, I can explore for kvm-unit-test in separate patch set.
> >
> > Please make tests your top priority. Without tests, I am not going
> > to spend any
> > time reviewing this series, or any other hardware enabling
> > series[*]. I don't
> > expect KVM specific tests for everything, i.e. it's ok to to rely
> > things like
> > running VMs that utilize LAM and/or running LAM selftests in the
> > guest, but I do
> > want a reasonably thorough explanation of how all the test pieces
> > fit
> > together to
> > validate KVM's implementation.
>
> Sure, and ack on unit test is part of development work.
>
> This patch set had always been unit tested before sent out, i.e.
> "running LAM selftests in guest" on both ept=Y/N.
>
> CR4.LAM_SUP, as Chao pointed out, could not be covered by kselftest,
> I
> may explore it in kvm-unit-test.
>
When I come to kvm-unit-test, just find that I had already developed
some test case on CR4.LAM_SUP toggle and carried out on this patch set.
Just forgot about it.
Is it all right? if so, I will include it in next version.
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 3d58ef7..c6b1db6 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -105,6 +105,8 @@
#define X86_CR4_CET BIT(X86_CR4_CET_BIT)
#define X86_CR4_PKS_BIT (24)
#define X86_CR4_PKS BIT(X86_CR4_PKS_BIT)
+#define X86_CR4_LAM_SUP_BIT (28)
+#define X86_CR4_LAM_SUP BIT(X86_CR4_LAM_SUP_BIT)
#define X86_EFLAGS_CF_BIT (0)
#define X86_EFLAGS_CF BIT(X86_EFLAGS_CF_BIT)
@@ -248,6 +250,7 @@ static inline bool is_intel(void)
#define X86_FEATURE_SPEC_CTRL (CPUID(0x7, 0, EDX,
26))
#define X86_FEATURE_ARCH_CAPABILITIES (CPUID(0x7, 0, EDX,
29))
#define X86_FEATURE_PKS (CPUID(0x7, 0, ECX,
31))
+#define X86_FEATURE_LAM (CPUID(0x7, 1, EAX,
26))
/*
* Extended Leafs, a.k.a. AMD defined
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
index f483dea..af626cc 100644
--- a/x86/Makefile.x86_64
+++ b/x86/Makefile.x86_64
@@ -34,6 +34,7 @@ tests += $(TEST_DIR)/rdpru.$(exe)
tests += $(TEST_DIR)/pks.$(exe)
tests += $(TEST_DIR)/pmu_lbr.$(exe)
tests += $(TEST_DIR)/pmu_pebs.$(exe)
+tests += $(TEST_DIR)/lam_sup.$(exe)
ifeq ($(CONFIG_EFI),y)
tests += $(TEST_DIR)/amd_sev.$(exe)
diff --git a/x86/lam_sup.c b/x86/lam_sup.c
new file mode 100644
index 0000000..3e05129
--- /dev/null
+++ b/x86/lam_sup.c
@@ -0,0 +1,37 @@
+#include "libcflat.h"
+#include "processor.h"
+#include "desc.h"
+
+int main(int ac, char **av)
+{
+ unsigned long cr4;
+ struct cpuid c = {0, 0, 0, 0};
+
+ c = cpuid_indexed(7, 1);
+
+ if (!(c.a & (1 << 26))) {
+ report_skip("LAM is not supported. EAX = 0x%0x", c.a);
+ abort();
+ }
+
+ report_info("set CR4.LAM_SUP(bit 28)");
+
+ cr4 = read_cr4();
+ write_cr4(cr4 | X86_CR4_LAM_SUP);
+
+ report((cr4 | X86_CR4_LAM_SUP) == read_cr4(), "Set CR4.LAM_SUP
succeeded.");
+
+ report_info("clear CR4.LAM_SUP(bit 28)");
+
+ cr4 = read_cr4();
+ write_cr4(cr4 & ~X86_CR4_LAM_SUP);
+
+ report((cr4 & ~X86_CR4_LAM_SUP) == read_cr4(), "Clear
CR4.LAM_SUP succeeded.");
+
+ return report_summary();
+}
+
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index f324e32..0c90dfe 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -478,3 +478,8 @@ file = cet.flat
arch = x86_64
smp = 2
extra_params = -enable-kvm -m 2048 -cpu host
+
+[intel-lam]
+file = lam_sup.flat
+arch = x86_64
+extra_params = -enable-kvm -cpu host
