* [PATCH V3 2/6] x86/Hyper-V: Free hv_panic_page when fail to register kmsg dump
From: ltykernel @ 2020-03-24 7:57 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, liuwe, tglx, mingo, bp, hpa, x86,
michael.h.kelley
Cc: Tianyu Lan, linux-hyperv, linux-kernel, vkuznets
In-Reply-To: <20200324075720.9462-1-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
If kmsg_dump_register() fails, hv_panic_page will not be used
anywhere. So free and reset it.
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v1:
- Update commit log
- Remove hv_free_hyperv_page() in the error path
---
drivers/hv/vmbus_drv.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 6478240d11ab..00a511f15926 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1385,9 +1385,13 @@ static int vmbus_bus_init(void)
hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
if (hv_panic_page) {
ret = kmsg_dump_register(&hv_kmsg_dumper);
- if (ret)
+ if (ret) {
pr_err("Hyper-V: kmsg dump register "
"error 0x%x\n", ret);
+ hv_free_hyperv_page(
+ (unsigned long)hv_panic_page);
+ hv_panic_page = NULL;
+ }
} else
pr_err("Hyper-V: panic message page memory "
"allocation failed");
@@ -1416,7 +1420,6 @@ static int vmbus_bus_init(void)
hv_remove_vmbus_irq();
bus_unregister(&hv_bus);
- hv_free_hyperv_page((unsigned long)hv_panic_page);
unregister_sysctl_table(hv_ctl_table_hdr);
hv_ctl_table_hdr = NULL;
return ret;
--
2.14.5
^ permalink raw reply related
* [PATCH V3 0/6] x86/Hyper-V: Panic code path fixes
From: ltykernel @ 2020-03-24 7:57 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, liuwe, tglx, mingo, bp, hpa, x86,
michael.h.kelley
Cc: Tianyu Lan, linux-hyperv, linux-kernel, vkuznets
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
This patchset fixes some issues in the Hyper-V panic code path.
Patch 1 resolves issue that panic system still responses network
packets.
Patch 2-3,5-6 resolves crash enlightenment issues.
Patch 4 is to set crash_kexec_post_notifiers to true for Hyper-V
VM in order to report crash data or kmsg to host before running
kdump kernel.
Tianyu Lan (6):
x86/Hyper-V: Unload vmbus channel in hv panic callback
x86/Hyper-V: Free hv_panic_page when fail to register kmsg dump
x86/Hyper-V: Trigger crash enlightenment only once during system
crash.
x86/Hyper-V: Report crash register data or ksmg before running crash
kernel
x86/Hyper-V: Report crash register data when sysctl_record_panic_msg
is not set
x86/Hyper-V: Report crash data in die() when panic_on_oops is set
arch/x86/kernel/cpu/mshyperv.c | 10 +++++++
drivers/hv/channel_mgmt.c | 3 +++
drivers/hv/vmbus_drv.c | 61 +++++++++++++++++++++++++++++-------------
3 files changed, 56 insertions(+), 18 deletions(-)
--
2.14.5
^ permalink raw reply
* [PATCH V3 3/6] x86/Hyper-V: Trigger crash enlightenment only once during system crash.
From: ltykernel @ 2020-03-24 7:57 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, liuwe, tglx, mingo, bp, hpa, x86,
michael.h.kelley
Cc: Tianyu Lan, linux-hyperv, linux-kernel, vkuznets
In-Reply-To: <20200324075720.9462-1-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
When a guest VM panics, Hyper-V should be notified only once via the
crash synthetic MSRs. Current Linux code might write these crash MSRs
twice during a system panic:
1) hyperv_panic/die_event() calling hyperv_report_panic()
2) hv_kmsg_dump() calling hyperv_report_panic_msg()
Fix this by not calling hyperv_report_panic() if a kmsg dump has been
successfully registered. The notification will happen later via
hyperv_report_panic_msg().
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v1:
- Update commit log
Change since v2:
- Update comment
---
drivers/hv/vmbus_drv.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 00a511f15926..333dad39b1c1 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -55,7 +55,13 @@ static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
vmbus_initiate_unload(true);
- if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+ /*
+ * Hyper-V should be notified only once about a panic. If we will be
+ * doing hyperv_report_panic_msg() later with kmsg data, don't do
+ * the notification here.
+ */
+ if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE
+ && !hv_panic_page) {
regs = current_pt_regs();
hyperv_report_panic(regs, val);
}
@@ -68,7 +74,13 @@ static int hyperv_die_event(struct notifier_block *nb, unsigned long val,
struct die_args *die = (struct die_args *)args;
struct pt_regs *regs = die->regs;
- hyperv_report_panic(regs, val);
+ /*
+ * Hyper-V should be notified only once about a panic. If we will be
+ * doing hyperv_report_panic_msg() later with kmsg data, don't do
+ * the notification here.
+ */
+ if (!hv_panic_page)
+ hyperv_report_panic(regs, val);
return NOTIFY_DONE;
}
--
2.14.5
^ permalink raw reply related
* Re: [PATCH v10 7/7] KVM: selftests: update hyperv_cpuid with SynDBG tests
From: Vitaly Kuznetsov @ 2020-03-24 9:05 UTC (permalink / raw)
To: Jon Doron; +Cc: kvm, linux-hyperv
In-Reply-To: <20200324074341.1770081-8-arilou@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 528 bytes --]
Jon Doron <arilou@gmail.com> writes:
> 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>
Something weird happened to this patch. It fails to apply on kvm/queue
but it's also a bit different from what I've sent yesterday. I fixed and
tested it, please find the result attached.
--
Vitaly
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-KVM-selftests-update-hyperv_cpuid-with-SynDBG-tests.patch --]
[-- Type: text/x-patch, Size: 6295 bytes --]
From ae2c688389c20a99d7457861e57ce97054780908 Mon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov <vkuznets () redhat ! com>
Date: Tue, 24 Mar 2020 09:43:41 +0200
Subject: [PATCH v10 7/7 FIXED] KVM: selftests: update hyperv_cpuid with SynDBG
tests
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.25.1
^ permalink raw reply related
* Re: [PATCH v10 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Vitaly Kuznetsov @ 2020-03-24 13:27 UTC (permalink / raw)
To: Jon Doron; +Cc: kvm, linux-hyperv
In-Reply-To: <20200324074341.1770081-5-arilou@gmail.com>
Jon Doron <arilou@gmail.com> writes:
> 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.
>
> 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 4872c47bbcff..fe992dcf4f93 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -5024,6 +5024,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 {
> @@ -5039,6 +5040,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 */
> @@ -5055,6 +5065,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 98959e8cd448..c09fa7401b13 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 5a7336d6b8ca..befe5b3b9e20 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 f194dd058470..bf6c3852868d 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -1515,6 +1515,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 3156e25b0774..7bfa30d7a525 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1214,6 +1214,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,
> @@ -2906,6 +2910,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:
> @@ -3151,6 +3157,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:
> @@ -3323,6 +3331,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:
> @@ -4178,6 +4187,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 7ee0ddc4c457..0b573e1155c8 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;
> };
>
> @@ -1012,6 +1022,7 @@ struct kvm_ppc_resize_hpt {
> #define KVM_CAP_ARM_NISV_TO_USER 177
> #define KVM_CAP_ARM_INJECT_EXT_DABT 178
> #define KVM_CAP_S390_VCPU_RESETS 179
> +#define KVM_CAP_HYPERV_SYNDBG 180
'180' is already occupied by KVM_CAP_S390_PROTECTED (kvm/queue) so
KVM_CAP_HYPERV_SYNDBG will have to become '181'. No need to resend I
guess, it can change again if there is a conflicting series.
>
> #ifdef KVM_CAP_IRQ_ROUTING
I don't know much about syndbg interface myself but the code looks good
and CPUID output is tested with PATCH7, so
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply
* Re: [PATCH v10 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg
From: Vitaly Kuznetsov @ 2020-03-24 13:28 UTC (permalink / raw)
To: Jon Doron, kvm, linux-hyperv; +Cc: Jon Doron
In-Reply-To: <20200324074341.1770081-6-arilou@gmail.com>
Jon Doron <arilou@gmail.com> writes:
> 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.
>
> 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 befe5b3b9e20..59c6eadb7eca 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)
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply
* Re: [PATCH v10 6/7] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
From: Vitaly Kuznetsov @ 2020-03-24 13:41 UTC (permalink / raw)
To: Jon Doron; +Cc: kvm, linux-hyperv
In-Reply-To: <20200324074341.1770081-7-arilou@gmail.com>
Jon Doron <arilou@gmail.com> writes:
> 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.
>
> 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 59c6eadb7eca..45ff3098e079 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;
We may want to split this into a helper in the future (as
HVCALL_POST_MESSAGE handling is not any different - basically, we just
pass the full hypercall input to the userspace, regardless of what the
hypercall was).
> + return 0;
> + }
> default:
> ret = HV_STATUS_INVALID_HYPERCALL_CODE;
> break;
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply
* [PATCH] x86/Hyper-V: Fix hv sched clock function return wrong time unit
From: Yubo Xie @ 2020-03-24 15:19 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, liuwe, daniel.lezcano, tglx,
michael.h.kelley
Cc: Yubo Xie, linux-hyperv, linux-kernel, vkuznets, stable,
Tianyu Lan
sched clock callback should return time with nano second as unit
but current hv callback returns time with 100ns. Fix it.
Cc: stable@vger.kernel.org
Signed-off-by: Yubo Xie <yuboxie@microsoft.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
Fixes: adb87ff4f96c ("clocksource/drivers/hyperv: Allocate Hyper-V TSC page statically")
---
drivers/clocksource/hyperv_timer.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index 9d808d595ca8..662ed978fa24 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -343,7 +343,8 @@ static u64 notrace read_hv_clock_tsc_cs(struct clocksource *arg)
static u64 read_hv_sched_clock_tsc(void)
{
- return read_hv_clock_tsc() - hv_sched_clock_offset;
+ return (read_hv_clock_tsc() - hv_sched_clock_offset)
+ * (NSEC_PER_SEC / HV_CLOCK_HZ);
}
static void suspend_hv_clock_tsc(struct clocksource *arg)
@@ -398,7 +399,8 @@ static u64 notrace read_hv_clock_msr_cs(struct clocksource *arg)
static u64 read_hv_sched_clock_msr(void)
{
- return read_hv_clock_msr() - hv_sched_clock_offset;
+ return (read_hv_clock_msr() - hv_sched_clock_offset)
+ * (NSEC_PER_SEC / HV_CLOCK_HZ);
}
static struct clocksource hyperv_cs_msr = {
--
2.14.5
^ permalink raw reply related
* Re: [PATCH] x86/Hyper-V: Fix hv sched clock function return wrong time unit
From: Vitaly Kuznetsov @ 2020-03-24 15:49 UTC (permalink / raw)
To: Yubo Xie
Cc: Yubo Xie, linux-hyperv, linux-kernel, stable, Tianyu Lan, kys,
haiyangz, sthemmin, liuwe, daniel.lezcano, tglx, michael.h.kelley
In-Reply-To: <20200324151935.15814-1-yuboxie@microsoft.com>
Yubo Xie <ltykernel@gmail.com> writes:
> sched clock callback should return time with nano second as unit
> but current hv callback returns time with 100ns. Fix it.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Yubo Xie <yuboxie@microsoft.com>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> Fixes: adb87ff4f96c ("clocksource/drivers/hyperv: Allocate Hyper-V TSC page statically")
I don't think this is the right commit to reference,
commit bd00cd52d5be655a2f217e2ed74b91a71cb2b14f
Author: Tianyu Lan <Tianyu.Lan@microsoft.com>
Date: Wed Aug 14 20:32:16 2019 +0800
clocksource/drivers/hyperv: Add Hyper-V specific sched clock function
looks like the one.
> ---
> drivers/clocksource/hyperv_timer.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index 9d808d595ca8..662ed978fa24 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -343,7 +343,8 @@ static u64 notrace read_hv_clock_tsc_cs(struct clocksource *arg)
>
> static u64 read_hv_sched_clock_tsc(void)
> {
> - return read_hv_clock_tsc() - hv_sched_clock_offset;
> + return (read_hv_clock_tsc() - hv_sched_clock_offset)
> + * (NSEC_PER_SEC / HV_CLOCK_HZ);
> }
>
> static void suspend_hv_clock_tsc(struct clocksource *arg)
> @@ -398,7 +399,8 @@ static u64 notrace read_hv_clock_msr_cs(struct clocksource *arg)
>
> static u64 read_hv_sched_clock_msr(void)
> {
> - return read_hv_clock_msr() - hv_sched_clock_offset;
> + return (read_hv_clock_msr() - hv_sched_clock_offset)
> + * (NSEC_PER_SEC / HV_CLOCK_HZ);
> }
kvmclock seems to have the same (pre-patch) code ...
>
> static struct clocksource hyperv_cs_msr = {
--
Vitaly
^ permalink raw reply
* Re: [PATCH] x86/Hyper-V: Fix hv sched clock function return wrong time unit
From: Sasha Levin @ 2020-03-25 1:02 UTC (permalink / raw)
To: Sasha Levin, Yubo Xie, kys, haiyangz, sthemmin
Cc: Yubo Xie, linux-hyperv, stable, stable
In-Reply-To: <20200324151935.15814-1-yuboxie@microsoft.com>
Hi
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag
fixing commit: adb87ff4f96c ("clocksource/drivers/hyperv: Allocate Hyper-V TSC page statically").
The bot has tested the following trees: v5.5.11, v5.4.27.
v5.5.11: Failed to apply! Possible dependencies:
0af3e137c144 ("clocksource/drivers/hyper-v: Untangle stimers and timesync from clocksources")
ddc61bbc4501 ("clocksource/drivers/hyper-v: Reserve PAGE_SIZE space for tsc page")
v5.4.27: Failed to apply! Possible dependencies:
0af3e137c144 ("clocksource/drivers/hyper-v: Untangle stimers and timesync from clocksources")
ddc61bbc4501 ("clocksource/drivers/hyper-v: Reserve PAGE_SIZE space for tsc page")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks
Sasha
^ permalink raw reply
* Re: [PATCH] x86/Hyper-V: Fix hv sched clock function return wrong time unit
From: Tianyu Lan @ 2020-03-25 13:57 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: Yubo Xie, linux-hyperv, linux-kernel, stable, Tianyu Lan, kys,
haiyangz, sthemmin, liuwe, daniel.lezcano, tglx, michael.h.kelley
In-Reply-To: <87ftdx7nxv.fsf@vitty.brq.redhat.com>
Hi Vitaly:
Thanks for your review.
On 3/24/2020 11:49 PM, Vitaly Kuznetsov wrote:
> Yubo Xie <ltykernel@gmail.com> writes:
>
>> sched clock callback should return time with nano second as unit
>> but current hv callback returns time with 100ns. Fix it.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Yubo Xie <yuboxie@microsoft.com>
>> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
>> Fixes: adb87ff4f96c ("clocksource/drivers/hyperv: Allocate Hyper-V TSC page statically")
>
> I don't think this is the right commit to reference,
>
> commit bd00cd52d5be655a2f217e2ed74b91a71cb2b14f
> Author: Tianyu Lan <Tianyu.Lan@microsoft.com>
> Date: Wed Aug 14 20:32:16 2019 +0800
>
> clocksource/drivers/hyperv: Add Hyper-V specific sched clock function
>
> looks like the one.
Sorry. You are right. Will update in the next version.
>
>> ---
>> drivers/clocksource/hyperv_timer.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
>> index 9d808d595ca8..662ed978fa24 100644
>> --- a/drivers/clocksource/hyperv_timer.c
>> +++ b/drivers/clocksource/hyperv_timer.c
>> @@ -343,7 +343,8 @@ static u64 notrace read_hv_clock_tsc_cs(struct clocksource *arg)
>>
>> static u64 read_hv_sched_clock_tsc(void)
>> {
>> - return read_hv_clock_tsc() - hv_sched_clock_offset;
>> + return (read_hv_clock_tsc() - hv_sched_clock_offset)
>> + * (NSEC_PER_SEC / HV_CLOCK_HZ);
>> }
>>
>> static void suspend_hv_clock_tsc(struct clocksource *arg)
>> @@ -398,7 +399,8 @@ static u64 notrace read_hv_clock_msr_cs(struct clocksource *arg)
>>
>> static u64 read_hv_sched_clock_msr(void)
>> {
>> - return read_hv_clock_msr() - hv_sched_clock_offset;
>> + return (read_hv_clock_msr() - hv_sched_clock_offset)
>> + * (NSEC_PER_SEC / HV_CLOCK_HZ);
>> }
>
> kvmclock seems to have the same (pre-patch) code ...
kvm sched clock gets time from pvclock_clocksource_read() and
the time unit is nanosecond. So there is such issue in KVM code.
>
>>
>> static struct clocksource hyperv_cs_msr = {
>
^ permalink raw reply
* Re: [PATCH] x86/Hyper-V: Fix hv sched clock function return wrong time unit
From: Vitaly Kuznetsov @ 2020-03-25 14:28 UTC (permalink / raw)
To: Tianyu Lan
Cc: Yubo Xie, linux-hyperv, linux-kernel, stable, Tianyu Lan, kys,
haiyangz, sthemmin, liuwe, daniel.lezcano, tglx, michael.h.kelley
In-Reply-To: <039e126f-f00d-d7e1-aa92-c049c9e3333b@gmail.com>
Tianyu Lan <ltykernel@gmail.com> writes:
>>> @@ -398,7 +399,8 @@ static u64 notrace read_hv_clock_msr_cs(struct clocksource *arg)
>>>
>>> static u64 read_hv_sched_clock_msr(void)
>>> {
>>> - return read_hv_clock_msr() - hv_sched_clock_offset;
>>> + return (read_hv_clock_msr() - hv_sched_clock_offset)
>>> + * (NSEC_PER_SEC / HV_CLOCK_HZ);
>>> }
>>
>> kvmclock seems to have the same (pre-patch) code ...
>
>
> kvm sched clock gets time from pvclock_clocksource_read() and
> the time unit is nanosecond. So there is such issue in KVM code.
>
Ah, true, kvmclock is always 1Ghz so it's reading 'naturally' converts
to ns.
--
Vitaly
^ permalink raw reply
* RE: [PATCH V3 1/6] x86/Hyper-V: Unload vmbus channel in hv panic callback
From: Michael Kelley @ 2020-03-25 18:28 UTC (permalink / raw)
To: ltykernel@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, hpa@zytor.com, x86@kernel.org
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <20200324075720.9462-2-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
>
> When kdump is not configured, a Hyper-V VM might still respond to
> network traffic after a kernel panic when kernel parameter panic=0.
> The panic CPU goes into an infinite loop with interrupts enabled,
> and the VMbus driver interrupt handler still works because the
> VMbus connection is unloaded only in the kdump path. The network
> responses make the other end of the connection think the VM is
> still functional even though it has panic'ed, which could affect any
> failover actions that should be taken.
>
> Fix this by unloading the VMbus connection during the panic process.
> vmbus_initiate_unload() could then be called twice (e.g., by
> hyperv_panic_event() and hv_crash_handler(), so reset the connection
> state in vmbus_initiate_unload() to ensure the unload is done only
> once.
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v1:
> - Update change log
> - Use xchg() to change vmbus connection status
> Change since v2:
> - Update comment of registering panic callback.
> ---
> drivers/hv/channel_mgmt.c | 3 +++
> drivers/hv/vmbus_drv.c | 21 +++++++++++++--------
> 2 files changed, 16 insertions(+), 8 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH V3 2/6] x86/Hyper-V: Free hv_panic_page when fail to register kmsg dump
From: Michael Kelley @ 2020-03-25 18:30 UTC (permalink / raw)
To: ltykernel@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, hpa@zytor.com, x86@kernel.org
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <20200324075720.9462-3-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
>
> If kmsg_dump_register() fails, hv_panic_page will not be used
> anywhere. So free and reset it.
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v1:
> - Update commit log
> - Remove hv_free_hyperv_page() in the error path
> ---
> drivers/hv/vmbus_drv.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH V3 3/6] x86/Hyper-V: Trigger crash enlightenment only once during system crash.
From: Michael Kelley @ 2020-03-25 18:31 UTC (permalink / raw)
To: ltykernel@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, hpa@zytor.com, x86@kernel.org
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <20200324075720.9462-4-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
>
> When a guest VM panics, Hyper-V should be notified only once via the
> crash synthetic MSRs. Current Linux code might write these crash MSRs
> twice during a system panic:
> 1) hyperv_panic/die_event() calling hyperv_report_panic()
> 2) hv_kmsg_dump() calling hyperv_report_panic_msg()
>
> Fix this by not calling hyperv_report_panic() if a kmsg dump has been
> successfully registered. The notification will happen later via
> hyperv_report_panic_msg().
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v1:
> - Update commit log
>
> Change since v2:
> - Update comment
> ---
> drivers/hv/vmbus_drv.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH V3 4/6] x86/Hyper-V: Report crash register data or ksmg before running crash kernel
From: Michael Kelley @ 2020-03-25 18:33 UTC (permalink / raw)
To: ltykernel@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, hpa@zytor.com, x86@kernel.org
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <20200324075720.9462-5-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
>
> We want to notify Hyper-V when a Linux guest VM crash occurs, so
> there is a record of the crash even when kdump is enabled. But
> crash_kexec_post_notifiers defaults to "false", so the kdump kernel
> runs before the notifiers and Hyper-V never gets notified. Fix this by
> always setting crash_kexec_post_notifiers to be true for Hyper-V VMs.
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v1:
> Update commit log
> ---
> arch/x86/kernel/cpu/mshyperv.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH V3 5/6] x86/Hyper-V: Report crash register data when sysctl_record_panic_msg is not set
From: Michael Kelley @ 2020-03-25 18:34 UTC (permalink / raw)
To: ltykernel@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <20200324075720.9462-6-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
>
> When sysctl_record_panic_msg is not set, the panic will
> not be reported to Hyper-V via hyperv_report_panic_msg().
> So the crash should be reported via hyperv_report_panic().
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> drivers/hv/vmbus_drv.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH V3 6/6] x86/Hyper-V: Report crash data in die() when panic_on_oops is set
From: Michael Kelley @ 2020-03-25 18:36 UTC (permalink / raw)
To: ltykernel@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <20200324075720.9462-7-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
>
> When oops happens with panic_on_oops unset, the oops
> thread is killed by die() and system continues to run.
> In such case, guest should not report crash register
> data to host since system still runs. Fix it.
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> drivers/hv/vmbus_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH] x86/Hyper-V: Fix hv sched clock function return wrong time unit
From: Dexuan Cui @ 2020-03-25 18:46 UTC (permalink / raw)
To: Yubo Xie, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, daniel.lezcano@linaro.org, tglx@linutronix.de,
Michael Kelley
Cc: Yubo Xie, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets, stable@vger.kernel.org,
Tianyu Lan
In-Reply-To: <20200324151935.15814-1-yuboxie@microsoft.com>
> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Yubo Xie
> Sent: Tuesday, March 24, 2020 8:20 AM
> ...
> sched clock callback should return time with nano second as unit
> but current hv callback returns time with 100ns. Fix it.
Hi Yubo,
I'm curious how you found the bug? :-) Did you notice some kind of symtom?
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [PATCH] x86/Hyper-V: Fix hv sched clock function return wrong time unit
From: Yubo Xie @ 2020-03-25 18:57 UTC (permalink / raw)
To: Dexuan Cui, Yubo Xie, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, daniel.lezcano@linaro.org,
tglx@linutronix.de, Michael Kelley
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
vkuznets, stable@vger.kernel.org, Tianyu Lan
In-Reply-To: <HK0P153MB0273AEB3D64694E8DF31C0B0BFCE0@HK0P153MB0273.APCP153.PROD.OUTLOOK.COM>
I use "time" to run some performance tests in our project under WSL2, but the result looks weird: the value of "real" is far from the sum of "user" + "sys".
-----Original Message-----
From: Dexuan Cui <decui@microsoft.com>
Sent: Wednesday, March 25, 2020 11:46 AM
To: Yubo Xie <ltykernel@gmail.com>; KY Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>; Wei Liu <liuwe@microsoft.com>; daniel.lezcano@linaro.org; tglx@linutronix.de; Michael Kelley <mikelley@microsoft.com>
Cc: Yubo Xie <yuboxie@microsoft.com>; linux-hyperv@vger.kernel.org; linux-kernel@vger.kernel.org; vkuznets <vkuznets@redhat.com>; stable@vger.kernel.org; Tianyu Lan <Tianyu.Lan@microsoft.com>
Subject: RE: [PATCH] x86/Hyper-V: Fix hv sched clock function return wrong time unit
> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Yubo Xie
> Sent: Tuesday, March 24, 2020 8:20 AM
> ...
> sched clock callback should return time with nano second as unit but
> current hv callback returns time with 100ns. Fix it.
Hi Yubo,
I'm curious how you found the bug? :-) Did you notice some kind of symtom?
Thanks,
-- Dexuan
^ permalink raw reply
* [RFC PATCH 00/11] VMBus channel interrupt reassignment
From: Andrea Parri (Microsoft) @ 2020-03-25 22:54 UTC (permalink / raw)
To: linux-kernel
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
linux-hyperv, Michael Kelley, Dexuan Cui, Boqun Feng,
Vitaly Kuznetsov, Andrea Parri (Microsoft), David S. Miller,
Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas,
James E.J. Bottomley, Martin K. Petersen
Hi all,
This series introduces changes in the VMBus drivers to reassign (at
runtime) the CPU that a VMbus channel will interrupt. This feature
can be used for load balancing or other purposes, e.g. to offline a
given vCPU (vCPUs with no interrupts assigned can be taken offline).
Thanks,
Andrea
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Andrew Murray <amurray@thegoodpenguin.co.uk>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Andrea Parri (Microsoft) (11):
Drivers: hv: vmbus: Always handle the VMBus messages on CPU0
Drivers: hv: vmbus: Don't bind the offer&rescind works to a specific
CPU
Drivers: hv: vmbus: Replace the per-CPU channel lists with a global
array of channels
hv_netvsc: Disable NAPI before closing the VMBus channel
hv_utils: Always execute the fcopy and vss callbacks in a tasklet
Drivers: hv: vmbus: Use a spin lock for synchronizing channel
scheduling vs. channel removal
PCI: hv: Prepare hv_compose_msi_msg() for the
VMBus-channel-interrupt-to-vCPU reassignment functionality
Drivers: hv: vmbus: Remove the unused HV_LOCALIZED channel affinity
logic
Drivers: hv: vmbus: Synchronize init_vp_index() vs. CPU hotplug
Drivers: hv: vmbus: Introduce the CHANNELMSG_MODIFYCHANNEL message
type
scsi: storvsc: Re-init stor_chns when a channel interrupt is
re-assigned
drivers/hv/channel.c | 58 +++--
drivers/hv/channel_mgmt.c | 326 ++++++++++++++--------------
drivers/hv/connection.c | 58 +----
drivers/hv/hv.c | 16 +-
drivers/hv/hv_fcopy.c | 2 +-
drivers/hv/hv_snapshot.c | 2 +-
drivers/hv/hv_trace.h | 19 ++
drivers/hv/hyperv_vmbus.h | 32 ++-
drivers/hv/vmbus_drv.c | 241 ++++++++++++++++----
drivers/net/hyperv/netvsc.c | 7 +-
drivers/pci/controller/pci-hyperv.c | 44 ++--
drivers/scsi/storvsc_drv.c | 95 +++++++-
include/linux/hyperv.h | 51 ++---
13 files changed, 599 insertions(+), 352 deletions(-)
--
2.24.0
^ permalink raw reply
* [RFC PATCH 01/11] Drivers: hv: vmbus: Always handle the VMBus messages on CPU0
From: Andrea Parri (Microsoft) @ 2020-03-25 22:54 UTC (permalink / raw)
To: linux-kernel
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
linux-hyperv, Michael Kelley, Dexuan Cui, Boqun Feng,
Vitaly Kuznetsov, Andrea Parri (Microsoft)
In-Reply-To: <20200325225505.23998-1-parri.andrea@gmail.com>
A Linux guest have to pick a "connect CPU" to communicate with the
Hyper-V host. This CPU can not be taken offline because Hyper-V does
not provide a way to change that CPU assignment.
Current code sets the connect CPU to whatever CPU ends up running the
function vmbus_negotiate_version(), and this will generate problems if
that CPU is taken offine.
Establish CPU0 as the connect CPU, and add logics to prevents the
connect CPU from being taken offline. We could pick some other CPU,
and we could pick that "other CPU" dynamically if there was a reason to
do so at some point in the future. But for now, #defining the connect
CPU to 0 is the most straightforward and least complex solution.
While on this, add inline comments explaining "why" offer and rescind
messages should not be handled by a same serialized work queue.
Suggested-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
drivers/hv/connection.c | 20 +-------------------
drivers/hv/hv.c | 7 +++++++
drivers/hv/hyperv_vmbus.h | 11 ++++++-----
drivers/hv/vmbus_drv.c | 20 +++++++++++++++++---
4 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 74e77de89b4f3..f4bd306d2cef9 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -69,7 +69,6 @@ MODULE_PARM_DESC(max_version,
int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
{
int ret = 0;
- unsigned int cur_cpu;
struct vmbus_channel_initiate_contact *msg;
unsigned long flags;
@@ -102,24 +101,7 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
msg->monitor_page1 = virt_to_phys(vmbus_connection.monitor_pages[0]);
msg->monitor_page2 = virt_to_phys(vmbus_connection.monitor_pages[1]);
- /*
- * We want all channel messages to be delivered on CPU 0.
- * This has been the behavior pre-win8. This is not
- * perf issue and having all channel messages delivered on CPU 0
- * would be ok.
- * For post win8 hosts, we support receiving channel messagges on
- * all the CPUs. This is needed for kexec to work correctly where
- * the CPU attempting to connect may not be CPU 0.
- */
- if (version >= VERSION_WIN8_1) {
- cur_cpu = get_cpu();
- msg->target_vcpu = hv_cpu_number_to_vp_number(cur_cpu);
- vmbus_connection.connect_cpu = cur_cpu;
- put_cpu();
- } else {
- msg->target_vcpu = 0;
- vmbus_connection.connect_cpu = 0;
- }
+ msg->target_vcpu = hv_cpu_number_to_vp_number(VMBUS_CONNECT_CPU);
/*
* Add to list before we send the request since we may
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 6098e0cbdb4b0..e2b3310454640 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -249,6 +249,13 @@ int hv_synic_cleanup(unsigned int cpu)
bool channel_found = false;
unsigned long flags;
+ /*
+ * Hyper-V does not provide a way to change the connect CPU once
+ * it is set; we must prevent the connect CPU from going offline.
+ */
+ if (cpu == VMBUS_CONNECT_CPU)
+ return -EBUSY;
+
/*
* Search for channels which are bound to the CPU we're about to
* cleanup. In case we find one and vmbus is still connected we need to
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 70b30e223a578..67fb1edcbf527 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -212,12 +212,13 @@ enum vmbus_connect_state {
#define MAX_SIZE_CHANNEL_MESSAGE HV_MESSAGE_PAYLOAD_BYTE_COUNT
-struct vmbus_connection {
- /*
- * CPU on which the initial host contact was made.
- */
- int connect_cpu;
+/*
+ * The CPU that Hyper-V will interrupt for VMBUS messages, such as
+ * CHANNELMSG_OFFERCHANNEL and CHANNELMSG_RESCIND_CHANNELOFFER.
+ */
+#define VMBUS_CONNECT_CPU 0
+struct vmbus_connection {
u32 msg_conn_id;
atomic_t offer_in_progress;
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 029378c27421d..7600615e13754 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1056,14 +1056,28 @@ void vmbus_on_msg_dpc(unsigned long data)
/*
* If we are handling the rescind message;
* schedule the work on the global work queue.
+ *
+ * The OFFER message and the RESCIND message should
+ * not be handled by the same serialized work queue,
+ * because the OFFER handler may call vmbus_open(),
+ * which tries to open the channel by sending an
+ * OPEN_CHANNEL message to the host and waits for
+ * the host's response; however, if the host has
+ * rescinded the channel before it receives the
+ * OPEN_CHANNEL message, the host just silently
+ * ignores the OPEN_CHANNEL message; as a result,
+ * the guest's OFFER handler hangs for ever, if we
+ * handle the RESCIND message in the same serialized
+ * work queue: the RESCIND handler can not start to
+ * run before the OFFER handler finishes.
*/
- schedule_work_on(vmbus_connection.connect_cpu,
+ schedule_work_on(VMBUS_CONNECT_CPU,
&ctx->work);
break;
case CHANNELMSG_OFFERCHANNEL:
atomic_inc(&vmbus_connection.offer_in_progress);
- queue_work_on(vmbus_connection.connect_cpu,
+ queue_work_on(VMBUS_CONNECT_CPU,
vmbus_connection.work_queue,
&ctx->work);
break;
@@ -1110,7 +1124,7 @@ static void vmbus_force_channel_rescinded(struct vmbus_channel *channel)
INIT_WORK(&ctx->work, vmbus_onmessage_work);
- queue_work_on(vmbus_connection.connect_cpu,
+ queue_work_on(VMBUS_CONNECT_CPU,
vmbus_connection.work_queue,
&ctx->work);
}
--
2.24.0
^ permalink raw reply related
* [RFC PATCH 02/11] Drivers: hv: vmbus: Don't bind the offer&rescind works to a specific CPU
From: Andrea Parri (Microsoft) @ 2020-03-25 22:54 UTC (permalink / raw)
To: linux-kernel
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
linux-hyperv, Michael Kelley, Dexuan Cui, Boqun Feng,
Vitaly Kuznetsov, Andrea Parri (Microsoft)
In-Reply-To: <20200325225505.23998-1-parri.andrea@gmail.com>
The offer and rescind works are currently scheduled on the so called
"connect CPU". However, this is not really needed: we can synchronize
the works by relying on the usage of the offer_in_progress counter and
of the channel_mutex mutex. This synchronization is already in place.
So, remove this unnecessary "bind to the connect CPU" constraint and
update the inline comments accordingly.
Suggested-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
drivers/hv/channel_mgmt.c | 21 ++++++++++++++++-----
drivers/hv/vmbus_drv.c | 39 ++++++++++++++++++++++++++++-----------
2 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 0370364169c4e..1191f3d76d111 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1025,11 +1025,22 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
* offer comes in first and then the rescind.
* Since we process these events in work elements,
* and with preemption, we may end up processing
- * the events out of order. Given that we handle these
- * work elements on the same CPU, this is possible only
- * in the case of preemption. In any case wait here
- * until the offer processing has moved beyond the
- * point where the channel is discoverable.
+ * the events out of order. We rely on the synchronization
+ * provided by offer_in_progress and by channel_mutex for
+ * ordering these events:
+ *
+ * { Initially: offer_in_progress = 1 }
+ *
+ * CPU1 CPU2
+ *
+ * [vmbus_process_offer()] [vmbus_onoffer_rescind()]
+ *
+ * LOCK channel_mutex WAIT_ON offer_in_progress == 0
+ * DECREMENT offer_in_progress LOCK channel_mutex
+ * INSERT chn_list SEARCH chn_list
+ * UNLOCK channel_mutex UNLOCK channel_mutex
+ *
+ * Forbids: CPU2's SEARCH from *not* seeing CPU1's INSERT
*/
while (atomic_read(&vmbus_connection.offer_in_progress) != 0) {
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 7600615e13754..903b1ec6a259e 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1048,8 +1048,9 @@ void vmbus_on_msg_dpc(unsigned long data)
/*
* The host can generate a rescind message while we
* may still be handling the original offer. We deal with
- * this condition by ensuring the processing is done on the
- * same CPU.
+ * this condition by relying on the synchronization provided
+ * by offer_in_progress and by channel_mutex. See also the
+ * inline comments in vmbus_onoffer_rescind().
*/
switch (hdr->msgtype) {
case CHANNELMSG_RESCIND_CHANNELOFFER:
@@ -1071,16 +1072,34 @@ void vmbus_on_msg_dpc(unsigned long data)
* work queue: the RESCIND handler can not start to
* run before the OFFER handler finishes.
*/
- schedule_work_on(VMBUS_CONNECT_CPU,
- &ctx->work);
+ schedule_work(&ctx->work);
break;
case CHANNELMSG_OFFERCHANNEL:
+ /*
+ * The host sends the offer message of a given channel
+ * before sending the rescind message of the same
+ * channel. These messages are sent to the guest's
+ * connect CPU; the guest then starts processing them
+ * in the tasklet handler on this CPU:
+ *
+ * VMBUS_CONNECT_CPU
+ *
+ * [vmbus_on_msg_dpc()]
+ * atomic_inc() // CHANNELMSG_OFFERCHANNEL
+ * queue_work()
+ * ...
+ * [vmbus_on_msg_dpc()]
+ * schedule_work() // CHANNELMSG_RESCIND_CHANNELOFFER
+ *
+ * We rely on the memory-ordering properties of the
+ * queue_work() and schedule_work() primitives, which
+ * guarantee that the atomic increment will be visible
+ * to the CPUs which will execute the offer & rescind
+ * works by the time these works will start execution.
+ */
atomic_inc(&vmbus_connection.offer_in_progress);
- queue_work_on(VMBUS_CONNECT_CPU,
- vmbus_connection.work_queue,
- &ctx->work);
- break;
+ fallthrough;
default:
queue_work(vmbus_connection.work_queue, &ctx->work);
@@ -1124,9 +1143,7 @@ static void vmbus_force_channel_rescinded(struct vmbus_channel *channel)
INIT_WORK(&ctx->work, vmbus_onmessage_work);
- queue_work_on(VMBUS_CONNECT_CPU,
- vmbus_connection.work_queue,
- &ctx->work);
+ queue_work(vmbus_connection.work_queue, &ctx->work);
}
#endif /* CONFIG_PM_SLEEP */
--
2.24.0
^ permalink raw reply related
* [RFC PATCH 03/11] Drivers: hv: vmbus: Replace the per-CPU channel lists with a global array of channels
From: Andrea Parri (Microsoft) @ 2020-03-25 22:54 UTC (permalink / raw)
To: linux-kernel
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
linux-hyperv, Michael Kelley, Dexuan Cui, Boqun Feng,
Vitaly Kuznetsov, Andrea Parri (Microsoft)
In-Reply-To: <20200325225505.23998-1-parri.andrea@gmail.com>
When Hyper-V sends an interrupt to the guest, the guest has to figure
out which channel the interrupt is associated with. Hyper-V sets a bit
in a memory page that is shared with the guest, indicating a particular
"relid" that the interrupt is associated with. The current Linux code
then uses a set of per-CPU linked lists to map a given "relid" to a
pointer to a channel structure.
This design introduces a synchronization problem if the CPU that Hyper-V
will interrupt for a certain channel is changed. If the interrupt comes
on the "old CPU" and the channel was already moved to the per-CPU list
of the "new CPU", then the relid -> channel mapping will fail and the
interrupt is dropped. Similarly, if the interrupt comes on the new CPU
but the channel was not moved to the per-CPU list of the new CPU, then
the mapping will fail and the interrupt is dropped.
Relids are integers ranging from 0 to 2047. The mapping from relids to
channel structures can be done by setting up an array with 2048 entries,
each entry being a pointer to a channel structure (hence total size ~16K
bytes, which is not a problem). The array is global, so there are no
per-CPU linked lists to update. The array can be searched and updated
by simply loading and storing the array at the specified index. With no
per-CPU data structures, the above mentioned synchronization problem is
avoided and the relid2channel() function gets simpler.
Suggested-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
drivers/hv/channel_mgmt.c | 158 ++++++++++++++++++++++----------------
drivers/hv/connection.c | 38 +++------
drivers/hv/hv.c | 2 -
drivers/hv/hyperv_vmbus.h | 14 ++--
drivers/hv/vmbus_drv.c | 48 +++++++-----
include/linux/hyperv.h | 5 --
6 files changed, 139 insertions(+), 126 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 1191f3d76d111..9b1449c839575 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -319,7 +319,6 @@ static struct vmbus_channel *alloc_channel(void)
init_completion(&channel->rescind_event);
INIT_LIST_HEAD(&channel->sc_list);
- INIT_LIST_HEAD(&channel->percpu_list);
tasklet_init(&channel->callback_event,
vmbus_on_event, (unsigned long)channel);
@@ -340,23 +339,28 @@ static void free_channel(struct vmbus_channel *channel)
kobject_put(&channel->kobj);
}
-static void percpu_channel_enq(void *arg)
+void vmbus_channel_map_relid(struct vmbus_channel *channel)
{
- struct vmbus_channel *channel = arg;
- struct hv_per_cpu_context *hv_cpu
- = this_cpu_ptr(hv_context.cpu_context);
-
- list_add_tail_rcu(&channel->percpu_list, &hv_cpu->chan_list);
+ if (WARN_ON(channel->offermsg.child_relid >= MAX_CHANNEL_RELIDS))
+ return;
+ /*
+ * Pairs with the READ_ONCE() in vmbus_chan_sched(). Guarantees
+ * that vmbus_chan_sched() will find up-to-date data.
+ */
+ smp_store_release(
+ &vmbus_connection.channels[channel->offermsg.child_relid],
+ channel);
}
-static void percpu_channel_deq(void *arg)
+void vmbus_channel_unmap_relid(struct vmbus_channel *channel)
{
- struct vmbus_channel *channel = arg;
-
- list_del_rcu(&channel->percpu_list);
+ if (WARN_ON(channel->offermsg.child_relid >= MAX_CHANNEL_RELIDS))
+ return;
+ WRITE_ONCE(
+ vmbus_connection.channels[channel->offermsg.child_relid],
+ NULL);
}
-
static void vmbus_release_relid(u32 relid)
{
struct vmbus_channel_relid_released msg;
@@ -376,17 +380,25 @@ void hv_process_channel_removal(struct vmbus_channel *channel)
struct vmbus_channel *primary_channel;
unsigned long flags;
- BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
+ lockdep_assert_held(&vmbus_connection.channel_mutex);
BUG_ON(!channel->rescind);
- if (channel->target_cpu != get_cpu()) {
- put_cpu();
- smp_call_function_single(channel->target_cpu,
- percpu_channel_deq, channel, true);
- } else {
- percpu_channel_deq(channel);
- put_cpu();
- }
+ /*
+ * hv_process_channel_removal() could find INVALID_RELID only for
+ * hv_sock channels. See the inline comments in vmbus_onoffer().
+ */
+ WARN_ON(channel->offermsg.child_relid == INVALID_RELID &&
+ !is_hvsock_channel(channel));
+
+ /*
+ * Upon suspend, an in-use hv_sock channel is removed from the array of
+ * channels and the relid is invalidated. After hibernation, when the
+ * user-space appplication destroys the channel, it's unnecessary and
+ * unsafe to remove the channel from the array of channels. See also
+ * the inline comments before the call of vmbus_release_relid() below.
+ */
+ if (channel->offermsg.child_relid != INVALID_RELID)
+ vmbus_channel_unmap_relid(channel);
if (channel->primary_channel == NULL) {
list_del(&channel->listentry);
@@ -447,16 +459,6 @@ static void vmbus_add_channel_work(struct work_struct *work)
init_vp_index(newchannel, dev_type);
- if (newchannel->target_cpu != get_cpu()) {
- put_cpu();
- smp_call_function_single(newchannel->target_cpu,
- percpu_channel_enq,
- newchannel, true);
- } else {
- percpu_channel_enq(newchannel);
- put_cpu();
- }
-
/*
* This state is used to indicate a successful open
* so that when we do close the channel normally, we
@@ -523,17 +525,10 @@ static void vmbus_add_channel_work(struct work_struct *work)
spin_unlock_irqrestore(&primary_channel->lock, flags);
}
- mutex_unlock(&vmbus_connection.channel_mutex);
+ /* vmbus_process_offer() has mapped the channel. */
+ vmbus_channel_unmap_relid(newchannel);
- if (newchannel->target_cpu != get_cpu()) {
- put_cpu();
- smp_call_function_single(newchannel->target_cpu,
- percpu_channel_deq,
- newchannel, true);
- } else {
- percpu_channel_deq(newchannel);
- put_cpu();
- }
+ mutex_unlock(&vmbus_connection.channel_mutex);
vmbus_release_relid(newchannel->offermsg.child_relid);
@@ -599,6 +594,8 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
spin_unlock_irqrestore(&channel->lock, flags);
}
+ vmbus_channel_map_relid(newchannel);
+
mutex_unlock(&vmbus_connection.channel_mutex);
/*
@@ -937,8 +934,6 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
oldchannel = find_primary_channel_by_offer(offer);
if (oldchannel != NULL) {
- atomic_dec(&vmbus_connection.offer_in_progress);
-
/*
* We're resuming from hibernation: all the sub-channel and
* hv_sock channels we had before the hibernation should have
@@ -946,36 +941,65 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
* primary channel that we had before the hibernation.
*/
+ /*
+ * { Initially: channel relid = INVALID_RELID,
+ * channels[valid_relid] = NULL }
+ *
+ * CPU1 CPU2
+ *
+ * [vmbus_onoffer()] [vmbus_device_release()]
+ *
+ * LOCK channel_mutex LOCK channel_mutex
+ * STORE channel relid = valid_relid LOAD r1 = channel relid
+ * MAP_RELID channel if (r1 != INVALID_RELID)
+ * UNLOCK channel_mutex UNMAP_RELID channel
+ * UNLOCK channel_mutex
+ *
+ * Forbids: r1 == valid_relid &&
+ * channels[valid_relid] == channel
+ *
+ * Note. r1 can be INVALID_RELID only for an hv_sock channel.
+ * None of the hv_sock channels which were present before the
+ * suspend are re-offered upon the resume. See the WARN_ON()
+ * in hv_process_channel_removal().
+ */
+ mutex_lock(&vmbus_connection.channel_mutex);
+
+ atomic_dec(&vmbus_connection.offer_in_progress);
+
WARN_ON(oldchannel->offermsg.child_relid != INVALID_RELID);
/* Fix up the relid. */
oldchannel->offermsg.child_relid = offer->child_relid;
offer_sz = sizeof(*offer);
- if (memcmp(offer, &oldchannel->offermsg, offer_sz) == 0) {
- check_ready_for_resume_event();
- return;
+ if (memcmp(offer, &oldchannel->offermsg, offer_sz) != 0) {
+ /*
+ * This is not an error, since the host can also change
+ * the other field(s) of the offer, e.g. on WS RS5
+ * (Build 17763), the offer->connection_id of the
+ * Mellanox VF vmbus device can change when the host
+ * reoffers the device upon resume.
+ */
+ pr_debug("vmbus offer changed: relid=%d\n",
+ offer->child_relid);
+
+ print_hex_dump_debug("Old vmbus offer: ",
+ DUMP_PREFIX_OFFSET, 16, 4,
+ &oldchannel->offermsg, offer_sz,
+ false);
+ print_hex_dump_debug("New vmbus offer: ",
+ DUMP_PREFIX_OFFSET, 16, 4,
+ offer, offer_sz, false);
+
+ /* Fix up the old channel. */
+ vmbus_setup_channel_state(oldchannel, offer);
}
- /*
- * This is not an error, since the host can also change the
- * other field(s) of the offer, e.g. on WS RS5 (Build 17763),
- * the offer->connection_id of the Mellanox VF vmbus device
- * can change when the host reoffers the device upon resume.
- */
- pr_debug("vmbus offer changed: relid=%d\n",
- offer->child_relid);
-
- print_hex_dump_debug("Old vmbus offer: ", DUMP_PREFIX_OFFSET,
- 16, 4, &oldchannel->offermsg, offer_sz,
- false);
- print_hex_dump_debug("New vmbus offer: ", DUMP_PREFIX_OFFSET,
- 16, 4, offer, offer_sz, false);
-
- /* Fix up the old channel. */
- vmbus_setup_channel_state(oldchannel, offer);
-
+ /* Add the channel back to the array of channels. */
+ vmbus_channel_map_relid(oldchannel);
check_ready_for_resume_event();
+ mutex_unlock(&vmbus_connection.channel_mutex);
return;
}
@@ -1033,14 +1057,14 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
*
* CPU1 CPU2
*
- * [vmbus_process_offer()] [vmbus_onoffer_rescind()]
+ * [vmbus_onoffer()] [vmbus_onoffer_rescind()]
*
* LOCK channel_mutex WAIT_ON offer_in_progress == 0
* DECREMENT offer_in_progress LOCK channel_mutex
- * INSERT chn_list SEARCH chn_list
+ * STORE channels[] LOAD channels[]
* UNLOCK channel_mutex UNLOCK channel_mutex
*
- * Forbids: CPU2's SEARCH from *not* seeing CPU1's INSERT
+ * Forbids: CPU2's LOAD from *not* seeing CPU1's STORE
*/
while (atomic_read(&vmbus_connection.offer_in_progress) != 0) {
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index f4bd306d2cef9..11170d9a2e1a5 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -248,6 +248,14 @@ int vmbus_connect(void)
pr_info("Vmbus version:%d.%d\n",
version >> 16, version & 0xFFFF);
+ vmbus_connection.channels = kcalloc(MAX_CHANNEL_RELIDS,
+ sizeof(struct vmbus_channel *),
+ GFP_KERNEL);
+ if (vmbus_connection.channels == NULL) {
+ ret = -ENOMEM;
+ goto cleanup;
+ }
+
kfree(msginfo);
return 0;
@@ -295,33 +303,9 @@ void vmbus_disconnect(void)
*/
struct vmbus_channel *relid2channel(u32 relid)
{
- struct vmbus_channel *channel;
- struct vmbus_channel *found_channel = NULL;
- struct list_head *cur, *tmp;
- struct vmbus_channel *cur_sc;
-
- BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
-
- list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
- if (channel->offermsg.child_relid == relid) {
- found_channel = channel;
- break;
- } else if (!list_empty(&channel->sc_list)) {
- /*
- * Deal with sub-channels.
- */
- list_for_each_safe(cur, tmp, &channel->sc_list) {
- cur_sc = list_entry(cur, struct vmbus_channel,
- sc_list);
- if (cur_sc->offermsg.child_relid == relid) {
- found_channel = cur_sc;
- break;
- }
- }
- }
- }
-
- return found_channel;
+ if (WARN_ON(relid >= MAX_CHANNEL_RELIDS))
+ return NULL;
+ return READ_ONCE(vmbus_connection.channels[relid]);
}
/*
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index e2b3310454640..17bf1f229152b 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -117,8 +117,6 @@ int hv_synic_alloc(void)
pr_err("Unable to allocate post msg page\n");
goto err;
}
-
- INIT_LIST_HEAD(&hv_cpu->chan_list);
}
return 0;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 67fb1edcbf527..2216cd5e8e8bf 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -132,12 +132,6 @@ struct hv_per_cpu_context {
* basis.
*/
struct tasklet_struct msg_dpc;
-
- /*
- * To optimize the mapping of relid to channel, maintain
- * per-cpu list of the channels based on their CPU affinity.
- */
- struct list_head chan_list;
};
struct hv_context {
@@ -202,6 +196,8 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
/* TODO: Need to make this configurable */
#define MAX_NUM_CHANNELS_SUPPORTED 256
+#define MAX_CHANNEL_RELIDS \
+ max(MAX_NUM_CHANNELS_SUPPORTED, HV_EVENT_FLAGS_COUNT)
enum vmbus_connect_state {
DISCONNECTED,
@@ -251,6 +247,9 @@ struct vmbus_connection {
struct list_head chn_list;
struct mutex channel_mutex;
+ /* Array of channels */
+ struct vmbus_channel **channels;
+
/*
* An offer message is handled first on the work_queue, and then
* is further handled on handle_primary_chan_wq or
@@ -337,6 +336,9 @@ int vmbus_add_channel_kobj(struct hv_device *device_obj,
void vmbus_remove_channel_attr_group(struct vmbus_channel *channel);
+void vmbus_channel_map_relid(struct vmbus_channel *channel);
+void vmbus_channel_unmap_relid(struct vmbus_channel *channel);
+
struct vmbus_channel *relid2channel(u32 relid);
void vmbus_free_channels(void);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 903b1ec6a259e..301e3f484bb1a 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1198,33 +1198,39 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
if (relid == 0)
continue;
+ /*
+ * Pairs with the kfree_rcu() in vmbus_chan_release().
+ * Guarantees that the channel data structure doesn't
+ * get freed while the channel pointer below is being
+ * dereferenced.
+ */
rcu_read_lock();
/* Find channel based on relid */
- list_for_each_entry_rcu(channel, &hv_cpu->chan_list, percpu_list) {
- if (channel->offermsg.child_relid != relid)
- continue;
+ channel = relid2channel(relid);
+ if (channel == NULL)
+ goto sched_unlock_rcu;
- if (channel->rescind)
- continue;
+ if (channel->rescind)
+ goto sched_unlock_rcu;
- trace_vmbus_chan_sched(channel);
+ trace_vmbus_chan_sched(channel);
- ++channel->interrupts;
+ ++channel->interrupts;
- switch (channel->callback_mode) {
- case HV_CALL_ISR:
- vmbus_channel_isr(channel);
- break;
+ switch (channel->callback_mode) {
+ case HV_CALL_ISR:
+ vmbus_channel_isr(channel);
+ break;
- case HV_CALL_BATCHED:
- hv_begin_read(&channel->inbound);
- /* fallthrough */
- case HV_CALL_DIRECT:
- tasklet_schedule(&channel->callback_event);
- }
+ case HV_CALL_BATCHED:
+ hv_begin_read(&channel->inbound);
+ fallthrough;
+ case HV_CALL_DIRECT:
+ tasklet_schedule(&channel->callback_event);
}
+sched_unlock_rcu:
rcu_read_unlock();
}
}
@@ -2208,9 +2214,12 @@ static int vmbus_bus_suspend(struct device *dev)
list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
/*
- * Invalidate the field. Upon resume, vmbus_onoffer() will fix
- * up the field, and the other fields (if necessary).
+ * Remove the channel from the array of channels and invalidate
+ * the channel's relid. Upon resume, vmbus_onoffer() will fix
+ * up the relid (and other fields, if necessary) and add the
+ * channel back to the array.
*/
+ vmbus_channel_unmap_relid(channel);
channel->offermsg.child_relid = INVALID_RELID;
if (is_hvsock_channel(channel)) {
@@ -2450,6 +2459,7 @@ static void __exit vmbus_exit(void)
hv_debug_rm_all_dir();
vmbus_free_channels();
+ kfree(vmbus_connection.channels);
if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
kmsg_dump_unregister(&hv_kmsg_dumper);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 692c89ccf5dfd..6c794fd5c903e 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -854,11 +854,6 @@ struct vmbus_channel {
* Support per-channel state for use by vmbus drivers.
*/
void *per_channel_state;
- /*
- * To support per-cpu lookup mapping of relid to channel,
- * link up channels based on their CPU affinity.
- */
- struct list_head percpu_list;
/*
* Defer freeing channel until after all cpu's have
--
2.24.0
^ permalink raw reply related
* [RFC PATCH 05/11] hv_utils: Always execute the fcopy and vss callbacks in a tasklet
From: Andrea Parri (Microsoft) @ 2020-03-25 22:54 UTC (permalink / raw)
To: linux-kernel
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
linux-hyperv, Michael Kelley, Dexuan Cui, Boqun Feng,
Vitaly Kuznetsov, Andrea Parri (Microsoft)
In-Reply-To: <20200325225505.23998-1-parri.andrea@gmail.com>
The fcopy and vss callback functions could be running in a tasklet
at the same time they are called in hv_poll_channel(). Current code
serializes the invocations of these functions, and their accesses to
the channel ring buffer, by sending an IPI to the CPU that is allowed
to access the ring buffer, cf. hv_poll_channel(). This IPI mechanism
becomes infeasible if we allow changing the CPU that a channel will
interrupt. Instead modify the callback wrappers to always execute
the fcopy and vss callbacks in a tasklet, thus mirroring the solution
for the kvp callback functions adopted since commit a3ade8cc474d8
("HV: properly delay KVP packets when negotiation is in progress").
This will ensure that the callback function can't run on two CPUs at
the same time.
Suggested-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
drivers/hv/hv_fcopy.c | 2 +-
drivers/hv/hv_snapshot.c | 2 +-
drivers/hv/hyperv_vmbus.h | 7 +------
3 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index bb9ba3f7c7949..5040d7e0cd9e9 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -71,7 +71,7 @@ static void fcopy_poll_wrapper(void *channel)
{
/* Transaction is finished, reset the state here to avoid races. */
fcopy_transaction.state = HVUTIL_READY;
- hv_fcopy_onchannelcallback(channel);
+ tasklet_schedule(&((struct vmbus_channel *)channel)->callback_event);
}
static void fcopy_timeout_func(struct work_struct *dummy)
diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index 1c75b38f0d6da..783779e4cc1a5 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -80,7 +80,7 @@ static void vss_poll_wrapper(void *channel)
{
/* Transaction is finished, reset the state here to avoid races. */
vss_transaction.state = HVUTIL_READY;
- hv_vss_onchannelcallback(channel);
+ tasklet_schedule(&((struct vmbus_channel *)channel)->callback_event);
}
/*
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 2216cd5e8e8bf..ff187cf5896b9 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -377,12 +377,7 @@ static inline void hv_poll_channel(struct vmbus_channel *channel,
{
if (!channel)
return;
-
- if (in_interrupt() && (channel->target_cpu == smp_processor_id())) {
- cb(channel);
- return;
- }
- smp_call_function_single(channel->target_cpu, cb, channel, true);
+ cb(channel);
}
enum hvutil_device_state {
--
2.24.0
^ permalink raw reply related
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