* [PATCH v11 2/7] x86/kvm/hyper-v: Simplify addition for custom cpuid leafs
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron
In-Reply-To: <20200424113746.3473563-1-arilou@gmail.com>
Simlify the code to define a new cpuid leaf group by enabled feature.
This also fixes a bug in which the max cpuid leaf was always set to
HYPERV_CPUID_NESTED_FEATURES regardless if nesting is supported or not.
Any new CPUID group needs to consider the max leaf and be added in the
correct order, in this method there are two rules:
1. Each cpuid leaf group must be order in an ascending order
2. The appending for the cpuid leafs by features also needs to happen by
ascending order.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
arch/x86/kvm/hyperv.c | 46 ++++++++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index bcefa9d4e57e..ab3e9dbcabbe 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1785,27 +1785,45 @@ int kvm_vm_ioctl_hv_eventfd(struct kvm *kvm, struct kvm_hyperv_eventfd *args)
return kvm_hv_eventfd_assign(kvm, args->conn_id, args->fd);
}
+/* Must be sorted in ascending order by function */
+static struct kvm_cpuid_entry2 core_cpuid_entries[] = {
+ { .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
+ { .function = HYPERV_CPUID_INTERFACE },
+ { .function = HYPERV_CPUID_VERSION },
+ { .function = HYPERV_CPUID_FEATURES },
+ { .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
+ { .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
+};
+
+static struct kvm_cpuid_entry2 evmcs_cpuid_entries[] = {
+ { .function = HYPERV_CPUID_NESTED_FEATURES },
+};
+
+#define HV_MAX_CPUID_ENTRIES \
+ (ARRAY_SIZE(core_cpuid_entries) +\
+ ARRAY_SIZE(evmcs_cpuid_entries))
+
int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
struct kvm_cpuid_entry2 __user *entries)
{
uint16_t evmcs_ver = 0;
- struct kvm_cpuid_entry2 cpuid_entries[] = {
- { .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
- { .function = HYPERV_CPUID_INTERFACE },
- { .function = HYPERV_CPUID_VERSION },
- { .function = HYPERV_CPUID_FEATURES },
- { .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
- { .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
- { .function = HYPERV_CPUID_NESTED_FEATURES },
- };
- int i, nent = ARRAY_SIZE(cpuid_entries);
+ struct kvm_cpuid_entry2 cpuid_entries[HV_MAX_CPUID_ENTRIES];
+ int i, nent = 0;
+
+ /* Set the core cpuid entries required for Hyper-V */
+ memcpy(&cpuid_entries[nent], &core_cpuid_entries,
+ sizeof(core_cpuid_entries));
+ nent = ARRAY_SIZE(core_cpuid_entries);
if (kvm_x86_ops.nested_get_evmcs_version)
evmcs_ver = kvm_x86_ops.nested_get_evmcs_version(vcpu);
- /* Skip NESTED_FEATURES if eVMCS is not supported */
- if (!evmcs_ver)
- --nent;
+ if (evmcs_ver) {
+ /* EVMCS is enabled, add the required EVMCS CPUID leafs */
+ memcpy(&cpuid_entries[nent], &evmcs_cpuid_entries,
+ sizeof(evmcs_cpuid_entries));
+ nent += ARRAY_SIZE(evmcs_cpuid_entries);
+ }
if (cpuid->nent < nent)
return -E2BIG;
@@ -1821,7 +1839,7 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
case HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS:
memcpy(signature, "Linux KVM Hv", 12);
- ent->eax = HYPERV_CPUID_NESTED_FEATURES;
+ ent->eax = cpuid_entries[nent - 1].function;
ent->ebx = signature[0];
ent->ecx = signature[1];
ent->edx = signature[2];
--
2.24.1
^ permalink raw reply related
* [PATCH v11 1/7] x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron
In-Reply-To: <20200424113746.3473563-1-arilou@gmail.com>
The problem the patch is trying to address is the fact that 'struct
kvm_hyperv_exit' has different layout on when compiling in 32 and 64 bit
modes.
In 64-bit mode the default alignment boundary is 64 bits thus
forcing extra gaps after 'type' and 'msr' but in 32-bit mode the
boundary is at 32 bits thus no extra gaps.
This is an issue as even when the kernel is 64 bit, the userspace using
the interface can be both 32 and 64 bit but the same 32 bit userspace has
to work with 32 bit kernel.
The issue is fixed by forcing the 64 bit layout, this leads to ABI
change for 32 bit builds and while we are obviously breaking '32 bit
userspace with 32 bit kernel' case, we're fixing the '32 bit userspace
with 64 bit kernel' one.
As the interface has no (known) users and 32 bit KVM is rather baroque
nowadays, this seems like a reasonable decision.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
Documentation/virt/kvm/api.rst | 2 ++
include/uapi/linux/kvm.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index efbbe570aa9b..750d005a75bc 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -5067,9 +5067,11 @@ EOI was received.
#define KVM_EXIT_HYPERV_SYNIC 1
#define KVM_EXIT_HYPERV_HCALL 2
__u32 type;
+ __u32 pad1;
union {
struct {
__u32 msr;
+ __u32 pad2;
__u64 control;
__u64 evt_page;
__u64 msg_page;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 428c7dde6b4b..9cdc5356f542 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -189,9 +189,11 @@ struct kvm_hyperv_exit {
#define KVM_EXIT_HYPERV_SYNIC 1
#define KVM_EXIT_HYPERV_HCALL 2
__u32 type;
+ __u32 pad1;
union {
struct {
__u32 msr;
+ __u32 pad2;
__u64 control;
__u64 evt_page;
__u64 msg_page;
--
2.24.1
^ permalink raw reply related
* [PATCH v11 3/7] x86/hyper-v: Add synthetic debugger definitions
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron, Michael Kelley
In-Reply-To: <20200424113746.3473563-1-arilou@gmail.com>
Hyper-V synthetic debugger has two modes, one that uses MSRs and
the other that use Hypercalls.
Add all the required definitions to both types of synthetic debugger
interface.
Some of the required new CPUIDs and MSRs are not documented in the TLFS
so they are in hyperv.h instead.
The reason they are not documented is because they are subjected to be
removed in future versions of Windows.
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
arch/x86/include/asm/hyperv-tlfs.h | 6 ++++++
arch/x86/kvm/hyperv.h | 27 +++++++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 29336574d0bc..53ef6b7bd380 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -131,6 +131,8 @@
#define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE BIT(8)
/* Crash MSR available */
#define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(10)
+/* Support for debug MSRs available */
+#define HV_FEATURE_DEBUG_MSRS_AVAILABLE BIT(11)
/* stimer Direct Mode is available */
#define HV_STIMER_DIRECT_MODE_AVAILABLE BIT(19)
@@ -376,6 +378,9 @@ struct hv_tsc_emulation_status {
#define HVCALL_SEND_IPI_EX 0x0015
#define HVCALL_POST_MESSAGE 0x005c
#define HVCALL_SIGNAL_EVENT 0x005d
+#define HVCALL_POST_DEBUG_DATA 0x0069
+#define HVCALL_RETRIEVE_DEBUG_DATA 0x006a
+#define HVCALL_RESET_DEBUG_SESSION 0x006b
#define HVCALL_RETARGET_INTERRUPT 0x007e
#define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
#define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
@@ -422,6 +427,7 @@ enum HV_GENERIC_SET_FORMAT {
#define HV_STATUS_INVALID_HYPERCALL_INPUT 3
#define HV_STATUS_INVALID_ALIGNMENT 4
#define HV_STATUS_INVALID_PARAMETER 5
+#define HV_STATUS_OPERATION_DENIED 8
#define HV_STATUS_INSUFFICIENT_MEMORY 11
#define HV_STATUS_INVALID_PORT_ID 17
#define HV_STATUS_INVALID_CONNECTION_ID 18
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 757cb578101c..7f50ff0bad07 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -23,6 +23,33 @@
#include <linux/kvm_host.h>
+/*
+ * The #defines related to the synthetic debugger are required by KDNet, but
+ * they are not documented in the Hyper-V TLFS because the synthetic debugger
+ * functionality has been deprecated and is subject to removal in future
+ * versions of Windows.
+ */
+#define HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
+#define HYPERV_CPUID_SYNDBG_INTERFACE 0x40000081
+#define HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
+
+/*
+ * Hyper-V synthetic debugger platform capabilities
+ * These are HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES.EAX bits.
+ */
+#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING BIT(1)
+
+/* Hyper-V Synthetic debug options MSR */
+#define HV_X64_MSR_SYNDBG_CONTROL 0x400000F1
+#define HV_X64_MSR_SYNDBG_STATUS 0x400000F2
+#define HV_X64_MSR_SYNDBG_SEND_BUFFER 0x400000F3
+#define HV_X64_MSR_SYNDBG_RECV_BUFFER 0x400000F4
+#define HV_X64_MSR_SYNDBG_PENDING_BUFFER 0x400000F5
+#define HV_X64_MSR_SYNDBG_OPTIONS 0x400000FF
+
+/* Hyper-V HV_X64_MSR_SYNDBG_OPTIONS bits */
+#define HV_X64_SYNDBG_OPTION_USE_HCALLS BIT(2)
+
static inline struct kvm_vcpu_hv *vcpu_to_hv_vcpu(struct kvm_vcpu *vcpu)
{
return &vcpu->arch.hyperv;
--
2.24.1
^ permalink raw reply related
* [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron
In-Reply-To: <20200424113746.3473563-1-arilou@gmail.com>
Add support for Hyper-V synthetic debugger (syndbg) interface.
The syndbg interface is using MSRs to emulate a way to send/recv packets
data.
The debug transport dll (kdvm/kdnet) will identify if Hyper-V is enabled
and if it supports the synthetic debugger interface it will attempt to
use it, instead of trying to initialize a network adapter.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
Documentation/virt/kvm/api.rst | 16 ++++
arch/x86/include/asm/kvm_host.h | 14 +++
arch/x86/kvm/hyperv.c | 165 +++++++++++++++++++++++++++++++-
arch/x86/kvm/hyperv.h | 6 ++
arch/x86/kvm/trace.h | 51 ++++++++++
arch/x86/kvm/x86.c | 13 +++
include/uapi/linux/kvm.h | 11 +++
7 files changed, 273 insertions(+), 3 deletions(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 750d005a75bc..52ba12758f7c 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -5066,6 +5066,7 @@ EOI was received.
struct kvm_hyperv_exit {
#define KVM_EXIT_HYPERV_SYNIC 1
#define KVM_EXIT_HYPERV_HCALL 2
+ #define KVM_EXIT_HYPERV_SYNDBG 3
__u32 type;
__u32 pad1;
union {
@@ -5081,6 +5082,15 @@ EOI was received.
__u64 result;
__u64 params[2];
} hcall;
+ struct {
+ __u32 msr;
+ __u32 pad2;
+ __u64 control;
+ __u64 status;
+ __u64 send_page;
+ __u64 recv_page;
+ __u64 pending_page;
+ } syndbg;
} u;
};
/* KVM_EXIT_HYPERV */
@@ -5097,6 +5107,12 @@ Hyper-V SynIC state change. Notification is used to remap SynIC
event/message pages and to enable/disable SynIC messages/events processing
in userspace.
+ - KVM_EXIT_HYPERV_SYNDBG -- synchronously notify user-space about
+
+Hyper-V Synthetic debugger state change. Notification is used to either update
+the pending_page location or to send a control command (send the buffer located
+in send_page or recv a buffer to recv_page).
+
::
/* KVM_EXIT_ARM_NISV */
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 42a2d0d3984a..563a9e69f113 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -854,6 +854,19 @@ struct kvm_apic_map {
struct kvm_lapic *phys_map[];
};
+/* Hyper-V synthetic debugger (SynDbg)*/
+struct kvm_hv_syndbg {
+ struct {
+ u64 control;
+ u64 status;
+ u64 send_page;
+ u64 recv_page;
+ u64 pending_page;
+ } control;
+ u64 options;
+ bool active;
+};
+
/* Hyper-V emulation context */
struct kvm_hv {
struct mutex hv_lock;
@@ -877,6 +890,7 @@ struct kvm_hv {
atomic_t num_mismatched_vp_indexes;
struct hv_partition_assist_pg *hv_pa_pg;
+ struct kvm_hv_syndbg hv_syndbg;
};
enum kvm_irqchip_mode {
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index ab3e9dbcabbe..435516595090 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -266,6 +266,117 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
return ret;
}
+void kvm_hv_activate_syndbg(struct kvm_vcpu *vcpu)
+{
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+
+ syndbg->active = true;
+}
+
+static int kvm_hv_syndbg_complete_userspace(struct kvm_vcpu *vcpu)
+{
+ struct kvm *kvm = vcpu->kvm;
+ struct kvm_hv *hv = &kvm->arch.hyperv;
+
+ if (vcpu->run->hyperv.u.syndbg.msr == HV_X64_MSR_SYNDBG_CONTROL)
+ hv->hv_syndbg.control.status =
+ vcpu->run->hyperv.u.syndbg.status;
+ return 1;
+}
+
+static void syndbg_exit(struct kvm_vcpu *vcpu, u32 msr)
+{
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+ struct kvm_vcpu_hv *hv_vcpu = &vcpu->arch.hyperv;
+
+ hv_vcpu->exit.type = KVM_EXIT_HYPERV_SYNDBG;
+ hv_vcpu->exit.u.syndbg.msr = msr;
+ hv_vcpu->exit.u.syndbg.control = syndbg->control.control;
+ hv_vcpu->exit.u.syndbg.send_page = syndbg->control.send_page;
+ hv_vcpu->exit.u.syndbg.recv_page = syndbg->control.recv_page;
+ hv_vcpu->exit.u.syndbg.pending_page = syndbg->control.pending_page;
+ vcpu->arch.complete_userspace_io =
+ kvm_hv_syndbg_complete_userspace;
+
+ kvm_make_request(KVM_REQ_HV_EXIT, vcpu);
+}
+
+static int syndbg_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
+{
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+
+ if (!syndbg->active && !host)
+ return 1;
+
+ trace_kvm_hv_syndbg_set_msr(vcpu->vcpu_id,
+ vcpu_to_hv_vcpu(vcpu)->vp_index, msr, data);
+ switch (msr) {
+ case HV_X64_MSR_SYNDBG_CONTROL:
+ syndbg->control.control = data;
+ if (!host)
+ syndbg_exit(vcpu, msr);
+ break;
+ case HV_X64_MSR_SYNDBG_STATUS:
+ syndbg->control.status = data;
+ break;
+ case HV_X64_MSR_SYNDBG_SEND_BUFFER:
+ syndbg->control.send_page = data;
+ break;
+ case HV_X64_MSR_SYNDBG_RECV_BUFFER:
+ syndbg->control.recv_page = data;
+ break;
+ case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ syndbg->control.pending_page = data;
+ if (!host)
+ syndbg_exit(vcpu, msr);
+ break;
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ syndbg->options = data;
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int syndbg_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
+{
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+
+ if (!syndbg->active && !host)
+ return 1;
+
+ switch (msr) {
+ case HV_X64_MSR_SYNDBG_CONTROL:
+ *pdata = syndbg->control.control;
+ break;
+ case HV_X64_MSR_SYNDBG_STATUS:
+ *pdata = syndbg->control.status;
+ break;
+ case HV_X64_MSR_SYNDBG_SEND_BUFFER:
+ *pdata = syndbg->control.send_page;
+ break;
+ case HV_X64_MSR_SYNDBG_RECV_BUFFER:
+ *pdata = syndbg->control.recv_page;
+ break;
+ case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ *pdata = syndbg->control.pending_page;
+ break;
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ *pdata = syndbg->options;
+ break;
+ default:
+ break;
+ }
+
+ trace_kvm_hv_syndbg_get_msr(vcpu->vcpu_id,
+ vcpu_to_hv_vcpu(vcpu)->vp_index, msr,
+ *pdata);
+
+ return 0;
+}
+
static int synic_get_msr(struct kvm_vcpu_hv_synic *synic, u32 msr, u64 *pdata,
bool host)
{
@@ -800,6 +911,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
case HV_X64_MSR_REENLIGHTENMENT_CONTROL:
case HV_X64_MSR_TSC_EMULATION_CONTROL:
case HV_X64_MSR_TSC_EMULATION_STATUS:
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
r = true;
break;
}
@@ -1061,6 +1174,9 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data,
if (!host)
return 1;
break;
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ return syndbg_set_msr(vcpu, msr, data, host);
default:
vcpu_unimpl(vcpu, "Hyper-V unhandled wrmsr: 0x%x data 0x%llx\n",
msr, data);
@@ -1190,7 +1306,8 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
return 0;
}
-static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
+static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata,
+ bool host)
{
u64 data = 0;
struct kvm *kvm = vcpu->kvm;
@@ -1227,6 +1344,9 @@ static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
case HV_X64_MSR_TSC_EMULATION_STATUS:
data = hv->hv_tsc_emulation_status;
break;
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ return syndbg_get_msr(vcpu, msr, pdata, host);
default:
vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
return 1;
@@ -1316,7 +1436,7 @@ int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
int r;
mutex_lock(&vcpu->kvm->arch.hyperv.hv_lock);
- r = kvm_hv_get_msr_pw(vcpu, msr, pdata);
+ r = kvm_hv_get_msr_pw(vcpu, msr, pdata, host);
mutex_unlock(&vcpu->kvm->arch.hyperv.hv_lock);
return r;
} else
@@ -1799,9 +1919,16 @@ static struct kvm_cpuid_entry2 evmcs_cpuid_entries[] = {
{ .function = HYPERV_CPUID_NESTED_FEATURES },
};
+static struct kvm_cpuid_entry2 syndbg_cpuid_entries[] = {
+ { .function = HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS },
+ { .function = HYPERV_CPUID_SYNDBG_INTERFACE },
+ { .function = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES },
+};
+
#define HV_MAX_CPUID_ENTRIES \
(ARRAY_SIZE(core_cpuid_entries) +\
- ARRAY_SIZE(evmcs_cpuid_entries))
+ ARRAY_SIZE(evmcs_cpuid_entries) +\
+ ARRAY_SIZE(syndbg_cpuid_entries))
int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
struct kvm_cpuid_entry2 __user *entries)
@@ -1809,6 +1936,7 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
uint16_t evmcs_ver = 0;
struct kvm_cpuid_entry2 cpuid_entries[HV_MAX_CPUID_ENTRIES];
int i, nent = 0;
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
/* Set the core cpuid entries required for Hyper-V */
memcpy(&cpuid_entries[nent], &core_cpuid_entries,
@@ -1825,6 +1953,13 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
nent += ARRAY_SIZE(evmcs_cpuid_entries);
}
+ if (syndbg->active) {
+ /* Syndbg is enabled, add the required Syndbg CPUID leafs */
+ memcpy(&cpuid_entries[nent], &syndbg_cpuid_entries,
+ sizeof(syndbg_cpuid_entries));
+ nent += ARRAY_SIZE(syndbg_cpuid_entries);
+ }
+
if (cpuid->nent < nent)
return -E2BIG;
@@ -1878,6 +2013,12 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
ent->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
+ if (syndbg->active) {
+ ent->ebx |= HV_X64_DEBUGGING;
+ ent->edx |= HV_X64_GUEST_DEBUGGING_AVAILABLE;
+ ent->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
+ }
+
/*
* Direct Synthetic timers only make sense with in-kernel
* LAPIC
@@ -1921,6 +2062,24 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
break;
+ case HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS:
+ memcpy(signature, "Linux KVM Hv", 12);
+
+ ent->eax = 0;
+ ent->ebx = signature[0];
+ ent->ecx = signature[1];
+ ent->edx = signature[2];
+ break;
+
+ case HYPERV_CPUID_SYNDBG_INTERFACE:
+ memcpy(signature, "VS#1\0\0\0\0\0\0\0\0", 12);
+ ent->eax = signature[0];
+ break;
+
+ case HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES:
+ ent->eax |= HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
+ break;
+
default:
break;
}
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 7f50ff0bad07..50cca85b5e48 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -73,6 +73,11 @@ static inline struct kvm_vcpu *synic_to_vcpu(struct kvm_vcpu_hv_synic *synic)
return hv_vcpu_to_vcpu(container_of(synic, struct kvm_vcpu_hv, synic));
}
+static inline struct kvm_hv_syndbg *vcpu_to_hv_syndbg(struct kvm_vcpu *vcpu)
+{
+ return &vcpu->kvm->arch.hyperv.hv_syndbg;
+}
+
int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host);
@@ -83,6 +88,7 @@ void kvm_hv_irq_routing_update(struct kvm *kvm);
int kvm_hv_synic_set_irq(struct kvm *kvm, u32 vcpu_id, u32 sint);
void kvm_hv_synic_send_eoi(struct kvm_vcpu *vcpu, int vector);
int kvm_hv_activate_synic(struct kvm_vcpu *vcpu, bool dont_zero_synic_pages);
+void kvm_hv_activate_syndbg(struct kvm_vcpu *vcpu);
void kvm_hv_vcpu_init(struct kvm_vcpu *vcpu);
void kvm_hv_vcpu_postcreate(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 249062f24b94..df95c45ec3bb 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -1539,6 +1539,57 @@ TRACE_EVENT(kvm_nested_vmenter_failed,
__print_symbolic(__entry->err, VMX_VMENTER_INSTRUCTION_ERRORS))
);
+/*
+ * Tracepoint for syndbg_set_msr.
+ */
+TRACE_EVENT(kvm_hv_syndbg_set_msr,
+ TP_PROTO(int vcpu_id, u32 vp_index, u32 msr, u64 data),
+ TP_ARGS(vcpu_id, vp_index, msr, data),
+
+ TP_STRUCT__entry(
+ __field(int, vcpu_id)
+ __field(u32, vp_index)
+ __field(u32, msr)
+ __field(u64, data)
+ ),
+
+ TP_fast_assign(
+ __entry->vcpu_id = vcpu_id;
+ __entry->vp_index = vp_index;
+ __entry->msr = msr;
+ __entry->data = data;
+ ),
+
+ TP_printk("vcpu_id %d vp_index %u msr 0x%x data 0x%llx",
+ __entry->vcpu_id, __entry->vp_index, __entry->msr,
+ __entry->data)
+);
+
+/*
+ * Tracepoint for syndbg_get_msr.
+ */
+TRACE_EVENT(kvm_hv_syndbg_get_msr,
+ TP_PROTO(int vcpu_id, u32 vp_index, u32 msr, u64 data),
+ TP_ARGS(vcpu_id, vp_index, msr, data),
+
+ TP_STRUCT__entry(
+ __field(int, vcpu_id)
+ __field(u32, vp_index)
+ __field(u32, msr)
+ __field(u64, data)
+ ),
+
+ TP_fast_assign(
+ __entry->vcpu_id = vcpu_id;
+ __entry->vp_index = vp_index;
+ __entry->msr = msr;
+ __entry->data = data;
+ ),
+
+ TP_printk("vcpu_id %d vp_index %u msr 0x%x data 0x%llx",
+ __entry->vcpu_id, __entry->vp_index, __entry->msr,
+ __entry->data)
+);
#endif /* _TRACE_KVM_H */
#undef TRACE_INCLUDE_PATH
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3bf2ecafd027..28b304be5419 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1241,6 +1241,10 @@ static const u32 emulated_msrs_all[] = {
HV_X64_MSR_VP_ASSIST_PAGE,
HV_X64_MSR_REENLIGHTENMENT_CONTROL, HV_X64_MSR_TSC_EMULATION_CONTROL,
HV_X64_MSR_TSC_EMULATION_STATUS,
+ HV_X64_MSR_SYNDBG_OPTIONS,
+ HV_X64_MSR_SYNDBG_CONTROL, HV_X64_MSR_SYNDBG_STATUS,
+ HV_X64_MSR_SYNDBG_SEND_BUFFER, HV_X64_MSR_SYNDBG_RECV_BUFFER,
+ HV_X64_MSR_SYNDBG_PENDING_BUFFER,
MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
MSR_KVM_PV_EOI_EN,
@@ -2940,6 +2944,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
*/
break;
case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
+ case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ case HV_X64_MSR_SYNDBG_OPTIONS:
case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
case HV_X64_MSR_CRASH_CTL:
case HV_X64_MSR_STIMER0_CONFIG ... HV_X64_MSR_STIMER3_COUNT:
@@ -3184,6 +3190,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
msr_info->data = 0x20000000;
break;
case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
+ case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ case HV_X64_MSR_SYNDBG_OPTIONS:
case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
case HV_X64_MSR_CRASH_CTL:
case HV_X64_MSR_STIMER0_CONFIG ... HV_X64_MSR_STIMER3_COUNT:
@@ -3355,6 +3363,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_HYPERV_TLBFLUSH:
case KVM_CAP_HYPERV_SEND_IPI:
case KVM_CAP_HYPERV_CPUID:
+ case KVM_CAP_HYPERV_SYNDBG:
case KVM_CAP_PCI_SEGMENT:
case KVM_CAP_DEBUGREGS:
case KVM_CAP_X86_ROBUST_SINGLESTEP:
@@ -4209,6 +4218,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
return -EINVAL;
switch (cap->cap) {
+ case KVM_CAP_HYPERV_SYNDBG:
+ kvm_hv_activate_syndbg(vcpu);
+ return 0;
+
case KVM_CAP_HYPERV_SYNIC2:
if (cap->args[0])
return -EINVAL;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 9cdc5356f542..ec1b2c7b449e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -188,6 +188,7 @@ struct kvm_s390_cmma_log {
struct kvm_hyperv_exit {
#define KVM_EXIT_HYPERV_SYNIC 1
#define KVM_EXIT_HYPERV_HCALL 2
+#define KVM_EXIT_HYPERV_SYNDBG 3
__u32 type;
__u32 pad1;
union {
@@ -203,6 +204,15 @@ struct kvm_hyperv_exit {
__u64 result;
__u64 params[2];
} hcall;
+ struct {
+ __u32 msr;
+ __u32 pad2;
+ __u64 control;
+ __u64 status;
+ __u64 send_page;
+ __u64 recv_page;
+ __u64 pending_page;
+ } syndbg;
} u;
};
@@ -1019,6 +1029,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_S390_VCPU_RESETS 179
#define KVM_CAP_S390_PROTECTED 180
#define KVM_CAP_PPC_SECURE_GUEST 181
+#define KVM_CAP_HYPERV_SYNDBG 182
#ifdef KVM_CAP_IRQ_ROUTING
--
2.24.1
^ permalink raw reply related
* [PATCH v11 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron
In-Reply-To: <20200424113746.3473563-1-arilou@gmail.com>
Microsoft's kdvm.dll dbgtransport module does not respect the hypercall
page and simply identifies the CPU being used (AMD/Intel) and according
to it simply makes hypercalls with the relevant instruction
(vmmcall/vmcall respectively).
The relevant function in kdvm is KdHvConnectHypervisor which first checks
if the hypercall page has been enabled via HV_X64_MSR_HYPERCALL_ENABLE,
and in case it was not it simply sets the HV_X64_MSR_GUEST_OS_ID to
0x1000101010001 which means:
build_number = 0x0001
service_version = 0x01
minor_version = 0x01
major_version = 0x01
os_id = 0x00 (Undefined)
vendor_id = 1 (Microsoft)
os_type = 0 (A value of 0 indicates a proprietary, closed source OS)
and starts issuing the hypercall without setting the hypercall page.
To resolve this issue simply enable hypercalls also if the guest_os_id
is not 0 and the syndbg feature is enabled.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
arch/x86/kvm/hyperv.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 435516595090..524b5466a515 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1650,7 +1650,10 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *current_vcpu, u64 ingpa, u64 outgpa,
bool kvm_hv_hypercall_enabled(struct kvm *kvm)
{
- return READ_ONCE(kvm->arch.hyperv.hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE;
+ struct kvm_hv *hv = &kvm->arch.hyperv;
+
+ return READ_ONCE(hv->hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE ||
+ (hv->hv_syndbg.active && READ_ONCE(hv->hv_guest_os_id) != 0);
}
static void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
--
2.24.1
^ permalink raw reply related
* [PATCH v11 6/7] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron
In-Reply-To: <20200424113746.3473563-1-arilou@gmail.com>
There is another mode for the synthetic debugger which uses hypercalls
to send/recv network data instead of the MSR interface.
This interface is much slower and less recommended since you might get
a lot of VMExits while KDVM polling for new packets to recv, rather
than simply checking the pending page to see if there is data avialble
and then request.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
arch/x86/kvm/hyperv.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 524b5466a515..744bcef88c70 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1832,6 +1832,34 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
}
ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, true, false);
break;
+ case HVCALL_POST_DEBUG_DATA:
+ case HVCALL_RETRIEVE_DEBUG_DATA:
+ if (unlikely(fast)) {
+ ret = HV_STATUS_INVALID_PARAMETER;
+ break;
+ }
+ fallthrough;
+ case HVCALL_RESET_DEBUG_SESSION: {
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+
+ if (!syndbg->active) {
+ ret = HV_STATUS_INVALID_HYPERCALL_CODE;
+ break;
+ }
+
+ if (!(syndbg->options & HV_X64_SYNDBG_OPTION_USE_HCALLS)) {
+ ret = HV_STATUS_OPERATION_DENIED;
+ break;
+ }
+ vcpu->run->exit_reason = KVM_EXIT_HYPERV;
+ vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
+ vcpu->run->hyperv.u.hcall.input = param;
+ vcpu->run->hyperv.u.hcall.params[0] = ingpa;
+ vcpu->run->hyperv.u.hcall.params[1] = outgpa;
+ vcpu->arch.complete_userspace_io =
+ kvm_hv_hypercall_complete_userspace;
+ return 0;
+ }
default:
ret = HV_STATUS_INVALID_HYPERCALL_CODE;
break;
--
2.24.1
^ permalink raw reply related
* [PATCH v11 7/7] KVM: selftests: update hyperv_cpuid with SynDBG tests
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets
In-Reply-To: <20200424113746.3473563-1-arilou@gmail.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Test all four combinations with eVMCS and SynDBG capabilities,
check that we get the right number of entries and that
0x40000000.EAX always returns the correct max leaf.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
.../selftests/kvm/x86_64/hyperv_cpuid.c | 143 ++++++++++++------
1 file changed, 95 insertions(+), 48 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 83323f3d7ca0..5268abf9ad80 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
@@ -26,18 +26,18 @@ static void guest_code(void)
{
}
-static int smt_possible(void)
+static bool smt_possible(void)
{
char buf[16];
FILE *f;
- bool res = 1;
+ bool res = true;
f = fopen("/sys/devices/system/cpu/smt/control", "r");
if (f) {
if (fread(buf, sizeof(*buf), sizeof(buf), f) > 0) {
if (!strncmp(buf, "forceoff", 8) ||
!strncmp(buf, "notsupported", 12))
- res = 0;
+ res = false;
}
fclose(f);
}
@@ -45,30 +45,48 @@ static int smt_possible(void)
return res;
}
+void vcpu_enable_syndbg(struct kvm_vm *vm, int vcpu_id)
+{
+ struct kvm_enable_cap enable_syndbg_cap = {
+ .cap = KVM_CAP_HYPERV_SYNDBG,
+ };
+
+ vcpu_ioctl(vm, vcpu_id, KVM_ENABLE_CAP, &enable_syndbg_cap);
+}
+
static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries,
- int evmcs_enabled)
+ bool evmcs_enabled, bool syndbg_enabled)
{
int i;
+ int nent = 6;
+ u32 test_val;
+
+ if (evmcs_enabled)
+ nent += 1; /* 0x4000000A */
- if (!evmcs_enabled)
- TEST_ASSERT(hv_cpuid_entries->nent == 6,
- "KVM_GET_SUPPORTED_HV_CPUID should return 6 entries"
- " when Enlightened VMCS is disabled (returned %d)",
- hv_cpuid_entries->nent);
- else
- TEST_ASSERT(hv_cpuid_entries->nent == 7,
- "KVM_GET_SUPPORTED_HV_CPUID should return 7 entries"
- " when Enlightened VMCS is enabled (returned %d)",
- hv_cpuid_entries->nent);
+ if (syndbg_enabled)
+ nent += 3; /* 0x40000080 - 0x40000082 */
+
+ TEST_ASSERT(hv_cpuid_entries->nent == nent,
+ "KVM_GET_SUPPORTED_HV_CPUID should return %d entries"
+ " with evmcs=%d syndbg=%d (returned %d)",
+ nent, evmcs_enabled, syndbg_enabled,
+ hv_cpuid_entries->nent);
for (i = 0; i < hv_cpuid_entries->nent; i++) {
struct kvm_cpuid_entry2 *entry = &hv_cpuid_entries->entries[i];
TEST_ASSERT((entry->function >= 0x40000000) &&
- (entry->function <= 0x4000000A),
+ (entry->function <= 0x40000082),
"function %x is our of supported range",
entry->function);
+ TEST_ASSERT(evmcs_enabled || (entry->function != 0x4000000A),
+ "0x4000000A leaf should not be reported");
+
+ TEST_ASSERT(syndbg_enabled || (entry->function <= 0x4000000A),
+ "SYNDBG leaves should not be reported");
+
TEST_ASSERT(entry->index == 0,
".index field should be zero");
@@ -78,12 +96,27 @@ static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries,
TEST_ASSERT(!entry->padding[0] && !entry->padding[1] &&
!entry->padding[2], "padding should be zero");
- if (entry->function == 0x40000004) {
- int nononarchcs = !!(entry->eax & (1UL << 18));
-
- TEST_ASSERT(nononarchcs == !smt_possible(),
+ switch (entry->function) {
+ case 0x40000000:
+ test_val = 0x40000005;
+ if (evmcs_enabled)
+ test_val = 0x4000000A;
+ if (syndbg_enabled)
+ test_val = 0x40000082;
+
+ TEST_ASSERT(entry->eax == test_val,
+ "Wrong max leaf report in 0x40000000.EAX: %x"
+ " (evmcs=%d syndbg=%d)",
+ entry->eax, evmcs_enabled, syndbg_enabled
+ );
+ break;
+ case 0x40000004:
+ test_val = entry->eax & (1UL << 18);
+
+ TEST_ASSERT(!!test_val == !smt_possible(),
"NoNonArchitecturalCoreSharing bit"
" doesn't reflect SMT setting");
+ break;
}
/*
@@ -133,8 +166,9 @@ struct kvm_cpuid2 *kvm_get_supported_hv_cpuid(struct kvm_vm *vm)
int main(int argc, char *argv[])
{
struct kvm_vm *vm;
- int rv;
+ int rv, stage;
struct kvm_cpuid2 *hv_cpuid_entries;
+ bool evmcs_enabled, syndbg_enabled;
/* Tell stdout not to buffer its content */
setbuf(stdout, NULL);
@@ -145,36 +179,49 @@ int main(int argc, char *argv[])
exit(KSFT_SKIP);
}
- /* Create VM */
- vm = vm_create_default(VCPU_ID, 0, guest_code);
-
- test_hv_cpuid_e2big(vm);
-
- hv_cpuid_entries = kvm_get_supported_hv_cpuid(vm);
- if (!hv_cpuid_entries)
- return 1;
-
- test_hv_cpuid(hv_cpuid_entries, 0);
-
- free(hv_cpuid_entries);
+ for (stage = 0; stage < 5; stage++) {
+ evmcs_enabled = false;
+ syndbg_enabled = false;
+
+ vm = vm_create_default(VCPU_ID, 0, guest_code);
+ switch (stage) {
+ case 0:
+ test_hv_cpuid_e2big(vm);
+ continue;
+ case 1:
+ break;
+ case 2:
+ if (!kvm_check_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
+ print_skip("Enlightened VMCS is unsupported");
+ continue;
+ }
+ vcpu_enable_evmcs(vm, VCPU_ID);
+ evmcs_enabled = true;
+ break;
+ case 3:
+ if (!kvm_check_cap(KVM_CAP_HYPERV_SYNDBG)) {
+ print_skip("Synthetic debugger is unsupported");
+ continue;
+ }
+ vcpu_enable_syndbg(vm, VCPU_ID);
+ syndbg_enabled = true;
+ break;
+ case 4:
+ if (!kvm_check_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS) ||
+ !kvm_check_cap(KVM_CAP_HYPERV_SYNDBG))
+ continue;
+ vcpu_enable_evmcs(vm, VCPU_ID);
+ vcpu_enable_syndbg(vm, VCPU_ID);
+ evmcs_enabled = true;
+ syndbg_enabled = true;
+ break;
+ }
- if (!kvm_check_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
- print_skip("Enlightened VMCS is unsupported");
- goto vm_free;
+ hv_cpuid_entries = kvm_get_supported_hv_cpuid(vm);
+ test_hv_cpuid(hv_cpuid_entries, evmcs_enabled, syndbg_enabled);
+ free(hv_cpuid_entries);
+ kvm_vm_free(vm);
}
- vcpu_enable_evmcs(vm, VCPU_ID);
-
- hv_cpuid_entries = kvm_get_supported_hv_cpuid(vm);
- if (!hv_cpuid_entries)
- return 1;
-
- test_hv_cpuid(hv_cpuid_entries, 1);
-
- free(hv_cpuid_entries);
-
-vm_free:
- kvm_vm_free(vm);
-
return 0;
}
--
2.24.1
^ permalink raw reply related
* Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM
From: Jon Doron @ 2020-04-24 12:20 UTC (permalink / raw)
To: Roman Kagan, kvm, linux-hyperv, vkuznets
In-Reply-To: <20200418064127.GB1917435@jondnuc>
On 18/04/2020, Jon Doron wrote:
>On 17/04/2020, Roman Kagan wrote:
>>On Thu, Apr 16, 2020 at 03:54:30PM +0300, Jon Doron wrote:
>>>On 16/04/2020, Roman Kagan wrote:
>>>> On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
>>>> > According to the TLFS:
>>>> > "A write to the end of message (EOM) register by the guest causes the
>>>> > hypervisor to scan the internal message buffer queue(s) associated with
>>>> > the virtual processor.
>>>> >
>>>> > If a message buffer queue contains a queued message buffer, the hypervisor
>>>> > attempts to deliver the message.
>>>> >
>>>> > Message delivery succeeds if the SIM page is enabled and the message slot
>>>> > corresponding to the SINTx is empty (that is, the message type in the
>>>> > header is set to HvMessageTypeNone).
>>>> > If a message is successfully delivered, its corresponding internal message
>>>> > buffer is dequeued and marked free.
>>>> > If the corresponding SINTx is not masked, an edge-triggered interrupt is
>>>> > delivered (that is, the corresponding bit in the IRR is set).
>>>> >
>>>> > This register can be used by guests to poll for messages. It can also be
>>>> > used as a way to drain the message queue for a SINTx that has
>>>> > been disabled (that is, masked)."
>>>>
>>>> Doesn't this work already?
>>>>
>>>
>>>Well if you dont have SCONTROL and a GSI associated with the SINT then it
>>>does not...
>>
>>Yes you do need both of these.
>>
>>>> > So basically this means that we need to exit on EOM so the hypervisor
>>>> > will have a chance to send all the pending messages regardless of the
>>>> > SCONTROL mechnaisim.
>>>>
>>>> I might be misinterpreting the spec, but my understanding is that
>>>> SCONTROL {en,dis}ables the message queueing completely. What the quoted
>>>> part means is that a write to EOM should trigger the message source to
>>>> push a new message into the slot, regardless of whether the SINT was
>>>> masked or not.
>>>>
>>>> And this (I think, haven't tested) should already work. The userspace
>>>> just keeps using the SINT route as it normally does, posting
>>>> notifications to the corresponding irqfd when posting a message, and
>>>> waiting on the resamplerfd for the message slot to become free. If the
>>>> SINT is masked KVM will skip injecting the interrupt, that's it.
>>>>
>>>> Roman.
>>>
>>>That's what I was thinking originally as well, but then i noticed KDNET as a
>>>VMBus client (and it basically runs before anything else) is working in this
>>>polling mode, where SCONTROL is disabled and it just loops, and if it saw
>>>there is a PENDING message flag it will issue an EOM to indicate it has free
>>>the slot.
>>
>>Who sets up the message page then? Doesn't it enabe SCONTROL as well?
>>
>
>KdNet is the one setting the SIMP and it's not setting the SCONTROL,
>ill paste output of KVM traces for the relevant MSRs
>
>>Note that, even if you don't see it being enabled by Windows, it can be
>>enabled by the firmware and/or by the bootloader.
>>
>>Can you perhaps try with the SeaBIOS from
>>https://src.openvz.org/projects/UP/repos/seabios branch hv-scsi? It
>>enables SCONTROL and leaves it that way.
>>
>>I'd also suggest tracing kvm_msr events (both reads and writes) for
>>SCONTROL and SIMP msrs, to better understand the picture.
>>
>>So far the change you propose appears too heavy to work around the
>>problem of disabled SCONTROL. You seem to be better off just making
>>sure it's enabled (either by the firmware or slighly violating the spec
>>and initializing to enabled from the start), and sticking to the
>>existing infrastructure for posting messages.
>>
>
>I guess there is something I'm missing here but let's say the BIOS
>would have set the SCONTROL but the OS is not setting it, who is in
>charge of handling the interrupts?
>
>>>(There are a bunch of patches i sent on the QEMU mailing list as well where
>>>i CCed you, I will probably revise it a bit but was hoping to get KVM
>>>sorted out first).
>>
>>I'll look through the archive, should be there, thanks.
>>
>>Roman.
>
>I tried testing with both the SeaBIOS branch you have suggested and
>the EDK2, unfortunately I could not get the EDK2 build to identify my
>VM drive to boot from (not sure why)
>
>Here is an output of KVM trace for the relevant MSRs (SCONTROL and SIMP)
>
>QEMU Default BIOS
>-----------------
> qemu-system-x86-613 [000] .... 1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-613 [000] .... 1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> qemu-system-x86-613 [000] .N.. 1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-613 [000] .N.. 1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>Choose Windows DebugEntry
> qemu-system-x86-613 [001] .... 1165.185227: kvm_msr: msr_read 40000083 = 0x0
> qemu-system-x86-613 [001] .... 1165.185255: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
> qemu-system-x86-613 [001] .... 1165.185255: kvm_msr: msr_write 40000083 = 0xfa1001
> qemu-system-x86-613 [001] .... 1165.193206: kvm_msr: msr_read 40000083 = 0xfa1001
> qemu-system-x86-613 [001] .... 1165.193236: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
> qemu-system-x86-613 [001] .... 1165.193237: kvm_msr: msr_write 40000083 = 0xfa1000
>
>
>SeaBIOS hv-scsci
>----------------
> qemu-system-x86-656 [001] .... 1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-656 [001] .... 1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> qemu-system-x86-656 [001] .... 1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-656 [001] .... 1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> qemu-system-x86-656 [001] .... 1313.156675: kvm_msr: msr_read 40000083 = 0x0
> qemu-system-x86-656 [001] .... 1313.156680: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x7fffe001 host 0
>Choose Windows DebugEntry
> qemu-system-x86-656 [001] .... 1313.156680: kvm_msr: msr_write 40000083 = 0x7fffe001
> qemu-system-x86-656 [001] .... 1313.162111: kvm_msr: msr_read 40000080 = 0x0
> qemu-system-x86-656 [001] .... 1313.162118: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x1 host 0
> qemu-system-x86-656 [001] .... 1313.162119: kvm_msr: msr_write 40000080 = 0x1
> qemu-system-x86-656 [001] .... 1313.246758: kvm_msr: msr_read 40000083 = 0x7fffe001
> qemu-system-x86-656 [001] .... 1313.246764: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 0
> qemu-system-x86-656 [001] .... 1313.246764: kvm_msr: msr_write 40000083 = 0x0
> qemu-system-x86-656 [001] .... 1348.904727: kvm_msr: msr_read 40000083 = 0x0
> qemu-system-x86-656 [001] .... 1348.904771: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
> qemu-system-x86-656 [001] .... 1348.904772: kvm_msr: msr_write 40000083 = 0xfa1001
> qemu-system-x86-656 [001] .... 1348.919170: kvm_msr: msr_read 40000083 = 0xfa1001
> qemu-system-x86-656 [001] .... 1348.919183: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
> qemu-system-x86-656 [001] .... 1348.919183: kvm_msr: msr_write 40000083 = 0xfa1000
>
>
> I could not get the EDK2 setup to work though
> (https://src.openvz.org/projects/UP/repos/edk2 branch hv-scsi)
>
>It does not detect my VM hard drive not sure why (this is how i
>configured it:
> -drive file=./win10.qcow2,format=qcow2,if=none,id=drive_disk0 \
> -device virtio-blk-pci,drive=drive_disk0 \
>
>(Is there something special i need to configure it order for it to
>work?, I tried building EDK2 with and without SMM_REQUIRE and
>SECURE_BOOT_ENABLE)
>
>
>But in general it sounds like there is something I dont fully
>understand when SCONTROL is enabled, then a GSI is associated with
>this SintRoute.
>
>Then when the guest triggers an EOI via the APIC we will trigger the
>GSI notification, which will give us another go on trying to copy the
>message into it's slot.
>
>So is it the OS that is in charge of setting the EOI? If so then it
>needs to be aware of SCONTROL being enabled and just having it left
>set by the BIOS might not be enough?
>
>Also in the TLFS (looking at v6) they mention that message queueing
>has "3 exit conditions", which will cause the hypervisor to try and
>attempt to deliver the additional messages.
>
>The 3 exit conditions they refer to are:
>* Another message buffer is queued.
>* The guest indicates the “end of interrupt” by writing to the APIC’s
>EOI register.
>* The guest indicates the “end of message” by writing to the SynIC’s
>EOM register.
>
>Also notice this additional exit is only if there is a pending message
>and not for every EOM.
>
>Thanks,
>-- Jon.
Hi Roman
Any other thoughts/suggestions about this?
Thanks,
-- Jon.
^ permalink raw reply
* [PATCH AUTOSEL 4.14 01/21] x86: hyperv: report value of misc_features
From: Sasha Levin @ 2020-04-24 12:23 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Olaf Hering, Wei Liu, Sasha Levin, linux-hyperv
From: Olaf Hering <olaf@aepfle.de>
[ Upstream commit 97d9f1c43bedd400301d6f1eff54d46e8c636e47 ]
A few kernel features depend on ms_hyperv.misc_features, but unlike its
siblings ->features and ->hints, the value was never reported during boot.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Link: https://lore.kernel.org/r/20200407172739.31371-1-olaf@aepfle.de
Signed-off-by: Wei Liu <wei.liu@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/x86/kernel/cpu/mshyperv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index c0201b11e9e2a..a6b323a3a6304 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -178,8 +178,8 @@ static void __init ms_hyperv_init_platform(void)
ms_hyperv.misc_features = cpuid_edx(HYPERV_CPUID_FEATURES);
ms_hyperv.hints = cpuid_eax(HYPERV_CPUID_ENLIGHTMENT_INFO);
- pr_info("Hyper-V: features 0x%x, hints 0x%x\n",
- ms_hyperv.features, ms_hyperv.hints);
+ pr_info("Hyper-V: features 0x%x, hints 0x%x, misc 0x%x\n",
+ ms_hyperv.features, ms_hyperv.hints, ms_hyperv.misc_features);
ms_hyperv.max_vp_index = cpuid_eax(HVCPUID_IMPLEMENTATION_LIMITS);
ms_hyperv.max_lp_index = cpuid_ebx(HVCPUID_IMPLEMENTATION_LIMITS);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 02/18] x86: hyperv: report value of misc_features
From: Sasha Levin @ 2020-04-24 12:23 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Olaf Hering, Wei Liu, Sasha Levin, linux-hyperv
In-Reply-To: <20200424122355.10453-1-sashal@kernel.org>
From: Olaf Hering <olaf@aepfle.de>
[ Upstream commit 97d9f1c43bedd400301d6f1eff54d46e8c636e47 ]
A few kernel features depend on ms_hyperv.misc_features, but unlike its
siblings ->features and ->hints, the value was never reported during boot.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Link: https://lore.kernel.org/r/20200407172739.31371-1-olaf@aepfle.de
Signed-off-by: Wei Liu <wei.liu@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/x86/kernel/cpu/mshyperv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index fc93ae3255153..f8b0fa2dbe374 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -214,8 +214,8 @@ static void __init ms_hyperv_init_platform(void)
ms_hyperv.misc_features = cpuid_edx(HYPERV_CPUID_FEATURES);
ms_hyperv.hints = cpuid_eax(HYPERV_CPUID_ENLIGHTMENT_INFO);
- pr_info("Hyper-V: features 0x%x, hints 0x%x\n",
- ms_hyperv.features, ms_hyperv.hints);
+ pr_info("Hyper-V: features 0x%x, hints 0x%x, misc 0x%x\n",
+ ms_hyperv.features, ms_hyperv.hints, ms_hyperv.misc_features);
ms_hyperv.max_vp_index = cpuid_eax(HYPERV_CPUID_IMPLEMENT_LIMITS);
ms_hyperv.max_lp_index = cpuid_ebx(HYPERV_CPUID_IMPLEMENT_LIMITS);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 03/26] x86: hyperv: report value of misc_features
From: Sasha Levin @ 2020-04-24 12:23 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Olaf Hering, Wei Liu, Sasha Levin, linux-hyperv
In-Reply-To: <20200424122323.10194-1-sashal@kernel.org>
From: Olaf Hering <olaf@aepfle.de>
[ Upstream commit 97d9f1c43bedd400301d6f1eff54d46e8c636e47 ]
A few kernel features depend on ms_hyperv.misc_features, but unlike its
siblings ->features and ->hints, the value was never reported during boot.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Link: https://lore.kernel.org/r/20200407172739.31371-1-olaf@aepfle.de
Signed-off-by: Wei Liu <wei.liu@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/x86/kernel/cpu/mshyperv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index fc8814faae62c..1c2f9baf84832 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -227,8 +227,8 @@ static void __init ms_hyperv_init_platform(void)
ms_hyperv.misc_features = cpuid_edx(HYPERV_CPUID_FEATURES);
ms_hyperv.hints = cpuid_eax(HYPERV_CPUID_ENLIGHTMENT_INFO);
- pr_info("Hyper-V: features 0x%x, hints 0x%x\n",
- ms_hyperv.features, ms_hyperv.hints);
+ pr_info("Hyper-V: features 0x%x, hints 0x%x, misc 0x%x\n",
+ ms_hyperv.features, ms_hyperv.hints, ms_hyperv.misc_features);
ms_hyperv.max_vp_index = cpuid_eax(HYPERV_CPUID_IMPLEMENT_LIMITS);
ms_hyperv.max_lp_index = cpuid_ebx(HYPERV_CPUID_IMPLEMENT_LIMITS);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 03/38] x86: hyperv: report value of misc_features
From: Sasha Levin @ 2020-04-24 12:22 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Olaf Hering, Wei Liu, Sasha Levin, linux-hyperv
In-Reply-To: <20200424122237.9831-1-sashal@kernel.org>
From: Olaf Hering <olaf@aepfle.de>
[ Upstream commit 97d9f1c43bedd400301d6f1eff54d46e8c636e47 ]
A few kernel features depend on ms_hyperv.misc_features, but unlike its
siblings ->features and ->hints, the value was never reported during boot.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Link: https://lore.kernel.org/r/20200407172739.31371-1-olaf@aepfle.de
Signed-off-by: Wei Liu <wei.liu@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/x86/kernel/cpu/mshyperv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 5e296a7e60363..ebf34c7bc8bc0 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -227,8 +227,8 @@ static void __init ms_hyperv_init_platform(void)
ms_hyperv.misc_features = cpuid_edx(HYPERV_CPUID_FEATURES);
ms_hyperv.hints = cpuid_eax(HYPERV_CPUID_ENLIGHTMENT_INFO);
- pr_info("Hyper-V: features 0x%x, hints 0x%x\n",
- ms_hyperv.features, ms_hyperv.hints);
+ pr_info("Hyper-V: features 0x%x, hints 0x%x, misc 0x%x\n",
+ ms_hyperv.features, ms_hyperv.hints, ms_hyperv.misc_features);
ms_hyperv.max_vp_index = cpuid_eax(HYPERV_CPUID_IMPLEMENT_LIMITS);
ms_hyperv.max_lp_index = cpuid_ebx(HYPERV_CPUID_IMPLEMENT_LIMITS);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM
From: Roman Kagan @ 2020-04-24 13:37 UTC (permalink / raw)
To: Jon Doron; +Cc: kvm, linux-hyperv, vkuznets
In-Reply-To: <20200418064127.GB1917435@jondnuc>
On Sat, Apr 18, 2020 at 09:41:27AM +0300, Jon Doron wrote:
> On 17/04/2020, Roman Kagan wrote:
> > On Thu, Apr 16, 2020 at 03:54:30PM +0300, Jon Doron wrote:
> > > On 16/04/2020, Roman Kagan wrote:
> > > > On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
> > > > > According to the TLFS:
> > > > > "A write to the end of message (EOM) register by the guest causes the
> > > > > hypervisor to scan the internal message buffer queue(s) associated with
> > > > > the virtual processor.
> > > > >
> > > > > If a message buffer queue contains a queued message buffer, the hypervisor
> > > > > attempts to deliver the message.
> > > > >
> > > > > Message delivery succeeds if the SIM page is enabled and the message slot
> > > > > corresponding to the SINTx is empty (that is, the message type in the
> > > > > header is set to HvMessageTypeNone).
> > > > > If a message is successfully delivered, its corresponding internal message
> > > > > buffer is dequeued and marked free.
> > > > > If the corresponding SINTx is not masked, an edge-triggered interrupt is
> > > > > delivered (that is, the corresponding bit in the IRR is set).
> > > > >
> > > > > This register can be used by guests to poll for messages. It can also be
> > > > > used as a way to drain the message queue for a SINTx that has
> > > > > been disabled (that is, masked)."
> > > >
> > > > Doesn't this work already?
> > > >
> > >
> > > Well if you dont have SCONTROL and a GSI associated with the SINT then it
> > > does not...
> >
> > Yes you do need both of these.
> >
> > > > > So basically this means that we need to exit on EOM so the hypervisor
> > > > > will have a chance to send all the pending messages regardless of the
> > > > > SCONTROL mechnaisim.
> > > >
> > > > I might be misinterpreting the spec, but my understanding is that
> > > > SCONTROL {en,dis}ables the message queueing completely. What the quoted
> > > > part means is that a write to EOM should trigger the message source to
> > > > push a new message into the slot, regardless of whether the SINT was
> > > > masked or not.
> > > >
> > > > And this (I think, haven't tested) should already work. The userspace
> > > > just keeps using the SINT route as it normally does, posting
> > > > notifications to the corresponding irqfd when posting a message, and
> > > > waiting on the resamplerfd for the message slot to become free. If the
> > > > SINT is masked KVM will skip injecting the interrupt, that's it.
> > > >
> > > > Roman.
> > >
> > > That's what I was thinking originally as well, but then i noticed KDNET as a
> > > VMBus client (and it basically runs before anything else) is working in this
> > > polling mode, where SCONTROL is disabled and it just loops, and if it saw
> > > there is a PENDING message flag it will issue an EOM to indicate it has free
> > > the slot.
> >
> > Who sets up the message page then? Doesn't it enabe SCONTROL as well?
> >
>
> KdNet is the one setting the SIMP and it's not setting the SCONTROL, ill
> paste output of KVM traces for the relevant MSRs
>
> > Note that, even if you don't see it being enabled by Windows, it can be
> > enabled by the firmware and/or by the bootloader.
> >
> > Can you perhaps try with the SeaBIOS from
> > https://src.openvz.org/projects/UP/repos/seabios branch hv-scsi? It
> > enables SCONTROL and leaves it that way.
> >
> > I'd also suggest tracing kvm_msr events (both reads and writes) for
> > SCONTROL and SIMP msrs, to better understand the picture.
> >
> > So far the change you propose appears too heavy to work around the
> > problem of disabled SCONTROL. You seem to be better off just making
> > sure it's enabled (either by the firmware or slighly violating the spec
> > and initializing to enabled from the start), and sticking to the
> > existing infrastructure for posting messages.
> >
>
> I guess there is something I'm missing here but let's say the BIOS would
> have set the SCONTROL but the OS is not setting it, who is in charge of
> handling the interrupts?
SCONTROL doesn't enable the interrupts, it enables SynIC as a whole.
The interrupts are enabled via individual SINTx msrs. This SeaBIOS
branch does exactly this: it enables the SynIC via SCONTROL, and then
specific SynIC functionality via SIMP/SIEFP, but doesn't activate SINTx
and works in polling mode.
I agree that this global SCONTROL switch seems redundant but it appears
to match the spec.
> > > (There are a bunch of patches i sent on the QEMU mailing list as well where
> > > i CCed you, I will probably revise it a bit but was hoping to get KVM
> > > sorted out first).
> >
> > I'll look through the archive, should be there, thanks.
> >
> > Roman.
>
> I tried testing with both the SeaBIOS branch you have suggested and the
> EDK2, unfortunately I could not get the EDK2 build to identify my VM drive
> to boot from (not sure why)
>
> Here is an output of KVM trace for the relevant MSRs (SCONTROL and SIMP)
>
> QEMU Default BIOS
> -----------------
> qemu-system-x86-613 [000] .... 1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-613 [000] .... 1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> qemu-system-x86-613 [000] .N.. 1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-613 [000] .N.. 1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> Choose Windows DebugEntry
> qemu-system-x86-613 [001] .... 1165.185227: kvm_msr: msr_read 40000083 = 0x0
> qemu-system-x86-613 [001] .... 1165.185255: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
> qemu-system-x86-613 [001] .... 1165.185255: kvm_msr: msr_write 40000083 = 0xfa1001
> qemu-system-x86-613 [001] .... 1165.193206: kvm_msr: msr_read 40000083 = 0xfa1001
> qemu-system-x86-613 [001] .... 1165.193236: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
> qemu-system-x86-613 [001] .... 1165.193237: kvm_msr: msr_write 40000083 = 0xfa1000
>
>
> SeaBIOS hv-scsci
> ----------------
> qemu-system-x86-656 [001] .... 1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-656 [001] .... 1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> qemu-system-x86-656 [001] .... 1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-656 [001] .... 1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
Initialization (host == 1)
> qemu-system-x86-656 [001] .... 1313.156675: kvm_msr: msr_read 40000083 = 0x0
> qemu-system-x86-656 [001] .... 1313.156680: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x7fffe001 host 0
> Choose Windows DebugEntry
I guess this is a bit misplaced timewise, BIOS is still working here
> qemu-system-x86-656 [001] .... 1313.156680: kvm_msr: msr_write 40000083 = 0x7fffe001
BIOS sets up message page
> qemu-system-x86-656 [001] .... 1313.162111: kvm_msr: msr_read 40000080 = 0x0
> qemu-system-x86-656 [001] .... 1313.162118: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x1 host 0
> qemu-system-x86-656 [001] .... 1313.162119: kvm_msr: msr_write 40000080 = 0x1
BIOS activates SCONTROL
> qemu-system-x86-656 [001] .... 1313.246758: kvm_msr: msr_read 40000083 = 0x7fffe001
> qemu-system-x86-656 [001] .... 1313.246764: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 0
> qemu-system-x86-656 [001] .... 1313.246764: kvm_msr: msr_write 40000083 = 0x0
BIOS clears message page (it's not needed once the VMBus device was
brought up)
I guess the choice of Windows DebugEntry appeared somewhere here.
> qemu-system-x86-656 [001] .... 1348.904727: kvm_msr: msr_read 40000083 = 0x0
> qemu-system-x86-656 [001] .... 1348.904771: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
> qemu-system-x86-656 [001] .... 1348.904772: kvm_msr: msr_write 40000083 = 0xfa1001
Bootloader (debug stub?) sets up the message page
> qemu-system-x86-656 [001] .... 1348.919170: kvm_msr: msr_read 40000083 = 0xfa1001
> qemu-system-x86-656 [001] .... 1348.919183: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
> qemu-system-x86-656 [001] .... 1348.919183: kvm_msr: msr_write 40000083 = 0xfa1000
Message page is being disabled again.
I guess you only filtered SCONTROL and SIMP, skipping e.g. SVERSION,
GUEST_OS_ID, HYPERCALL, etc., which are also part of the exchange here.
> I could not get the EDK2 setup to work though
> (https://src.openvz.org/projects/UP/repos/edk2 branch hv-scsi)
>
> It does not detect my VM hard drive not sure why (this is how i configured
> it:
> -drive file=./win10.qcow2,format=qcow2,if=none,id=drive_disk0 \
> -device virtio-blk-pci,drive=drive_disk0 \
>
> (Is there something special i need to configure it order for it to work?, I
> tried building EDK2 with and without SMM_REQUIRE and SECURE_BOOT_ENABLE)
No special configuration I can think of.
> But in general it sounds like there is something I dont fully understand
> when SCONTROL is enabled, then a GSI is associated with this SintRoute.
>
> Then when the guest triggers an EOI via the APIC we will trigger the GSI
> notification, which will give us another go on trying to copy the message
> into it's slot.
Right.
> So is it the OS that is in charge of setting the EOI?
Yes.
> If so then it needs to
> be aware of SCONTROL being enabled and just having it left set by the BIOS
> might not be enough?
Yes it needs to be aware of SCONTROL being enabled. However, this
awareness may be based on a pure assumption that the previous entity
(BIOS or bootloader) did it already.
> Also in the TLFS (looking at v6) they mention that message queueing has "3
> exit conditions", which will cause the hypervisor to try and attempt to
> deliver the additional messages.
>
> The 3 exit conditions they refer to are:
> * Another message buffer is queued.
> * The guest indicates the “end of interrupt” by writing to the APIC’s EOI
> register.
> * The guest indicates the “end of message” by writing to the SynIC’s EOM
> register.
>
> Also notice this additional exit is only if there is a pending message and
> not for every EOM.
This meaning of "exit" doesn't trivially correspond to what we have in
KVM. A write to an msr does cause a vmexit. Then KVM notifies resample
eventfds for all SINTs that have them set up, no matter if there's a
pending message in the slot. It may be slightly more optimal to only
notify those having indicated a pending message, but I don't see the
current behavior break anything or violate the spec, so, as EOMs are not
used on fast paths, I woudn't bother optimizing.
Roman.
^ permalink raw reply
* Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM
From: Jon Doron @ 2020-04-25 6:16 UTC (permalink / raw)
To: Roman Kagan, kvm, linux-hyperv, vkuznets
In-Reply-To: <20200424133742.GA2439920@rvkaganb>
On 24/04/2020, Roman Kagan wrote:
>On Sat, Apr 18, 2020 at 09:41:27AM +0300, Jon Doron wrote:
>> On 17/04/2020, Roman Kagan wrote:
>> > On Thu, Apr 16, 2020 at 03:54:30PM +0300, Jon Doron wrote:
>> > > On 16/04/2020, Roman Kagan wrote:
>> > > > On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
>> > > > > According to the TLFS:
>> > > > > "A write to the end of message (EOM) register by the guest causes the
>> > > > > hypervisor to scan the internal message buffer queue(s) associated with
>> > > > > the virtual processor.
>> > > > >
>> > > > > If a message buffer queue contains a queued message buffer, the hypervisor
>> > > > > attempts to deliver the message.
>> > > > >
>> > > > > Message delivery succeeds if the SIM page is enabled and the message slot
>> > > > > corresponding to the SINTx is empty (that is, the message type in the
>> > > > > header is set to HvMessageTypeNone).
>> > > > > If a message is successfully delivered, its corresponding internal message
>> > > > > buffer is dequeued and marked free.
>> > > > > If the corresponding SINTx is not masked, an edge-triggered interrupt is
>> > > > > delivered (that is, the corresponding bit in the IRR is set).
>> > > > >
>> > > > > This register can be used by guests to poll for messages. It can also be
>> > > > > used as a way to drain the message queue for a SINTx that has
>> > > > > been disabled (that is, masked)."
>> > > >
>> > > > Doesn't this work already?
>> > > >
>> > >
>> > > Well if you dont have SCONTROL and a GSI associated with the SINT then it
>> > > does not...
>> >
>> > Yes you do need both of these.
>> >
>> > > > > So basically this means that we need to exit on EOM so the hypervisor
>> > > > > will have a chance to send all the pending messages regardless of the
>> > > > > SCONTROL mechnaisim.
>> > > >
>> > > > I might be misinterpreting the spec, but my understanding is that
>> > > > SCONTROL {en,dis}ables the message queueing completely. What the quoted
>> > > > part means is that a write to EOM should trigger the message source to
>> > > > push a new message into the slot, regardless of whether the SINT was
>> > > > masked or not.
>> > > >
>> > > > And this (I think, haven't tested) should already work. The userspace
>> > > > just keeps using the SINT route as it normally does, posting
>> > > > notifications to the corresponding irqfd when posting a message, and
>> > > > waiting on the resamplerfd for the message slot to become free. If the
>> > > > SINT is masked KVM will skip injecting the interrupt, that's it.
>> > > >
>> > > > Roman.
>> > >
>> > > That's what I was thinking originally as well, but then i noticed KDNET as a
>> > > VMBus client (and it basically runs before anything else) is working in this
>> > > polling mode, where SCONTROL is disabled and it just loops, and if it saw
>> > > there is a PENDING message flag it will issue an EOM to indicate it has free
>> > > the slot.
>> >
>> > Who sets up the message page then? Doesn't it enabe SCONTROL as well?
>> >
>>
>> KdNet is the one setting the SIMP and it's not setting the SCONTROL, ill
>> paste output of KVM traces for the relevant MSRs
>>
>> > Note that, even if you don't see it being enabled by Windows, it can be
>> > enabled by the firmware and/or by the bootloader.
>> >
>> > Can you perhaps try with the SeaBIOS from
>> > https://src.openvz.org/projects/UP/repos/seabios branch hv-scsi? It
>> > enables SCONTROL and leaves it that way.
>> >
>> > I'd also suggest tracing kvm_msr events (both reads and writes) for
>> > SCONTROL and SIMP msrs, to better understand the picture.
>> >
>> > So far the change you propose appears too heavy to work around the
>> > problem of disabled SCONTROL. You seem to be better off just making
>> > sure it's enabled (either by the firmware or slighly violating the spec
>> > and initializing to enabled from the start), and sticking to the
>> > existing infrastructure for posting messages.
>> >
>>
>> I guess there is something I'm missing here but let's say the BIOS would
>> have set the SCONTROL but the OS is not setting it, who is in charge of
>> handling the interrupts?
>
>SCONTROL doesn't enable the interrupts, it enables SynIC as a whole.
>The interrupts are enabled via individual SINTx msrs. This SeaBIOS
>branch does exactly this: it enables the SynIC via SCONTROL, and then
>specific SynIC functionality via SIMP/SIEFP, but doesn't activate SINTx
>and works in polling mode.
>
>I agree that this global SCONTROL switch seems redundant but it appears
>to match the spec.
>
>> > > (There are a bunch of patches i sent on the QEMU mailing list as well where
>> > > i CCed you, I will probably revise it a bit but was hoping to get KVM
>> > > sorted out first).
>> >
>> > I'll look through the archive, should be there, thanks.
>> >
>> > Roman.
>>
>> I tried testing with both the SeaBIOS branch you have suggested and the
>> EDK2, unfortunately I could not get the EDK2 build to identify my VM drive
>> to boot from (not sure why)
>>
>> Here is an output of KVM trace for the relevant MSRs (SCONTROL and SIMP)
>>
>> QEMU Default BIOS
>> -----------------
>> qemu-system-x86-613 [000] .... 1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>> qemu-system-x86-613 [000] .... 1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>> qemu-system-x86-613 [000] .N.. 1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>> qemu-system-x86-613 [000] .N.. 1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>> Choose Windows DebugEntry
>> qemu-system-x86-613 [001] .... 1165.185227: kvm_msr: msr_read 40000083 = 0x0
>> qemu-system-x86-613 [001] .... 1165.185255: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
>> qemu-system-x86-613 [001] .... 1165.185255: kvm_msr: msr_write 40000083 = 0xfa1001
>> qemu-system-x86-613 [001] .... 1165.193206: kvm_msr: msr_read 40000083 = 0xfa1001
>> qemu-system-x86-613 [001] .... 1165.193236: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
>> qemu-system-x86-613 [001] .... 1165.193237: kvm_msr: msr_write 40000083 = 0xfa1000
>>
>>
>> SeaBIOS hv-scsci
>> ----------------
>> qemu-system-x86-656 [001] .... 1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>> qemu-system-x86-656 [001] .... 1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>> qemu-system-x86-656 [001] .... 1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>> qemu-system-x86-656 [001] .... 1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>
>Initialization (host == 1)
>
>> qemu-system-x86-656 [001] .... 1313.156675: kvm_msr: msr_read 40000083 = 0x0
>> qemu-system-x86-656 [001] .... 1313.156680: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x7fffe001 host 0
>> Choose Windows DebugEntry
>
>I guess this is a bit misplaced timewise, BIOS is still working here
>
>> qemu-system-x86-656 [001] .... 1313.156680: kvm_msr: msr_write 40000083 = 0x7fffe001
>
>BIOS sets up message page
>
>> qemu-system-x86-656 [001] .... 1313.162111: kvm_msr: msr_read 40000080 = 0x0
>> qemu-system-x86-656 [001] .... 1313.162118: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x1 host 0
>> qemu-system-x86-656 [001] .... 1313.162119: kvm_msr: msr_write 40000080 = 0x1
>
>BIOS activates SCONTROL
>
>> qemu-system-x86-656 [001] .... 1313.246758: kvm_msr: msr_read 40000083 = 0x7fffe001
>> qemu-system-x86-656 [001] .... 1313.246764: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 0
>> qemu-system-x86-656 [001] .... 1313.246764: kvm_msr: msr_write 40000083 = 0x0
>
>BIOS clears message page (it's not needed once the VMBus device was
>brought up)
>
>I guess the choice of Windows DebugEntry appeared somewhere here.
>
>> qemu-system-x86-656 [001] .... 1348.904727: kvm_msr: msr_read 40000083 = 0x0
>> qemu-system-x86-656 [001] .... 1348.904771: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
>> qemu-system-x86-656 [001] .... 1348.904772: kvm_msr: msr_write 40000083 = 0xfa1001
>
>Bootloader (debug stub?) sets up the message page
>
>> qemu-system-x86-656 [001] .... 1348.919170: kvm_msr: msr_read 40000083 = 0xfa1001
>> qemu-system-x86-656 [001] .... 1348.919183: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
>> qemu-system-x86-656 [001] .... 1348.919183: kvm_msr: msr_write 40000083 = 0xfa1000
>
>Message page is being disabled again.
>
>I guess you only filtered SCONTROL and SIMP, skipping e.g. SVERSION,
>GUEST_OS_ID, HYPERCALL, etc., which are also part of the exchange here.
>
Right my bad :( if you want I can re-run the test with the others as
well (do you need me to?)
>> I could not get the EDK2 setup to work though
>> (https://src.openvz.org/projects/UP/repos/edk2 branch hv-scsi)
>>
>> It does not detect my VM hard drive not sure why (this is how i configured
>> it:
>> -drive file=./win10.qcow2,format=qcow2,if=none,id=drive_disk0 \
>> -device virtio-blk-pci,drive=drive_disk0 \
>>
>> (Is there something special i need to configure it order for it to work?, I
>> tried building EDK2 with and without SMM_REQUIRE and SECURE_BOOT_ENABLE)
>
>No special configuration I can think of.
>
>> But in general it sounds like there is something I dont fully understand
>> when SCONTROL is enabled, then a GSI is associated with this SintRoute.
>>
>> Then when the guest triggers an EOI via the APIC we will trigger the GSI
>> notification, which will give us another go on trying to copy the message
>> into it's slot.
>
>Right.
>
>> So is it the OS that is in charge of setting the EOI?
>
>Yes.
>
>> If so then it needs to
>> be aware of SCONTROL being enabled and just having it left set by the BIOS
>> might not be enough?
>
>Yes it needs to be aware of SCONTROL being enabled. However, this
>awareness may be based on a pure assumption that the previous entity
>(BIOS or bootloader) did it already.
>
>> Also in the TLFS (looking at v6) they mention that message queueing has "3
>> exit conditions", which will cause the hypervisor to try and attempt to
>> deliver the additional messages.
>>
>> The 3 exit conditions they refer to are:
>> * Another message buffer is queued.
>> * The guest indicates the “end of interrupt” by writing to the APIC’s EOI
>> register.
>> * The guest indicates the “end of message” by writing to the SynIC’s EOM
>> register.
>>
>> Also notice this additional exit is only if there is a pending message and
>> not for every EOM.
>
>This meaning of "exit" doesn't trivially correspond to what we have in
>KVM. A write to an msr does cause a vmexit. Then KVM notifies resample
>eventfds for all SINTs that have them set up, no matter if there's a
>pending message in the slot. It may be slightly more optimal to only
>notify those having indicated a pending message, but I don't see the
>current behavior break anything or violate the spec, so, as EOMs are not
>used on fast paths, I woudn't bother optimizing.
>
>Roman.
Hi Roman,
So based on your answer I got to the following conclusions (correct if
they are wrong).
First of the one in charge of setting the SCONTROL in the 1st place is
the BIOS (I dont have a real Hyper-V setup so I cannot really debug it
and see, not sure which BIOS they have or if we can "rip" it out and run
it through KVM and see how things look like this way).
If the BIOS has not set the SCONTROL I would expect the OS to have
something along the lines:
if (!(get_scontrol() & ENABLED))
set_scontrol(ENABLED);
So I started looking through the entire Windows system looking what can
set SCONTROL, I believe I have found the flow to be the following:
VMBus.sys imports winhv.sys (which is an export library) winhv.sys will
set the SCONTROL prior to VMBus DriverEntry starting here is the
complete flow:
winhv!DllInitialize -> winhv!WinHvpInitialize ->
winhv!WinHvReportPresentHypervisor -> winhv!WinHvpConnectToSynic ->
winhv!WinHvpEnableSynic
Eventually WinHvpEnableSynic will simply set SCONTROL (for future
reference if anyone needs to look into how HyperV register access works
in Windows it seems like there is an enum representing all the HyperV
registers and to access it there are helper functions to Get/Set.
SCONTROL value in the enum is 0x0a0010 .
winhv.sys simply provides very simple API to access the Sints i.e
(WinHvSetSint / WinHvSetEndOfMessage / WinHvSetSintOnCurrentProcessor /
WinHvGetSintMessage / etc.)
So basically it seems like the OS does not really care if the BIOS has
setup the SCONTROL or not, and does so always (if it can) unfortunately
in my flow (via kdnet) VMBus is not loaded yet and so does winhv.sys so
they "fallback" into this Polling mode.
So that covers the OS part, after that I have tried looking for relevant
code in bootmgr and winload (which are Windows boot loader part (like
grub) and I could not find any code that might setup SCONTROL.
From your experience with this did you see Hyper-V BIOS simply setting
the SCONTROL? Perhaps if that's the case then the correct fix needs to
be in the SeaBIOS and the EDK .
I tried to see if Hyper-V supports giving it a BIOS but could not find
anyway of doing this, so it just might be that Hyper-V assumes the BIOS
is in charge of setting up SCONTROL for all the boot loader components.
But in a way it sounds weird because I would expect to see KDNet working
with the ACPI to trigger the GSI but I could not find any relevant code
that might do that.
As I write this I think I'm starting to get your point just to make sure
I understand it:
1. When a new SintRoute is created we associate it with a GSI
2. When an EOM is set, we trigger all the GSIs so QEMU will get
execution time and send all pending messages if it can.
So basically like you said everything "works" from our perspective
regardless if the system has setup SCONTROL or not, because you trigger
the interrupt to QEMU regardless of SCONTROL so it can clear the pending
message.
If that's indeed the case then probably the only thing needs fixing in
my scenario is in QEMU where it should not really care for the SCONTROL
if it's enabled or not.
Sounds about right?
Thanks,
-- Jon.
^ permalink raw reply
* [PATCH] PCI: pci-hyperv: Retry PCI bus D0 entry when the first attempt failed with invalid device state 0xC0000184.
From: Wei Hu @ 2020-04-26 13:24 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, wei.liu, lorenzo.pieralisi, robh,
bhelgaas, linux-hyperv, linux-pci, linux-kernel, decui, mikelley
Cc: Wei Hu
In the case of kdump, the PCI device was not cleanly shut down
before the kdump kernel starts. This causes the initial
attempt of entering D0 state in the kdump kernel to fail with
invalid device state 0xC0000184 returned from Hyper-V host.
When this happens, explicitly call PCI bus exit and retry to
enter the D0 state.
Also fix the PCI probe failure path to release the PCI device
resource properly.
Signed-off-by: Wei Hu <weh@microsoft.com>
---
drivers/pci/controller/pci-hyperv.c | 34 ++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index e15022ff63e3..eb4781fa058d 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -2736,6 +2736,10 @@ static void hv_free_config_window(struct hv_pcibus_device *hbus)
vmbus_free_mmio(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH);
}
+#define STATUS_INVALID_DEVICE_STATE 0xC0000184
+
+static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating);
+
/**
* hv_pci_enter_d0() - Bring the "bus" into the D0 power state
* @hdev: VMBus's tracking struct for this root PCI bus
@@ -2748,8 +2752,10 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
struct pci_bus_d0_entry *d0_entry;
struct hv_pci_compl comp_pkt;
struct pci_packet *pkt;
+ bool retry = true;
int ret;
+enter_d0_retry:
/*
* Tell the host that the bus is ready to use, and moved into the
* powered-on state. This includes telling the host which region
@@ -2780,6 +2786,30 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
dev_err(&hdev->device,
"PCI Pass-through VSP failed D0 Entry with status %x\n",
comp_pkt.completion_status);
+
+ /*
+ * In certain case (Kdump) the pci device of interest was
+ * not cleanly shut down and resource is still held on host
+ * side, the host could return STATUS_INVALID_DEVICE_STATE.
+ * We need to explicitly request host to release the resource
+ * and try to enter D0 again.
+ */
+ if (comp_pkt.completion_status == STATUS_INVALID_DEVICE_STATE &&
+ retry) {
+ ret = hv_pci_bus_exit(hdev, true);
+
+ retry = false;
+
+ if (ret == 0) {
+ kfree(pkt);
+ goto enter_d0_retry;
+ } else {
+ dev_err(&hdev->device,
+ "PCI bus D0 exit failed with ret %d\n",
+ ret);
+ }
+ }
+
ret = -EPROTO;
goto exit;
}
@@ -3136,7 +3166,7 @@ static int hv_pci_probe(struct hv_device *hdev,
ret = hv_pci_allocate_bridge_windows(hbus);
if (ret)
- goto free_irq_domain;
+ goto exit_d0;
ret = hv_send_resources_allocated(hdev);
if (ret)
@@ -3154,6 +3184,8 @@ static int hv_pci_probe(struct hv_device *hdev,
free_windows:
hv_pci_free_bridge_windows(hbus);
+exit_d0:
+ (void) hv_pci_bus_exit(hdev, true);
free_irq_domain:
irq_domain_remove(hbus->irq_domain);
free_fwnode:
--
2.20.1
^ permalink raw reply related
* RE: [PATCH] PCI: pci-hyperv: Retry PCI bus D0 entry when the first attempt failed with invalid device state 0xC0000184.
From: Michael Kelley @ 2020-04-27 0:15 UTC (permalink / raw)
To: Wei Hu, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
wei.liu@kernel.org, lorenzo.pieralisi@arm.com, robh@kernel.org,
bhelgaas@google.com, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
Dexuan Cui
In-Reply-To: <20200426132430.1756-1-weh@microsoft.com>
From: Wei Hu <weh@microsoft.com> Sent: Sunday, April 26, 2020 6:25 AM
>
> In the case of kdump, the PCI device was not cleanly shut down
> before the kdump kernel starts. This causes the initial
> attempt of entering D0 state in the kdump kernel to fail with
> invalid device state 0xC0000184 returned from Hyper-V host.
> When this happens, explicitly call PCI bus exit and retry to
> enter the D0 state.
>
> Also fix the PCI probe failure path to release the PCI device
> resource properly.
>
> Signed-off-by: Wei Hu <weh@microsoft.com>
> ---
> drivers/pci/controller/pci-hyperv.c | 34 ++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index e15022ff63e3..eb4781fa058d 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -2736,6 +2736,10 @@ static void hv_free_config_window(struct hv_pcibus_device
> *hbus)
> vmbus_free_mmio(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH);
> }
>
> +#define STATUS_INVALID_DEVICE_STATE 0xC0000184
> +
> +static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating);
> +
> /**
> * hv_pci_enter_d0() - Bring the "bus" into the D0 power state
> * @hdev: VMBus's tracking struct for this root PCI bus
> @@ -2748,8 +2752,10 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
> struct pci_bus_d0_entry *d0_entry;
> struct hv_pci_compl comp_pkt;
> struct pci_packet *pkt;
> + bool retry = true;
> int ret;
>
> +enter_d0_retry:
> /*
> * Tell the host that the bus is ready to use, and moved into the
> * powered-on state. This includes telling the host which region
> @@ -2780,6 +2786,30 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
> dev_err(&hdev->device,
> "PCI Pass-through VSP failed D0 Entry with status %x\n",
> comp_pkt.completion_status);
The above error message will be output even if a retry is attempted.
And if the retry succeeds, there's no further message, which could leave an
incorrect impression for someone looking at the boot logs. If the error message
is output, there should be a follow-up message indicating the retry succeeded.
Or don't output the above message at all -- output only a message that says
"doing a retry". This could be accomplished by doing a separate test for
STATUS_INVALID_DEVICE_STATE that is not nested under checking the
completion_status for 0. Here's a structure that also has the benefit of
reducing the indentation levels:
if ((comp_pkt.completion_status == STATUS_INVALID_DEVICE_STATE) && retry) {
retry = false;
dev_err(&hdev->device, "Retrying D0 Entry\n");
ret = hv_pci_bus_exit(hdev, true);
if (ret == 0) {
kfree(pkt);
goto enter_do_retry;
}
dev_err(&hdev->device, "Retrying D0 Entry failed with %d\n", ret);
}
if (comp_pkt.completion_status < 0) {
dev_err(&hdev->device,
"PCI Pass-through VSP failed D0 Entry with status %x\n",
comp_pkt.completion_status);
ret = -EPROTO;
goto exit;
}
> +
> + /*
> + * In certain case (Kdump) the pci device of interest was
> + * not cleanly shut down and resource is still held on host
> + * side, the host could return STATUS_INVALID_DEVICE_STATE.
> + * We need to explicitly request host to release the resource
> + * and try to enter D0 again.
> + */
> + if (comp_pkt.completion_status == STATUS_INVALID_DEVICE_STATE &&
> + retry) {
> + ret = hv_pci_bus_exit(hdev, true);
> +
> + retry = false;
> +
> + if (ret == 0) {
> + kfree(pkt);
> + goto enter_d0_retry;
> + } else {
> + dev_err(&hdev->device,
> + "PCI bus D0 exit failed with ret %d\n",
> + ret);
> + }
> + }
> +
> ret = -EPROTO;
> goto exit;
> }
> @@ -3136,7 +3166,7 @@ static int hv_pci_probe(struct hv_device *hdev,
>
> ret = hv_pci_allocate_bridge_windows(hbus);
> if (ret)
> - goto free_irq_domain;
> + goto exit_d0;
>
> ret = hv_send_resources_allocated(hdev);
> if (ret)
The above is good. But there's another error case that isn't handled
correctly. If create_root_hv_pci_bus() fails, hv_send_resources_released()
should be called.
Fixing these two error cases should probably go in a separate patch: One
patch for the retry problem in kdump, and a separate patch for these error
cases.
Michael
> @@ -3154,6 +3184,8 @@ static int hv_pci_probe(struct hv_device *hdev,
>
> free_windows:
> hv_pci_free_bridge_windows(hbus);
> +exit_d0:
> + (void) hv_pci_bus_exit(hdev, true);
> free_irq_domain:
> irq_domain_remove(hbus->irq_domain);
> free_fwnode:
> --
> 2.20.1
^ permalink raw reply
* RE: [PATCH] PCI: pci-hyperv: Retry PCI bus D0 entry when the first attempt failed with invalid device state 0xC0000184.
From: Dexuan Cui @ 2020-04-27 1:30 UTC (permalink / raw)
To: Wei Hu, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
wei.liu@kernel.org, lorenzo.pieralisi@arm.com, robh@kernel.org,
bhelgaas@google.com, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
Michael Kelley
> From: Wei Hu <weh@microsoft.com>
> Sent: Sunday, April 26, 2020 6:25 AM
> Subject: [PATCH] PCI: pci-hyperv: Retry PCI bus D0 entry when the first attempt
> failed with invalid device state 0xC0000184.
The title looks too long. :-)
Ideally it should be shorter than 75 chars. I suggest the part
"with invalid device state 0xC0000184. " should be removed.
> +#define STATUS_INVALID_DEVICE_STATE 0xC0000184
> +
> +static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating);
Should we change the name of the parameter 'hibernating'?
> /**
> * hv_pci_enter_d0() - Bring the "bus" into the D0 power state
> * @hdev: VMBus's tracking struct for this root PCI bus
> @@ -2748,8 +2752,10 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
> struct pci_bus_d0_entry *d0_entry;
> struct hv_pci_compl comp_pkt;
> struct pci_packet *pkt;
> + bool retry = true;
> int ret;
>
> +enter_d0_retry:
> /*
> * Tell the host that the bus is ready to use, and moved into the
> * powered-on state. This includes telling the host which region
> @@ -2780,6 +2786,30 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
> dev_err(&hdev->device,
> "PCI Pass-through VSP failed D0 Entry with status %x\n",
> comp_pkt.completion_status);
> +
> + /*
> + * In certain case (Kdump) the pci device of interest was
> + * not cleanly shut down and resource is still held on host
> + * side, the host could return STATUS_INVALID_DEVICE_STATE.
> + * We need to explicitly request host to release the resource
> + * and try to enter D0 again.
> + */
> + if (comp_pkt.completion_status == STATUS_INVALID_DEVICE_STATE
> &&
> + retry) {
Maybe it's better to just retry for any error in comp_pkt.completion_status?
Just in case the host returns a slightly different error code in future.
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [PATCH] PCI: pci-hyperv: Retry PCI bus D0 entry when the first attempt failed with invalid device state 0xC0000184.
From: Wei Hu @ 2020-04-27 7:05 UTC (permalink / raw)
To: Michael Kelley, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
wei.liu@kernel.org, linux-hyperv@vger.kernel.org, Dexuan Cui
In-Reply-To: <MW2PR2101MB10523247FBE97CDA56E7F5A8D7AF0@MW2PR2101MB1052.namprd21.prod.outlook.com>
> -----Original Message-----
> From: Michael Kelley <mikelley@microsoft.com>
> > @@ -3136,7 +3166,7 @@ static int hv_pci_probe(struct hv_device *hdev,
> >
> > ret = hv_pci_allocate_bridge_windows(hbus);
> > if (ret)
> > - goto free_irq_domain;
> > + goto exit_d0;
> >
> > ret = hv_send_resources_allocated(hdev);
> > if (ret)
>
> The above is good. But there's another error case that isn't handled
> correctly. If create_root_hv_pci_bus() fails, hv_send_resources_released()
> should be called.
>
> Fixing these two error cases should probably go in a separate patch: One
> patch for the retry problem in kdump, and a separate patch for these error
> cases.
[Wei Hu]
hv_send_resources_released() is called in the added hv_pci_bus_exit().
If hv_send_resources_allocated() fails, is it correct to call hv_send_resources_released()?
Allocation can fail in the middle. So I am not sure if calling hv_send_resources_released()
won't cause any side effect.
Agree it should to into a separate patch.
Wei
>
> Michael
>
>
> > @@ -3154,6 +3184,8 @@ static int hv_pci_probe(struct hv_device *hdev,
> >
> > free_windows:
> > hv_pci_free_bridge_windows(hbus);
> > +exit_d0:
> > + (void) hv_pci_bus_exit(hdev, true);
> > free_irq_domain:
> > irq_domain_remove(hbus->irq_domain);
> > free_fwnode:
> > --
> > 2.20.1
^ permalink raw reply
* Re: [PATCH] PCI: pci-hyperv: Retry PCI bus D0 entry when the first attempt failed with invalid device state 0xC0000184.
From: Wei Liu @ 2020-04-27 10:21 UTC (permalink / raw)
To: Wei Hu
Cc: kys, haiyangz, sthemmin, wei.liu, lorenzo.pieralisi, robh,
bhelgaas, linux-hyperv, linux-pci, linux-kernel, decui, mikelley
In-Reply-To: <20200426132430.1756-1-weh@microsoft.com>
On Sun, Apr 26, 2020 at 09:24:30PM +0800, Wei Hu wrote:
> In the case of kdump, the PCI device was not cleanly shut down
> before the kdump kernel starts. This causes the initial
> attempt of entering D0 state in the kdump kernel to fail with
> invalid device state 0xC0000184 returned from Hyper-V host.
> When this happens, explicitly call PCI bus exit and retry to
> enter the D0 state.
>
> Also fix the PCI probe failure path to release the PCI device
> resource properly.
>
> Signed-off-by: Wei Hu <weh@microsoft.com>
> ---
> drivers/pci/controller/pci-hyperv.c | 34 ++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index e15022ff63e3..eb4781fa058d 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -2736,6 +2736,10 @@ static void hv_free_config_window(struct hv_pcibus_device *hbus)
> vmbus_free_mmio(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH);
> }
>
> +#define STATUS_INVALID_DEVICE_STATE 0xC0000184
> +
Can you please move this along side STATUS_REVISION_MISMATCH?
Wei.
^ permalink raw reply
* [GIT PULL] Hyper-V commits for 5.7-rc4
From: Wei Liu @ 2020-04-27 11:19 UTC (permalink / raw)
To: Linus Torvalds
Cc: Wei Liu, Linux on Hyper-V List, Linux Kernel List, Michael Kelley,
kys, sthemmin, haiyangz
Hi Linus
Please pull the following changes since commit
f3a99e761efa616028b255b4de58e9b5b87c5545:
x86/Hyper-V: Report crash data in die() when panic_on_oops is set (2020-04-11 17:19:07 +0100)
are available in the Git repository at:
ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-fixes-signed
for you to fetch changes up to f081bbb3fd03f949bcdc5aed95a827d7c65e0f30:
hyper-v: Remove internal types from UAPI header (2020-04-22 21:10:05 +0100)
----------------------------------------------------------------
hyperv-fixes for 5.7-rc4
- Two patches from Dexuan fixing suspension bugs
- Three cleanup patches from Andy and Michael
----------------------------------------------------------------
Andy Shevchenko (2):
hyper-v: Use UUID API for exporting the GUID
hyper-v: Remove internal types from UAPI header
Dexuan Cui (2):
Drivers: hv: vmbus: Fix Suspend-to-Idle for Generation-2 VM
x86/hyperv: Suspend/resume the VP assist page for hibernation
Michael Kelley (1):
Drivers: hv: Move AEOI determination to architecture dependent code
arch/x86/hyperv/hv_init.c | 12 ++++++++++--
arch/x86/include/asm/mshyperv.h | 2 ++
drivers/hv/hv.c | 6 +-----
drivers/hv/hv_trace.h | 4 ++--
drivers/hv/vmbus_drv.c | 43 ++++++++++++++++++++++++++++++++---------
include/uapi/linux/hyperv.h | 4 ++--
6 files changed, 51 insertions(+), 20 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next 0/3] vsock: support network namespace
From: Stefano Garzarella @ 2020-04-27 14:25 UTC (permalink / raw)
To: davem, Stefan Hajnoczi, Michael S. Tsirkin
Cc: linux-kernel, Jorgen Hansen, Jason Wang, kvm, virtualization,
linux-hyperv, Dexuan Cui, netdev
In-Reply-To: <20200116172428.311437-1-sgarzare@redhat.com>
Hi David, Michael, Stefan,
I'm restarting to work on this topic since Kata guys are interested to
have that, especially on the guest side.
While working on the v2 I had few doubts, and I'd like to have your
suggestions:
1. netns assigned to the device inside the guest
Currently I assigned this device to 'init_net'. Maybe it is better
if we allow the user to decide which netns assign to the device
or to disable this new feature to have the same behavior as before
(host reachable from any netns).
I think we can handle this in the vsock core and not in the single
transports.
The simplest way that I found, is to add a new
IOCTL_VM_SOCKETS_ASSIGN_G2H_NETNS to /dev/vsock to enable the feature
and assign the device to the same netns of the process that do the
ioctl(), but I'm not sure it is clean enough.
Maybe it is better to add new rtnetlink messages, but I'm not sure if
it is feasible since we don't have a netdev device.
What do you suggest?
2. netns assigned in the host
As Michael suggested, I added a new /dev/vhost-vsock-netns to allow
userspace application to use this new feature, leaving to
/dev/vhost-vsock the previous behavior (guest reachable from any
netns).
I like this approach, but I had these doubts:
- I need to allocate a new minor for that device (e.g.
VHOST_VSOCK_NETNS_MINOR) or is there an alternative way that I can
use?
- It is vhost-vsock specific, should we provide something handled in
the vsock core, maybe centralizing the CID allocation and adding a
new IOCTL or rtnetlink message like for the guest side?
(maybe it could be a second step, and for now we can continue with
the new device)
Thanks for the help,
Stefano
On Thu, Jan 16, 2020 at 06:24:25PM +0100, Stefano Garzarella wrote:
> RFC -> v1:
> * added 'netns' module param to vsock.ko to enable the
> network namespace support (disabled by default)
> * added 'vsock_net_eq()' to check the "net" assigned to a socket
> only when 'netns' support is enabled
>
> RFC: https://patchwork.ozlabs.org/cover/1202235/
>
> Now that we have multi-transport upstream, I started to take a look to
> support network namespace in vsock.
>
> As we partially discussed in the multi-transport proposal [1], it could
> be nice to support network namespace in vsock to reach the following
> goals:
> - isolate host applications from guest applications using the same ports
> with CID_ANY
> - assign the same CID of VMs running in different network namespaces
> - partition VMs between VMMs or at finer granularity
>
> This new feature is disabled by default, because it changes vsock's
> behavior with network namespaces and could break existing applications.
> It can be enabled with the new 'netns' module parameter of vsock.ko.
>
> This implementation provides the following behavior:
> - packets received from the host (received by G2H transports) are
> assigned to the default netns (init_net)
> - packets received from the guest (received by H2G - vhost-vsock) are
> assigned to the netns of the process that opens /dev/vhost-vsock
> (usually the VMM, qemu in my tests, opens the /dev/vhost-vsock)
> - for vmci I need some suggestions, because I don't know how to do
> and test the same in the vmci driver, for now vmci uses the
> init_net
> - loopback packets are exchanged only in the same netns
>
> I tested the series in this way:
> l0_host$ qemu-system-x86_64 -m 4G -M accel=kvm -smp 4 \
> -drive file=/tmp/vsockvm0.img,if=virtio --nographic \
> -device vhost-vsock-pci,guest-cid=3
>
> l1_vm$ echo 1 > /sys/module/vsock/parameters/netns
>
> l1_vm$ ip netns add ns1
> l1_vm$ ip netns add ns2
> # same CID on different netns
> l1_vm$ ip netns exec ns1 qemu-system-x86_64 -m 1G -M accel=kvm -smp 2 \
> -drive file=/tmp/vsockvm1.img,if=virtio --nographic \
> -device vhost-vsock-pci,guest-cid=4
> l1_vm$ ip netns exec ns2 qemu-system-x86_64 -m 1G -M accel=kvm -smp 2 \
> -drive file=/tmp/vsockvm2.img,if=virtio --nographic \
> -device vhost-vsock-pci,guest-cid=4
>
> # all iperf3 listen on CID_ANY and port 5201, but in different netns
> l1_vm$ ./iperf3 --vsock -s # connection from l0 or guests started
> # on default netns (init_net)
> l1_vm$ ip netns exec ns1 ./iperf3 --vsock -s
> l1_vm$ ip netns exec ns1 ./iperf3 --vsock -s
>
> l0_host$ ./iperf3 --vsock -c 3
> l2_vm1$ ./iperf3 --vsock -c 2
> l2_vm2$ ./iperf3 --vsock -c 2
>
> [1] https://www.spinics.net/lists/netdev/msg575792.html
>
> Stefano Garzarella (3):
> vsock: add network namespace support
> vsock/virtio_transport_common: handle netns of received packets
> vhost/vsock: use netns of process that opens the vhost-vsock device
>
> drivers/vhost/vsock.c | 29 ++++++++++++-----
> include/linux/virtio_vsock.h | 2 ++
> include/net/af_vsock.h | 7 +++--
> net/vmw_vsock/af_vsock.c | 41 +++++++++++++++++++------
> net/vmw_vsock/hyperv_transport.c | 5 +--
> net/vmw_vsock/virtio_transport.c | 2 ++
> net/vmw_vsock/virtio_transport_common.c | 12 ++++++--
> net/vmw_vsock/vmci_transport.c | 5 +--
> 8 files changed, 78 insertions(+), 25 deletions(-)
>
> --
> 2.24.1
>
^ permalink raw reply
* Re: [PATCH net-next 0/3] vsock: support network namespace
From: Michael S. Tsirkin @ 2020-04-27 14:31 UTC (permalink / raw)
To: Stefano Garzarella
Cc: davem, Stefan Hajnoczi, linux-kernel, Jorgen Hansen, Jason Wang,
kvm, virtualization, linux-hyperv, Dexuan Cui, netdev
In-Reply-To: <20200427142518.uwssa6dtasrp3bfc@steredhat>
On Mon, Apr 27, 2020 at 04:25:18PM +0200, Stefano Garzarella wrote:
> Hi David, Michael, Stefan,
> I'm restarting to work on this topic since Kata guys are interested to
> have that, especially on the guest side.
>
> While working on the v2 I had few doubts, and I'd like to have your
> suggestions:
>
> 1. netns assigned to the device inside the guest
>
> Currently I assigned this device to 'init_net'. Maybe it is better
> if we allow the user to decide which netns assign to the device
> or to disable this new feature to have the same behavior as before
> (host reachable from any netns).
> I think we can handle this in the vsock core and not in the single
> transports.
>
> The simplest way that I found, is to add a new
> IOCTL_VM_SOCKETS_ASSIGN_G2H_NETNS to /dev/vsock to enable the feature
> and assign the device to the same netns of the process that do the
> ioctl(), but I'm not sure it is clean enough.
>
> Maybe it is better to add new rtnetlink messages, but I'm not sure if
> it is feasible since we don't have a netdev device.
>
> What do you suggest?
Maybe /dev/vsock-netns here too, like in the host?
>
> 2. netns assigned in the host
>
> As Michael suggested, I added a new /dev/vhost-vsock-netns to allow
> userspace application to use this new feature, leaving to
> /dev/vhost-vsock the previous behavior (guest reachable from any
> netns).
>
> I like this approach, but I had these doubts:
>
> - I need to allocate a new minor for that device (e.g.
> VHOST_VSOCK_NETNS_MINOR) or is there an alternative way that I can
> use?
Not that I see. I agree it's a bit annoying. I'll think about it a bit.
> - It is vhost-vsock specific, should we provide something handled in
> the vsock core, maybe centralizing the CID allocation and adding a
> new IOCTL or rtnetlink message like for the guest side?
> (maybe it could be a second step, and for now we can continue with
> the new device)
>
>
> Thanks for the help,
> Stefano
>
>
> On Thu, Jan 16, 2020 at 06:24:25PM +0100, Stefano Garzarella wrote:
> > RFC -> v1:
> > * added 'netns' module param to vsock.ko to enable the
> > network namespace support (disabled by default)
> > * added 'vsock_net_eq()' to check the "net" assigned to a socket
> > only when 'netns' support is enabled
> >
> > RFC: https://patchwork.ozlabs.org/cover/1202235/
> >
> > Now that we have multi-transport upstream, I started to take a look to
> > support network namespace in vsock.
> >
> > As we partially discussed in the multi-transport proposal [1], it could
> > be nice to support network namespace in vsock to reach the following
> > goals:
> > - isolate host applications from guest applications using the same ports
> > with CID_ANY
> > - assign the same CID of VMs running in different network namespaces
> > - partition VMs between VMMs or at finer granularity
> >
> > This new feature is disabled by default, because it changes vsock's
> > behavior with network namespaces and could break existing applications.
> > It can be enabled with the new 'netns' module parameter of vsock.ko.
> >
> > This implementation provides the following behavior:
> > - packets received from the host (received by G2H transports) are
> > assigned to the default netns (init_net)
> > - packets received from the guest (received by H2G - vhost-vsock) are
> > assigned to the netns of the process that opens /dev/vhost-vsock
> > (usually the VMM, qemu in my tests, opens the /dev/vhost-vsock)
> > - for vmci I need some suggestions, because I don't know how to do
> > and test the same in the vmci driver, for now vmci uses the
> > init_net
> > - loopback packets are exchanged only in the same netns
> >
> > I tested the series in this way:
> > l0_host$ qemu-system-x86_64 -m 4G -M accel=kvm -smp 4 \
> > -drive file=/tmp/vsockvm0.img,if=virtio --nographic \
> > -device vhost-vsock-pci,guest-cid=3
> >
> > l1_vm$ echo 1 > /sys/module/vsock/parameters/netns
> >
> > l1_vm$ ip netns add ns1
> > l1_vm$ ip netns add ns2
> > # same CID on different netns
> > l1_vm$ ip netns exec ns1 qemu-system-x86_64 -m 1G -M accel=kvm -smp 2 \
> > -drive file=/tmp/vsockvm1.img,if=virtio --nographic \
> > -device vhost-vsock-pci,guest-cid=4
> > l1_vm$ ip netns exec ns2 qemu-system-x86_64 -m 1G -M accel=kvm -smp 2 \
> > -drive file=/tmp/vsockvm2.img,if=virtio --nographic \
> > -device vhost-vsock-pci,guest-cid=4
> >
> > # all iperf3 listen on CID_ANY and port 5201, but in different netns
> > l1_vm$ ./iperf3 --vsock -s # connection from l0 or guests started
> > # on default netns (init_net)
> > l1_vm$ ip netns exec ns1 ./iperf3 --vsock -s
> > l1_vm$ ip netns exec ns1 ./iperf3 --vsock -s
> >
> > l0_host$ ./iperf3 --vsock -c 3
> > l2_vm1$ ./iperf3 --vsock -c 2
> > l2_vm2$ ./iperf3 --vsock -c 2
> >
> > [1] https://www.spinics.net/lists/netdev/msg575792.html
> >
> > Stefano Garzarella (3):
> > vsock: add network namespace support
> > vsock/virtio_transport_common: handle netns of received packets
> > vhost/vsock: use netns of process that opens the vhost-vsock device
> >
> > drivers/vhost/vsock.c | 29 ++++++++++++-----
> > include/linux/virtio_vsock.h | 2 ++
> > include/net/af_vsock.h | 7 +++--
> > net/vmw_vsock/af_vsock.c | 41 +++++++++++++++++++------
> > net/vmw_vsock/hyperv_transport.c | 5 +--
> > net/vmw_vsock/virtio_transport.c | 2 ++
> > net/vmw_vsock/virtio_transport_common.c | 12 ++++++--
> > net/vmw_vsock/vmci_transport.c | 5 +--
> > 8 files changed, 78 insertions(+), 25 deletions(-)
> >
> > --
> > 2.24.1
> >
^ permalink raw reply
* Re: [PATCH net-next 0/3] vsock: support network namespace
From: Stefano Garzarella @ 2020-04-27 15:21 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: davem, Stefan Hajnoczi, linux-kernel, Jorgen Hansen, Jason Wang,
kvm, virtualization, linux-hyperv, Dexuan Cui, netdev
In-Reply-To: <20200427102828-mutt-send-email-mst@kernel.org>
On Mon, Apr 27, 2020 at 10:31:57AM -0400, Michael S. Tsirkin wrote:
> On Mon, Apr 27, 2020 at 04:25:18PM +0200, Stefano Garzarella wrote:
> > Hi David, Michael, Stefan,
> > I'm restarting to work on this topic since Kata guys are interested to
> > have that, especially on the guest side.
> >
> > While working on the v2 I had few doubts, and I'd like to have your
> > suggestions:
> >
> > 1. netns assigned to the device inside the guest
> >
> > Currently I assigned this device to 'init_net'. Maybe it is better
> > if we allow the user to decide which netns assign to the device
> > or to disable this new feature to have the same behavior as before
> > (host reachable from any netns).
> > I think we can handle this in the vsock core and not in the single
> > transports.
> >
> > The simplest way that I found, is to add a new
> > IOCTL_VM_SOCKETS_ASSIGN_G2H_NETNS to /dev/vsock to enable the feature
> > and assign the device to the same netns of the process that do the
> > ioctl(), but I'm not sure it is clean enough.
> >
> > Maybe it is better to add new rtnetlink messages, but I'm not sure if
> > it is feasible since we don't have a netdev device.
> >
> > What do you suggest?
>
> Maybe /dev/vsock-netns here too, like in the host?
>
I'm not sure I get it.
In the guest, /dev/vsock is only used to get the CID assigned to the
guest through an ioctl().
In the virtio-vsock case, the guest transport is loaded when it is discovered
on the PCI bus, so we need a way to "move" it to a netns or to specify
which netns should be used when it is probed.
>
> >
> > 2. netns assigned in the host
> >
> > As Michael suggested, I added a new /dev/vhost-vsock-netns to allow
> > userspace application to use this new feature, leaving to
> > /dev/vhost-vsock the previous behavior (guest reachable from any
> > netns).
> >
> > I like this approach, but I had these doubts:
> >
> > - I need to allocate a new minor for that device (e.g.
> > VHOST_VSOCK_NETNS_MINOR) or is there an alternative way that I can
> > use?
>
> Not that I see. I agree it's a bit annoying. I'll think about it a bit.
>
Thanks for that!
An idea that I had, was to add a new ioctl to /dev/vhost-vsock to enable
the netns support, but I'm not sure it is a clean approach.
> > - It is vhost-vsock specific, should we provide something handled in
> > the vsock core, maybe centralizing the CID allocation and adding a
> > new IOCTL or rtnetlink message like for the guest side?
> > (maybe it could be a second step, and for now we can continue with
> > the new device)
> >
Thanks,
Stefano
^ permalink raw reply
* RE: [PATCH] PCI: pci-hyperv: Retry PCI bus D0 entry when the first attempt failed with invalid device state 0xC0000184.
From: Michael Kelley @ 2020-04-27 15:36 UTC (permalink / raw)
To: Wei Hu, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
wei.liu@kernel.org, linux-hyperv@vger.kernel.org, Dexuan Cui
In-Reply-To: <SG2P153MB0213FD4A0061C892B38D995ABBAF0@SG2P153MB0213.APCP153.PROD.OUTLOOK.COM>
From: Wei Hu <weh@microsoft.com> Sent: Monday, April 27, 2020 12:06 AM
> > -----Original Message-----
> > From: Michael Kelley <mikelley@microsoft.com>
> > > @@ -3136,7 +3166,7 @@ static int hv_pci_probe(struct hv_device *hdev,
> > >
> > > ret = hv_pci_allocate_bridge_windows(hbus);
> > > if (ret)
> > > - goto free_irq_domain;
> > > + goto exit_d0;
> > >
> > > ret = hv_send_resources_allocated(hdev);
> > > if (ret)
> >
> > The above is good. But there's another error case that isn't handled
> > correctly. If create_root_hv_pci_bus() fails, hv_send_resources_released()
> > should be called.
> >
> > Fixing these two error cases should probably go in a separate patch: One
> > patch for the retry problem in kdump, and a separate patch for these error
> > cases.
>
> [Wei Hu]
> hv_send_resources_released() is called in the added hv_pci_bus_exit().
> If hv_send_resources_allocated() fails, is it correct to call hv_send_resources_released()?
> Allocation can fail in the middle. So I am not sure if calling hv_send_resources_released()
> won't cause any side effect.
>
Ah yes, you are right. But that brings up a separate problem.
If hv_pci_allocate_bridge_windows() or hv_send_resources_allocated() fails, then the
error path will call hv_pci_bus_exit(), which will call hv_send_resources_released(),
even if hv_send_resources_allocated() was never called or didn't fully succeed. As you
noted, hv_send_resources_allocated() does multiple steps, some of which might have
succeeded, and some of which didn't. The mismatch might cause problems. That means
fixing this error handling is going to be a bit more complex. Each operation needs
to be individually undone, and only if it previously succeeded. Could you follow up
with the Hyper-V people to see if there's a problem with doing the RESOURCES_RELEASED
message on a slot where RESOURCES_ASSIGNED was not done or wasn't successful?
If doing a spurious RESOURCES_RELEASED is harmless, that will make the error cleanup
easier.
Michael
^ permalink raw reply
* Re: [GIT PULL] Hyper-V commits for 5.7-rc4
From: pr-tracker-bot @ 2020-04-27 20:40 UTC (permalink / raw)
To: Wei Liu
Cc: Linus Torvalds, Wei Liu, Linux on Hyper-V List, Linux Kernel List,
Michael Kelley, kys, sthemmin, haiyangz
In-Reply-To: <20200427111945.6qdvgimt3nt3ja57@liuwe-devbox-debian-v2.j3c5onc20sse1dnehy4noqpfcg.zx.internal.cloudapp.net>
The pull request you sent on Mon, 27 Apr 2020 11:19:45 +0000:
> ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-fixes-signed
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/869997be0e3d4b994aa04bfdf3e534b9072e6a17
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox