* [XTF PATCH 01/16] vvmx: test whether VMX feature is present in CPUID
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 14:40 ` Andrew Cooper
2016-12-16 13:43 ` [XTF PATCH 02/16] vvmx: test whether MSR_IA32_FEATURE_CONTROL is set correctly Haozhong Zhang
` (16 subsequent siblings)
17 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
cpuid.1:ecx[5] is expected to be set in this test.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/Makefile | 11 +++++++++++
tests/vvmx/cpuid.c | 24 ++++++++++++++++++++++++
| 1 +
tests/vvmx/main.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 80 insertions(+)
create mode 100644 tests/vvmx/Makefile
create mode 100644 tests/vvmx/cpuid.c
create mode 100644 tests/vvmx/extra.cfg.in
create mode 100644 tests/vvmx/main.c
diff --git a/tests/vvmx/Makefile b/tests/vvmx/Makefile
new file mode 100644
index 0000000..80a6629
--- /dev/null
+++ b/tests/vvmx/Makefile
@@ -0,0 +1,11 @@
+include $(ROOT)/build/common.mk
+
+NAME := vvmx
+CATEGORY := functional
+TEST-ENVS := hvm64
+
+TEST-EXTRA-CFG := extra.cfg.in
+
+obj-perenv += main.o cpuid.o
+
+include $(ROOT)/build/gen.mk
diff --git a/tests/vvmx/cpuid.c b/tests/vvmx/cpuid.c
new file mode 100644
index 0000000..9a4cdae
--- /dev/null
+++ b/tests/vvmx/cpuid.c
@@ -0,0 +1,24 @@
+#include <xtf.h>
+
+bool test_cpuid_vmx_feat(void)
+{
+ uint32_t ecx = cpuid_ecx(1);
+
+ if ( !(ecx & X86_FEATURE_VMX) )
+ {
+ xtf_failure("Fail: cpuid.1:ecx[5] is not set.\n");
+ return false;
+ }
+
+ return true;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--git a/tests/vvmx/extra.cfg.in b/tests/vvmx/extra.cfg.in
new file mode 100644
index 0000000..ae494f8
--- /dev/null
+++ b/tests/vvmx/extra.cfg.in
@@ -0,0 +1 @@
+nestedhvm = 1
diff --git a/tests/vvmx/main.c b/tests/vvmx/main.c
new file mode 100644
index 0000000..8506b7b
--- /dev/null
+++ b/tests/vvmx/main.c
@@ -0,0 +1,44 @@
+/**
+ * @file tests/vvmx/main.c
+ * @ref test-vvmx
+ *
+ * @page test-vvmx vvmx
+ *
+ * Test VMX features (CPUID, MSR, VMX instructions, EPT, APIC etc.) in
+ * the nested VMX environment.
+ *
+ * @see tests/vvmx/main.c
+ */
+#include <xtf.h>
+
+const char test_title[] = "Test vvmx";
+
+extern bool test_cpuid_vmx_feat(void);
+
+void test_main(void)
+{
+ if ( !vendor_is(X86_VENDOR_INTEL) )
+ {
+ xtf_skip("Skip: non-Intel processors\n");
+ return;
+ }
+
+ if ( !test_cpuid_vmx_feat() )
+ goto fail;
+
+ xtf_success(NULL);
+ return;
+
+fail:
+ xtf_failure(NULL);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [XTF PATCH 01/16] vvmx: test whether VMX feature is present in CPUID
2016-12-16 13:43 ` [XTF PATCH 01/16] vvmx: test whether VMX feature is present in CPUID Haozhong Zhang
@ 2016-12-16 14:40 ` Andrew Cooper
2016-12-19 1:51 ` Haozhong Zhang
0 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 14:40 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel
On 16/12/16 13:43, Haozhong Zhang wrote:
> diff --git a/tests/vvmx/cpuid.c b/tests/vvmx/cpuid.c
> new file mode 100644
> index 0000000..9a4cdae
> --- /dev/null
> +++ b/tests/vvmx/cpuid.c
> @@ -0,0 +1,24 @@
> +#include <xtf.h>
> +
> +bool test_cpuid_vmx_feat(void)
> +{
> + uint32_t ecx = cpuid_ecx(1);
> +
> + if ( !(ecx & X86_FEATURE_VMX) )
> + {
> + xtf_failure("Fail: cpuid.1:ecx[5] is not set.\n");
> + return false;
> + }
Some cpuid information is cached at boot.
It turns out that there is already a suitable cpu_has_vmx define.
> +#include <xtf.h>
> +
> +const char test_title[] = "Test vvmx";
> +
> +extern bool test_cpuid_vmx_feat(void);
> +
> +void test_main(void)
> +{
> + if ( !vendor_is(X86_VENDOR_INTEL) )
There is a slightly shorter vendor_is_intel which you can use.
> + {
> + xtf_skip("Skip: non-Intel processors\n");
"processor"
> + return;
Where it makes the code easier to read, I tend to use return
xtf_skip("Skip: non-Intel processor\n"), which in this case allows the
braces to be dropped. However, I am not overly fussed if you prefer
this style.
> + }
> +
> + if ( !test_cpuid_vmx_feat() )
> + goto fail;
Are you intending to do converse tests? We have had security issues in
the past where some of the nested-virt code in Xen was reachable from a
guest even through the feature was intended to be fully disabled.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 01/16] vvmx: test whether VMX feature is present in CPUID
2016-12-16 14:40 ` Andrew Cooper
@ 2016-12-19 1:51 ` Haozhong Zhang
0 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 1:51 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/16/16 14:40 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>> diff --git a/tests/vvmx/cpuid.c b/tests/vvmx/cpuid.c
>> new file mode 100644
>> index 0000000..9a4cdae
>> --- /dev/null
>> +++ b/tests/vvmx/cpuid.c
>> @@ -0,0 +1,24 @@
>> +#include <xtf.h>
>> +
>> +bool test_cpuid_vmx_feat(void)
>> +{
>> + uint32_t ecx = cpuid_ecx(1);
>> +
>> + if ( !(ecx & X86_FEATURE_VMX) )
>> + {
>> + xtf_failure("Fail: cpuid.1:ecx[5] is not set.\n");
>> + return false;
>> + }
>
>Some cpuid information is cached at boot.
>
>It turns out that there is already a suitable cpu_has_vmx define.
>
Ah yes, it's what I want.
>> +#include <xtf.h>
>> +
>> +const char test_title[] = "Test vvmx";
>> +
>> +extern bool test_cpuid_vmx_feat(void);
>> +
>> +void test_main(void)
>> +{
>> + if ( !vendor_is(X86_VENDOR_INTEL) )
>
>There is a slightly shorter vendor_is_intel which you can use.
>
ditto
>> + {
>> + xtf_skip("Skip: non-Intel processors\n");
>
>"processor"
>
will change
>> + return;
>
>Where it makes the code easier to read, I tend to use return
>xtf_skip("Skip: non-Intel processor\n"), which in this case allows the
>braces to be dropped. However, I am not overly fussed if you prefer
>this style.
>
Your suggestion is more clear and I'll change.
>> + }
>> +
>> + if ( !test_cpuid_vmx_feat() )
>> + goto fail;
>
>Are you intending to do converse tests? We have had security issues in
>the past where some of the nested-virt code in Xen was reachable from a
>guest even through the feature was intended to be fully disabled.
>
I should follow the pseudo code in Intel SDM more closely, as the
pseudo code has a branch saying if vmxon is executed w/o VMX support,
there will be some errors. I'll add test cases for such circumstance.
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [XTF PATCH 02/16] vvmx: test whether MSR_IA32_FEATURE_CONTROL is set correctly
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
2016-12-16 13:43 ` [XTF PATCH 01/16] vvmx: test whether VMX feature is present in CPUID Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 16:17 ` Andrew Cooper
2016-12-16 13:43 ` [XTF PATCH 03/16] vvmx: test whether MSR_IA32_VMX_BASIC " Haozhong Zhang
` (15 subsequent siblings)
17 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
Guest MSR_IA32_FEATURE_CONTROL is set by Xen hypervisor instead by
guest firmware or hvmloader, so this test instead checks whether bits
in MSR_IA32_FEATURE_CONTROL are set correctly, rather than requiring
they are all zeroed.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
include/arch/x86/msr-index.h | 5 ++++
tests/vvmx/Makefile | 2 +-
tests/vvmx/main.c | 4 +++
tests/vvmx/msr.c | 67 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 77 insertions(+), 1 deletion(-)
create mode 100644 tests/vvmx/msr.c
diff --git a/include/arch/x86/msr-index.h b/include/arch/x86/msr-index.h
index d479239..f9867d5 100644
--- a/include/arch/x86/msr-index.h
+++ b/include/arch/x86/msr-index.h
@@ -3,6 +3,11 @@
#include <xtf/numbers.h>
+#define MSR_IA32_FEATURE_CONTROL 0x0000003a
+#define IA32_FEATURE_CONTROL_LOCK 0x0001
+#define IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX 0x0002
+#define IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX 0x0004
+
#define MSR_INTEL_PLATFORM_INFO 0x000000ce
#define _MSR_PLATFORM_INFO_CPUID_FAULTING 31
#define MSR_PLATFORM_INFO_CPUID_FAULTING (1ULL << _MSR_PLATFORM_INFO_CPUID_FAULTING)
diff --git a/tests/vvmx/Makefile b/tests/vvmx/Makefile
index 80a6629..54769fa 100644
--- a/tests/vvmx/Makefile
+++ b/tests/vvmx/Makefile
@@ -6,6 +6,6 @@ TEST-ENVS := hvm64
TEST-EXTRA-CFG := extra.cfg.in
-obj-perenv += main.o cpuid.o
+obj-perenv += main.o cpuid.o msr.o
include $(ROOT)/build/gen.mk
diff --git a/tests/vvmx/main.c b/tests/vvmx/main.c
index 8506b7b..bd36f10 100644
--- a/tests/vvmx/main.c
+++ b/tests/vvmx/main.c
@@ -14,6 +14,7 @@
const char test_title[] = "Test vvmx";
extern bool test_cpuid_vmx_feat(void);
+extern bool test_msr_vmx(void);
void test_main(void)
{
@@ -26,6 +27,9 @@ void test_main(void)
if ( !test_cpuid_vmx_feat() )
goto fail;
+ if ( !test_msr_vmx() )
+ goto fail;
+
xtf_success(NULL);
return;
diff --git a/tests/vvmx/msr.c b/tests/vvmx/msr.c
new file mode 100644
index 0000000..100491d
--- /dev/null
+++ b/tests/vvmx/msr.c
@@ -0,0 +1,67 @@
+#include <xtf.h>
+
+#include <arch/x86/msr-index.h>
+
+/*
+ * NB. Guest MSR_IA32_FEATURE_CONTROL is set by Xen hypervisor instead
+ * by guest firmware or hvmloader, so this test checks whether bits in
+ * MSR_IA32_FEATURE_CONTROL are set correctly and does not require they
+ * are all zero.
+ *
+ * TODO: If the behavior in above NB is changed in future, remember to
+ * adjust this test.
+ */
+static bool test_msr_feature_control(void)
+{
+ bool passed = true;
+ uint32_t cpuid_feat_ecx = cpuid_ecx(1);
+ uint64_t msr_feat_ctrl;
+
+ if ( rdmsr_safe(MSR_IA32_FEATURE_CONTROL, &msr_feat_ctrl) )
+ {
+ xtf_failure("Fail: fault when rdmsr MSR_IA32_FEATURE_CONTROL\n");
+ passed = false;
+ goto out;
+ }
+
+ if ( (msr_feat_ctrl & IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX) &&
+ !(cpuid_feat_ecx & X86_FEATURE_SMX) )
+ {
+ xtf_failure("Fail: MSR_IA32_FEATURE_CONTROL[1] is set "
+ "but SMX is not supported\n");
+ passed = false;
+ }
+
+ if ( !(msr_feat_ctrl & IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX) )
+ {
+ xtf_failure("Fail: MSR_IA32_FEATURE_CONTROL[2] is not set\n");
+ passed = false;
+ }
+
+ if ( !(msr_feat_ctrl & IA32_FEATURE_CONTROL_LOCK) )
+ {
+ xtf_failure("Fail: MSR_IA32_FEATURE_CONTROL[0] is not set\n");
+ passed = false;
+ }
+
+out:
+ return passed;
+}
+
+bool test_msr_vmx(void)
+{
+ if ( !test_msr_feature_control() )
+ return false;
+
+ return true;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [XTF PATCH 02/16] vvmx: test whether MSR_IA32_FEATURE_CONTROL is set correctly
2016-12-16 13:43 ` [XTF PATCH 02/16] vvmx: test whether MSR_IA32_FEATURE_CONTROL is set correctly Haozhong Zhang
@ 2016-12-16 16:17 ` Andrew Cooper
2016-12-19 2:20 ` Haozhong Zhang
0 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 16:17 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel
On 16/12/16 13:43, Haozhong Zhang wrote:
> diff --git a/tests/vvmx/msr.c b/tests/vvmx/msr.c
> new file mode 100644
> index 0000000..100491d
> --- /dev/null
> +++ b/tests/vvmx/msr.c
> @@ -0,0 +1,67 @@
> +#include <xtf.h>
> +
> +#include <arch/x86/msr-index.h>
> +
> +/*
> + * NB. Guest MSR_IA32_FEATURE_CONTROL is set by Xen hypervisor instead
> + * by guest firmware or hvmloader, so this test checks whether bits in
> + * MSR_IA32_FEATURE_CONTROL are set correctly and does not require they
> + * are all zero.
> + *
> + * TODO: If the behavior in above NB is changed in future, remember to
> + * adjust this test.
What are the purposes of these lock bits on real hardware? I presume it
is to allow a user choice in the BIOS, but to force the choice to be
immutable once the OS loads?
If so, I don't expect Xen's behaviour to ever change. We'd prefer to do
all configuration like that at the toolstack/hypervisor level, rather
than the in-guest firmware (if any).
> + */
> +static bool test_msr_feature_control(void)
> +{
> + bool passed = true;
> + uint32_t cpuid_feat_ecx = cpuid_ecx(1);
> + uint64_t msr_feat_ctrl;
> +
> + if ( rdmsr_safe(MSR_IA32_FEATURE_CONTROL, &msr_feat_ctrl) )
> + {
> + xtf_failure("Fail: fault when rdmsr MSR_IA32_FEATURE_CONTROL\n");
"Fail: Fault when reading MSR_IA32_FEATURE_CONTROL\n" would be slightly
clearer.
> + passed = false;
The status functions including xtf_failure and xtf_success are sticky,
worst-takes-priority. Therefore, you don't need the xtf_failure(NULL)
case in test_main(). You can also even leave via the xtf_success(NULL)
case and the overall report will be FAILURE.
> + goto out;
In the past, I have found it very useful to proceed with as many checks
as reasonable, even in the face of a failure.
Sometime, a subtle change in Xen can have a cascade failure for the
guest, and seeing all the failures at once can help identify which area
went wrong.
> + }
> +
> + if ( (msr_feat_ctrl & IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX) &&
> + !(cpuid_feat_ecx & X86_FEATURE_SMX) )
cpu_has_smx please.
~Andrew
> + {
> + xtf_failure("Fail: MSR_IA32_FEATURE_CONTROL[1] is set "
> + "but SMX is not supported\n");
> + passed = false;
> + }
> +
> + if ( !(msr_feat_ctrl & IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX) )
> + {
> + xtf_failure("Fail: MSR_IA32_FEATURE_CONTROL[2] is not set\n");
> + passed = false;
> + }
> +
> + if ( !(msr_feat_ctrl & IA32_FEATURE_CONTROL_LOCK) )
> + {
> + xtf_failure("Fail: MSR_IA32_FEATURE_CONTROL[0] is not set\n");
> + passed = false;
> + }
> +
> +out:
> + return passed;
> +}
> +
> +bool test_msr_vmx(void)
> +{
> + if ( !test_msr_feature_control() )
> + return false;
> +
> + return true;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 02/16] vvmx: test whether MSR_IA32_FEATURE_CONTROL is set correctly
2016-12-16 16:17 ` Andrew Cooper
@ 2016-12-19 2:20 ` Haozhong Zhang
2016-12-19 15:29 ` Andrew Cooper
0 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 2:20 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/16/16 16:17 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>> diff --git a/tests/vvmx/msr.c b/tests/vvmx/msr.c
>> new file mode 100644
>> index 0000000..100491d
>> --- /dev/null
>> +++ b/tests/vvmx/msr.c
>> @@ -0,0 +1,67 @@
>> +#include <xtf.h>
>> +
>> +#include <arch/x86/msr-index.h>
>> +
>> +/*
>> + * NB. Guest MSR_IA32_FEATURE_CONTROL is set by Xen hypervisor instead
>> + * by guest firmware or hvmloader, so this test checks whether bits in
>> + * MSR_IA32_FEATURE_CONTROL are set correctly and does not require they
>> + * are all zero.
>> + *
>> + * TODO: If the behavior in above NB is changed in future, remember to
>> + * adjust this test.
>
>What are the purposes of these lock bits on real hardware? I presume it
>is to allow a user choice in the BIOS, but to force the choice to be
>immutable once the OS loads?
>
same as my understanding
>If so, I don't expect Xen's behaviour to ever change. We'd prefer to do
>all configuration like that at the toolstack/hypervisor level, rather
>than the in-guest firmware (if any).
>
OK, then TODO is not necessary at all. My concern was that Intel SDM
says MSR_IA32_FEATURE_CONTROL is cleared to zero when a logical
processor is reset that is not consistent to what Xen currently
presents in HVM domains.
>> + */
>> +static bool test_msr_feature_control(void)
>> +{
>> + bool passed = true;
>> + uint32_t cpuid_feat_ecx = cpuid_ecx(1);
>> + uint64_t msr_feat_ctrl;
>> +
>> + if ( rdmsr_safe(MSR_IA32_FEATURE_CONTROL, &msr_feat_ctrl) )
>> + {
>> + xtf_failure("Fail: fault when rdmsr MSR_IA32_FEATURE_CONTROL\n");
>
>"Fail: Fault when reading MSR_IA32_FEATURE_CONTROL\n" would be slightly
>clearer.
>
will change
>> + passed = false;
>
>The status functions including xtf_failure and xtf_success are sticky,
>worst-takes-priority. Therefore, you don't need the xtf_failure(NULL)
>case in test_main(). You can also even leave via the xtf_success(NULL)
>case and the overall report will be FAILURE.
>
Thanks for reminding me of this which I didn't notice.
>> + goto out;
>
>In the past, I have found it very useful to proceed with as many checks
>as reasonable, even in the face of a failure.
>
>Sometime, a subtle change in Xen can have a cascade failure for the
>guest, and seeing all the failures at once can help identify which area
>went wrong.
>
I'll add case to test vmxon w/ VMX is turned off by hypervisor.
>> + }
>> +
>> + if ( (msr_feat_ctrl & IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX) &&
>> + !(cpuid_feat_ecx & X86_FEATURE_SMX) )
>
>cpu_has_smx please.
>
Will change.
Thanks,
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 02/16] vvmx: test whether MSR_IA32_FEATURE_CONTROL is set correctly
2016-12-19 2:20 ` Haozhong Zhang
@ 2016-12-19 15:29 ` Andrew Cooper
0 siblings, 0 replies; 56+ messages in thread
From: Andrew Cooper @ 2016-12-19 15:29 UTC (permalink / raw)
To: Haozhong Zhang; +Cc: xen-devel
On 19/12/16 02:20, Haozhong Zhang wrote:
> On 12/16/16 16:17 +0000, Andrew Cooper wrote:
>> If so, I don't expect Xen's behaviour to ever change. We'd prefer to do
>> all configuration like that at the toolstack/hypervisor level, rather
>> than the in-guest firmware (if any).
>>
>
> OK, then TODO is not necessary at all. My concern was that Intel SDM
> says MSR_IA32_FEATURE_CONTROL is cleared to zero when a logical
> processor is reset that is not consistent to what Xen currently
> presents in HVM domains.
The firmware line is somewhat blurred in virtualisation.
Even with reset, I'd still expect Xen to handle this part of the state,
in accordance with the toolstack settings.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [XTF PATCH 03/16] vvmx: test whether MSR_IA32_VMX_BASIC is set correctly
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
2016-12-16 13:43 ` [XTF PATCH 01/16] vvmx: test whether VMX feature is present in CPUID Haozhong Zhang
2016-12-16 13:43 ` [XTF PATCH 02/16] vvmx: test whether MSR_IA32_FEATURE_CONTROL is set correctly Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 17:19 ` Andrew Cooper
2016-12-16 13:43 ` [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld Haozhong Zhang
` (14 subsequent siblings)
17 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
It tests whether bit 31 and bit 48 are 0, and VMCS size is in the
range (0, 4096].
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
include/arch/x86/msr-index.h | 4 ++++
tests/vvmx/msr.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
diff --git a/include/arch/x86/msr-index.h b/include/arch/x86/msr-index.h
index f9867d5..b7aeef0 100644
--- a/include/arch/x86/msr-index.h
+++ b/include/arch/x86/msr-index.h
@@ -16,6 +16,10 @@
#define _MSR_MISC_FEATURES_CPUID_FAULTING 0
#define MSR_MISC_FEATURES_CPUID_FAULTING (1ULL << _MSR_MISC_FEATURES_CPUID_FAULTING)
+#define MSR_IA32_VMX_BASIC 0x00000480
+#define VMX_BASIC_VMCS_SIZE_MASK (0x1fffULL << 32)
+#define VMX_BASIC_32BIT_ADDRESSES (1ULL << 48)
+
#define MSR_EFER 0xc0000080 /* Extended Feature register. */
#define _EFER_SCE 0 /* SYSCALL Enable. */
#define EFER_SCE (_AC(1, L) << _EFER_SCE)
diff --git a/tests/vvmx/msr.c b/tests/vvmx/msr.c
index 100491d..ad01f26 100644
--- a/tests/vvmx/msr.c
+++ b/tests/vvmx/msr.c
@@ -48,11 +48,58 @@ static bool test_msr_feature_control(void)
return passed;
}
+static bool test_msr_vmx_basic(void)
+{
+ bool passed = true;
+ uint64_t vmx_basic;
+ uint64_t vmcs_size;
+
+ if ( rdmsr_safe(MSR_IA32_VMX_BASIC, &vmx_basic) )
+ {
+ xtf_failure("Fail: fault when rdmsr MSR_IA32_VMX_BASIC\n");
+ passed = false;
+ goto out;
+ }
+
+ if ( vmx_basic & (1ULL << 31) )
+ {
+ xtf_failure("Fail: MSR_IA32_VMX_BASIC[31] is not 0\n");
+ passed = false;
+ }
+
+ vmcs_size = (vmx_basic & VMX_BASIC_VMCS_SIZE_MASK) >> 32;
+ if ( vmcs_size > PAGE_SIZE )
+ {
+ xtf_failure("Fail: "
+ "VMCS size (%"PRIu64") in MSR_IA32_VMX_BASIC is > %ld\n",
+ vmcs_size, PAGE_SIZE);
+ passed = false;
+ }
+ else if ( vmcs_size == 0 )
+ {
+ xtf_failure("Fail: VMCS size in MSR_IA32_VMX_BASIC cannot be 0\n");
+ passed = false;
+ }
+
+ /* test is running on hvm64, so this bit should be 0 */
+ if ( vmx_basic & VMX_BASIC_32BIT_ADDRESSES )
+ {
+ xtf_failure("Fail: MSR_IA32_VMX_BASIC[48] is not 0\n");
+ passed = false;
+ }
+
+out:
+ return passed;
+}
+
bool test_msr_vmx(void)
{
if ( !test_msr_feature_control() )
return false;
+ if ( !test_msr_vmx_basic() )
+ return false;
+
return true;
}
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [XTF PATCH 03/16] vvmx: test whether MSR_IA32_VMX_BASIC is set correctly
2016-12-16 13:43 ` [XTF PATCH 03/16] vvmx: test whether MSR_IA32_VMX_BASIC " Haozhong Zhang
@ 2016-12-16 17:19 ` Andrew Cooper
2016-12-19 2:44 ` Haozhong Zhang
0 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 17:19 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel
On 16/12/16 13:43, Haozhong Zhang wrote:
> It tests whether bit 31 and bit 48 are 0, and VMCS size is in the
> range (0, 4096].
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
> include/arch/x86/msr-index.h | 4 ++++
> tests/vvmx/msr.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/include/arch/x86/msr-index.h b/include/arch/x86/msr-index.h
> index f9867d5..b7aeef0 100644
> --- a/include/arch/x86/msr-index.h
> +++ b/include/arch/x86/msr-index.h
> @@ -16,6 +16,10 @@
> #define _MSR_MISC_FEATURES_CPUID_FAULTING 0
> #define MSR_MISC_FEATURES_CPUID_FAULTING (1ULL << _MSR_MISC_FEATURES_CPUID_FAULTING)
>
> +#define MSR_IA32_VMX_BASIC 0x00000480
> +#define VMX_BASIC_VMCS_SIZE_MASK (0x1fffULL << 32)
> +#define VMX_BASIC_32BIT_ADDRESSES (1ULL << 48)
> +
> #define MSR_EFER 0xc0000080 /* Extended Feature register. */
> #define _EFER_SCE 0 /* SYSCALL Enable. */
> #define EFER_SCE (_AC(1, L) << _EFER_SCE)
> diff --git a/tests/vvmx/msr.c b/tests/vvmx/msr.c
> index 100491d..ad01f26 100644
> --- a/tests/vvmx/msr.c
> +++ b/tests/vvmx/msr.c
> @@ -48,11 +48,58 @@ static bool test_msr_feature_control(void)
> return passed;
> }
>
> +static bool test_msr_vmx_basic(void)
> +{
> + bool passed = true;
> + uint64_t vmx_basic;
> + uint64_t vmcs_size;
> +
> + if ( rdmsr_safe(MSR_IA32_VMX_BASIC, &vmx_basic) )
> + {
> + xtf_failure("Fail: fault when rdmsr MSR_IA32_VMX_BASIC\n");
> + passed = false;
> + goto out;
> + }
> +
> + if ( vmx_basic & (1ULL << 31) )
> + {
> + xtf_failure("Fail: MSR_IA32_VMX_BASIC[31] is not 0\n");
Out of interest, what is the reason for requiring this bit to be 0? I
can see that the manual insists that it is, but not why.
> + passed = false;
> + }
> +
> + vmcs_size = (vmx_basic & VMX_BASIC_VMCS_SIZE_MASK) >> 32;
> + if ( vmcs_size > PAGE_SIZE )
> + {
> + xtf_failure("Fail: "
> + "VMCS size (%"PRIu64") in MSR_IA32_VMX_BASIC is > %ld\n",
> + vmcs_size, PAGE_SIZE);
> + passed = false;
> + }
> + else if ( vmcs_size == 0 )
> + {
> + xtf_failure("Fail: VMCS size in MSR_IA32_VMX_BASIC cannot be 0\n");
> + passed = false;
> + }
> +
> + /* test is running on hvm64, so this bit should be 0 */
> + if ( vmx_basic & VMX_BASIC_32BIT_ADDRESSES )
There is nothing in principle wrong with Xen setting this bit. It would
be odd certainly, but not erroneous.
~Andrew
> + {
> + xtf_failure("Fail: MSR_IA32_VMX_BASIC[48] is not 0\n");
> + passed = false;
> + }
> +
> +out:
> + return passed;
> +}
> +
> bool test_msr_vmx(void)
> {
> if ( !test_msr_feature_control() )
> return false;
>
> + if ( !test_msr_vmx_basic() )
> + return false;
> +
> return true;
> }
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 03/16] vvmx: test whether MSR_IA32_VMX_BASIC is set correctly
2016-12-16 17:19 ` Andrew Cooper
@ 2016-12-19 2:44 ` Haozhong Zhang
2016-12-19 15:41 ` Andrew Cooper
0 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 2:44 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/16/16 17:19 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>> It tests whether bit 31 and bit 48 are 0, and VMCS size is in the
>> range (0, 4096].
>>
>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>> ---
>> include/arch/x86/msr-index.h | 4 ++++
>> tests/vvmx/msr.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 51 insertions(+)
>>
>> diff --git a/include/arch/x86/msr-index.h b/include/arch/x86/msr-index.h
>> index f9867d5..b7aeef0 100644
>> --- a/include/arch/x86/msr-index.h
>> +++ b/include/arch/x86/msr-index.h
>> @@ -16,6 +16,10 @@
>> #define _MSR_MISC_FEATURES_CPUID_FAULTING 0
>> #define MSR_MISC_FEATURES_CPUID_FAULTING (1ULL << _MSR_MISC_FEATURES_CPUID_FAULTING)
>>
>> +#define MSR_IA32_VMX_BASIC 0x00000480
>> +#define VMX_BASIC_VMCS_SIZE_MASK (0x1fffULL << 32)
>> +#define VMX_BASIC_32BIT_ADDRESSES (1ULL << 48)
>> +
>> #define MSR_EFER 0xc0000080 /* Extended Feature register. */
>> #define _EFER_SCE 0 /* SYSCALL Enable. */
>> #define EFER_SCE (_AC(1, L) << _EFER_SCE)
>> diff --git a/tests/vvmx/msr.c b/tests/vvmx/msr.c
>> index 100491d..ad01f26 100644
>> --- a/tests/vvmx/msr.c
>> +++ b/tests/vvmx/msr.c
>> @@ -48,11 +48,58 @@ static bool test_msr_feature_control(void)
>> return passed;
>> }
>>
>> +static bool test_msr_vmx_basic(void)
>> +{
>> + bool passed = true;
>> + uint64_t vmx_basic;
>> + uint64_t vmcs_size;
>> +
>> + if ( rdmsr_safe(MSR_IA32_VMX_BASIC, &vmx_basic) )
>> + {
>> + xtf_failure("Fail: fault when rdmsr MSR_IA32_VMX_BASIC\n");
>> + passed = false;
>> + goto out;
>> + }
>> +
>> + if ( vmx_basic & (1ULL << 31) )
>> + {
>> + xtf_failure("Fail: MSR_IA32_VMX_BASIC[31] is not 0\n");
>
>Out of interest, what is the reason for requiring this bit to be 0? I
>can see that the manual insists that it is, but not why.
>
I'm not sure the exact reason, but guess it's because bit 31 of VMCS
revision id is reserved to indicate whether it's a shadow one.
>> + passed = false;
>> + }
>> +
>> + vmcs_size = (vmx_basic & VMX_BASIC_VMCS_SIZE_MASK) >> 32;
>> + if ( vmcs_size > PAGE_SIZE )
>> + {
>> + xtf_failure("Fail: "
>> + "VMCS size (%"PRIu64") in MSR_IA32_VMX_BASIC is > %ld\n",
>> + vmcs_size, PAGE_SIZE);
>> + passed = false;
>> + }
>> + else if ( vmcs_size == 0 )
>> + {
>> + xtf_failure("Fail: VMCS size in MSR_IA32_VMX_BASIC cannot be 0\n");
>> + passed = false;
>> + }
>> +
>> + /* test is running on hvm64, so this bit should be 0 */
>> + if ( vmx_basic & VMX_BASIC_32BIT_ADDRESSES )
>
>There is nothing in principle wrong with Xen setting this bit. It would
>be odd certainly, but not erroneous.
>
The reason I added this test is because Intel SDM Vol 3, Appendix A.1
Basic VMX Information says "This bit (bit 48) is always 0 for
processors that support Intel 64 architecture."
I don't know whether there is SW in real world that depends on the
consistency between this bit and Intel SDM, but if there is any, it
might be confused by an odd configuration. So I think it's still worth
to keep such test.
Thanks,
Haozhong
>~Andrew
>
>> + {
>> + xtf_failure("Fail: MSR_IA32_VMX_BASIC[48] is not 0\n");
>> + passed = false;
>> + }
>> +
>> +out:
>> + return passed;
>> +}
>> +
>> bool test_msr_vmx(void)
>> {
>> if ( !test_msr_feature_control() )
>> return false;
>>
>> + if ( !test_msr_vmx_basic() )
>> + return false;
>> +
>> return true;
>> }
>>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 03/16] vvmx: test whether MSR_IA32_VMX_BASIC is set correctly
2016-12-19 2:44 ` Haozhong Zhang
@ 2016-12-19 15:41 ` Andrew Cooper
2016-12-20 2:45 ` Haozhong Zhang
0 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-19 15:41 UTC (permalink / raw)
To: Haozhong Zhang; +Cc: xen-devel
On 19/12/16 02:44, Haozhong Zhang wrote:
>>> + passed = false;
>>> + }
>>> +
>>> + vmcs_size = (vmx_basic & VMX_BASIC_VMCS_SIZE_MASK) >> 32;
>>> + if ( vmcs_size > PAGE_SIZE )
>>> + {
>>> + xtf_failure("Fail: "
>>> + "VMCS size (%"PRIu64") in MSR_IA32_VMX_BASIC is
>>> > %ld\n",
>>> + vmcs_size, PAGE_SIZE);
>>> + passed = false;
>>> + }
>>> + else if ( vmcs_size == 0 )
>>> + {
>>> + xtf_failure("Fail: VMCS size in MSR_IA32_VMX_BASIC cannot
>>> be 0\n");
>>> + passed = false;
>>> + }
>>> +
>>> + /* test is running on hvm64, so this bit should be 0 */
>>> + if ( vmx_basic & VMX_BASIC_32BIT_ADDRESSES )
>>
>> There is nothing in principle wrong with Xen setting this bit. It would
>> be odd certainly, but not erroneous.
>>
>
> The reason I added this test is because Intel SDM Vol 3, Appendix A.1
> Basic VMX Information says "This bit (bit 48) is always 0 for
> processors that support Intel 64 architecture."
Right, and Xen, being 64bit only, can expect to find this bit always clear.
>
> I don't know whether there is SW in real world that depends on the
> consistency between this bit and Intel SDM, but if there is any, it
> might be confused by an odd configuration. So I think it's still worth
> to keep such test.
It is a valid administrator choice to restrict a VM to 32bit-only by
hiding the long mode bit. Under those circumstances, it would be
logical to expect this bit to be set in the virtual environment, despite
not being required at the physical level.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 03/16] vvmx: test whether MSR_IA32_VMX_BASIC is set correctly
2016-12-19 15:41 ` Andrew Cooper
@ 2016-12-20 2:45 ` Haozhong Zhang
0 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-20 2:45 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/19/16 15:41 +0000, Andrew Cooper wrote:
>On 19/12/16 02:44, Haozhong Zhang wrote:
>>>> + passed = false;
>>>> + }
>>>> +
>>>> + vmcs_size = (vmx_basic & VMX_BASIC_VMCS_SIZE_MASK) >> 32;
>>>> + if ( vmcs_size > PAGE_SIZE )
>>>> + {
>>>> + xtf_failure("Fail: "
>>>> + "VMCS size (%"PRIu64") in MSR_IA32_VMX_BASIC is
>>>> > %ld\n",
>>>> + vmcs_size, PAGE_SIZE);
>>>> + passed = false;
>>>> + }
>>>> + else if ( vmcs_size == 0 )
>>>> + {
>>>> + xtf_failure("Fail: VMCS size in MSR_IA32_VMX_BASIC cannot
>>>> be 0\n");
>>>> + passed = false;
>>>> + }
>>>> +
>>>> + /* test is running on hvm64, so this bit should be 0 */
>>>> + if ( vmx_basic & VMX_BASIC_32BIT_ADDRESSES )
>>>
>>> There is nothing in principle wrong with Xen setting this bit. It would
>>> be odd certainly, but not erroneous.
>>>
>>
>> The reason I added this test is because Intel SDM Vol 3, Appendix A.1
>> Basic VMX Information says "This bit (bit 48) is always 0 for
>> processors that support Intel 64 architecture."
>
>Right, and Xen, being 64bit only, can expect to find this bit always clear.
>
>>
>> I don't know whether there is SW in real world that depends on the
>> consistency between this bit and Intel SDM, but if there is any, it
>> might be confused by an odd configuration. So I think it's still worth
>> to keep such test.
>
>It is a valid administrator choice to restrict a VM to 32bit-only by
>hiding the long mode bit. Under those circumstances, it would be
>logical to expect this bit to be set in the virtual environment, despite
>not being required at the physical level.
>
If we want to limit the L1 hypervisor to only use 32-bit VMCS/VMXON
address, I think the right way is to limit the L1 vcpu's physical
address width to 32, instead of introducing spec inconsistency, though
it also limits the physical address space that can be used by L1
hypervisor.
Thanks,
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (2 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 03/16] vvmx: test whether MSR_IA32_VMX_BASIC " Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 19:03 ` [licensing] was: " Andrew Cooper
2016-12-16 19:47 ` Andrew Cooper
2016-12-16 13:43 ` [XTF PATCH 05/16] vvmx: add a general error handler for VMX instructions Haozhong Zhang
` (13 subsequent siblings)
17 siblings, 2 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
These C wrappers record the faults and VMfails in the execution of vmx
instructions vmxon, vmread and vmptrld. Further tests can use those
records to check if a VMX instruction is implemented correctly by Xen
in the nested VMX. Other VMX instructions will be added once their
test cases are added.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
include/arch/x86/hvm/vmx/vmcs.h | 179 ++++++++++++++++++++++++++++++++++++++++
tests/vvmx/Makefile | 2 +-
tests/vvmx/util.c | 83 +++++++++++++++++++
tests/vvmx/util.h | 78 +++++++++++++++++
tests/vvmx/vmxon.c | 0
5 files changed, 341 insertions(+), 1 deletion(-)
create mode 100644 include/arch/x86/hvm/vmx/vmcs.h
create mode 100644 tests/vvmx/util.c
create mode 100644 tests/vvmx/util.h
create mode 100644 tests/vvmx/vmxon.c
diff --git a/include/arch/x86/hvm/vmx/vmcs.h b/include/arch/x86/hvm/vmx/vmcs.h
new file mode 100644
index 0000000..e1a6ef8
--- /dev/null
+++ b/include/arch/x86/hvm/vmx/vmcs.h
@@ -0,0 +1,179 @@
+#ifndef XTF_X86_HVM_VMX_VMCS_H
+#define XTF_X86_HVM_VMX_VMCS_H
+
+/* VMCS field encodings. */
+#define VMCS_HIGH(x) ((x) | 1)
+enum vmcs_field {
+ VIRTUAL_PROCESSOR_ID = 0x00000000,
+ POSTED_INTR_NOTIFICATION_VECTOR = 0x00000002,
+ EPTP_INDEX = 0x00000004,
+#define GUEST_SEG_SELECTOR(sel) (GUEST_ES_SELECTOR + (sel) * 2) /* ES ... GS */
+ GUEST_ES_SELECTOR = 0x00000800,
+ GUEST_CS_SELECTOR = 0x00000802,
+ GUEST_SS_SELECTOR = 0x00000804,
+ GUEST_DS_SELECTOR = 0x00000806,
+ GUEST_FS_SELECTOR = 0x00000808,
+ GUEST_GS_SELECTOR = 0x0000080a,
+ GUEST_LDTR_SELECTOR = 0x0000080c,
+ GUEST_TR_SELECTOR = 0x0000080e,
+ GUEST_INTR_STATUS = 0x00000810,
+ GUEST_PML_INDEX = 0x00000812,
+ HOST_ES_SELECTOR = 0x00000c00,
+ HOST_CS_SELECTOR = 0x00000c02,
+ HOST_SS_SELECTOR = 0x00000c04,
+ HOST_DS_SELECTOR = 0x00000c06,
+ HOST_FS_SELECTOR = 0x00000c08,
+ HOST_GS_SELECTOR = 0x00000c0a,
+ HOST_TR_SELECTOR = 0x00000c0c,
+ IO_BITMAP_A = 0x00002000,
+ IO_BITMAP_B = 0x00002002,
+ MSR_BITMAP = 0x00002004,
+ VM_EXIT_MSR_STORE_ADDR = 0x00002006,
+ VM_EXIT_MSR_LOAD_ADDR = 0x00002008,
+ VM_ENTRY_MSR_LOAD_ADDR = 0x0000200a,
+ PML_ADDRESS = 0x0000200e,
+ TSC_OFFSET = 0x00002010,
+ VIRTUAL_APIC_PAGE_ADDR = 0x00002012,
+ APIC_ACCESS_ADDR = 0x00002014,
+ PI_DESC_ADDR = 0x00002016,
+ VM_FUNCTION_CONTROL = 0x00002018,
+ EPT_POINTER = 0x0000201a,
+ EOI_EXIT_BITMAP0 = 0x0000201c,
+#define EOI_EXIT_BITMAP(n) (EOI_EXIT_BITMAP0 + (n) * 2) /* n = 0...3 */
+ EPTP_LIST_ADDR = 0x00002024,
+ VMREAD_BITMAP = 0x00002026,
+ VMWRITE_BITMAP = 0x00002028,
+ VIRT_EXCEPTION_INFO = 0x0000202a,
+ XSS_EXIT_BITMAP = 0x0000202c,
+ TSC_MULTIPLIER = 0x00002032,
+ GUEST_PHYSICAL_ADDRESS = 0x00002400,
+ VMCS_LINK_POINTER = 0x00002800,
+ GUEST_IA32_DEBUGCTL = 0x00002802,
+ GUEST_PAT = 0x00002804,
+ GUEST_EFER = 0x00002806,
+ GUEST_PERF_GLOBAL_CTRL = 0x00002808,
+ GUEST_PDPTE0 = 0x0000280a,
+#define GUEST_PDPTE(n) (GUEST_PDPTE0 + (n) * 2) /* n = 0...3 */
+ GUEST_BNDCFGS = 0x00002812,
+ HOST_PAT = 0x00002c00,
+ HOST_EFER = 0x00002c02,
+ HOST_PERF_GLOBAL_CTRL = 0x00002c04,
+ PIN_BASED_VM_EXEC_CONTROL = 0x00004000,
+ CPU_BASED_VM_EXEC_CONTROL = 0x00004002,
+ EXCEPTION_BITMAP = 0x00004004,
+ PAGE_FAULT_ERROR_CODE_MASK = 0x00004006,
+ PAGE_FAULT_ERROR_CODE_MATCH = 0x00004008,
+ CR3_TARGET_COUNT = 0x0000400a,
+ VM_EXIT_CONTROLS = 0x0000400c,
+ VM_EXIT_MSR_STORE_COUNT = 0x0000400e,
+ VM_EXIT_MSR_LOAD_COUNT = 0x00004010,
+ VM_ENTRY_CONTROLS = 0x00004012,
+ VM_ENTRY_MSR_LOAD_COUNT = 0x00004014,
+ VM_ENTRY_INTR_INFO = 0x00004016,
+ VM_ENTRY_EXCEPTION_ERROR_CODE = 0x00004018,
+ VM_ENTRY_INSTRUCTION_LEN = 0x0000401a,
+ TPR_THRESHOLD = 0x0000401c,
+ SECONDARY_VM_EXEC_CONTROL = 0x0000401e,
+ PLE_GAP = 0x00004020,
+ PLE_WINDOW = 0x00004022,
+ VM_INSTRUCTION_ERROR = 0x00004400,
+ VM_EXIT_REASON = 0x00004402,
+ VM_EXIT_INTR_INFO = 0x00004404,
+ VM_EXIT_INTR_ERROR_CODE = 0x00004406,
+ IDT_VECTORING_INFO = 0x00004408,
+ IDT_VECTORING_ERROR_CODE = 0x0000440a,
+ VM_EXIT_INSTRUCTION_LEN = 0x0000440c,
+ VMX_INSTRUCTION_INFO = 0x0000440e,
+#define GUEST_SEG_LIMIT(sel) (GUEST_ES_LIMIT + (sel) * 2) /* ES ... GS */
+ GUEST_ES_LIMIT = 0x00004800,
+ GUEST_CS_LIMIT = 0x00004802,
+ GUEST_SS_LIMIT = 0x00004804,
+ GUEST_DS_LIMIT = 0x00004806,
+ GUEST_FS_LIMIT = 0x00004808,
+ GUEST_GS_LIMIT = 0x0000480a,
+ GUEST_LDTR_LIMIT = 0x0000480c,
+ GUEST_TR_LIMIT = 0x0000480e,
+ GUEST_GDTR_LIMIT = 0x00004810,
+ GUEST_IDTR_LIMIT = 0x00004812,
+#define GUEST_SEG_AR_BYTES(sel) (GUEST_ES_AR_BYTES + (sel) * 2) /* ES ... GS */
+ GUEST_ES_AR_BYTES = 0x00004814,
+ GUEST_CS_AR_BYTES = 0x00004816,
+ GUEST_SS_AR_BYTES = 0x00004818,
+ GUEST_DS_AR_BYTES = 0x0000481a,
+ GUEST_FS_AR_BYTES = 0x0000481c,
+ GUEST_GS_AR_BYTES = 0x0000481e,
+ GUEST_LDTR_AR_BYTES = 0x00004820,
+ GUEST_TR_AR_BYTES = 0x00004822,
+ GUEST_INTERRUPTIBILITY_INFO = 0x00004824,
+ GUEST_ACTIVITY_STATE = 0x00004826,
+ GUEST_SMBASE = 0x00004828,
+ GUEST_SYSENTER_CS = 0x0000482a,
+ GUEST_PREEMPTION_TIMER = 0x0000482e,
+ HOST_SYSENTER_CS = 0x00004c00,
+ CR0_GUEST_HOST_MASK = 0x00006000,
+ CR4_GUEST_HOST_MASK = 0x00006002,
+ CR0_READ_SHADOW = 0x00006004,
+ CR4_READ_SHADOW = 0x00006006,
+ CR3_TARGET_VALUE0 = 0x00006008,
+#define CR3_TARGET_VALUE(n) (CR3_TARGET_VALUE0 + (n) * 2) /* n < CR3_TARGET_COUNT */
+ EXIT_QUALIFICATION = 0x00006400,
+ GUEST_LINEAR_ADDRESS = 0x0000640a,
+ GUEST_CR0 = 0x00006800,
+ GUEST_CR3 = 0x00006802,
+ GUEST_CR4 = 0x00006804,
+#define GUEST_SEG_BASE(sel) (GUEST_ES_BASE + (sel) * 2) /* ES ... GS */
+ GUEST_ES_BASE = 0x00006806,
+ GUEST_CS_BASE = 0x00006808,
+ GUEST_SS_BASE = 0x0000680a,
+ GUEST_DS_BASE = 0x0000680c,
+ GUEST_FS_BASE = 0x0000680e,
+ GUEST_GS_BASE = 0x00006810,
+ GUEST_LDTR_BASE = 0x00006812,
+ GUEST_TR_BASE = 0x00006814,
+ GUEST_GDTR_BASE = 0x00006816,
+ GUEST_IDTR_BASE = 0x00006818,
+ GUEST_DR7 = 0x0000681a,
+ GUEST_RSP = 0x0000681c,
+ GUEST_RIP = 0x0000681e,
+ GUEST_RFLAGS = 0x00006820,
+ GUEST_PENDING_DBG_EXCEPTIONS = 0x00006822,
+ GUEST_SYSENTER_ESP = 0x00006824,
+ GUEST_SYSENTER_EIP = 0x00006826,
+ HOST_CR0 = 0x00006c00,
+ HOST_CR3 = 0x00006c02,
+ HOST_CR4 = 0x00006c04,
+ HOST_FS_BASE = 0x00006c06,
+ HOST_GS_BASE = 0x00006c08,
+ HOST_TR_BASE = 0x00006c0a,
+ HOST_GDTR_BASE = 0x00006c0c,
+ HOST_IDTR_BASE = 0x00006c0e,
+ HOST_SYSENTER_ESP = 0x00006c10,
+ HOST_SYSENTER_EIP = 0x00006c12,
+ HOST_RSP = 0x00006c14,
+ HOST_RIP = 0x00006c16,
+};
+
+/* VM Instruction error numbers */
+enum vmx_insn_errno
+{
+ VMX_INSN_VMCLEAR_INVALID_PHYADDR = 2,
+ VMX_INSN_VMLAUNCH_NONCLEAR_VMCS = 4,
+ VMX_INSN_VMRESUME_NONLAUNCHED_VMCS = 5,
+ VMX_INSN_INVALID_CONTROL_STATE = 7,
+ VMX_INSN_INVALID_HOST_STATE = 8,
+ VMX_INSN_VMPTRLD_INVALID_PHYADDR = 9,
+ VMX_INSN_UNSUPPORTED_VMCS_COMPONENT = 12,
+ VMX_INSN_VMXON_IN_VMX_ROOT = 15,
+};
+
+#endif /* XTF_X86_HVM_VMX_VMCS_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tests/vvmx/Makefile b/tests/vvmx/Makefile
index 54769fa..24d3720 100644
--- a/tests/vvmx/Makefile
+++ b/tests/vvmx/Makefile
@@ -6,6 +6,6 @@ TEST-ENVS := hvm64
TEST-EXTRA-CFG := extra.cfg.in
-obj-perenv += main.o cpuid.o msr.o
+obj-perenv += main.o cpuid.o msr.o util.o
include $(ROOT)/build/gen.mk
diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c
new file mode 100644
index 0000000..8cd35c5
--- /dev/null
+++ b/tests/vvmx/util.c
@@ -0,0 +1,83 @@
+#include <xtf.h>
+#include <arch/x86/hvm/vmx/vmcs.h>
+#include "util.h"
+
+#define VMPTRLD_OPCODE ".byte 0x0f,0xc7\n" /* reg/opcode: /6 */
+#define VMREAD_OPCODE ".byte 0x0f,0x78\n"
+#define VMXON_OPCODE ".byte 0xf3,0x0f,0xc7\n"
+
+#define MODRM_EAX_06 ".byte 0x30\n" /* [EAX], with reg/opcode: /6 */
+#define MODRM_EAX_ECX ".byte 0xc1\n" /* EAX, ECX */
+
+uint8_t vmxon(uint64_t paddr, exinfo_t *fault_info)
+{
+ exinfo_t fault = 0;
+ uint8_t valid = 0, invalid = 0;
+
+ asm volatile("1: "VMXON_OPCODE MODRM_EAX_06 "\n\t"
+ " setc %0; setz %1 \n\t"
+ "2: \n\t"
+ _ASM_EXTABLE_HANDLER(1b, 2b, ex_record_fault_edi)
+ : "=q" (invalid), "=q" (valid), "+D" (fault)
+ : "a" (&paddr)
+ : "memory", "cc");
+
+ if ( fault && fault_info )
+ *fault_info = fault;
+
+ return (fault ? VMXERR_FAULT : 0) |
+ (valid ? VMXERR_VMFAIL_VALID : 0) |
+ (invalid ? VMXERR_VMFAIL_INVALID : 0);
+}
+
+uint8_t vmread(enum vmcs_field field, uint64_t *val, exinfo_t *fault_info)
+{
+ exinfo_t fault = 0;
+ uint8_t valid = 0, invalid = 0;
+
+ asm volatile("1: "VMREAD_OPCODE MODRM_EAX_ECX "\n\t"
+ " setc %0; setz %1 \n\t"
+ "2: \n\t"
+ _ASM_EXTABLE_HANDLER(1b, 2b, ex_record_fault_edi)
+ : "=q" (invalid), "=q" (valid), "+D" (fault), "=c" (*val)
+ : "a" (field)
+ : "memory", "cc");
+
+ if ( fault && fault_info )
+ *fault_info = fault;
+
+ return (fault ? VMXERR_FAULT : 0) |
+ (valid ? VMXERR_VMFAIL_VALID : 0) |
+ (invalid ? VMXERR_VMFAIL_INVALID : 0);
+}
+
+uint8_t vmptrld(uint64_t paddr, exinfo_t *fault_info)
+{
+ exinfo_t fault = 0;
+ uint8_t valid = 0, invalid = 0;
+
+ asm volatile("1: "VMPTRLD_OPCODE MODRM_EAX_06 "\n\t"
+ " setc %0; setz %1 \n\t"
+ "2: \n\t"
+ _ASM_EXTABLE_HANDLER(1b, 2b, ex_record_fault_edi)
+ : "=q" (invalid), "=q" (valid), "+D" (fault)
+ : "a" (&paddr)
+ : "memory", "cc");
+
+ if ( fault && fault_info )
+ *fault_info = fault;
+
+ return (fault ? VMXERR_FAULT : 0) |
+ (valid ? VMXERR_VMFAIL_VALID : 0) |
+ (invalid ? VMXERR_VMFAIL_INVALID : 0);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tests/vvmx/util.h b/tests/vvmx/util.h
new file mode 100644
index 0000000..57d3398
--- /dev/null
+++ b/tests/vvmx/util.h
@@ -0,0 +1,78 @@
+#ifndef XTF_TESTS_VVMX_UTIL_H
+#define XTF_TESTS_VVMX_UTIL_H
+
+#include <arch/x86/exinfo.h>
+#include <arch/x86/hvm/vmx/vmcs.h>
+
+/**
+ * Flags for errors during the execution of a VMX instruction.
+ *
+ * NB. Besides VMXERR_NOERR, other flags are not mutually exclusive,
+ * because we should not assume Xen implements the nested VMX
+ * instructions correctly.
+ */
+#define VMXERR_NOERR 0
+#define VMXERR_VMFAIL_VALID (1 << 0)
+#define VMXERR_VMFAIL_INVALID (1 << 1)
+#define VMXERR_FAULT (1 << 2)
+
+/**
+ * vmxon
+ *
+ * Parameters:
+ * @paddr: the physical address of the VMXON region
+ * @fault: return the information of fault encountered in the execution
+ *
+ * Return:
+ * VMXERR_ flags.
+ *
+ * If VMXERR_FAULT is present and @fault is not NULL, the fault
+ * information will be returned via @fault.
+ */
+uint8_t vmxon(uint64_t paddr, exinfo_t *fault);
+
+/**
+ * vmread
+ *
+ * Parameters:
+ * @field: the encoding of the VMCS field to read
+ * @value: return the value of the VMCS field if no error is encountered
+ * @fault: return the information of fault encountered in the execution
+ *
+ * Return:
+ * VMXERR_ flags.
+ *
+ * If VMXERR_FAULT is present and @fault is not NULL, the fault
+ * information will be returned via @fault.
+ *
+ * If VMXERR_NOERR is returned and @value is not NULL, the value of
+ * VMCS field @field will be returned via @value.
+ */
+uint8_t vmread(enum vmcs_field field, uint64_t *value, exinfo_t *fault);
+
+/**
+ * vmptrld
+ *
+ * Parameters:
+ * @paddr: the physical address of VMCS
+ * @fault: return the information of fault encountered in the execution
+ *
+ * Return:
+ * VMXERR_ flags.
+ *
+ * If VMXERR_FAULT is present and @fault is not NULL, the fault
+ * information will be returned via @fault.
+ */
+uint8_t vmptrld(uint64_t paddr, exinfo_t *fault);
+
+#endif /* XTF_TESTS_VVMX_UTIL_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
new file mode 100644
index 0000000..e69de29
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* [licensing] was: [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld
2016-12-16 13:43 ` [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld Haozhong Zhang
@ 2016-12-16 19:03 ` Andrew Cooper
2016-12-19 3:20 ` Haozhong Zhang
2016-12-19 15:20 ` Roger Pau Monné
2016-12-16 19:47 ` Andrew Cooper
1 sibling, 2 replies; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 19:03 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel; +Cc: Lars Kurth, Kevin Tian, Ian Jackson
On 16/12/16 13:43, Haozhong Zhang wrote:
> diff --git a/include/arch/x86/hvm/vmx/vmcs.h b/include/arch/x86/hvm/vmx/vmcs.h
> new file mode 100644
> index 0000000..e1a6ef8
> --- /dev/null
> +++ b/include/arch/x86/hvm/vmx/vmcs.h
> @@ -0,0 +1,179 @@
> +#ifndef XTF_X86_HVM_VMX_VMCS_H
> +#define XTF_X86_HVM_VMX_VMCS_H
> +
> +/* VMCS field encodings. */
> +#define VMCS_HIGH(x) ((x) | 1)
> +enum vmcs_field {
> + VIRTUAL_PROCESSOR_ID = 0x00000000,
> + POSTED_INTR_NOTIFICATION_VECTOR = 0x00000002,
> + EPTP_INDEX = 0x00000004,
> +#define GUEST_SEG_SELECTOR(sel) (GUEST_ES_SELECTOR + (sel) * 2) /* ES ... GS */
Unfortunately, there is probably a BSD/GPL licensing issue here.
XTF is BSD clause 2 licensed, because a lot of the core microkernel bits
are generally useful to other microkernel projects, and I have specific
plans to contribute improvements back to the likes of Mini-OS and
HVMLoader. I would specifically like to maintain this property.
Xen, following its Linux heritage, is strictly GPLv2 (other than the
public headers, which are specifically different).
Having XTF use the same naming as Xen is convenient for development, and
I specifically don't want to get caught up in tricks like renaming
constants; these names inherit from the architecture manual and calling
them anything else would be even worse. If we were to go down this
route, being able to keep the header file in sync would be useful, but
dual licensing it Xen would be complicated and confusing.
BSD and GPL are compatible licenses. One option Ian suggested would be
to have a GPL subdirectory in XTF which clearly separates GPL content
from BSD content. The resulting tests would become GPL, but the primary
distribution method is "git clone && make", so there are no issues
there. I think it is reasonable to take this approach for header file
content like this, describing an external ABI, but I'd certainly like to
avoid doing anything similar for other content, as it complicates
contributing microkernel content back to other projects.
CC'ing various people for opinions.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [licensing] was: [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld
2016-12-16 19:03 ` [licensing] was: " Andrew Cooper
@ 2016-12-19 3:20 ` Haozhong Zhang
2016-12-19 14:41 ` Lars Kurth
2016-12-19 15:58 ` Andrew Cooper
2016-12-19 15:20 ` Roger Pau Monné
1 sibling, 2 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 3:20 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Kevin Tian, Ian Jackson, Lars Kurth
On 12/16/16 19:03 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>> diff --git a/include/arch/x86/hvm/vmx/vmcs.h b/include/arch/x86/hvm/vmx/vmcs.h
>> new file mode 100644
>> index 0000000..e1a6ef8
>> --- /dev/null
>> +++ b/include/arch/x86/hvm/vmx/vmcs.h
>> @@ -0,0 +1,179 @@
>> +#ifndef XTF_X86_HVM_VMX_VMCS_H
>> +#define XTF_X86_HVM_VMX_VMCS_H
>> +
>> +/* VMCS field encodings. */
>> +#define VMCS_HIGH(x) ((x) | 1)
>> +enum vmcs_field {
>> + VIRTUAL_PROCESSOR_ID = 0x00000000,
>> + POSTED_INTR_NOTIFICATION_VECTOR = 0x00000002,
>> + EPTP_INDEX = 0x00000004,
>> +#define GUEST_SEG_SELECTOR(sel) (GUEST_ES_SELECTOR + (sel) * 2) /* ES ... GS */
>
>Unfortunately, there is probably a BSD/GPL licensing issue here.
>
>XTF is BSD clause 2 licensed, because a lot of the core microkernel bits
>are generally useful to other microkernel projects, and I have specific
>plans to contribute improvements back to the likes of Mini-OS and
>HVMLoader. I would specifically like to maintain this property.
>
>Xen, following its Linux heritage, is strictly GPLv2 (other than the
>public headers, which are specifically different).
>
>
>Having XTF use the same naming as Xen is convenient for development, and
>I specifically don't want to get caught up in tricks like renaming
>constants;
but renaming or taking names from other BSD-licensed projects [1] could
keep the whole project as purely BSD-licensed.
[1] https://github.com/freebsd/freebsd/blob/master/sys/amd64/vmm/intel/vmcs.h
> these names inherit from the architecture manual and calling
>them anything else would be even worse. If we were to go down this
>route, being able to keep the header file in sync would be useful, but
>dual licensing it Xen would be complicated and confusing.
>
>BSD and GPL are compatible licenses. One option Ian suggested would be
>to have a GPL subdirectory in XTF which clearly separates GPL content
>from BSD content. The resulting tests would become GPL, but the primary
>distribution method is "git clone && make", so there are no issues
>there.
If I want to use the BSD-licensed files individually in other
projects, will I need to keep a GPL license with them?
Thanks,
Haozhong
> I think it is reasonable to take this approach for header file
>content like this, describing an external ABI, but I'd certainly like to
>avoid doing anything similar for other content, as it complicates
>contributing microkernel content back to other projects.
>
>CC'ing various people for opinions.
>
>~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [licensing] was: [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld
2016-12-19 3:20 ` Haozhong Zhang
@ 2016-12-19 14:41 ` Lars Kurth
2016-12-19 15:51 ` Ian Jackson
2016-12-19 15:58 ` Andrew Cooper
1 sibling, 1 reply; 56+ messages in thread
From: Lars Kurth @ 2016-12-19 14:41 UTC (permalink / raw)
To: Haozhong Zhang, Andrew Cooper; +Cc: xen-devel, Kevin Tian, Ian Jackson
On 18/12/2016 21:20, "Haozhong Zhang" <haozhong.zhang@intel.com> wrote:
>On 12/16/16 19:03 +0000, Andrew Cooper wrote:
>>On 16/12/16 13:43, Haozhong Zhang wrote:
>>> diff --git a/include/arch/x86/hvm/vmx/vmcs.h
>>>b/include/arch/x86/hvm/vmx/vmcs.h
>>> new file mode 100644
>>> index 0000000..e1a6ef8
>>> --- /dev/null
>>> +++ b/include/arch/x86/hvm/vmx/vmcs.h
>>> @@ -0,0 +1,179 @@
>>> +#ifndef XTF_X86_HVM_VMX_VMCS_H
>>> +#define XTF_X86_HVM_VMX_VMCS_H
>>> +
>>> +/* VMCS field encodings. */
>>> +#define VMCS_HIGH(x) ((x) | 1)
>>> +enum vmcs_field {
>>> + VIRTUAL_PROCESSOR_ID = 0x00000000,
>>> + POSTED_INTR_NOTIFICATION_VECTOR = 0x00000002,
>>> + EPTP_INDEX = 0x00000004,
>>> +#define GUEST_SEG_SELECTOR(sel) (GUEST_ES_SELECTOR + (sel) * 2) /* ES
>>>... GS */
>>
>>Unfortunately, there is probably a BSD/GPL licensing issue here.
>>
>>XTF is BSD clause 2 licensed, because a lot of the core microkernel bits
>>are generally useful to other microkernel projects, and I have specific
>>plans to contribute improvements back to the likes of Mini-OS and
>>HVMLoader. I would specifically like to maintain this property.
>>
>>Xen, following its Linux heritage, is strictly GPLv2 (other than the
>>public headers, which are specifically different).
>>
>>
>>Having XTF use the same naming as Xen is convenient for development, and
>>I specifically don't want to get caught up in tricks like renaming
>>constants;
>
>but renaming or taking names from other BSD-licensed projects [1] could
>keep the whole project as purely BSD-licensed.
>
>[1]
>https://github.com/freebsd/freebsd/blob/master/sys/amd64/vmm/intel/vmcs.h
I am not sure what you mean.
>> these names inherit from the architecture manual and calling
>>them anything else would be even worse. If we were to go down this
>>route, being able to keep the header file in sync would be useful, but
>>dual licensing it Xen would be complicated and confusing.
>>
>>BSD and GPL are compatible licenses. One option Ian suggested would be
>>to have a GPL subdirectory in XTF which clearly separates GPL content
>>from BSD content. The resulting tests would become GPL, but the primary
>>distribution method is "git clone && make", so there are no issues
>>there.
This seems to be a workable approach.
Whether the test suite itself (when combined with minios or other non-GPL
baselines) are GPL or another license probably does not matter.
Separating the GPL code out, would be useful if the suite needed to be
combined with a baseline which is non-GPL compatible.
@Andy: you may want to have a chat with Ian and check whether the MPL 2.0
may be a better choice for XTF than BSD in this case. I have not looked
into how copyleft and MPL 2 work together though. Just a thought.
>If I want to use the BSD-licensed files individually in other
>projects, will I need to keep a GPL license with them?
I am assuming you mean whether the file needs a GPL (c) header? The answer
is no.
But the combined work will be GPL: for example let's say a folder A has
BSD files in it and a folder B has GPL files in it and you compile A and B
together to AB.exe, then AB.exe as a whole is licensed under the GPL.
>
>Thanks,
>Haozhong
>
>> I think it is reasonable to take this approach for header file
>>content like this, describing an external ABI, but I'd certainly like to
>>avoid doing anything similar for other content, as it complicates
>>contributing microkernel content back to other projects.
>>
>>CC'ing various people for opinions.
>>
>>~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [licensing] was: [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld
2016-12-19 14:41 ` Lars Kurth
@ 2016-12-19 15:51 ` Ian Jackson
0 siblings, 0 replies; 56+ messages in thread
From: Ian Jackson @ 2016-12-19 15:51 UTC (permalink / raw)
To: Lars Kurth; +Cc: Haozhong Zhang, Kevin Tian, xen-devel, Andrew Cooper
Lars Kurth writes ("Re: [licensing] was: [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld"):
> @Andy: you may want to have a chat with Ian and check whether the MPL 2.0
> may be a better choice for XTF than BSD in this case. I have not looked
> into how copyleft and MPL 2 work together though. Just a thought.
They work badly together. I would avoid the MPL for anything.
BSD2/MIT (ie a permissive licence) is correct for Andy's requirements.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [licensing] was: [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld
2016-12-19 3:20 ` Haozhong Zhang
2016-12-19 14:41 ` Lars Kurth
@ 2016-12-19 15:58 ` Andrew Cooper
1 sibling, 0 replies; 56+ messages in thread
From: Andrew Cooper @ 2016-12-19 15:58 UTC (permalink / raw)
To: Haozhong Zhang; +Cc: xen-devel, Kevin Tian, Ian Jackson, Lars Kurth
On 19/12/16 03:20, Haozhong Zhang wrote:
> On 12/16/16 19:03 +0000, Andrew Cooper wrote:
>> On 16/12/16 13:43, Haozhong Zhang wrote:
>>> diff --git a/include/arch/x86/hvm/vmx/vmcs.h
>>> b/include/arch/x86/hvm/vmx/vmcs.h
>>> new file mode 100644
>>> index 0000000..e1a6ef8
>>> --- /dev/null
>>> +++ b/include/arch/x86/hvm/vmx/vmcs.h
>>> @@ -0,0 +1,179 @@
>>> +#ifndef XTF_X86_HVM_VMX_VMCS_H
>>> +#define XTF_X86_HVM_VMX_VMCS_H
>>> +
>>> +/* VMCS field encodings. */
>>> +#define VMCS_HIGH(x) ((x) | 1)
>>> +enum vmcs_field {
>>> + VIRTUAL_PROCESSOR_ID = 0x00000000,
>>> + POSTED_INTR_NOTIFICATION_VECTOR = 0x00000002,
>>> + EPTP_INDEX = 0x00000004,
>>> +#define GUEST_SEG_SELECTOR(sel) (GUEST_ES_SELECTOR + (sel) * 2) /*
>>> ES ... GS */
>>
>> Unfortunately, there is probably a BSD/GPL licensing issue here.
>>
>> XTF is BSD clause 2 licensed, because a lot of the core microkernel bits
>> are generally useful to other microkernel projects, and I have specific
>> plans to contribute improvements back to the likes of Mini-OS and
>> HVMLoader. I would specifically like to maintain this property.
>>
>> Xen, following its Linux heritage, is strictly GPLv2 (other than the
>> public headers, which are specifically different).
>>
>>
>> Having XTF use the same naming as Xen is convenient for development, and
>> I specifically don't want to get caught up in tricks like renaming
>> constants;
>
> but renaming or taking names from other BSD-licensed projects [1] could
> keep the whole project as purely BSD-licensed.
That would also work.
I also tend to only pull in defines which are actually used, but that is
just personal choice and to keep the size of the commits down.
>
> [1]
> https://github.com/freebsd/freebsd/blob/master/sys/amd64/vmm/intel/vmcs.h
>
>> these names inherit from the architecture manual and calling
>> them anything else would be even worse. If we were to go down this
>> route, being able to keep the header file in sync would be useful, but
>> dual licensing it Xen would be complicated and confusing.
>>
>> BSD and GPL are compatible licenses. One option Ian suggested would be
>> to have a GPL subdirectory in XTF which clearly separates GPL content
>> from BSD content. The resulting tests would become GPL, but the primary
>> distribution method is "git clone && make", so there are no issues
>> there.
>
> If I want to use the BSD-licensed files individually in other
> projects, will I need to keep a GPL license with them?
Only if the GPL file needed to be ported as well. In this case, it is
only the VMX test which uses it, rather than any of the core code.
I don't plan on porting entire files to other projects, but individual
functions certainly. (Having said that, I do plan on moving the entire
vsnprintf() implementation to hvmloader so it can actually format 64bit
integers).
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [licensing] was: [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld
2016-12-16 19:03 ` [licensing] was: " Andrew Cooper
2016-12-19 3:20 ` Haozhong Zhang
@ 2016-12-19 15:20 ` Roger Pau Monné
2016-12-19 15:24 ` Andrew Cooper
1 sibling, 1 reply; 56+ messages in thread
From: Roger Pau Monné @ 2016-12-19 15:20 UTC (permalink / raw)
To: Andrew Cooper
Cc: Haozhong Zhang, Lars Kurth, Ian Jackson, Kevin Tian, xen-devel
On Fri, Dec 16, 2016 at 07:03:49PM +0000, Andrew Cooper wrote:
> On 16/12/16 13:43, Haozhong Zhang wrote:
> > diff --git a/include/arch/x86/hvm/vmx/vmcs.h b/include/arch/x86/hvm/vmx/vmcs.h
> > new file mode 100644
> > index 0000000..e1a6ef8
> > --- /dev/null
> > +++ b/include/arch/x86/hvm/vmx/vmcs.h
> > @@ -0,0 +1,179 @@
> > +#ifndef XTF_X86_HVM_VMX_VMCS_H
> > +#define XTF_X86_HVM_VMX_VMCS_H
> > +
> > +/* VMCS field encodings. */
> > +#define VMCS_HIGH(x) ((x) | 1)
> > +enum vmcs_field {
> > + VIRTUAL_PROCESSOR_ID = 0x00000000,
> > + POSTED_INTR_NOTIFICATION_VECTOR = 0x00000002,
> > + EPTP_INDEX = 0x00000004,
> > +#define GUEST_SEG_SELECTOR(sel) (GUEST_ES_SELECTOR + (sel) * 2) /* ES ... GS */
>
> Unfortunately, there is probably a BSD/GPL licensing issue here.
>
> XTF is BSD clause 2 licensed, because a lot of the core microkernel bits
> are generally useful to other microkernel projects, and I have specific
> plans to contribute improvements back to the likes of Mini-OS and
> HVMLoader. I would specifically like to maintain this property.
>
> Xen, following its Linux heritage, is strictly GPLv2 (other than the
> public headers, which are specifically different).
IANAL, but it seems quite weird to me that a set of defines or enums (without
any logic behind) can be put under a specific license, which seems to be the
case here. Certainly this is publicly available on the SDM, and it won't be
surprising for someone to code them in the exact same way AFAICT.
Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [licensing] was: [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld
2016-12-19 15:20 ` Roger Pau Monné
@ 2016-12-19 15:24 ` Andrew Cooper
0 siblings, 0 replies; 56+ messages in thread
From: Andrew Cooper @ 2016-12-19 15:24 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Haozhong Zhang, Lars Kurth, Ian Jackson, Kevin Tian, xen-devel
On 19/12/16 15:20, Roger Pau Monné wrote:
> On Fri, Dec 16, 2016 at 07:03:49PM +0000, Andrew Cooper wrote:
>> On 16/12/16 13:43, Haozhong Zhang wrote:
>>> diff --git a/include/arch/x86/hvm/vmx/vmcs.h b/include/arch/x86/hvm/vmx/vmcs.h
>>> new file mode 100644
>>> index 0000000..e1a6ef8
>>> --- /dev/null
>>> +++ b/include/arch/x86/hvm/vmx/vmcs.h
>>> @@ -0,0 +1,179 @@
>>> +#ifndef XTF_X86_HVM_VMX_VMCS_H
>>> +#define XTF_X86_HVM_VMX_VMCS_H
>>> +
>>> +/* VMCS field encodings. */
>>> +#define VMCS_HIGH(x) ((x) | 1)
>>> +enum vmcs_field {
>>> + VIRTUAL_PROCESSOR_ID = 0x00000000,
>>> + POSTED_INTR_NOTIFICATION_VECTOR = 0x00000002,
>>> + EPTP_INDEX = 0x00000004,
>>> +#define GUEST_SEG_SELECTOR(sel) (GUEST_ES_SELECTOR + (sel) * 2) /* ES ... GS */
>> Unfortunately, there is probably a BSD/GPL licensing issue here.
>>
>> XTF is BSD clause 2 licensed, because a lot of the core microkernel bits
>> are generally useful to other microkernel projects, and I have specific
>> plans to contribute improvements back to the likes of Mini-OS and
>> HVMLoader. I would specifically like to maintain this property.
>>
>> Xen, following its Linux heritage, is strictly GPLv2 (other than the
>> public headers, which are specifically different).
> IANAL, but it seems quite weird to me that a set of defines or enums (without
> any logic behind) can be put under a specific license, which seems to be the
> case here. Certainly this is publicly available on the SDM, and it won't be
> surprising for someone to code them in the exact same way AFAICT.
Frankly, I was already on the fence with this, but Ian had a stronger
opinion.
If it were just the names, I wouldn't have raised an issue at all.
After all, it is just a C implementation of an ABI described in the
Intel manual, and there are only a handful of ways to actually do that
and also end up something which compiles.
The extra comments and macros however, are more dubious.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld
2016-12-16 13:43 ` [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld Haozhong Zhang
2016-12-16 19:03 ` [licensing] was: " Andrew Cooper
@ 2016-12-16 19:47 ` Andrew Cooper
2016-12-19 3:00 ` Haozhong Zhang
1 sibling, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 19:47 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel
On 16/12/16 13:43, Haozhong Zhang wrote:
Review of the technical side of the patch, leaving the licensing to the
other thread.
Reordering for better logical clarity for my suggestion.
> diff --git a/tests/vvmx/util.h b/tests/vvmx/util.h
> new file mode 100644
> index 0000000..57d3398
> --- /dev/null
> +++ b/tests/vvmx/util.h
> @@ -0,0 +1,78 @@
> +#ifndef XTF_TESTS_VVMX_UTIL_H
> +#define XTF_TESTS_VVMX_UTIL_H
> +
> +#include <arch/x86/exinfo.h>
> +#include <arch/x86/hvm/vmx/vmcs.h>
> +
> +/**
> + * Flags for errors during the execution of a VMX instruction.
> + *
> + * NB. Besides VMXERR_NOERR, other flags are not mutually exclusive,
> + * because we should not assume Xen implements the nested VMX
> + * instructions correctly.
I understand what you are trying to say here, although phrasing it like
this isn't great.
How about "these flags are not mutually exclusive because we need to
test all aspects of Xen's behaviour" ?
> + */
> +#define VMXERR_NOERR 0
> +#define VMXERR_VMFAIL_VALID (1 << 0)
> +#define VMXERR_VMFAIL_INVALID (1 << 1)
> +#define VMXERR_FAULT (1 << 2)
As for the flags themselves, they are very closely related to the
existing exinfo_t infrastructure, and I see that you pass around quite a
lot of fault/error state below.
There are 23 spare bits in exinfo_t, and I don't forsee any more general
use in it.
XTF already has things like GDTE_AVAIL{0...3} which are dedicated
resources available to tests. If it helps simplify the logic later, I
think it would be entirely reasonable to formally reserve some bits in
extinfo_t for test-specific use.
For example, something like:
diff --git a/include/arch/x86/exinfo.h b/include/arch/x86/exinfo.h
index eef39b6..1e9c10b 100644
--- a/include/arch/x86/exinfo.h
+++ b/include/arch/x86/exinfo.h
@@ -13,6 +13,7 @@
*
* - Bottom 16 bits are error code
* - Next 8 bits are the entry vector
+ * - Next 4 bits reserved for test-specific information
* - Top bit it set to disambiguate @#DE from no exception
*/
typedef unsigned int exinfo_t;
@@ -33,6 +34,9 @@ static inline unsigned int exinfo_ec(exinfo_t info)
return info & 0xffff;
}
+/* Bits reserved for test-specific information. */
+#define EXINFO_BIT_AVAIL0 24
+#define EXINFO_BIT_AVAIL1 25
+#define EXINFO_BIT_AVAIL2 26
+#define EXINFO_BIT_AVAIL3 27
+#define EXINFO_AVAIL_MASK (0xfu << EXINFO_BIT_AVAIL0)
+
#endif /* XTF_X86_EXINFO_H */
/*
and,
#define VMXERR_VMFAIL_VALID (1u << EXINFO_BIT_AVAIL0)
#define VMXERR_VMFAIL_INVALID (1u << EXINFO_BIT_AVAIL1)
which would allow you to return all vmx-instruction related error
information in a single exinfo_t.
> diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c
> new file mode 100644
> index 0000000..8cd35c5
> --- /dev/null
> +++ b/tests/vvmx/util.c
> @@ -0,0 +1,83 @@
> +#include <xtf.h>
> +#include <arch/x86/hvm/vmx/vmcs.h>
> +#include "util.h"
> +
> +#define VMPTRLD_OPCODE ".byte 0x0f,0xc7\n" /* reg/opcode: /6 */
> +#define VMREAD_OPCODE ".byte 0x0f,0x78\n"
> +#define VMXON_OPCODE ".byte 0xf3,0x0f,0xc7\n"
> +
> +#define MODRM_EAX_06 ".byte 0x30\n" /* [EAX], with reg/opcode: /6 */
> +#define MODRM_EAX_ECX ".byte 0xc1\n" /* EAX, ECX */
> +
> +uint8_t vmxon(uint64_t paddr, exinfo_t *fault_info)
paddr_t please.
> +{
> + exinfo_t fault = 0;
> + uint8_t valid = 0, invalid = 0;
> +
> + asm volatile("1: "VMXON_OPCODE MODRM_EAX_06 "\n\t"
> + " setc %0; setz %1 \n\t"
I tend not to use \n\t in inline assembly, because it detracts from the
C code, and have never found it useful for the few occasions I have
needed to debug a .E file.
> + "2: \n\t"
> + _ASM_EXTABLE_HANDLER(1b, 2b, ex_record_fault_edi)
> + : "=q" (invalid), "=q" (valid), "+D" (fault)
> + : "a" (&paddr)
> + : "memory", "cc");
"cc" clobbers are unilaterally assumed for x86. No need to specify them.
> +
> + if ( fault && fault_info )
> + *fault_info = fault;
> +
> + return (fault ? VMXERR_FAULT : 0) |
> + (valid ? VMXERR_VMFAIL_VALID : 0) |
> + (invalid ? VMXERR_VMFAIL_INVALID : 0);
If you chose to use the exinfo_t proposal above, you can do something
rather more cunning here.
bool ex_record_vmxerr_edi(struct cpu_regs *regs, const struct
extable_entry *ex)
{
if ( regs->flags & X86_EFLAGS_CF )
regs->di |= VMXERR_VMFAIL_INVALID;
if ( regs->flags & X86_EFLAGS_ZF )
regs->di |= VMXERR_VMFAIL_VALID;
regs->ip = ex->fixup;
return true;
}
and
asm volatile ("1:" VMXON_OPCODE MODRM_EAX_06
"2: ud2a;"
"3:"
_ASM_EXTABLE_HANDLER(1b, 3b, ex_record_fault_edi)
_ASM_EXTABLE_HANDLER(2b, 3b, ex_record_vmxerr_edi)
...
This would avoid the repetitive common tail to all of these functions,
and get all of your error information into edi in one go.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld
2016-12-16 19:47 ` Andrew Cooper
@ 2016-12-19 3:00 ` Haozhong Zhang
0 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 3:00 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/16/16 19:47 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>
>Review of the technical side of the patch, leaving the licensing to the
>other thread.
>
>Reordering for better logical clarity for my suggestion.
>
>> diff --git a/tests/vvmx/util.h b/tests/vvmx/util.h
>> new file mode 100644
>> index 0000000..57d3398
>> --- /dev/null
>> +++ b/tests/vvmx/util.h
>> @@ -0,0 +1,78 @@
>> +#ifndef XTF_TESTS_VVMX_UTIL_H
>> +#define XTF_TESTS_VVMX_UTIL_H
>> +
>> +#include <arch/x86/exinfo.h>
>> +#include <arch/x86/hvm/vmx/vmcs.h>
>> +
>> +/**
>> + * Flags for errors during the execution of a VMX instruction.
>> + *
>> + * NB. Besides VMXERR_NOERR, other flags are not mutually exclusive,
>> + * because we should not assume Xen implements the nested VMX
>> + * instructions correctly.
>
>I understand what you are trying to say here, although phrasing it like
>this isn't great.
>
>How about "these flags are not mutually exclusive because we need to
>test all aspects of Xen's behaviour" ?
>
Yes, I will use your suggestion.
>> + */
>> +#define VMXERR_NOERR 0
>> +#define VMXERR_VMFAIL_VALID (1 << 0)
>> +#define VMXERR_VMFAIL_INVALID (1 << 1)
>> +#define VMXERR_FAULT (1 << 2)
>
>As for the flags themselves, they are very closely related to the
>existing exinfo_t infrastructure, and I see that you pass around quite a
>lot of fault/error state below.
>
>There are 23 spare bits in exinfo_t, and I don't forsee any more general
>use in it.
>
>XTF already has things like GDTE_AVAIL{0...3} which are dedicated
>resources available to tests. If it helps simplify the logic later, I
>think it would be entirely reasonable to formally reserve some bits in
>extinfo_t for test-specific use.
>
>For example, something like:
>
>diff --git a/include/arch/x86/exinfo.h b/include/arch/x86/exinfo.h
>index eef39b6..1e9c10b 100644
>--- a/include/arch/x86/exinfo.h
>+++ b/include/arch/x86/exinfo.h
>@@ -13,6 +13,7 @@
> *
> * - Bottom 16 bits are error code
> * - Next 8 bits are the entry vector
>+ * - Next 4 bits reserved for test-specific information
Great! It doesn't only help to simplify the logic in this patch, but
also helps later patches that pass VMXERR_ flags from
exec_user(). Thanks for the suggestion!
> * - Top bit it set to disambiguate @#DE from no exception
> */
> typedef unsigned int exinfo_t;
>@@ -33,6 +34,9 @@ static inline unsigned int exinfo_ec(exinfo_t info)
> return info & 0xffff;
> }
>
>+/* Bits reserved for test-specific information. */
>+#define EXINFO_BIT_AVAIL0 24
>+#define EXINFO_BIT_AVAIL1 25
>+#define EXINFO_BIT_AVAIL2 26
>+#define EXINFO_BIT_AVAIL3 27
>+#define EXINFO_AVAIL_MASK (0xfu << EXINFO_BIT_AVAIL0)
>+
> #endif /* XTF_X86_EXINFO_H */
>
> /*
>
>and,
>
>#define VMXERR_VMFAIL_VALID (1u << EXINFO_BIT_AVAIL0)
>#define VMXERR_VMFAIL_INVALID (1u << EXINFO_BIT_AVAIL1)
>
>which would allow you to return all vmx-instruction related error
>information in a single exinfo_t.
>
>> diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c
>> new file mode 100644
>> index 0000000..8cd35c5
>> --- /dev/null
>> +++ b/tests/vvmx/util.c
>> @@ -0,0 +1,83 @@
>> +#include <xtf.h>
>> +#include <arch/x86/hvm/vmx/vmcs.h>
>> +#include "util.h"
>> +
>> +#define VMPTRLD_OPCODE ".byte 0x0f,0xc7\n" /* reg/opcode: /6 */
>> +#define VMREAD_OPCODE ".byte 0x0f,0x78\n"
>> +#define VMXON_OPCODE ".byte 0xf3,0x0f,0xc7\n"
>> +
>> +#define MODRM_EAX_06 ".byte 0x30\n" /* [EAX], with reg/opcode: /6 */
>> +#define MODRM_EAX_ECX ".byte 0xc1\n" /* EAX, ECX */
>> +
>> +uint8_t vmxon(uint64_t paddr, exinfo_t *fault_info)
>
>paddr_t please.
>
will change
>> +{
>> + exinfo_t fault = 0;
>> + uint8_t valid = 0, invalid = 0;
>> +
>> + asm volatile("1: "VMXON_OPCODE MODRM_EAX_06 "\n\t"
>> + " setc %0; setz %1 \n\t"
>
>I tend not to use \n\t in inline assembly, because it detracts from the
>C code, and have never found it useful for the few occasions I have
>needed to debug a .E file.
>
will remove them
>> + "2: \n\t"
>> + _ASM_EXTABLE_HANDLER(1b, 2b, ex_record_fault_edi)
>> + : "=q" (invalid), "=q" (valid), "+D" (fault)
>> + : "a" (&paddr)
>> + : "memory", "cc");
>
>"cc" clobbers are unilaterally assumed for x86. No need to specify them.
>
ditto
>> +
>> + if ( fault && fault_info )
>> + *fault_info = fault;
>> +
>> + return (fault ? VMXERR_FAULT : 0) |
>> + (valid ? VMXERR_VMFAIL_VALID : 0) |
>> + (invalid ? VMXERR_VMFAIL_INVALID : 0);
>
>If you chose to use the exinfo_t proposal above, you can do something
>rather more cunning here.
>
>bool ex_record_vmxerr_edi(struct cpu_regs *regs, const struct
>extable_entry *ex)
>{
> if ( regs->flags & X86_EFLAGS_CF )
> regs->di |= VMXERR_VMFAIL_INVALID;
> if ( regs->flags & X86_EFLAGS_ZF )
> regs->di |= VMXERR_VMFAIL_VALID;
>
> regs->ip = ex->fixup;
>
> return true;
>}
>
>and
>
> asm volatile ("1:" VMXON_OPCODE MODRM_EAX_06
> "2: ud2a;"
> "3:"
> _ASM_EXTABLE_HANDLER(1b, 3b, ex_record_fault_edi)
> _ASM_EXTABLE_HANDLER(2b, 3b, ex_record_vmxerr_edi)
> ...
>
>This would avoid the repetitive common tail to all of these functions,
>and get all of your error information into edi in one go.
>
Exactly, thanks!
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [XTF PATCH 05/16] vvmx: add a general error handler for VMX instructions
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (3 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 04/16] vvmx: add C wrappers of vmxon/vmread/vmptrld Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 20:16 ` Andrew Cooper
2016-12-16 13:43 ` [XTF PATCH 06/16] vvmx: test vmxon with CR4.VMXE cleared Haozhong Zhang
` (12 subsequent siblings)
17 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
handle_vmxinsn_err() is added to check and output the mismatch between
errors in the execution of a VMX instruction and the expected errors.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/util.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
tests/vvmx/util.h | 17 +++++++++
2 files changed, 121 insertions(+)
diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c
index 8cd35c5..74b4d01 100644
--- a/tests/vvmx/util.c
+++ b/tests/vvmx/util.c
@@ -2,6 +2,110 @@
#include <arch/x86/hvm/vmx/vmcs.h>
#include "util.h"
+#define vvmx_failure(prefix, fmt, ...) \
+ do { \
+ xtf_failure("Fail: %s: "fmt, prefix, ##__VA_ARGS__); \
+ } while (0)
+
+bool handle_vmxinsn_err(const char *name, uint8_t err, exinfo_t fault_info,
+ uint8_t exp_err, exinfo_t exp_fault_info,
+ enum vmx_insn_errno exp_insn_errno)
+{
+ bool passed = true;
+
+ bool has_fault = !!(err & VMXERR_FAULT);
+ unsigned int fault_vec = exinfo_vec(fault_info);
+ unsigned int fault_ec = exinfo_ec(fault_info);
+ bool exp_fault = !!(exp_err & VMXERR_FAULT);
+ unsigned int exp_fault_vec = exinfo_vec(exp_fault_info);
+ bool ec = !!(exp_fault_vec & X86_EXC_HAVE_EC);
+ unsigned int exp_fault_ec = ec ? exinfo_ec(exp_fault_info) : 0;
+
+ if ( !exp_fault && has_fault )
+ {
+ vvmx_failure(name,
+ "unexpected fault #%u(%u), but no fault is expected\n",
+ fault_vec, fault_ec);
+ passed = false;
+ }
+ else if ( exp_fault && !has_fault )
+ {
+ vvmx_failure(name, "no fault, but fault #%u(%u) is expected\n",
+ exp_fault_vec, exp_fault_ec);
+ passed = false;
+ }
+ else if ( has_fault &&
+ (fault_vec != exp_fault_vec || (ec && fault_ec != exp_fault_ec)) )
+ {
+ vvmx_failure(name,
+ "unexpected fault #%u(%u), but #%u(%u) is expected\n",
+ fault_vec, fault_ec, exp_fault_vec, exp_fault_ec);
+ passed = false;
+ }
+
+ /* VMfail bits are not set by VMX instructions when a fault happens */
+ if ( has_fault )
+ goto out;
+
+ bool has_vmfail_valid = !!(err & VMXERR_VMFAIL_VALID);
+ bool exp_vmfail_valid = !!(exp_err & VMXERR_VMFAIL_VALID);
+ bool has_insn_errno = false;
+ uint64_t insn_errno = 0;
+
+ if ( has_vmfail_valid )
+ {
+ has_insn_errno = !!vmread(VM_INSTRUCTION_ERROR, &insn_errno, NULL);
+
+ if ( has_insn_errno )
+ {
+ vvmx_failure(name,
+ "unexpected error when vmread VM_INSTRUCTION_ERROR\n");
+ passed = false;
+ }
+ }
+
+ if ( !exp_vmfail_valid && has_vmfail_valid )
+ {
+ vvmx_failure(name, "unexpected VMfailvalid(%d), "
+ "but no VMfailvalid is expected\n",
+ (enum vmx_insn_errno)insn_errno);
+ passed = false;
+ }
+ else if ( exp_vmfail_valid && !has_vmfail_valid )
+ {
+ vvmx_failure(name, "no VMfailvalid, but VMfailvalid(%d) is expected\n",
+ exp_insn_errno);
+ passed = false;
+ }
+ else if ( has_vmfail_valid &&
+ has_insn_errno && insn_errno != exp_insn_errno )
+ {
+ vvmx_failure(name, "unexpected VMfailvalid(%d), "
+ "but VMfailvalid(%d) is expected\n",
+ (enum vmx_insn_errno)insn_errno, exp_insn_errno);
+ passed = false;
+ }
+
+ bool has_vmfail_invalid = !!(err & VMXERR_VMFAIL_INVALID);
+ bool exp_vmfail_invalid = !!(exp_err & VMXERR_VMFAIL_INVALID);
+
+ if ( !exp_vmfail_invalid && has_vmfail_invalid )
+ {
+ vvmx_failure(name, "unexpected VMfailInvalid, "
+ "but no VMfailInvalid is expected\n");
+ passed = false;
+ }
+ else if ( exp_vmfail_invalid && !has_vmfail_invalid )
+ {
+ vvmx_failure(name,
+ "no VMfailInvalid, but VMfailInvalid is expected\n");
+ passed = false;
+ }
+
+out:
+ return passed;
+}
+
#define VMPTRLD_OPCODE ".byte 0x0f,0xc7\n" /* reg/opcode: /6 */
#define VMREAD_OPCODE ".byte 0x0f,0x78\n"
#define VMXON_OPCODE ".byte 0xf3,0x0f,0xc7\n"
diff --git a/tests/vvmx/util.h b/tests/vvmx/util.h
index 57d3398..f18b9cc 100644
--- a/tests/vvmx/util.h
+++ b/tests/vvmx/util.h
@@ -17,6 +17,23 @@
#define VMXERR_FAULT (1 << 2)
/**
+ * A general function to check and output the mismatch between the
+ * error in the execution of a VMX instruction and the expected error.
+ *
+ * Parameters:
+ * @name: the name of the test and is included in the error messages
+ * @err: VMXERR_ flags returned by the execution of a VMX instruction
+ * @fault: the information of the fault if (@err & VMXERR_FAULT)
+ * @exp_err: expected VMXERR_ flags
+ * @exp_fault: the information of the expected fault if (exp_err & VMXERR_FAULT)
+ * @exp_errno: the expected VMX instruction error number if
+ * (exp_err & VMXERR_VMFAIL_VALID)
+ */
+bool handle_vmxinsn_err(const char *name, uint8_t err, exinfo_t fault,
+ uint8_t exp_err, exinfo_t exp_fault,
+ enum vmx_insn_errno exp_errno);
+
+/**
* vmxon
*
* Parameters:
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [XTF PATCH 05/16] vvmx: add a general error handler for VMX instructions
2016-12-16 13:43 ` [XTF PATCH 05/16] vvmx: add a general error handler for VMX instructions Haozhong Zhang
@ 2016-12-16 20:16 ` Andrew Cooper
2016-12-19 3:24 ` Haozhong Zhang
0 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 20:16 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel
On 16/12/16 13:43, Haozhong Zhang wrote:
> handle_vmxinsn_err() is added to check and output the mismatch between
> errors in the execution of a VMX instruction and the expected errors.
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
> tests/vvmx/util.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/vvmx/util.h | 17 +++++++++
> 2 files changed, 121 insertions(+)
>
> diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c
> index 8cd35c5..74b4d01 100644
> --- a/tests/vvmx/util.c
> +++ b/tests/vvmx/util.c
> @@ -2,6 +2,110 @@
> #include <arch/x86/hvm/vmx/vmcs.h>
> #include "util.h"
>
> +#define vvmx_failure(prefix, fmt, ...) \
> + do { \
> + xtf_failure("Fail: %s: "fmt, prefix, ##__VA_ARGS__); \
> + } while (0)
I don't see this used anywhere else in the series. Is it only for the
function below?
> +
> +bool handle_vmxinsn_err(const char *name, uint8_t err, exinfo_t fault_info,
> + uint8_t exp_err, exinfo_t exp_fault_info,
> + enum vmx_insn_errno exp_insn_errno)
> +{
> + bool passed = true;
> +
> + bool has_fault = !!(err & VMXERR_FAULT);
C99 bools don't need !!, as they have that property guaranteed by the
standard.
It came as a shock to me when Xen's old bool_t's didn't have that
property, which is what prompted me to fix Xen. In particular,
static inline bool_t (void) { return x & 0x1000; }
used to truncate to 0 regardless of the value of x, which was
desperately unhelpful.
> + unsigned int fault_vec = exinfo_vec(fault_info);
> + unsigned int fault_ec = exinfo_ec(fault_info);
> + bool exp_fault = !!(exp_err & VMXERR_FAULT);
> + unsigned int exp_fault_vec = exinfo_vec(exp_fault_info);
> + bool ec = !!(exp_fault_vec & X86_EXC_HAVE_EC);
> + unsigned int exp_fault_ec = ec ? exinfo_ec(exp_fault_info) : 0;
> +
> + if ( !exp_fault && has_fault )
> + {
> + vvmx_failure(name,
> + "unexpected fault #%u(%u), but no fault is expected\n",
> + fault_vec, fault_ec);
Please use x86_decode_exinfo(), which formats an exinfo_t with mnemonics.
I already have half a patch series to introduce a %p for exinfo_t. I
should clean the series up and post it.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 05/16] vvmx: add a general error handler for VMX instructions
2016-12-16 20:16 ` Andrew Cooper
@ 2016-12-19 3:24 ` Haozhong Zhang
0 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 3:24 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/16/16 20:16 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>> handle_vmxinsn_err() is added to check and output the mismatch between
>> errors in the execution of a VMX instruction and the expected errors.
>>
>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>> ---
>> tests/vvmx/util.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> tests/vvmx/util.h | 17 +++++++++
>> 2 files changed, 121 insertions(+)
>>
>> diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c
>> index 8cd35c5..74b4d01 100644
>> --- a/tests/vvmx/util.c
>> +++ b/tests/vvmx/util.c
>> @@ -2,6 +2,110 @@
>> #include <arch/x86/hvm/vmx/vmcs.h>
>> #include "util.h"
>>
>> +#define vvmx_failure(prefix, fmt, ...) \
>> + do { \
>> + xtf_failure("Fail: %s: "fmt, prefix, ##__VA_ARGS__); \
>> + } while (0)
>
>I don't see this used anywhere else in the series. Is it only for the
>function below?
>
Yes, only for handle_vmxinsn_err().
>> +
>> +bool handle_vmxinsn_err(const char *name, uint8_t err, exinfo_t fault_info,
>> + uint8_t exp_err, exinfo_t exp_fault_info,
>> + enum vmx_insn_errno exp_insn_errno)
>> +{
>> + bool passed = true;
>> +
>> + bool has_fault = !!(err & VMXERR_FAULT);
>
>C99 bools don't need !!, as they have that property guaranteed by the
>standard.
>
I'll remove !! in all these patches.
>It came as a shock to me when Xen's old bool_t's didn't have that
>property, which is what prompted me to fix Xen. In particular,
>
>static inline bool_t (void) { return x & 0x1000; }
>
>used to truncate to 0 regardless of the value of x, which was
>desperately unhelpful.
>
>> + unsigned int fault_vec = exinfo_vec(fault_info);
>> + unsigned int fault_ec = exinfo_ec(fault_info);
>> + bool exp_fault = !!(exp_err & VMXERR_FAULT);
>> + unsigned int exp_fault_vec = exinfo_vec(exp_fault_info);
>> + bool ec = !!(exp_fault_vec & X86_EXC_HAVE_EC);
>> + unsigned int exp_fault_ec = ec ? exinfo_ec(exp_fault_info) : 0;
>> +
>> + if ( !exp_fault && has_fault )
>> + {
>> + vvmx_failure(name,
>> + "unexpected fault #%u(%u), but no fault is expected\n",
>> + fault_vec, fault_ec);
>
>Please use x86_decode_exinfo(), which formats an exinfo_t with mnemonics.
>
Sure, will change.
>I already have half a patch series to introduce a %p for exinfo_t. I
>should clean the series up and post it.
>
>~Andrew
Thanks,
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [XTF PATCH 06/16] vvmx: test vmxon with CR4.VMXE cleared
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (4 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 05/16] vvmx: add a general error handler for VMX instructions Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 20:25 ` Andrew Cooper
2016-12-16 13:43 ` [XTF PATCH 07/16] vvmx: test vmxon in CPL=3 and out of VMX operation Haozhong Zhang
` (11 subsequent siblings)
17 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
Fault #UD is expected in this test.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
include/arch/x86/msr-index.h | 1 +
tests/vvmx/Makefile | 2 +-
tests/vvmx/main.c | 4 ++++
tests/vvmx/util.c | 24 ++++++++++++++++++++++
tests/vvmx/util.h | 18 +++++++++++++++++
tests/vvmx/vmxon.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/include/arch/x86/msr-index.h b/include/arch/x86/msr-index.h
index b7aeef0..ce5d068 100644
--- a/include/arch/x86/msr-index.h
+++ b/include/arch/x86/msr-index.h
@@ -17,6 +17,7 @@
#define MSR_MISC_FEATURES_CPUID_FAULTING (1ULL << _MSR_MISC_FEATURES_CPUID_FAULTING)
#define MSR_IA32_VMX_BASIC 0x00000480
+#define VMX_BASIC_REVISION_MASK 0x7fffffff
#define VMX_BASIC_VMCS_SIZE_MASK (0x1fffULL << 32)
#define VMX_BASIC_32BIT_ADDRESSES (1ULL << 48)
diff --git a/tests/vvmx/Makefile b/tests/vvmx/Makefile
index 24d3720..fecb7c1 100644
--- a/tests/vvmx/Makefile
+++ b/tests/vvmx/Makefile
@@ -6,6 +6,6 @@ TEST-ENVS := hvm64
TEST-EXTRA-CFG := extra.cfg.in
-obj-perenv += main.o cpuid.o msr.o util.o
+obj-perenv += main.o cpuid.o msr.o util.o vmxon.o
include $(ROOT)/build/gen.mk
diff --git a/tests/vvmx/main.c b/tests/vvmx/main.c
index bd36f10..cec9057 100644
--- a/tests/vvmx/main.c
+++ b/tests/vvmx/main.c
@@ -15,6 +15,7 @@ const char test_title[] = "Test vvmx";
extern bool test_cpuid_vmx_feat(void);
extern bool test_msr_vmx(void);
+extern bool test_vmxon(void);
void test_main(void)
{
@@ -30,6 +31,9 @@ void test_main(void)
if ( !test_msr_vmx() )
goto fail;
+ if ( !test_vmxon() )
+ goto fail;
+
xtf_success(NULL);
return;
diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c
index 74b4d01..f30fc26 100644
--- a/tests/vvmx/util.c
+++ b/tests/vvmx/util.c
@@ -1,7 +1,31 @@
#include <xtf.h>
+#include <arch/x86/msr-index.h>
#include <arch/x86/hvm/vmx/vmcs.h>
#include "util.h"
+#define INVALID_VMCS_REVID (~(uint32_t)0)
+
+static uint32_t vmcs_revid = INVALID_VMCS_REVID;
+
+uint32_t get_vmcs_revid(void)
+{
+ if ( vmcs_revid != INVALID_VMCS_REVID )
+ goto out;
+
+ uint64_t vmx_basic = rdmsr(MSR_IA32_VMX_BASIC);
+
+ vmcs_revid = (uint32_t)vmx_basic & VMX_BASIC_REVISION_MASK;
+
+out:
+ return vmcs_revid;
+}
+
+void clear_vmcs(void *vmcs, uint32_t revid)
+{
+ memset(vmcs, 0, PAGE_SIZE);
+ *((uint32_t *)vmcs) = revid;
+}
+
#define vvmx_failure(prefix, fmt, ...) \
do { \
xtf_failure("Fail: %s: "fmt, prefix, ##__VA_ARGS__); \
diff --git a/tests/vvmx/util.h b/tests/vvmx/util.h
index f18b9cc..c7b1c99 100644
--- a/tests/vvmx/util.h
+++ b/tests/vvmx/util.h
@@ -5,6 +5,24 @@
#include <arch/x86/hvm/vmx/vmcs.h>
/**
+ * Get a valid VMCS revision ID from MSR_IA32_VMX_BASIC.
+ *
+ * Return:
+ * VMCS revision ID
+ */
+uint32_t get_vmcs_revid(void);
+
+/**
+ * Zero a VMXON region or VMCS and set the VMCS revision ID.
+ *
+ * Parameters:
+ * @vmcs: pointer to VMXON region or VMCS
+ * @revid: the VMCS revision ID to be used. It's not required
+ * to be a valid revision ID.
+ */
+void clear_vmcs(void *vmcs, uint32_t revid);
+
+/**
* Flags for errors during the execution of a VMX instruction.
*
* NB. Besides VMXERR_NOERR, other flags are not mutually exclusive,
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
index e69de29..31f074c 100644
--- a/tests/vvmx/vmxon.c
+++ b/tests/vvmx/vmxon.c
@@ -0,0 +1,47 @@
+#include <xtf.h>
+
+#include "util.h"
+
+static uint8_t vmxon_region[PAGE_SIZE] __aligned(PAGE_SIZE);
+
+/**
+ * vmxon with CR4.VMXE cleared
+ *
+ * Expect: #UD
+ */
+static bool test_vmxon_novmxe(void)
+{
+ uint8_t ret;
+ exinfo_t fault;
+
+ if ( read_cr4() & X86_CR4_VMXE )
+ {
+ xtf_skip("Skip: CR4.VMXE is already set, "
+ "skip testing vmxon w/ CR4.VMXE=0\n");
+ return true;
+ }
+
+ clear_vmcs(vmxon_region, get_vmcs_revid());
+ ret = vmxon((uint64_t)vmxon_region, &fault);
+
+ return handle_vmxinsn_err(__func__, ret, fault,
+ VMXERR_FAULT, EXINFO_SYM(UD, 0), 0);
+}
+
+bool test_vmxon(void)
+{
+ if ( !test_vmxon_novmxe() )
+ return false;
+
+ return true;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [XTF PATCH 06/16] vvmx: test vmxon with CR4.VMXE cleared
2016-12-16 13:43 ` [XTF PATCH 06/16] vvmx: test vmxon with CR4.VMXE cleared Haozhong Zhang
@ 2016-12-16 20:25 ` Andrew Cooper
2016-12-19 3:26 ` Haozhong Zhang
0 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 20:25 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel
On 16/12/16 13:43, Haozhong Zhang wrote:
> diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c
> index 74b4d01..f30fc26 100644
> --- a/tests/vvmx/util.c
> +++ b/tests/vvmx/util.c
> @@ -1,7 +1,31 @@
> #include <xtf.h>
> +#include <arch/x86/msr-index.h>
> #include <arch/x86/hvm/vmx/vmcs.h>
> #include "util.h"
>
> +#define INVALID_VMCS_REVID (~(uint32_t)0)
> +
> +static uint32_t vmcs_revid = INVALID_VMCS_REVID;
> +
> +uint32_t get_vmcs_revid(void)
> +{
> + if ( vmcs_revid != INVALID_VMCS_REVID )
> + goto out;
> +
> + uint64_t vmx_basic = rdmsr(MSR_IA32_VMX_BASIC);
> +
> + vmcs_revid = (uint32_t)vmx_basic & VMX_BASIC_REVISION_MASK;
> +
> +out:
> + return vmcs_revid;
> +}
> +
> +void clear_vmcs(void *vmcs, uint32_t revid)
Strictly speaking, this should be setup_vmcs() as it does more than just
clearing it.
> +{
> + memset(vmcs, 0, PAGE_SIZE);
> + *((uint32_t *)vmcs) = revid;
> +}
> +
> #define vvmx_failure(prefix, fmt, ...) \
> do { \
> xtf_failure("Fail: %s: "fmt, prefix, ##__VA_ARGS__); \
> diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
> index e69de29..31f074c 100644
> --- a/tests/vvmx/vmxon.c
> +++ b/tests/vvmx/vmxon.c
> @@ -0,0 +1,47 @@
> +#include <xtf.h>
> +
> +#include "util.h"
> +
> +static uint8_t vmxon_region[PAGE_SIZE] __aligned(PAGE_SIZE);
> +
> +/**
> + * vmxon with CR4.VMXE cleared
> + *
> + * Expect: #UD
> + */
> +static bool test_vmxon_novmxe(void)
> +{
> + uint8_t ret;
> + exinfo_t fault;
> +
> + if ( read_cr4() & X86_CR4_VMXE )
> + {
> + xtf_skip("Skip: CR4.VMXE is already set, "
> + "skip testing vmxon w/ CR4.VMXE=0\n");
> + return true;
Tests should, wherever possible, not depend on the starting state of the
microkernel. They are also entirely free to mess with any state they
want to construct a test scenario.
In this case, I would suggest...
> + }
> +
> + clear_vmcs(vmxon_region, get_vmcs_revid());
> + ret = vmxon((uint64_t)vmxon_region, &fault);
> +
> + return handle_vmxinsn_err(__func__, ret, fault,
> + VMXERR_FAULT, EXINFO_SYM(UD, 0), 0);
> +}
> +
> +bool test_vmxon(void)
> +{
unsigned long cr4 = read_cr4();
if ( cr4 & X86_CR4_VMXE )
write_cr4(cr4 & ~X86_CR4_VMXE);
to explicitly set up state as intended.
~Andrew
> + if ( !test_vmxon_novmxe() )
> + return false;
> +
> + return true;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 06/16] vvmx: test vmxon with CR4.VMXE cleared
2016-12-16 20:25 ` Andrew Cooper
@ 2016-12-19 3:26 ` Haozhong Zhang
0 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 3:26 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/16/16 20:25 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>> diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c
>> index 74b4d01..f30fc26 100644
>> --- a/tests/vvmx/util.c
>> +++ b/tests/vvmx/util.c
>> @@ -1,7 +1,31 @@
>> #include <xtf.h>
>> +#include <arch/x86/msr-index.h>
>> #include <arch/x86/hvm/vmx/vmcs.h>
>> #include "util.h"
>>
>> +#define INVALID_VMCS_REVID (~(uint32_t)0)
>> +
>> +static uint32_t vmcs_revid = INVALID_VMCS_REVID;
>> +
>> +uint32_t get_vmcs_revid(void)
>> +{
>> + if ( vmcs_revid != INVALID_VMCS_REVID )
>> + goto out;
>> +
>> + uint64_t vmx_basic = rdmsr(MSR_IA32_VMX_BASIC);
>> +
>> + vmcs_revid = (uint32_t)vmx_basic & VMX_BASIC_REVISION_MASK;
>> +
>> +out:
>> + return vmcs_revid;
>> +}
>> +
>> +void clear_vmcs(void *vmcs, uint32_t revid)
>
>Strictly speaking, this should be setup_vmcs() as it does more than just
>clearing it.
>
will change the name
>> +{
>> + memset(vmcs, 0, PAGE_SIZE);
>> + *((uint32_t *)vmcs) = revid;
>> +}
>> +
>> #define vvmx_failure(prefix, fmt, ...) \
>> do { \
>> xtf_failure("Fail: %s: "fmt, prefix, ##__VA_ARGS__); \
>> diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
>> index e69de29..31f074c 100644
>> --- a/tests/vvmx/vmxon.c
>> +++ b/tests/vvmx/vmxon.c
>> @@ -0,0 +1,47 @@
>> +#include <xtf.h>
>> +
>> +#include "util.h"
>> +
>> +static uint8_t vmxon_region[PAGE_SIZE] __aligned(PAGE_SIZE);
>> +
>> +/**
>> + * vmxon with CR4.VMXE cleared
>> + *
>> + * Expect: #UD
>> + */
>> +static bool test_vmxon_novmxe(void)
>> +{
>> + uint8_t ret;
>> + exinfo_t fault;
>> +
>> + if ( read_cr4() & X86_CR4_VMXE )
>> + {
>> + xtf_skip("Skip: CR4.VMXE is already set, "
>> + "skip testing vmxon w/ CR4.VMXE=0\n");
>> + return true;
>
>Tests should, wherever possible, not depend on the starting state of the
>microkernel. They are also entirely free to mess with any state they
>want to construct a test scenario.
>
>In this case, I would suggest...
>
>> + }
>> +
>> + clear_vmcs(vmxon_region, get_vmcs_revid());
>> + ret = vmxon((uint64_t)vmxon_region, &fault);
>> +
>> + return handle_vmxinsn_err(__func__, ret, fault,
>> + VMXERR_FAULT, EXINFO_SYM(UD, 0), 0);
>> +}
>> +
>> +bool test_vmxon(void)
>> +{
>
>unsigned long cr4 = read_cr4();
>
>if ( cr4 & X86_CR4_VMXE )
> write_cr4(cr4 & ~X86_CR4_VMXE);
>
>to explicitly set up state as intended.
>
will change to your suggestion.
Thanks,
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [XTF PATCH 07/16] vvmx: test vmxon in CPL=3 and out of VMX operation
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (5 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 06/16] vvmx: test vmxon with CR4.VMXE cleared Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 20:33 ` Andrew Cooper
2016-12-16 13:43 ` [XTF PATCH 08/16] vvmx: test vmxon with invalidly wide VMXON region address Haozhong Zhang
` (10 subsequent siblings)
17 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
Fault #GP(0) is expected in this test.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/main.c | 2 ++
tests/vvmx/vmxon.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/tests/vvmx/main.c b/tests/vvmx/main.c
index cec9057..c1852fd 100644
--- a/tests/vvmx/main.c
+++ b/tests/vvmx/main.c
@@ -13,6 +13,8 @@
const char test_title[] = "Test vvmx";
+bool test_wants_user_mappings = true;
+
extern bool test_cpuid_vmx_feat(void);
extern bool test_msr_vmx(void);
extern bool test_vmxon(void);
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
index 31f074c..ca33b3c 100644
--- a/tests/vvmx/vmxon.c
+++ b/tests/vvmx/vmxon.c
@@ -28,11 +28,42 @@ static bool test_vmxon_novmxe(void)
VMXERR_FAULT, EXINFO_SYM(UD, 0), 0);
}
+static unsigned long vmxon_in_user(void)
+{
+ exinfo_t fault;
+ unsigned long ret = vmxon((uint64_t)vmxon_region, &fault);
+
+ return (ret << 32) | fault;
+}
+
+/**
+ * vmxon in CPL=3 and out of VMX operation
+ *
+ * Expect: #GP(0)
+ */
+static bool test_vmxon_in_user(void)
+{
+ clear_vmcs(vmxon_region, get_vmcs_revid());
+
+ unsigned long ret = exec_user(vmxon_in_user);
+ uint8_t err = (ret >> 32) & 0xff;
+ exinfo_t fault = ret & 0xFFFFFFFF;
+
+ return handle_vmxinsn_err(__func__, err, fault,
+ VMXERR_FAULT, EXINFO_SYM(GP, 0), 0);
+}
+
bool test_vmxon(void)
{
if ( !test_vmxon_novmxe() )
return false;
+ unsigned long cr4 = read_cr4();
+ write_cr4(cr4 | X86_CR4_VMXE);
+
+ if ( !test_vmxon_in_user() )
+ return false;
+
return true;
}
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [XTF PATCH 07/16] vvmx: test vmxon in CPL=3 and out of VMX operation
2016-12-16 13:43 ` [XTF PATCH 07/16] vvmx: test vmxon in CPL=3 and out of VMX operation Haozhong Zhang
@ 2016-12-16 20:33 ` Andrew Cooper
2016-12-19 3:35 ` Haozhong Zhang
0 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 20:33 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel
On 16/12/16 13:43, Haozhong Zhang wrote:
> diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
> index 31f074c..ca33b3c 100644
> --- a/tests/vvmx/vmxon.c
> +++ b/tests/vvmx/vmxon.c
> @@ -28,11 +28,42 @@ static bool test_vmxon_novmxe(void)
> VMXERR_FAULT, EXINFO_SYM(UD, 0), 0);
> }
>
> +static unsigned long vmxon_in_user(void)
I'd name this user_vmxon() as it is slightly shorter, but I'm not
terribly fussed.
> +{
> + exinfo_t fault;
> + unsigned long ret = vmxon((uint64_t)vmxon_region, &fault);
> +
> + return (ret << 32) | fault;
> +}
> +
> +/**
> + * vmxon in CPL=3 and out of VMX operation
> + *
> + * Expect: #GP(0)
> + */
> +static bool test_vmxon_in_user(void)
Similarly, test_user_vmxon()
> +{
> + clear_vmcs(vmxon_region, get_vmcs_revid());
> +
> + unsigned long ret = exec_user(vmxon_in_user);
> + uint8_t err = (ret >> 32) & 0xff;
> + exinfo_t fault = ret & 0xFFFFFFFF;
> +
> + return handle_vmxinsn_err(__func__, err, fault,
> + VMXERR_FAULT, EXINFO_SYM(GP, 0), 0);
> +}
> +
> bool test_vmxon(void)
> {
> if ( !test_vmxon_novmxe() )
> return false;
Your subject says out of VMX operation, but the implementation is inside
VMX operation.
It would be worth testing both scenarios, as they should be
distinguished by #UD vs #GP[0].
~Andrew
>
> + unsigned long cr4 = read_cr4();
> + write_cr4(cr4 | X86_CR4_VMXE);
> +
> + if ( !test_vmxon_in_user() )
> + return false;
> +
> return true;
> }
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 07/16] vvmx: test vmxon in CPL=3 and out of VMX operation
2016-12-16 20:33 ` Andrew Cooper
@ 2016-12-19 3:35 ` Haozhong Zhang
2016-12-19 16:02 ` Andrew Cooper
2016-12-19 16:02 ` Andrew Cooper
0 siblings, 2 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 3:35 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/16/16 20:33 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>> diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
>> index 31f074c..ca33b3c 100644
>> --- a/tests/vvmx/vmxon.c
>> +++ b/tests/vvmx/vmxon.c
>> @@ -28,11 +28,42 @@ static bool test_vmxon_novmxe(void)
>> VMXERR_FAULT, EXINFO_SYM(UD, 0), 0);
>> }
>>
>> +static unsigned long vmxon_in_user(void)
>
>I'd name this user_vmxon() as it is slightly shorter, but I'm not
>terribly fussed.
>
>> +{
>> + exinfo_t fault;
>> + unsigned long ret = vmxon((uint64_t)vmxon_region, &fault);
>> +
>> + return (ret << 32) | fault;
>> +}
>> +
>> +/**
>> + * vmxon in CPL=3 and out of VMX operation
>> + *
>> + * Expect: #GP(0)
>> + */
>> +static bool test_vmxon_in_user(void)
>
>Similarly, test_user_vmxon()
>
I'll turn to shorter names.
>> +{
>> + clear_vmcs(vmxon_region, get_vmcs_revid());
>> +
>> + unsigned long ret = exec_user(vmxon_in_user);
>> + uint8_t err = (ret >> 32) & 0xff;
>> + exinfo_t fault = ret & 0xFFFFFFFF;
>> +
>> + return handle_vmxinsn_err(__func__, err, fault,
>> + VMXERR_FAULT, EXINFO_SYM(GP, 0), 0);
>> +}
>> +
>> bool test_vmxon(void)
>> {
>> if ( !test_vmxon_novmxe() )
>> return false;
>
>Your subject says out of VMX operation, but the implementation is inside
>VMX operation.
>
vmxon in test_vmxon_novmxe() fails if test_vmxon_novmxe() return true,
so here we are still out of VMX operation.
>It would be worth testing both scenarios, as they should be
>distinguished by #UD vs #GP[0].
>
Yes, patch 13 - 16 are for this purpose.
Thanks,
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 07/16] vvmx: test vmxon in CPL=3 and out of VMX operation
2016-12-19 3:35 ` Haozhong Zhang
@ 2016-12-19 16:02 ` Andrew Cooper
2016-12-19 16:02 ` Andrew Cooper
1 sibling, 0 replies; 56+ messages in thread
From: Andrew Cooper @ 2016-12-19 16:02 UTC (permalink / raw)
To: Haozhong Zhang; +Cc: xen-devel
On 19/12/16 03:35, Haozhong Zhang wrote:
> On 12/16/16 20:33 +0000, Andrew Cooper wrote:
>> On 16/12/16 13:43, Haozhong Zhang wrote:
>>> diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
>>> index 31f074c..ca33b3c 100644
>>> --- a/tests/vvmx/vmxon.c
>>> +++ b/tests/vvmx/vmxon.c
>>> @@ -28,11 +28,42 @@ static bool test_vmxon_novmxe(void)
>>> VMXERR_FAULT, EXINFO_SYM(UD, 0), 0);
>>> }
>>>
>>> +static unsigned long vmxon_in_user(void)
>>
>> I'd name this user_vmxon() as it is slightly shorter, but I'm not
>> terribly fussed.
>>
>>> +{
>>> + exinfo_t fault;
>>> + unsigned long ret = vmxon((uint64_t)vmxon_region, &fault);
>>> +
>>> + return (ret << 32) | fault;
>>> +}
>>> +
>>> +/**
>>> + * vmxon in CPL=3 and out of VMX operation
>>> + *
>>> + * Expect: #GP(0)
>>> + */
>>> +static bool test_vmxon_in_user(void)
>>
>> Similarly, test_user_vmxon()
>>
>
> I'll turn to shorter names.
>
>>> +{
>>> + clear_vmcs(vmxon_region, get_vmcs_revid());
>>> +
>>> + unsigned long ret = exec_user(vmxon_in_user);
>>> + uint8_t err = (ret >> 32) & 0xff;
>>> + exinfo_t fault = ret & 0xFFFFFFFF;
>>> +
>>> + return handle_vmxinsn_err(__func__, err, fault,
>>> + VMXERR_FAULT, EXINFO_SYM(GP, 0), 0);
>>> +}
>>> +
>>> bool test_vmxon(void)
>>> {
>>> if ( !test_vmxon_novmxe() )
>>> return false;
>>
>> Your subject says out of VMX operation, but the implementation is inside
>> VMX operation.
>>
>
> vmxon in test_vmxon_novmxe() fails if test_vmxon_novmxe() return true,
> so here we are still out of VMX operation.
Very good point. Its obvious now you point it out. Sorry for the noise.
~andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 07/16] vvmx: test vmxon in CPL=3 and out of VMX operation
2016-12-19 3:35 ` Haozhong Zhang
2016-12-19 16:02 ` Andrew Cooper
@ 2016-12-19 16:02 ` Andrew Cooper
1 sibling, 0 replies; 56+ messages in thread
From: Andrew Cooper @ 2016-12-19 16:02 UTC (permalink / raw)
To: Haozhong Zhang; +Cc: xen-devel
On 19/12/16 03:35, Haozhong Zhang wrote:
>
>>> +{
>>> + clear_vmcs(vmxon_region, get_vmcs_revid());
>>> +
>>> + unsigned long ret = exec_user(vmxon_in_user);
>>> + uint8_t err = (ret >> 32) & 0xff;
>>> + exinfo_t fault = ret & 0xFFFFFFFF;
>>> +
>>> + return handle_vmxinsn_err(__func__, err, fault,
>>> + VMXERR_FAULT, EXINFO_SYM(GP, 0), 0);
>>> +}
>>> +
>>> bool test_vmxon(void)
>>> {
>>> if ( !test_vmxon_novmxe() )
>>> return false;
>>
>> Your subject says out of VMX operation, but the implementation is inside
>> VMX operation.
>>
>
> vmxon in test_vmxon_novmxe() fails if test_vmxon_novmxe() return true,
> so here we are still out of VMX operation.
Very good point. Its obvious now you point it out. Sorry for the noise.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [XTF PATCH 08/16] vvmx: test vmxon with invalidly wide VMXON region address
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (6 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 07/16] vvmx: test vmxon in CPL=3 and out of VMX operation Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 20:40 ` Andrew Cooper
2016-12-16 13:43 ` [XTF PATCH 09/16] vvmx: test vmxon with unaligned " Haozhong Zhang
` (9 subsequent siblings)
17 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
VMfailInvalid is expected in this test.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/vmxon.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
index ca33b3c..8147679 100644
--- a/tests/vvmx/vmxon.c
+++ b/tests/vvmx/vmxon.c
@@ -53,6 +53,40 @@ static bool test_vmxon_in_user(void)
VMXERR_FAULT, EXINFO_SYM(GP, 0), 0);
}
+static uint8_t get_cpu_paddr_bits(void)
+{
+ uint8_t paddr_bits = 36;
+ uint32_t eax;
+
+ eax = cpuid_eax(0x80000000);
+ if ( (eax >> 16) == 0x8000 && eax >= 0x80000008 )
+ {
+ eax = cpuid_eax(0x80000008);
+ paddr_bits = eax & 0xff;
+ if ( paddr_bits > 52 )
+ paddr_bits = 52;
+ }
+
+ return paddr_bits;
+}
+
+/**
+ * vmxon with VMXON region address that expires the maximum physical
+ * address width
+ *
+ * Expect: VMfailInvalid
+ */
+static bool test_vmxon_invalid_paddr_width(void)
+{
+ uint8_t paddr_bits = get_cpu_paddr_bits();
+ uint64_t invalid_vmxon_address = (1UL << paddr_bits);
+ exinfo_t fault;
+ uint8_t ret = vmxon(invalid_vmxon_address, &fault);
+
+ return handle_vmxinsn_err(__func__, ret, fault,
+ VMXERR_VMFAIL_INVALID, 0, 0);
+}
+
bool test_vmxon(void)
{
if ( !test_vmxon_novmxe() )
@@ -64,6 +98,9 @@ bool test_vmxon(void)
if ( !test_vmxon_in_user() )
return false;
+ if ( !test_vmxon_invalid_paddr_width() )
+ return false;
+
return true;
}
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [XTF PATCH 08/16] vvmx: test vmxon with invalidly wide VMXON region address
2016-12-16 13:43 ` [XTF PATCH 08/16] vvmx: test vmxon with invalidly wide VMXON region address Haozhong Zhang
@ 2016-12-16 20:40 ` Andrew Cooper
2016-12-19 3:36 ` Haozhong Zhang
0 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 20:40 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel
On 16/12/16 13:43, Haozhong Zhang wrote:
> +static uint8_t get_cpu_paddr_bits(void)
> +{
> + uint8_t paddr_bits = 36;
> + uint32_t eax;
> +
> + eax = cpuid_eax(0x80000000);
> + if ( (eax >> 16) == 0x8000 && eax >= 0x80000008 )
> + {
> + eax = cpuid_eax(0x80000008);
> + paddr_bits = eax & 0xff;
> + if ( paddr_bits > 52 )
> + paddr_bits = 52;
> + }
> +
> + return paddr_bits;
This is useful core functionality (and I can't believe I haven't
published a test which needs it yet).
Please extend collect_cpuid() and make maxphysaddr and maxvirtaddr
available globally along with the other vendor/family/feature
information, although I'd drop the clipping at 52 bits.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 08/16] vvmx: test vmxon with invalidly wide VMXON region address
2016-12-16 20:40 ` Andrew Cooper
@ 2016-12-19 3:36 ` Haozhong Zhang
0 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 3:36 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/16/16 20:40 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>> +static uint8_t get_cpu_paddr_bits(void)
>> +{
>> + uint8_t paddr_bits = 36;
>> + uint32_t eax;
>> +
>> + eax = cpuid_eax(0x80000000);
>> + if ( (eax >> 16) == 0x8000 && eax >= 0x80000008 )
>> + {
>> + eax = cpuid_eax(0x80000008);
>> + paddr_bits = eax & 0xff;
>> + if ( paddr_bits > 52 )
>> + paddr_bits = 52;
>> + }
>> +
>> + return paddr_bits;
>
>This is useful core functionality (and I can't believe I haven't
>published a test which needs it yet).
>
>Please extend collect_cpuid() and make maxphysaddr and maxvirtaddr
>available globally along with the other vendor/family/feature
>information, although I'd drop the clipping at 52 bits.
>
Sure.
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [XTF PATCH 09/16] vvmx: test vmxon with unaligned VMXON region address
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (7 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 08/16] vvmx: test vmxon with invalidly wide VMXON region address Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 13:43 ` [XTF PATCH 10/16] vvmx: test vmxon with mismatched VMCS revision ID Haozhong Zhang
` (8 subsequent siblings)
17 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
VMfailInvalid is expected in this case.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/vmxon.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
index 8147679..f230fdf 100644
--- a/tests/vvmx/vmxon.c
+++ b/tests/vvmx/vmxon.c
@@ -87,6 +87,21 @@ static bool test_vmxon_invalid_paddr_width(void)
VMXERR_VMFAIL_INVALID, 0, 0);
}
+/**
+ * vmxon with VMXON region address not aligned to 4096
+ *
+ * Expect: VMfailInvalid
+ */
+static bool test_vmxon_unaligned_addr(void)
+{
+ uint64_t unaligned_addr = (uint64_t)vmxon_region | 0xff;
+ exinfo_t fault;
+ uint8_t ret = vmxon(unaligned_addr, &fault);
+
+ return handle_vmxinsn_err(__func__, ret, fault,
+ VMXERR_VMFAIL_INVALID, 0, 0);
+}
+
bool test_vmxon(void)
{
if ( !test_vmxon_novmxe() )
@@ -101,6 +116,9 @@ bool test_vmxon(void)
if ( !test_vmxon_invalid_paddr_width() )
return false;
+ if ( !test_vmxon_unaligned_addr() )
+ return false;
+
return true;
}
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* [XTF PATCH 10/16] vvmx: test vmxon with mismatched VMCS revision ID
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (8 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 09/16] vvmx: test vmxon with unaligned " Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 13:43 ` [XTF PATCH 11/16] vvmx: test vmxon with bit 31 of VMCS revision ID set Haozhong Zhang
` (7 subsequent siblings)
17 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
VMfailInvalid is expected in this test.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/vmxon.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
index f230fdf..75b6dcb 100644
--- a/tests/vvmx/vmxon.c
+++ b/tests/vvmx/vmxon.c
@@ -102,6 +102,22 @@ static bool test_vmxon_unaligned_addr(void)
VMXERR_VMFAIL_INVALID, 0, 0);
}
+/**
+ * vmxon with VMCS revision ID mismatched with MSR_IA32_VMX_BASIC
+ *
+ * Expect: VMfailInvalid
+ */
+static bool test_vmxon_mismatched_revid(void)
+{
+ clear_vmcs(vmxon_region, get_vmcs_revid() + 1);
+
+ exinfo_t fault;
+ uint8_t ret = vmxon((uint64_t)vmxon_region, &fault);
+
+ return handle_vmxinsn_err(__func__, ret, fault,
+ VMXERR_VMFAIL_INVALID, 0, 0);
+}
+
bool test_vmxon(void)
{
if ( !test_vmxon_novmxe() )
@@ -119,6 +135,9 @@ bool test_vmxon(void)
if ( !test_vmxon_unaligned_addr() )
return false;
+ if ( !test_vmxon_mismatched_revid() )
+ return false;
+
return true;
}
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* [XTF PATCH 11/16] vvmx: test vmxon with bit 31 of VMCS revision ID set
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (9 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 10/16] vvmx: test vmxon with mismatched VMCS revision ID Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 13:43 ` [XTF PATCH 12/16] vvmx: test the correct vmxon Haozhong Zhang
` (6 subsequent siblings)
17 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
VMfailInvalid is expected in this test.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/vmxon.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
index 75b6dcb..268baa8 100644
--- a/tests/vvmx/vmxon.c
+++ b/tests/vvmx/vmxon.c
@@ -118,6 +118,22 @@ static bool test_vmxon_mismatched_revid(void)
VMXERR_VMFAIL_INVALID, 0, 0);
}
+/**
+ * vmxon with VMCS revision ID[31] set
+ *
+ * Expect: VMfailInvalid
+ */
+static bool test_vmxon_revid_bit31(void)
+{
+ clear_vmcs(vmxon_region, get_vmcs_revid() | (1UL << 31));
+
+ exinfo_t fault;
+ uint8_t ret = vmxon((uint64_t)vmxon_region, &fault);
+
+ return handle_vmxinsn_err(__func__, ret, fault,
+ VMXERR_VMFAIL_INVALID, 0, 0);
+}
+
bool test_vmxon(void)
{
if ( !test_vmxon_novmxe() )
@@ -138,6 +154,9 @@ bool test_vmxon(void)
if ( !test_vmxon_mismatched_revid() )
return false;
+ if ( !test_vmxon_revid_bit31() )
+ return false;
+
return true;
}
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* [XTF PATCH 12/16] vvmx: test the correct vmxon
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (10 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 11/16] vvmx: test vmxon with bit 31 of VMCS revision ID set Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 13:43 ` [XTF PATCH 13/16] vvmx: test vmxon in VMX root w/ CPL = 0 and w/o current VMCS Haozhong Zhang
` (5 subsequent siblings)
17 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
No error is expected in this test.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/vmxon.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
index 268baa8..a41f101 100644
--- a/tests/vvmx/vmxon.c
+++ b/tests/vvmx/vmxon.c
@@ -134,6 +134,21 @@ static bool test_vmxon_revid_bit31(void)
VMXERR_VMFAIL_INVALID, 0, 0);
}
+/**
+ * vmxon expected to succeed
+ *
+ * Expect: not errors
+ */
+static bool test_vmxon_correct(void)
+{
+ clear_vmcs(vmxon_region, get_vmcs_revid());
+
+ exinfo_t fault;
+ uint8_t ret = vmxon((uint64_t)vmxon_region, &fault);
+
+ return handle_vmxinsn_err(__func__, ret, fault, VMXERR_NOERR, 0, 0);
+}
+
bool test_vmxon(void)
{
if ( !test_vmxon_novmxe() )
@@ -157,6 +172,9 @@ bool test_vmxon(void)
if ( !test_vmxon_revid_bit31() )
return false;
+ if ( !test_vmxon_correct() )
+ return false;
+
return true;
}
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* [XTF PATCH 13/16] vvmx: test vmxon in VMX root w/ CPL = 0 and w/o current VMCS
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (11 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 12/16] vvmx: test the correct vmxon Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 13:43 ` [XTF PATCH 14/16] vvmx: test vmxon in VMX root w/ CPL = 3 " Haozhong Zhang
` (4 subsequent siblings)
17 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
VMfailInvalid is expected in this test.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/vmxon.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
index a41f101..0664a48 100644
--- a/tests/vvmx/vmxon.c
+++ b/tests/vvmx/vmxon.c
@@ -149,6 +149,22 @@ static bool test_vmxon_correct(void)
return handle_vmxinsn_err(__func__, ret, fault, VMXERR_NOERR, 0, 0);
}
+/**
+ * vmxon in VMX root w/ CPL = 0 and w/o current VMCS
+ *
+ * Expect: VMfailInvalid
+ */
+static bool test_vmxon_in_root_cpl0_novmcs(void)
+{
+ clear_vmcs(vmxon_region_2nd, get_vmcs_revid());
+
+ exinfo_t fault;
+ uint8_t ret = vmxon((uint64_t)vmxon_region_2nd, &fault);
+
+ return handle_vmxinsn_err(__func__, ret, fault,
+ VMXERR_VMFAIL_INVALID, 0, 0);
+}
+
bool test_vmxon(void)
{
if ( !test_vmxon_novmxe() )
@@ -175,6 +191,9 @@ bool test_vmxon(void)
if ( !test_vmxon_correct() )
return false;
+ if ( !test_vmxon_in_root_cpl0_novmcs() )
+ return false;
+
return true;
}
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* [XTF PATCH 14/16] vvmx: test vmxon in VMX root w/ CPL = 3 and w/o current VMCS
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (12 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 13/16] vvmx: test vmxon in VMX root w/ CPL = 0 and w/o current VMCS Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 20:59 ` Andrew Cooper
2016-12-16 13:43 ` [XTF PATCH 15/16] vvmx: test vmxon in VMX root w/ CPL = 0 and w/ " Haozhong Zhang
` (3 subsequent siblings)
17 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
Fault #GP(0) is expected in this test.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/vmxon.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
index 0664a48..ec7ee7e 100644
--- a/tests/vvmx/vmxon.c
+++ b/tests/vvmx/vmxon.c
@@ -3,6 +3,7 @@
#include "util.h"
static uint8_t vmxon_region[PAGE_SIZE] __aligned(PAGE_SIZE);
+static uint8_t vmxon_region_2nd[PAGE_SIZE] __aligned(PAGE_SIZE);
/**
* vmxon with CR4.VMXE cleared
@@ -165,6 +166,31 @@ static bool test_vmxon_in_root_cpl0_novmcs(void)
VMXERR_VMFAIL_INVALID, 0, 0);
}
+static unsigned long vmxon_in_root_user(void)
+{
+ exinfo_t fault;
+ unsigned long ret = vmxon((uint64_t)vmxon_region_2nd, &fault);
+
+ return (ret << 32) | fault;
+}
+
+/**
+ * vmxon in VMX root w/ CPL = 3 and w/o current VMCS
+ *
+ * Expect: #GP(0)
+ */
+static bool test_vmxon_in_root_user_novmcs(void)
+{
+ clear_vmcs(vmxon_region_2nd, get_vmcs_revid());
+
+ unsigned long ret = exec_user(vmxon_in_root_user);
+ uint8_t err = (ret >> 32) & 0xff;
+ exinfo_t fault = ret & 0xffffffff;
+
+ return handle_vmxinsn_err(__func__, err, fault,
+ VMXERR_FAULT, EXINFO_SYM(GP, 0), 0);
+}
+
bool test_vmxon(void)
{
if ( !test_vmxon_novmxe() )
@@ -194,6 +220,9 @@ bool test_vmxon(void)
if ( !test_vmxon_in_root_cpl0_novmcs() )
return false;
+ if ( !test_vmxon_in_root_user_novmcs() )
+ return false;
+
return true;
}
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [XTF PATCH 14/16] vvmx: test vmxon in VMX root w/ CPL = 3 and w/o current VMCS
2016-12-16 13:43 ` [XTF PATCH 14/16] vvmx: test vmxon in VMX root w/ CPL = 3 " Haozhong Zhang
@ 2016-12-16 20:59 ` Andrew Cooper
2016-12-19 3:46 ` Haozhong Zhang
0 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 20:59 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel
On 16/12/16 13:43, Haozhong Zhang wrote:
> Fault #GP(0) is expected in this test.
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
> tests/vvmx/vmxon.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
> index 0664a48..ec7ee7e 100644
> --- a/tests/vvmx/vmxon.c
> +++ b/tests/vvmx/vmxon.c
> @@ -3,6 +3,7 @@
> #include "util.h"
>
> static uint8_t vmxon_region[PAGE_SIZE] __aligned(PAGE_SIZE);
> +static uint8_t vmxon_region_2nd[PAGE_SIZE] __aligned(PAGE_SIZE);
You introduce vmxon_region_2nd here, but reference it in the previous patch.
However, I am not sure of its purpose. Why cant you reuse the previous
host state area?
>
> /**
> * vmxon with CR4.VMXE cleared
> @@ -165,6 +166,31 @@ static bool test_vmxon_in_root_cpl0_novmcs(void)
> VMXERR_VMFAIL_INVALID, 0, 0);
> }
>
> +static unsigned long vmxon_in_root_user(void)
> +{
> + exinfo_t fault;
> + unsigned long ret = vmxon((uint64_t)vmxon_region_2nd, &fault);
> +
> + return (ret << 32) | fault;
> +}
I have a pending patch for exec_user_param() (originally written for a
test which I subsequently reimplemented differently).
I have just pushed it for you to use. You can have one common
user_vmxon() and pass a variable host region to it, rather than having
an almost-entirely duplicate function.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 14/16] vvmx: test vmxon in VMX root w/ CPL = 3 and w/o current VMCS
2016-12-16 20:59 ` Andrew Cooper
@ 2016-12-19 3:46 ` Haozhong Zhang
2016-12-19 16:07 ` Andrew Cooper
0 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 3:46 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/16/16 20:59 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>> Fault #GP(0) is expected in this test.
>>
>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>> ---
>> tests/vvmx/vmxon.c | 29 +++++++++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
>> index 0664a48..ec7ee7e 100644
>> --- a/tests/vvmx/vmxon.c
>> +++ b/tests/vvmx/vmxon.c
>> @@ -3,6 +3,7 @@
>> #include "util.h"
>>
>> static uint8_t vmxon_region[PAGE_SIZE] __aligned(PAGE_SIZE);
>> +static uint8_t vmxon_region_2nd[PAGE_SIZE] __aligned(PAGE_SIZE);
>
>You introduce vmxon_region_2nd here, but reference it in the previous patch.
>
I must have made mistakes when separating patch 13 - 16.
>However, I am not sure of its purpose. Why cant you reuse the previous
>host state area?
>
Intel SDM says SW should not access or modify the VXMON rgion of a
logical processor between vmxon and vmxoff. Though I have tested on
real hardware whether reusing VMXON region would cause any trouble, I
still want to exclude that possibility/noise from this test.
>>
>> /**
>> * vmxon with CR4.VMXE cleared
>> @@ -165,6 +166,31 @@ static bool test_vmxon_in_root_cpl0_novmcs(void)
>> VMXERR_VMFAIL_INVALID, 0, 0);
>> }
>>
>> +static unsigned long vmxon_in_root_user(void)
>> +{
>> + exinfo_t fault;
>> + unsigned long ret = vmxon((uint64_t)vmxon_region_2nd, &fault);
>> +
>> + return (ret << 32) | fault;
>> +}
>
>I have a pending patch for exec_user_param() (originally written for a
>test which I subsequently reimplemented differently).
>
>I have just pushed it for you to use. You can have one common
>user_vmxon() and pass a variable host region to it, rather than having
>an almost-entirely duplicate function.
>
Thank you! I can use the dedicated bits in exinfo_t as you suggested
in previous patch.
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 14/16] vvmx: test vmxon in VMX root w/ CPL = 3 and w/o current VMCS
2016-12-19 3:46 ` Haozhong Zhang
@ 2016-12-19 16:07 ` Andrew Cooper
2016-12-20 2:50 ` Haozhong Zhang
0 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-19 16:07 UTC (permalink / raw)
To: Haozhong Zhang; +Cc: xen-devel
On 19/12/16 03:46, Haozhong Zhang wrote:
>> However, I am not sure of its purpose. Why cant you reuse the previous
>> host state area?
>>
>
> Intel SDM says SW should not access or modify the VXMON rgion of a
> logical processor between vmxon and vmxoff. Though I have tested on
> real hardware whether reusing VMXON region would cause any trouble, I
> still want to exclude that possibility/noise from this test.
That is a good reason. Could you please add a comment to this effect?
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [XTF PATCH 14/16] vvmx: test vmxon in VMX root w/ CPL = 3 and w/o current VMCS
2016-12-19 16:07 ` Andrew Cooper
@ 2016-12-20 2:50 ` Haozhong Zhang
0 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-20 2:50 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/19/16 16:07 +0000, Andrew Cooper wrote:
>On 19/12/16 03:46, Haozhong Zhang wrote:
>>> However, I am not sure of its purpose. Why cant you reuse the previous
>>> host state area?
>>>
>>
>> Intel SDM says SW should not access or modify the VXMON rgion of a
>> logical processor between vmxon and vmxoff. Though I have tested on
^
I missed a 'not' here, hope you didn't misunderstand what I meant here
>> real hardware whether reusing VMXON region would cause any trouble, I
>> still want to exclude that possibility/noise from this test.
>
>That is a good reason. Could you please add a comment to this effect?
>
Sure, I'll add a comment around vmxon_region_2nd.
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [XTF PATCH 15/16] vvmx: test vmxon in VMX root w/ CPL = 0 and w/ current VMCS
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (13 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 14/16] vvmx: test vmxon in VMX root w/ CPL = 3 " Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 13:43 ` [XTF PATCH 16/16] vvmx: test vmxon in VMX root w/ CPL = 3 " Haozhong Zhang
` (2 subsequent siblings)
17 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
VMfailvalid(15) is expected in this test.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/vmxon.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
index ec7ee7e..705a04d 100644
--- a/tests/vvmx/vmxon.c
+++ b/tests/vvmx/vmxon.c
@@ -4,6 +4,7 @@
static uint8_t vmxon_region[PAGE_SIZE] __aligned(PAGE_SIZE);
static uint8_t vmxon_region_2nd[PAGE_SIZE] __aligned(PAGE_SIZE);
+static uint8_t vmcs[PAGE_SIZE] __aligned(PAGE_SIZE);
/**
* vmxon with CR4.VMXE cleared
@@ -191,6 +192,23 @@ static bool test_vmxon_in_root_user_novmcs(void)
VMXERR_FAULT, EXINFO_SYM(GP, 0), 0);
}
+/**
+ * vmxon in VMX root w/ CPL = 0 and w/ current VMCS
+ *
+ * Expect: VMfailvalid(15)
+ */
+static bool test_vmxon_in_root_cpl0_vmcs(void)
+{
+ clear_vmcs(vmxon_region_2nd, get_vmcs_revid());
+
+ exinfo_t fault;
+ uint8_t ret = vmxon((uint64_t)vmxon_region_2nd, &fault);
+
+ return handle_vmxinsn_err(__func__, ret, fault,
+ VMXERR_VMFAIL_VALID, 0,
+ VMX_INSN_VMXON_IN_VMX_ROOT);
+}
+
bool test_vmxon(void)
{
if ( !test_vmxon_novmxe() )
@@ -223,6 +241,17 @@ bool test_vmxon(void)
if ( !test_vmxon_in_root_user_novmcs() )
return false;
+ clear_vmcs(vmcs, get_vmcs_revid());
+ if ( vmptrld((uint64_t)vmcs, NULL) )
+ {
+ xtf_failure("Fail: %s: unexpected failure from vmptrld 0x%"PRIx64"\n",
+ __func__, (uint64_t)vmcs);
+ return false;
+ }
+
+ if ( !test_vmxon_in_root_cpl0_vmcs() )
+ return false;
+
return true;
}
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* [XTF PATCH 16/16] vvmx: test vmxon in VMX root w/ CPL = 3 and w/ current VMCS
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (14 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 15/16] vvmx: test vmxon in VMX root w/ CPL = 0 and w/ " Haozhong Zhang
@ 2016-12-16 13:43 ` Haozhong Zhang
2016-12-16 14:12 ` [XTF PATCH 00/16] Add test cases for nested vmxon Andrew Cooper
2016-12-16 21:26 ` Andrew Cooper
17 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-16 13:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Haozhong Zhang
Fault #GP(0) is expected in this test.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
tests/vvmx/vmxon.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c
index 705a04d..c8f2852 100644
--- a/tests/vvmx/vmxon.c
+++ b/tests/vvmx/vmxon.c
@@ -209,6 +209,23 @@ static bool test_vmxon_in_root_cpl0_vmcs(void)
VMX_INSN_VMXON_IN_VMX_ROOT);
}
+/**
+ * vmxon in VMX root w/ CPL = 3 and w/ current VMCS
+ *
+ * Expect: #GP(0)
+ */
+static bool test_vmxon_in_root_user_vmcs(void)
+{
+ clear_vmcs(vmxon_region_2nd, get_vmcs_revid());
+
+ unsigned long ret = exec_user(vmxon_in_root_user);
+ uint8_t err = (ret >> 32) & 0xff;
+ exinfo_t fault = ret & 0xffffffff;
+
+ return handle_vmxinsn_err(__func__, err, fault,
+ VMXERR_FAULT, EXINFO_SYM(GP, 0), 0);
+}
+
bool test_vmxon(void)
{
if ( !test_vmxon_novmxe() )
@@ -252,6 +269,9 @@ bool test_vmxon(void)
if ( !test_vmxon_in_root_cpl0_vmcs() )
return false;
+ if ( !test_vmxon_in_root_user_vmcs() )
+ return false;
+
return true;
}
--
2.10.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [XTF PATCH 00/16] Add test cases for nested vmxon
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (15 preceding siblings ...)
2016-12-16 13:43 ` [XTF PATCH 16/16] vvmx: test vmxon in VMX root w/ CPL = 3 " Haozhong Zhang
@ 2016-12-16 14:12 ` Andrew Cooper
2016-12-19 1:44 ` Haozhong Zhang
2016-12-16 21:26 ` Andrew Cooper
17 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 14:12 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel
On 16/12/16 13:43, Haozhong Zhang wrote:
> This patch series starts to add a test selection "test-hvm64-vvmx" for
> nested VMX. This first series focuses mostly on nested vmxon.
>
> * Patch 01 - 03 test the basic environment (cpuid and MSR).
>
> * Patch 04 - 05 add functions to execute VMX instructions and to
> collect and handle errors.
>
> * Patch 06 - 16 construct a bunch of test cases for nested vmxon per
> its pseudo code in section "VMXON - Enter VMX Operation" of Intel
> SDM Vol 3.
Thankyou very much for contributing this.
As a general question though, how have you found using XTF? I am eager
for any feedback, especially if you found problem areas.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 00/16] Add test cases for nested vmxon
2016-12-16 14:12 ` [XTF PATCH 00/16] Add test cases for nested vmxon Andrew Cooper
@ 2016-12-19 1:44 ` Haozhong Zhang
2016-12-19 16:18 ` Andrew Cooper
0 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 1:44 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/16/16 14:12 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>> This patch series starts to add a test selection "test-hvm64-vvmx" for
>> nested VMX. This first series focuses mostly on nested vmxon.
>>
>> * Patch 01 - 03 test the basic environment (cpuid and MSR).
>>
>> * Patch 04 - 05 add functions to execute VMX instructions and to
>> collect and handle errors.
>>
>> * Patch 06 - 16 construct a bunch of test cases for nested vmxon per
>> its pseudo code in section "VMXON - Enter VMX Operation" of Intel
>> SDM Vol 3.
>
>Thankyou very much for contributing this.
>
>As a general question though, how have you found using XTF? I am eager
>for any feedback, especially if you found problem areas.
>
Lars Kurth and George Dunlap visited out site recently and mentioned
XTF. When I started to look at and fix nested VMX bugs, I used KVM as
L1 hypervisor. However, KVM is sometimes too heavy and not quite
controllable for the quick verification and test, so I turned to XTF and
found it was exactly what I wanted for this purpose.
So far I feel XTF is pretty handy for me to construct quick and short
test cases for VMX instructions.
One feature I desire is to configure the dependency among tests. Take
this patch series as an example: some tests in patch 09 - 16 make
sense only when VMX is available. If I can indicate they depend on the
successful result of tests in patch 01 - 03, then I can separate patch
01 - 03 and patch 09 - 16 into two test selections, and will not need
to introduce additional code in patch 09 - 16 to check the availability
of VMX.
Thanks,
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [XTF PATCH 00/16] Add test cases for nested vmxon
2016-12-19 1:44 ` Haozhong Zhang
@ 2016-12-19 16:18 ` Andrew Cooper
0 siblings, 0 replies; 56+ messages in thread
From: Andrew Cooper @ 2016-12-19 16:18 UTC (permalink / raw)
To: Haozhong Zhang; +Cc: xen-devel
On 19/12/16 01:44, Haozhong Zhang wrote:
> On 12/16/16 14:12 +0000, Andrew Cooper wrote:
>> On 16/12/16 13:43, Haozhong Zhang wrote:
>>> This patch series starts to add a test selection "test-hvm64-vvmx" for
>>> nested VMX. This first series focuses mostly on nested vmxon.
>>>
>>> * Patch 01 - 03 test the basic environment (cpuid and MSR).
>>>
>>> * Patch 04 - 05 add functions to execute VMX instructions and to
>>> collect and handle errors.
>>>
>>> * Patch 06 - 16 construct a bunch of test cases for nested vmxon per
>>> its pseudo code in section "VMXON - Enter VMX Operation" of Intel
>>> SDM Vol 3.
>>
>> Thankyou very much for contributing this.
>>
>> As a general question though, how have you found using XTF? I am eager
>> for any feedback, especially if you found problem areas.
>>
>
> Lars Kurth and George Dunlap visited out site recently and mentioned
> XTF. When I started to look at and fix nested VMX bugs, I used KVM as
> L1 hypervisor. However, KVM is sometimes too heavy and not quite
> controllable for the quick verification and test, so I turned to XTF and
> found it was exactly what I wanted for this purpose.
>
> So far I feel XTF is pretty handy for me to construct quick and short
> test cases for VMX instructions.
This reason is precisely why I developed it to start with. I now find
that I do almost all my debugging and development work with it.
>
> One feature I desire is to configure the dependency among tests. Take
> this patch series as an example: some tests in patch 09 - 16 make
> sense only when VMX is available. If I can indicate they depend on the
> successful result of tests in patch 01 - 03, then I can separate patch
> 01 - 03 and patch 09 - 16 into two test selections, and will not need
> to introduce additional code in patch 09 - 16 to check the availability
> of VMX.
Do you mean dependencies between specific test functions?
From XTF's point of view, each microkernel constitutes a single test
with an overall result, which typically consists of multiple test
steps. Within a single microkernel, you are free to make some of the
test steps conditional.
Particularly when testing bits which differ subtly between Intel and
AMD, I find it common to have test steps conditional on cpu_has_*. One
thing I tend to do which I notice you haven't is that I tend to have a
printk() before major test steps so in the success case, there is a bit
of a log of what was tested.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [XTF PATCH 00/16] Add test cases for nested vmxon
2016-12-16 13:43 [XTF PATCH 00/16] Add test cases for nested vmxon Haozhong Zhang
` (16 preceding siblings ...)
2016-12-16 14:12 ` [XTF PATCH 00/16] Add test cases for nested vmxon Andrew Cooper
@ 2016-12-16 21:26 ` Andrew Cooper
2016-12-19 5:31 ` Haozhong Zhang
17 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-16 21:26 UTC (permalink / raw)
To: Haozhong Zhang, xen-devel
On 16/12/16 13:43, Haozhong Zhang wrote:
> This patch series starts to add a test selection "test-hvm64-vvmx" for
> nested VMX. This first series focuses mostly on nested vmxon.
>
> * Patch 01 - 03 test the basic environment (cpuid and MSR).
>
> * Patch 04 - 05 add functions to execute VMX instructions and to
> collect and handle errors.
>
> * Patch 06 - 16 construct a bunch of test cases for nested vmxon per
> its pseudo code in section "VMXON - Enter VMX Operation" of Intel
> SDM Vol 3.
>
Thankyou for this series. I have reviewed it now, and have no major
problems. It is in quite good shape, other than the licensing concerns
with patch 4.
One limitation (because I haven't gotten around to implementing it yet)
is that once a test is accepted, it can't be logically extended, because
it isn't bisection-safe as far as OSSTest is concerned.
I will prioritise my work to introduce the test-revision plan, which
will allow the OSSTest bisector to work properly in the case that new
functionality in a test causes a previously-passing scenario to now fail.
Is this a complete set of vmxon scenarios, or are you working on more?
Some which come to mind are:
* a register operand (to check that Xen decodes properly and rejects the
instruction)
* duplicate vmxon in root operation
Another area I had on my nested-virt plan was to allow rather finer
grain configuration of the vmx options, but that depends on the start of
the MSR levelling work, which is still blocked behind getting enough
time to finish the CPUID levelling plans.
In particular, I eventually want an ability for fine-grained toolstack
settings of the available VMX configuration (these being a subset of the
MSRs in the system), (all subject to auditing by Xen to keep it within
hardware and supported bounds), at which point it would be plausible to
spin up a VM, asking for VMX_BASIC_32BIT_ADDRESSES, and checking that
suitable limits were observed/enforced inside the VM.
Now, the above is part of some larger issues I need to fix for
XenServer, and I am not suggesting or hinting for you to do any of the
work (This is a multi-year plan on my behalf). However, I hope it
helps to see where I am trying to get to in the long term.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [XTF PATCH 00/16] Add test cases for nested vmxon
2016-12-16 21:26 ` Andrew Cooper
@ 2016-12-19 5:31 ` Haozhong Zhang
2016-12-19 16:34 ` Andrew Cooper
0 siblings, 1 reply; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-19 5:31 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/16/16 21:26 +0000, Andrew Cooper wrote:
>On 16/12/16 13:43, Haozhong Zhang wrote:
>> This patch series starts to add a test selection "test-hvm64-vvmx" for
>> nested VMX. This first series focuses mostly on nested vmxon.
>>
>> * Patch 01 - 03 test the basic environment (cpuid and MSR).
>>
>> * Patch 04 - 05 add functions to execute VMX instructions and to
>> collect and handle errors.
>>
>> * Patch 06 - 16 construct a bunch of test cases for nested vmxon per
>> its pseudo code in section "VMXON - Enter VMX Operation" of Intel
>> SDM Vol 3.
>>
>
>Thankyou for this series. I have reviewed it now, and have no major
>problems. It is in quite good shape, other than the licensing concerns
>with patch 4.
>
>One limitation (because I haven't gotten around to implementing it yet)
>is that once a test is accepted, it can't be logically extended, because
>it isn't bisection-safe as far as OSSTest is concerned.
>
I'm going to add more test cases in the process of fixing nested VMX,
so I think I'd better to put each case in a single test, instead of
all cases in one test-hvm64-vvmx.
>I will prioritise my work to introduce the test-revision plan, which
>will allow the OSSTest bisector to work properly in the case that new
>functionality in a test causes a previously-passing scenario to now fail.
>
>Is this a complete set of vmxon scenarios, or are you working on more?
>Some which come to mind are:
>
>* a register operand (to check that Xen decodes properly and rejects the
>instruction)
I'll add this one, and
* vmxon w/ nestedhvm=0.
>* duplicate vmxon in root operation
>
>Another area I had on my nested-virt plan was to allow rather finer
>grain configuration of the vmx options, but that depends on the start of
>the MSR levelling work, which is still blocked behind getting enough
>time to finish the CPUID levelling plans.
>
I'll keep this possible change in my mind while I'm preparing test
cases which use CPUID/MSR/... to detect the environment instead of
replying on assumptions only satisfied on the current Xen implementation.
>In particular, I eventually want an ability for fine-grained toolstack
>settings of the available VMX configuration (these being a subset of the
>MSRs in the system), (all subject to auditing by Xen to keep it within
>hardware and supported bounds), at which point it would be plausible to
>spin up a VM, asking for VMX_BASIC_32BIT_ADDRESSES, and checking that
>suitable limits were observed/enforced inside the VM.
>
I do not quite understand the purpose for the fine-grained VMX
options. Is it to provide users with a chance to workaround bugs in
certain parts of nested virtualization?
Thanks,
Haozhong
>Now, the above is part of some larger issues I need to fix for
>XenServer, and I am not suggesting or hinting for you to do any of the
>work (This is a multi-year plan on my behalf). However, I hope it
>helps to see where I am trying to get to in the long term.
>
>~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [XTF PATCH 00/16] Add test cases for nested vmxon
2016-12-19 5:31 ` Haozhong Zhang
@ 2016-12-19 16:34 ` Andrew Cooper
2016-12-20 2:32 ` Haozhong Zhang
0 siblings, 1 reply; 56+ messages in thread
From: Andrew Cooper @ 2016-12-19 16:34 UTC (permalink / raw)
To: Haozhong Zhang; +Cc: xen-devel
On 19/12/16 05:31, Haozhong Zhang wrote:
> On 12/16/16 21:26 +0000, Andrew Cooper wrote:
>> On 16/12/16 13:43, Haozhong Zhang wrote:
>>> This patch series starts to add a test selection "test-hvm64-vvmx" for
>>> nested VMX. This first series focuses mostly on nested vmxon.
>>>
>>> * Patch 01 - 03 test the basic environment (cpuid and MSR).
>>>
>>> * Patch 04 - 05 add functions to execute VMX instructions and to
>>> collect and handle errors.
>>>
>>> * Patch 06 - 16 construct a bunch of test cases for nested vmxon per
>>> its pseudo code in section "VMXON - Enter VMX Operation" of Intel
>>> SDM Vol 3.
>>>
>>
>> Thankyou for this series. I have reviewed it now, and have no major
>> problems. It is in quite good shape, other than the licensing concerns
>> with patch 4.
>>
>> One limitation (because I haven't gotten around to implementing it yet)
>> is that once a test is accepted, it can't be logically extended, because
>> it isn't bisection-safe as far as OSSTest is concerned.
>>
>
> I'm going to add more test cases in the process of fixing nested VMX,
> so I think I'd better to put each case in a single test, instead of
> all cases in one test-hvm64-vvmx.
Ah - this clarifies what you mean by dependences of tests on the other
thread.
At the moment there is an implicit dependency used by both automated
test systems running XTF at the moment, which says that a test-$ENV-$foo
shouldn't be run if the test-$ENV-selftest didn't complete successfully.
I haven't yet encountered a need for other tests to be co-dependent.
Let me consider how this might be made to work.
>
>> I will prioritise my work to introduce the test-revision plan, which
>> will allow the OSSTest bisector to work properly in the case that new
>> functionality in a test causes a previously-passing scenario to now
>> fail.
>>
>> Is this a complete set of vmxon scenarios, or are you working on more?
>> Some which come to mind are:
>>
>> * a register operand (to check that Xen decodes properly and rejects the
>> instruction)
>
> I'll add this one, and
>
> * vmxon w/ nestedhvm=0.
You should probably look into using a test variation to explicitly set
the test configuration to =0 and =1
See the invlpg and pv-iopl tests for examples of this.
>
>> * duplicate vmxon in root operation
>>
>> Another area I had on my nested-virt plan was to allow rather finer
>> grain configuration of the vmx options, but that depends on the start of
>> the MSR levelling work, which is still blocked behind getting enough
>> time to finish the CPUID levelling plans.
>>
>
> I'll keep this possible change in my mind while I'm preparing test
> cases which use CPUID/MSR/... to detect the environment instead of
> replying on assumptions only satisfied on the current Xen implementation.
>
>> In particular, I eventually want an ability for fine-grained toolstack
>> settings of the available VMX configuration (these being a subset of the
>> MSRs in the system), (all subject to auditing by Xen to keep it within
>> hardware and supported bounds), at which point it would be plausible to
>> spin up a VM, asking for VMX_BASIC_32BIT_ADDRESSES, and checking that
>> suitable limits were observed/enforced inside the VM.
>>
>
> I do not quite understand the purpose for the fine-grained VMX
> options. Is it to provide users with a chance to workaround bugs in
> certain parts of nested virtualization?
I have two usecases in mind.
First is being able to check that Xen functions correctly when certain
VMX features are unavailable. Recently, we have found a number of
security issues which only affect older hardware, and older hardware is
getting harder and harder to come by. Using fine grained features can
simulate older hardware in some cases.
Second, and more importantly, that I need to eventually make this all
work with live migration, including between non-identical hardware.
Therefore, the fine-grained nature would be used to implement feature
levelling.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [XTF PATCH 00/16] Add test cases for nested vmxon
2016-12-19 16:34 ` Andrew Cooper
@ 2016-12-20 2:32 ` Haozhong Zhang
0 siblings, 0 replies; 56+ messages in thread
From: Haozhong Zhang @ 2016-12-20 2:32 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12/19/16 16:34 +0000, Andrew Cooper wrote:
>On 19/12/16 05:31, Haozhong Zhang wrote:
>> On 12/16/16 21:26 +0000, Andrew Cooper wrote:
>>> On 16/12/16 13:43, Haozhong Zhang wrote:
>>>> This patch series starts to add a test selection "test-hvm64-vvmx" for
>>>> nested VMX. This first series focuses mostly on nested vmxon.
>>>>
>>>> * Patch 01 - 03 test the basic environment (cpuid and MSR).
>>>>
>>>> * Patch 04 - 05 add functions to execute VMX instructions and to
>>>> collect and handle errors.
>>>>
>>>> * Patch 06 - 16 construct a bunch of test cases for nested vmxon per
>>>> its pseudo code in section "VMXON - Enter VMX Operation" of Intel
>>>> SDM Vol 3.
>>>>
>>>
>>> Thankyou for this series. I have reviewed it now, and have no major
>>> problems. It is in quite good shape, other than the licensing concerns
>>> with patch 4.
>>>
>>> One limitation (because I haven't gotten around to implementing it yet)
>>> is that once a test is accepted, it can't be logically extended, because
>>> it isn't bisection-safe as far as OSSTest is concerned.
>>>
>>
>> I'm going to add more test cases in the process of fixing nested VMX,
>> so I think I'd better to put each case in a single test, instead of
>> all cases in one test-hvm64-vvmx.
>
>Ah - this clarifies what you mean by dependences of tests on the other
>thread.
>
>At the moment there is an implicit dependency used by both automated
>test systems running XTF at the moment, which says that a test-$ENV-$foo
>shouldn't be run if the test-$ENV-selftest didn't complete successfully.
>
>I haven't yet encountered a need for other tests to be co-dependent.
>Let me consider how this might be made to work.
>
OK. Right now let me organize these test cases by instructions,
e.g. test-hvm64-vvmxon, test-hvm64-vvmxoff, etc.
>>
>>> I will prioritise my work to introduce the test-revision plan, which
>>> will allow the OSSTest bisector to work properly in the case that new
>>> functionality in a test causes a previously-passing scenario to now
>>> fail.
>>>
>>> Is this a complete set of vmxon scenarios, or are you working on more?
>>> Some which come to mind are:
>>>
>>> * a register operand (to check that Xen decodes properly and rejects the
>>> instruction)
>>
>> I'll add this one, and
>>
>> * vmxon w/ nestedhvm=0.
>
>You should probably look into using a test variation to explicitly set
>the test configuration to =0 and =1
>
>See the invlpg and pv-iopl tests for examples of this.
>
Yes, that is what I'm going to follow.
>>
>>> * duplicate vmxon in root operation
>>>
>>> Another area I had on my nested-virt plan was to allow rather finer
>>> grain configuration of the vmx options, but that depends on the start of
>>> the MSR levelling work, which is still blocked behind getting enough
>>> time to finish the CPUID levelling plans.
>>>
>>
>> I'll keep this possible change in my mind while I'm preparing test
>> cases which use CPUID/MSR/... to detect the environment instead of
>> replying on assumptions only satisfied on the current Xen implementation.
>>
>>> In particular, I eventually want an ability for fine-grained toolstack
>>> settings of the available VMX configuration (these being a subset of the
>>> MSRs in the system), (all subject to auditing by Xen to keep it within
>>> hardware and supported bounds), at which point it would be plausible to
>>> spin up a VM, asking for VMX_BASIC_32BIT_ADDRESSES, and checking that
>>> suitable limits were observed/enforced inside the VM.
>>>
>>
>> I do not quite understand the purpose for the fine-grained VMX
>> options. Is it to provide users with a chance to workaround bugs in
>> certain parts of nested virtualization?
>
>I have two usecases in mind.
>
>First is being able to check that Xen functions correctly when certain
>VMX features are unavailable. Recently, we have found a number of
>security issues which only affect older hardware, and older hardware is
>getting harder and harder to come by. Using fine grained features can
>simulate older hardware in some cases.
make sense
>
>Second, and more importantly, that I need to eventually make this all
>work with live migration, including between non-identical hardware.
>Therefore, the fine-grained nature would be used to implement feature
>levelling.
Ah yes, for the live migration! Thanks for the explanation.
Thanks,
Haozhong
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread