"KVM: x86: hyper-v: Fine-grained access check to Hyper-V hypercalls and
MSRs" and "Add support for XMM fast hypercalls" series were developed
at the same time so the later landed without a proper feature bit check
for 'strict' (KVM_CAP_HYPERV_ENFORCE_CPUID) mode. Add it now.
TLFS states that "Availability of the XMM fast hypercall interface is
indicated via the “Hypervisor Feature Identification” CPUID Leaf
(0x40000003, see section 2.4.4) ... Any attempt to use this interface
when the hypervisor does not indicate availability will result in a #UD
fault."
Vitaly Kuznetsov (4):
KVM: x86: hyper-v: Check access to hypercall before reading XMM
registers
KVM: x86: Introduce trace_kvm_hv_hypercall_done()
KVM: x86: hyper-v: Check if guest is allowed to use XMM registers for
hypercall input
KVM: selftests: Test access to XMM fast hypercalls
arch/x86/kvm/hyperv.c | 18 ++++++--
arch/x86/kvm/trace.h | 15 +++++++
.../selftests/kvm/include/x86_64/hyperv.h | 5 ++-
.../selftests/kvm/x86_64/hyperv_features.c | 41 +++++++++++++++++--
4 files changed, 71 insertions(+), 8 deletions(-)
--
2.31.1
Hypercall failures are unusual with potentially far going consequences
so it would be useful to see their results when tracing.
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
arch/x86/kvm/hyperv.c | 1 +
arch/x86/kvm/trace.h | 15 +++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index cb7e045905a5..2945b93dbadd 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -2016,6 +2016,7 @@ static void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
static int kvm_hv_hypercall_complete(struct kvm_vcpu *vcpu, u64 result)
{
+ trace_kvm_hv_hypercall_done(result);
kvm_hv_hypercall_set_result(vcpu, result);
++vcpu->stat.hypercalls;
return kvm_skip_emulated_instruction(vcpu);
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index b484141ea15b..03ebe368333e 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -92,6 +92,21 @@ TRACE_EVENT(kvm_hv_hypercall,
__entry->outgpa)
);
+TRACE_EVENT(kvm_hv_hypercall_done,
+ TP_PROTO(u64 result),
+ TP_ARGS(result),
+
+ TP_STRUCT__entry(
+ __field(__u64, result)
+ ),
+
+ TP_fast_assign(
+ __entry->result = result;
+ ),
+
+ TP_printk("result 0x%llx", __entry->result)
+);
+
/*
* Tracepoint for Xen hypercall.
*/
--
2.31.1
In case guest doesn't have access to the particular hypercall we can avoid
reading XMM registers.
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
arch/x86/kvm/hyperv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index b07592ca92f0..cb7e045905a5 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -2173,9 +2173,6 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
hc.rep_idx = (hc.param >> HV_HYPERCALL_REP_START_OFFSET) & 0xfff;
hc.rep = !!(hc.rep_cnt || hc.rep_idx);
- if (hc.fast && is_xmm_fast_hypercall(&hc))
- kvm_hv_hypercall_read_xmm(&hc);
-
trace_kvm_hv_hypercall(hc.code, hc.fast, hc.rep_cnt, hc.rep_idx,
hc.ingpa, hc.outgpa);
@@ -2184,6 +2181,9 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
goto hypercall_complete;
}
+ if (hc.fast && is_xmm_fast_hypercall(&hc))
+ kvm_hv_hypercall_read_xmm(&hc);
+
switch (hc.code) {
case HVCALL_NOTIFY_LONG_SPIN_WAIT:
if (unlikely(hc.rep)) {
--
2.31.1
HYPERV_CPUID_FEATURES.EDX and an 'XMM fast' hypercall is issued.
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
.../selftests/kvm/include/x86_64/hyperv.h | 5 ++-
.../selftests/kvm/x86_64/hyperv_features.c | 41 +++++++++++++++++--
2 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
index 412eaee7884a..b66910702c0a 100644
--- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h
+++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
@@ -117,7 +117,7 @@
#define HV_X64_GUEST_DEBUGGING_AVAILABLE BIT(1)
#define HV_X64_PERF_MONITOR_AVAILABLE BIT(2)
#define HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE BIT(3)
-#define HV_X64_HYPERCALL_PARAMS_XMM_AVAILABLE BIT(4)
+#define HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE BIT(4)
#define HV_X64_GUEST_IDLE_STATE_AVAILABLE BIT(5)
#define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE BIT(8)
#define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(10)
@@ -182,4 +182,7 @@
#define HV_STATUS_INVALID_CONNECTION_ID 18
#define HV_STATUS_INSUFFICIENT_BUFFERS 19
+/* hypercall options */
+#define HV_HYPERCALL_FAST_BIT BIT(16)
+
#endif /* !SELFTEST_KVM_HYPERV_H */
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
index af27c7e829c1..91d88aaa9899 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
@@ -47,6 +47,7 @@ static void do_wrmsr(u32 idx, u64 val)
}
static int nr_gp;
+static int nr_ud;
static inline u64 hypercall(u64 control, vm_vaddr_t input_address,
vm_vaddr_t output_address)
@@ -80,6 +81,12 @@ static void guest_gp_handler(struct ex_regs *regs)
regs->rip = (uint64_t)&wrmsr_end;
}
+static void guest_ud_handler(struct ex_regs *regs)
+{
+ nr_ud++;
+ regs->rip += 3;
+}
+
struct msr_data {
uint32_t idx;
bool available;
@@ -90,6 +97,7 @@ struct msr_data {
struct hcall_data {
uint64_t control;
uint64_t expect;
+ bool ud_expected;
};
static void guest_msr(struct msr_data *msr)
@@ -117,13 +125,26 @@ static void guest_msr(struct msr_data *msr)
static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall)
{
int i = 0;
+ u64 res, input, output;
wrmsr(HV_X64_MSR_GUEST_OS_ID, LINUX_OS_ID);
wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
while (hcall->control) {
- GUEST_ASSERT(hypercall(hcall->control, pgs_gpa,
- pgs_gpa + 4096) == hcall->expect);
+ nr_ud = 0;
+ if (!(hcall->control & HV_HYPERCALL_FAST_BIT)) {
+ input = pgs_gpa;
+ output = pgs_gpa + 4096;
+ } else {
+ input = output = 0;
+ }
+
+ res = hypercall(hcall->control, input, output);
+ if (hcall->ud_expected)
+ GUEST_ASSERT(nr_ud == 1);
+ else
+ GUEST_ASSERT(res == hcall->expect);
+
GUEST_SYNC(i++);
}
@@ -552,8 +573,18 @@ static void guest_test_hcalls_access(struct kvm_vm *vm, struct hcall_data *hcall
recomm.ebx = 0xfff;
hcall->expect = HV_STATUS_SUCCESS;
break;
-
case 17:
+ /* XMM fast hypercall */
+ hcall->control = HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE | HV_HYPERCALL_FAST_BIT;
+ hcall->ud_expected = true;
+ break;
+ case 18:
+ feat.edx |= HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE;
+ hcall->ud_expected = false;
+ hcall->expect = HV_STATUS_SUCCESS;
+ break;
+
+ case 19:
/* END */
hcall->control = 0;
break;
@@ -625,6 +656,10 @@ int main(void)
/* Test hypercalls */
vm = vm_create_default(VCPU_ID, 0, guest_hcall);
+ vm_init_descriptor_tables(vm);
+ vcpu_init_descriptor_tables(vm, VCPU_ID);
+ vm_install_exception_handler(vm, UD_VECTOR, guest_ud_handler);
+
/* Hypercall input/output */
hcall_page = vm_vaddr_alloc_pages(vm, 2);
memset(addr_gva2hva(vm, hcall_page), 0x0, 2 * getpagesize());
--
2.31.1
TLFS states that "Availability of the XMM fast hypercall interface is
indicated via the “Hypervisor Feature Identification” CPUID Leaf
(0x40000003, see section 2.4.4) ... Any attempt to use this interface
when the hypervisor does not indicate availability will result in a #UD
fault."
Implement the check for 'strict' mode (KVM_CAP_HYPERV_ENFORCE_CPUID).
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
arch/x86/kvm/hyperv.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 2945b93dbadd..0b38f944c6b6 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -2140,6 +2140,7 @@ static bool hv_check_hypercall_access(struct kvm_vcpu_hv *hv_vcpu, u16 code)
int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
{
+ struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
struct kvm_hv_hcall hc;
u64 ret = HV_STATUS_SUCCESS;
@@ -2177,13 +2178,21 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
trace_kvm_hv_hypercall(hc.code, hc.fast, hc.rep_cnt, hc.rep_idx,
hc.ingpa, hc.outgpa);
- if (unlikely(!hv_check_hypercall_access(to_hv_vcpu(vcpu), hc.code))) {
+ if (unlikely(!hv_check_hypercall_access(hv_vcpu, hc.code))) {
ret = HV_STATUS_ACCESS_DENIED;
goto hypercall_complete;
}
- if (hc.fast && is_xmm_fast_hypercall(&hc))
+ if (hc.fast && is_xmm_fast_hypercall(&hc)) {
+ if (unlikely(hv_vcpu->enforce_cpuid &&
+ !(hv_vcpu->cpuid_cache.features_edx &
+ HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE))) {
+ kvm_queue_exception(vcpu, UD_VECTOR);
+ return 1;
+ }
+
kvm_hv_hypercall_read_xmm(&hc);
+ }
switch (hc.code) {
case HVCALL_NOTIFY_LONG_SPIN_WAIT:
--
2.31.1
On Fri, Jul 30, 2021 at 02:26:22PM +0200, Vitaly Kuznetsov wrote:
> In case guest doesn't have access to the particular hypercall we can avoid
> reading XMM registers.
>
> Signed-off-by: Vitaly Kuznetsov <[email protected]>
Reviewed-by: Siddharth Chandrasekaran <[email protected]>
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
On Fri, Jul 30, 2021 at 02:26:23PM +0200, Vitaly Kuznetsov wrote:
> Hypercall failures are unusual with potentially far going consequences
> so it would be useful to see their results when tracing.
>
> Signed-off-by: Vitaly Kuznetsov <[email protected]>
Reviewed-by: Siddharth Chandrasekaran <[email protected]>
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
On Fri, Jul 30, 2021 at 02:26:24PM +0200, Vitaly Kuznetsov wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> TLFS states that "Availability of the XMM fast hypercall interface is
> indicated via the “Hypervisor Feature Identification” CPUID Leaf
> (0x40000003, see section 2.4.4) ... Any attempt to use this interface
> when the hypervisor does not indicate availability will result in a #UD
> fault."
>
> Implement the check for 'strict' mode (KVM_CAP_HYPERV_ENFORCE_CPUID).
>
> Signed-off-by: Vitaly Kuznetsov <[email protected]>
Reviewed-by: Siddharth Chandrasekaran <[email protected]>
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
On Fri, Jul 30, 2021 at 02:26:25PM +0200, Vitaly Kuznetsov wrote:
> HYPERV_CPUID_FEATURES.EDX and an 'XMM fast' hypercall is issued.
>
> Signed-off-by: Vitaly Kuznetsov <[email protected]>
> ---
> .../selftests/kvm/include/x86_64/hyperv.h | 5 ++-
> .../selftests/kvm/x86_64/hyperv_features.c | 41 +++++++++++++++++--
> 2 files changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> index 412eaee7884a..b66910702c0a 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> @@ -117,7 +117,7 @@
> #define HV_X64_GUEST_DEBUGGING_AVAILABLE BIT(1)
> #define HV_X64_PERF_MONITOR_AVAILABLE BIT(2)
> #define HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE BIT(3)
> -#define HV_X64_HYPERCALL_PARAMS_XMM_AVAILABLE BIT(4)
> +#define HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE BIT(4)
> #define HV_X64_GUEST_IDLE_STATE_AVAILABLE BIT(5)
> #define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE BIT(8)
> #define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(10)
> @@ -182,4 +182,7 @@
> #define HV_STATUS_INVALID_CONNECTION_ID 18
> #define HV_STATUS_INSUFFICIENT_BUFFERS 19
>
> +/* hypercall options */
> +#define HV_HYPERCALL_FAST_BIT BIT(16)
> +
> #endif /* !SELFTEST_KVM_HYPERV_H */
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> index af27c7e829c1..91d88aaa9899 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> @@ -47,6 +47,7 @@ static void do_wrmsr(u32 idx, u64 val)
> }
>
> static int nr_gp;
> +static int nr_ud;
>
> static inline u64 hypercall(u64 control, vm_vaddr_t input_address,
> vm_vaddr_t output_address)
> @@ -80,6 +81,12 @@ static void guest_gp_handler(struct ex_regs *regs)
> regs->rip = (uint64_t)&wrmsr_end;
> }
>
> +static void guest_ud_handler(struct ex_regs *regs)
> +{
> + nr_ud++;
> + regs->rip += 3;
> +}
> +
> struct msr_data {
> uint32_t idx;
> bool available;
> @@ -90,6 +97,7 @@ struct msr_data {
> struct hcall_data {
> uint64_t control;
> uint64_t expect;
> + bool ud_expected;
> };
>
> static void guest_msr(struct msr_data *msr)
> @@ -117,13 +125,26 @@ static void guest_msr(struct msr_data *msr)
> static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall)
> {
> int i = 0;
> + u64 res, input, output;
>
> wrmsr(HV_X64_MSR_GUEST_OS_ID, LINUX_OS_ID);
> wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
>
> while (hcall->control) {
> - GUEST_ASSERT(hypercall(hcall->control, pgs_gpa,
> - pgs_gpa + 4096) == hcall->expect);
> + nr_ud = 0;
> + if (!(hcall->control & HV_HYPERCALL_FAST_BIT)) {
> + input = pgs_gpa;
> + output = pgs_gpa + 4096;
> + } else {
> + input = output = 0;
> + }
> +
> + res = hypercall(hcall->control, input, output);
> + if (hcall->ud_expected)
> + GUEST_ASSERT(nr_ud == 1);
Should we also do WRITE_ONCE(nr_ur, 0) here? or perhaps pass the the
expected value of nr_ud + 1 in hcall->ud_expected from caller and do,
if (hcall->ud_expected)
GUEST_ASSERT(nr_ud == hcall->ud_expected);
This way there can be other test that can also expect a UD.
> + else
> + GUEST_ASSERT(res == hcall->expect);
> +
> GUEST_SYNC(i++);
> }
>
> @@ -552,8 +573,18 @@ static void guest_test_hcalls_access(struct kvm_vm *vm, struct hcall_data *hcall
> recomm.ebx = 0xfff;
> hcall->expect = HV_STATUS_SUCCESS;
> break;
> -
> case 17:
> + /* XMM fast hypercall */
> + hcall->control = HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE | HV_HYPERCALL_FAST_BIT;
> + hcall->ud_expected = true;
> + break;
> + case 18:
> + feat.edx |= HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE;
> + hcall->ud_expected = false;
> + hcall->expect = HV_STATUS_SUCCESS;
> + break;
> +
> + case 19:
> /* END */
> hcall->control = 0;
> break;
> @@ -625,6 +656,10 @@ int main(void)
> /* Test hypercalls */
> vm = vm_create_default(VCPU_ID, 0, guest_hcall);
>
> + vm_init_descriptor_tables(vm);
> + vcpu_init_descriptor_tables(vm, VCPU_ID);
> + vm_install_exception_handler(vm, UD_VECTOR, guest_ud_handler);
> +
> /* Hypercall input/output */
> hcall_page = vm_vaddr_alloc_pages(vm, 2);
> memset(addr_gva2hva(vm, hcall_page), 0x0, 2 * getpagesize());
> --
> 2.31.1
>
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
Siddharth Chandrasekaran <[email protected]> writes:
> On Fri, Jul 30, 2021 at 02:26:25PM +0200, Vitaly Kuznetsov wrote:
>> HYPERV_CPUID_FEATURES.EDX and an 'XMM fast' hypercall is issued.
>>
>> Signed-off-by: Vitaly Kuznetsov <[email protected]>
>> ---
>> .../selftests/kvm/include/x86_64/hyperv.h | 5 ++-
>> .../selftests/kvm/x86_64/hyperv_features.c | 41 +++++++++++++++++--
>> 2 files changed, 42 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
>> index 412eaee7884a..b66910702c0a 100644
>> --- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h
>> +++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
>> @@ -117,7 +117,7 @@
>> #define HV_X64_GUEST_DEBUGGING_AVAILABLE BIT(1)
>> #define HV_X64_PERF_MONITOR_AVAILABLE BIT(2)
>> #define HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE BIT(3)
>> -#define HV_X64_HYPERCALL_PARAMS_XMM_AVAILABLE BIT(4)
>> +#define HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE BIT(4)
>> #define HV_X64_GUEST_IDLE_STATE_AVAILABLE BIT(5)
>> #define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE BIT(8)
>> #define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(10)
>> @@ -182,4 +182,7 @@
>> #define HV_STATUS_INVALID_CONNECTION_ID 18
>> #define HV_STATUS_INSUFFICIENT_BUFFERS 19
>>
>> +/* hypercall options */
>> +#define HV_HYPERCALL_FAST_BIT BIT(16)
>> +
>> #endif /* !SELFTEST_KVM_HYPERV_H */
>> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
>> index af27c7e829c1..91d88aaa9899 100644
>> --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
>> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
>> @@ -47,6 +47,7 @@ static void do_wrmsr(u32 idx, u64 val)
>> }
>>
>> static int nr_gp;
>> +static int nr_ud;
>>
>> static inline u64 hypercall(u64 control, vm_vaddr_t input_address,
>> vm_vaddr_t output_address)
>> @@ -80,6 +81,12 @@ static void guest_gp_handler(struct ex_regs *regs)
>> regs->rip = (uint64_t)&wrmsr_end;
>> }
>>
>> +static void guest_ud_handler(struct ex_regs *regs)
>> +{
>> + nr_ud++;
>> + regs->rip += 3;
>> +}
>> +
>> struct msr_data {
>> uint32_t idx;
>> bool available;
>> @@ -90,6 +97,7 @@ struct msr_data {
>> struct hcall_data {
>> uint64_t control;
>> uint64_t expect;
>> + bool ud_expected;
>> };
>>
>> static void guest_msr(struct msr_data *msr)
>> @@ -117,13 +125,26 @@ static void guest_msr(struct msr_data *msr)
>> static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall)
>> {
>> int i = 0;
>> + u64 res, input, output;
>>
>> wrmsr(HV_X64_MSR_GUEST_OS_ID, LINUX_OS_ID);
>> wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
>>
>> while (hcall->control) {
>> - GUEST_ASSERT(hypercall(hcall->control, pgs_gpa,
>> - pgs_gpa + 4096) == hcall->expect);
>> + nr_ud = 0;
>> + if (!(hcall->control & HV_HYPERCALL_FAST_BIT)) {
>> + input = pgs_gpa;
>> + output = pgs_gpa + 4096;
>> + } else {
>> + input = output = 0;
>> + }
>> +
>> + res = hypercall(hcall->control, input, output);
>> + if (hcall->ud_expected)
>> + GUEST_ASSERT(nr_ud == 1);
>
> Should we also do WRITE_ONCE(nr_ur, 0) here?
It could probably make sense to replace 'nr_ud = 0' above with this so
compiler doesn't screw us up one day..
> or perhaps pass the the
> expected value of nr_ud + 1 in hcall->ud_expected from caller and do,
>
> if (hcall->ud_expected)
> GUEST_ASSERT(nr_ud == hcall->ud_expected);
>
> This way there can be other test that can also expect a UD.
My idea was that we don't really need to count #UDs for now, just
checking the fact that it happened is OK so I reset nr_ud before doing
the hypercall and check it after. It is possible to add more tests with
'ud_expected' this way.
>
>> + else
>> + GUEST_ASSERT(res == hcall->expect);
>> +
>> GUEST_SYNC(i++);
>> }
>>
>> @@ -552,8 +573,18 @@ static void guest_test_hcalls_access(struct kvm_vm *vm, struct hcall_data *hcall
>> recomm.ebx = 0xfff;
>> hcall->expect = HV_STATUS_SUCCESS;
>> break;
>> -
>> case 17:
>> + /* XMM fast hypercall */
>> + hcall->control = HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE | HV_HYPERCALL_FAST_BIT;
>> + hcall->ud_expected = true;
>> + break;
>> + case 18:
>> + feat.edx |= HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE;
>> + hcall->ud_expected = false;
>> + hcall->expect = HV_STATUS_SUCCESS;
>> + break;
>> +
>> + case 19:
>> /* END */
>> hcall->control = 0;
>> break;
>> @@ -625,6 +656,10 @@ int main(void)
>> /* Test hypercalls */
>> vm = vm_create_default(VCPU_ID, 0, guest_hcall);
>>
>> + vm_init_descriptor_tables(vm);
>> + vcpu_init_descriptor_tables(vm, VCPU_ID);
>> + vm_install_exception_handler(vm, UD_VECTOR, guest_ud_handler);
>> +
>> /* Hypercall input/output */
>> hcall_page = vm_vaddr_alloc_pages(vm, 2);
>> memset(addr_gva2hva(vm, hcall_page), 0x0, 2 * getpagesize());
>> --
>> 2.31.1
>>
>
>
>
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879
>
>
>
--
Vitaly
On Fri, Jul 30, 2021 at 04:50:06PM +0200, Vitaly Kuznetsov wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> Siddharth Chandrasekaran <[email protected]> writes:
>
> > On Fri, Jul 30, 2021 at 02:26:25PM +0200, Vitaly Kuznetsov wrote:
> >> HYPERV_CPUID_FEATURES.EDX and an 'XMM fast' hypercall is issued.
> >>
> >> Signed-off-by: Vitaly Kuznetsov <[email protected]>
> >> ---
> >> .../selftests/kvm/include/x86_64/hyperv.h | 5 ++-
> >> .../selftests/kvm/x86_64/hyperv_features.c | 41 +++++++++++++++++--
> >> 2 files changed, 42 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> >> index 412eaee7884a..b66910702c0a 100644
> >> --- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> >> +++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> >> @@ -117,7 +117,7 @@
> >> #define HV_X64_GUEST_DEBUGGING_AVAILABLE BIT(1)
> >> #define HV_X64_PERF_MONITOR_AVAILABLE BIT(2)
> >> #define HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE BIT(3)
> >> -#define HV_X64_HYPERCALL_PARAMS_XMM_AVAILABLE BIT(4)
> >> +#define HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE BIT(4)
> >> #define HV_X64_GUEST_IDLE_STATE_AVAILABLE BIT(5)
> >> #define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE BIT(8)
> >> #define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(10)
> >> @@ -182,4 +182,7 @@
> >> #define HV_STATUS_INVALID_CONNECTION_ID 18
> >> #define HV_STATUS_INSUFFICIENT_BUFFERS 19
> >>
> >> +/* hypercall options */
> >> +#define HV_HYPERCALL_FAST_BIT BIT(16)
> >> +
> >> #endif /* !SELFTEST_KVM_HYPERV_H */
> >> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> >> index af27c7e829c1..91d88aaa9899 100644
> >> --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> >> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> >> @@ -47,6 +47,7 @@ static void do_wrmsr(u32 idx, u64 val)
> >> }
> >>
> >> static int nr_gp;
> >> +static int nr_ud;
> >>
> >> static inline u64 hypercall(u64 control, vm_vaddr_t input_address,
> >> vm_vaddr_t output_address)
> >> @@ -80,6 +81,12 @@ static void guest_gp_handler(struct ex_regs *regs)
> >> regs->rip = (uint64_t)&wrmsr_end;
> >> }
> >>
> >> +static void guest_ud_handler(struct ex_regs *regs)
> >> +{
> >> + nr_ud++;
> >> + regs->rip += 3;
> >> +}
> >> +
> >> struct msr_data {
> >> uint32_t idx;
> >> bool available;
> >> @@ -90,6 +97,7 @@ struct msr_data {
> >> struct hcall_data {
> >> uint64_t control;
> >> uint64_t expect;
> >> + bool ud_expected;
> >> };
> >>
> >> static void guest_msr(struct msr_data *msr)
> >> @@ -117,13 +125,26 @@ static void guest_msr(struct msr_data *msr)
> >> static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall)
> >> {
> >> int i = 0;
> >> + u64 res, input, output;
> >>
> >> wrmsr(HV_X64_MSR_GUEST_OS_ID, LINUX_OS_ID);
> >> wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
> >>
> >> while (hcall->control) {
> >> - GUEST_ASSERT(hypercall(hcall->control, pgs_gpa,
> >> - pgs_gpa + 4096) == hcall->expect);
> >> + nr_ud = 0;
> >> + if (!(hcall->control & HV_HYPERCALL_FAST_BIT)) {
> >> + input = pgs_gpa;
> >> + output = pgs_gpa + 4096;
> >> + } else {
> >> + input = output = 0;
> >> + }
> >> +
> >> + res = hypercall(hcall->control, input, output);
> >> + if (hcall->ud_expected)
> >> + GUEST_ASSERT(nr_ud == 1);
> >
> > Should we also do WRITE_ONCE(nr_ur, 0) here?
>
> It could probably make sense to replace 'nr_ud = 0' above with this so
> compiler doesn't screw us up one day..
>
> > or perhaps pass the the
> > expected value of nr_ud + 1 in hcall->ud_expected from caller and do,
> >
> > if (hcall->ud_expected)
> > GUEST_ASSERT(nr_ud == hcall->ud_expected);
> >
> > This way there can be other test that can also expect a UD.
>
> My idea was that we don't really need to count #UDs for now, just
> checking the fact that it happened is OK so I reset nr_ud before doing
> the hypercall and check it after. It is possible to add more tests with
> 'ud_expected' this way.
Oops, my bad, didn't notice that you were resetting nr_ud before
hypercall(). Thanks for clarifying.
Reviewed-by: Siddharth Chandrasekaran <[email protected]>
~ Sid.
> >
> >> + else
> >> + GUEST_ASSERT(res == hcall->expect);
> >> +
> >> GUEST_SYNC(i++);
> >> }
> >>
> >> @@ -552,8 +573,18 @@ static void guest_test_hcalls_access(struct kvm_vm *vm, struct hcall_data *hcall
> >> recomm.ebx = 0xfff;
> >> hcall->expect = HV_STATUS_SUCCESS;
> >> break;
> >> -
> >> case 17:
> >> + /* XMM fast hypercall */
> >> + hcall->control = HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE | HV_HYPERCALL_FAST_BIT;
> >> + hcall->ud_expected = true;
> >> + break;
> >> + case 18:
> >> + feat.edx |= HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE;
> >> + hcall->ud_expected = false;
> >> + hcall->expect = HV_STATUS_SUCCESS;
> >> + break;
> >> +
> >> + case 19:
> >> /* END */
> >> hcall->control = 0;
> >> break;
> >> @@ -625,6 +656,10 @@ int main(void)
> >> /* Test hypercalls */
> >> vm = vm_create_default(VCPU_ID, 0, guest_hcall);
> >>
> >> + vm_init_descriptor_tables(vm);
> >> + vcpu_init_descriptor_tables(vm, VCPU_ID);
> >> + vm_install_exception_handler(vm, UD_VECTOR, guest_ud_handler);
> >> +
> >> /* Hypercall input/output */
> >> hcall_page = vm_vaddr_alloc_pages(vm, 2);
> >> memset(addr_gva2hva(vm, hcall_page), 0x0, 2 * getpagesize());
> >> --
> >> 2.31.1
> >>
> >
> >
> >
> > Amazon Development Center Germany GmbH
> > Krausenstr. 38
> > 10117 Berlin
> > Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
> > Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> > Sitz: Berlin
> > Ust-ID: DE 289 237 879
> >
> >
> >
>
> --
> Vitaly
>
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
On 30/07/21 16:50, Vitaly Kuznetsov wrote:
>> Should we also do WRITE_ONCE(nr_ur, 0) here?
> It could probably make sense to replace 'nr_ud = 0' above with this so
> compiler doesn't screw us up one day..
>
It should be okay with the "memory" clobber.
Paolo
On 30/07/21 14:26, Vitaly Kuznetsov wrote:
> "KVM: x86: hyper-v: Fine-grained access check to Hyper-V hypercalls and
> MSRs" and "Add support for XMM fast hypercalls" series were developed
> at the same time so the later landed without a proper feature bit check
> for 'strict' (KVM_CAP_HYPERV_ENFORCE_CPUID) mode. Add it now.
>
> TLFS states that "Availability of the XMM fast hypercall interface is
> indicated via the “Hypervisor Feature Identification” CPUID Leaf
> (0x40000003, see section 2.4.4) ... Any attempt to use this interface
> when the hypervisor does not indicate availability will result in a #UD
> fault."
>
> Vitaly Kuznetsov (4):
> KVM: x86: hyper-v: Check access to hypercall before reading XMM
> registers
> KVM: x86: Introduce trace_kvm_hv_hypercall_done()
> KVM: x86: hyper-v: Check if guest is allowed to use XMM registers for
> hypercall input
> KVM: selftests: Test access to XMM fast hypercalls
>
> arch/x86/kvm/hyperv.c | 18 ++++++--
> arch/x86/kvm/trace.h | 15 +++++++
> .../selftests/kvm/include/x86_64/hyperv.h | 5 ++-
> .../selftests/kvm/x86_64/hyperv_features.c | 41 +++++++++++++++++--
> 4 files changed, 71 insertions(+), 8 deletions(-)
>
Queued, thanks.
Paolo
On 30/07/21 14:26, Vitaly Kuznetsov wrote:
> "KVM: x86: hyper-v: Fine-grained access check to Hyper-V hypercalls and
> MSRs" and "Add support for XMM fast hypercalls" series were developed
> at the same time so the later landed without a proper feature bit check
> for 'strict' (KVM_CAP_HYPERV_ENFORCE_CPUID) mode. Add it now.
>
> TLFS states that "Availability of the XMM fast hypercall interface is
> indicated via the “Hypervisor Feature Identification” CPUID Leaf
> (0x40000003, see section 2.4.4) ... Any attempt to use this interface
> when the hypervisor does not indicate availability will result in a #UD
> fault."
>
> Vitaly Kuznetsov (4):
> KVM: x86: hyper-v: Check access to hypercall before reading XMM
> registers
> KVM: x86: Introduce trace_kvm_hv_hypercall_done()
> KVM: x86: hyper-v: Check if guest is allowed to use XMM registers for
> hypercall input
> KVM: selftests: Test access to XMM fast hypercalls
>
> arch/x86/kvm/hyperv.c | 18 ++++++--
> arch/x86/kvm/trace.h | 15 +++++++
> .../selftests/kvm/include/x86_64/hyperv.h | 5 ++-
> .../selftests/kvm/x86_64/hyperv_features.c | 41 +++++++++++++++++--
> 4 files changed, 71 insertions(+), 8 deletions(-)
>
Queued, thanks.
Paolo