* Re: [PATCH v2 2/4] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Jon Doron @ 2020-03-06 16:34 UTC (permalink / raw)
To: Vitaly Kuznetsov; +Cc: kvm, linux-hyperv
In-Reply-To: <874kv1ec7f.fsf@vitty.brq.redhat.com>
On 06/03/2020, Vitaly Kuznetsov wrote:
>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.
>>
>
>I would suggest you split TLFS changes into it's own patch so Hyper-V
>folks can ACK (or they can ack the whole patch with KVM changes of
>course :-)
>
Done.
>> Signed-off-by: Jon Doron <arilou@gmail.com>
>> ---
>> arch/x86/include/asm/hyperv-tlfs.h | 16 ++++
>> arch/x86/include/asm/kvm_host.h | 13 ++++
>> arch/x86/kvm/hyperv.c | 114 ++++++++++++++++++++++++++++-
>> arch/x86/kvm/hyperv.h | 5 ++
>> arch/x86/kvm/trace.h | 25 +++++++
>> arch/x86/kvm/x86.c | 9 +++
>> include/uapi/linux/kvm.h | 10 +++
>> 7 files changed, 191 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
>> index 92abc1e42bfc..8efdf974c23f 100644
>> --- a/arch/x86/include/asm/hyperv-tlfs.h
>> +++ b/arch/x86/include/asm/hyperv-tlfs.h
>> @@ -33,6 +33,9 @@
>> #define HYPERV_CPUID_ENLIGHTMENT_INFO 0x40000004
>> #define HYPERV_CPUID_IMPLEMENT_LIMITS 0x40000005
>> #define HYPERV_CPUID_NESTED_FEATURES 0x4000000A
>> +#define HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
>> +#define HYPERV_CPUID_SYNDBG_INTERFACE 0x40000081
>> +#define HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
>>
>> #define HYPERV_HYPERVISOR_PRESENT_BIT 0x80000000
>> #define HYPERV_CPUID_MIN 0x40000005
>> @@ -131,6 +134,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)
>>
>> @@ -194,6 +199,9 @@
>> #define HV_X64_NESTED_GUEST_MAPPING_FLUSH BIT(18)
>> #define HV_X64_NESTED_MSR_BITMAP BIT(19)
>>
>> +/* Hyper-V synthetic debugger platform capabilities */
>> +#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING BIT(1)
>> +
>
>hyperv-tlfs.h is not perfectly structured but still there is some
>structure there,
>e.g. HV_X64_NESTED_GUEST_MAPPING_FLUSH/HV_X64_NESTED_MSR_BITMAP/... are
>said to be HYPERV_CPUID_ENLIGHTMENT_INFO.EAX
>bits (see above HV_X64_AS_SWITCH_RECOMMENDED).
>
>To make it clear that HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING doesn't
>belong to these bits I'd suggest you add a comment like
>
>/*
> * Hyper-V synthetic debugger platform capabilities.
> * These are HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES.EAX bits.
> *
>*/
>
>to make it clear.
>
Done.
>> /* Hyper-V specific model specific registers (MSRs) */
>>
>> /* MSR used to identify the guest OS. */
>> @@ -267,6 +275,14 @@
>> /* Hyper-V guest idle MSR */
>> #define HV_X64_MSR_GUEST_IDLE 0x400000F0
>>
>> +/* 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 guest crash notification MSR's */
>> #define HV_X64_MSR_CRASH_P0 0x40000100
>> #define HV_X64_MSR_CRASH_P1 0x40000101
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 98959e8cd448..f8e58e8866bb 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -854,6 +854,18 @@ 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;
>> +};
>> +
>> /* Hyper-V emulation context */
>> struct kvm_hv {
>> struct mutex hv_lock;
>> @@ -877,6 +889,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 a86fda7a1d03..7cbc4afe9d07 100644
>> --- a/arch/x86/kvm/hyperv.c
>> +++ b/arch/x86/kvm/hyperv.c
>> @@ -266,6 +266,71 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
>> return ret;
>> }
>>
>> +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)
>> +{
>> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
>> + int ret;
>> +
>> + trace_kvm_hv_syndbg_set_msr(vcpu->vcpu_id,
>> + vcpu_to_hv_vcpu(vcpu)->vp_index, msr, data);
>> + ret = 0;
>> + switch (msr) {
>> + case HV_X64_MSR_SYNDBG_CONTROL:
>> + syndbg->control.control = data;
>> + 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;
>> + syndbg_exit(vcpu, msr);
>> + break;
>> + case HV_X64_MSR_SYNDBG_OPTIONS:
>> + syndbg->options = data;
>> + break;
>> + default:
>> + ret = 1;
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static int synic_get_msr(struct kvm_vcpu_hv_synic *synic, u32 msr, u64 *pdata,
>> bool host)
>> {
>> @@ -800,6 +865,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 +1128,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);
>> default:
>> vcpu_unimpl(vcpu, "Hyper-V unhandled wrmsr: 0x%x data 0x%llx\n",
>> msr, data);
>> @@ -1227,6 +1297,24 @@ 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:
>> + data = hv->hv_syndbg.options;
>> + break;
>> + case HV_X64_MSR_SYNDBG_CONTROL:
>> + data = hv->hv_syndbg.control.control;
>> + break;
>> + case HV_X64_MSR_SYNDBG_STATUS:
>> + data = hv->hv_syndbg.control.status;
>> + break;
>> + case HV_X64_MSR_SYNDBG_SEND_BUFFER:
>> + data = hv->hv_syndbg.control.send_page;
>> + break;
>> + case HV_X64_MSR_SYNDBG_RECV_BUFFER:
>> + data = hv->hv_syndbg.control.recv_page;
>> + break;
>> + case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
>> + data = hv->hv_syndbg.control.pending_page;
>> + break;
>> default:
>> vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
>> return 1;
>> @@ -1797,6 +1885,9 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>> { .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
>> { .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
>> { .function = HYPERV_CPUID_NESTED_FEATURES },
>> + { .function = HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS },
>> + { .function = HYPERV_CPUID_SYNDBG_INTERFACE },
>> + { .function = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES },
>> };
>> int i, nent = ARRAY_SIZE(cpuid_entries);
>>
>> @@ -1821,7 +1912,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 = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES;
>> ent->ebx = signature[0];
>> ent->ecx = signature[1];
>> ent->edx = signature[2];
>> @@ -1856,9 +1947,12 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>>
>> ent->ebx |= HV_X64_POST_MESSAGES;
>> ent->ebx |= HV_X64_SIGNAL_EVENTS;
>> + ent->ebx |= HV_X64_DEBUGGING;
>>
>> ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
>> ent->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
>> + ent->edx |= HV_X64_GUEST_DEBUGGING_AVAILABLE;
>> + ent->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
>>
>> /*
>> * Direct Synthetic timers only make sense with in-kernel
>> @@ -1903,6 +1997,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];
>
>or maybe just copy HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS.EBX if it's not
>any different?
>
So few things about this, HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS
should return "Microsoft VS" and EAX 0, while
HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS should return "Microsoft Hv" with
EAX as MAX CPUID leaf, with that said the implementation would look
pretty weird, it would look something like
ent->ebx = cpuid_entries[0].ebx;
...
I thought it's a bit confusing that I'll be addressing the index and
make the order of the entries in cpuid_entries important.
>> + 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 757cb578101c..6a86151fac53 100644
>> --- a/arch/x86/kvm/hyperv.h
>> +++ b/arch/x86/kvm/hyperv.h
>> @@ -46,6 +46,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);
>>
>> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
>> index f194dd058470..5fd600916fd4 100644
>> --- a/arch/x86/kvm/trace.h
>> +++ b/arch/x86/kvm/trace.h
>> @@ -1515,6 +1515,31 @@ 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)
>> +);
>
>To be consistent, do we also want a kvm_hv_syndbg_get_msr tracepoint?
>
Done.
>> #endif /* _TRACE_KVM_H */
>>
>> #undef TRACE_INCLUDE_PATH
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 5de200663f51..619c24bac79e 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_DEBUGGING:
>> case KVM_CAP_PCI_SEGMENT:
>> case KVM_CAP_DEBUGREGS:
>> case KVM_CAP_X86_ROBUST_SINGLESTEP:
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 9b4d449f4d20..ca28ea04d1d5 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;
>> union {
>> struct {
>> @@ -202,6 +203,14 @@ struct kvm_hyperv_exit {
>> __u64 params[2];
>> __u32 pad;
>> } hcall;
>> + struct {
>> + __u32 msr;
>> + __u64 control;
>> + __u64 status;
>> + __u64 send_page;
>> + __u64 recv_page;
>> + __u64 pending_page;
>> + } syndbg;
>> } u;
>> };
>>
>> @@ -1011,6 +1020,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_DEBUGGING 180
>>
>> #ifdef KVM_CAP_IRQ_ROUTING
>
>--
>Vitaly
>
^ permalink raw reply
* RE: [PATCH v2 2/4] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Michael Kelley @ 2020-03-06 15:45 UTC (permalink / raw)
To: vkuznets, Jon Doron; +Cc: kvm@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <874kv1ec7f.fsf@vitty.brq.redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Friday, March 6, 2020 7:27 AM
>
> 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.
> >
>
> I would suggest you split TLFS changes into it's own patch so Hyper-V
> folks can ACK (or they can ack the whole patch with KVM changes of
> course :-)
I have contacted the Hyper-V team for clarification of the status
of the synthetic debugging feature and the associated CPUID leaves and
MSRs. The first statement I got is that these are "deprecated", but I'm
trying to get more specifics about exactly what that means, if they
are going to be removed in a future release.
Michael
>
> > Signed-off-by: Jon Doron <arilou@gmail.com>
> > ---
> > arch/x86/include/asm/hyperv-tlfs.h | 16 ++++
> > arch/x86/include/asm/kvm_host.h | 13 ++++
> > arch/x86/kvm/hyperv.c | 114 ++++++++++++++++++++++++++++-
> > arch/x86/kvm/hyperv.h | 5 ++
> > arch/x86/kvm/trace.h | 25 +++++++
> > arch/x86/kvm/x86.c | 9 +++
> > include/uapi/linux/kvm.h | 10 +++
> > 7 files changed, 191 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> > index 92abc1e42bfc..8efdf974c23f 100644
> > --- a/arch/x86/include/asm/hyperv-tlfs.h
> > +++ b/arch/x86/include/asm/hyperv-tlfs.h
> > @@ -33,6 +33,9 @@
> > #define HYPERV_CPUID_ENLIGHTMENT_INFO 0x40000004
> > #define HYPERV_CPUID_IMPLEMENT_LIMITS 0x40000005
> > #define HYPERV_CPUID_NESTED_FEATURES 0x4000000A
> > +#define HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
> > +#define HYPERV_CPUID_SYNDBG_INTERFACE 0x40000081
> > +#define HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
> >
> > #define HYPERV_HYPERVISOR_PRESENT_BIT 0x80000000
> > #define HYPERV_CPUID_MIN 0x40000005
> > @@ -131,6 +134,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)
> >
> > @@ -194,6 +199,9 @@
> > #define HV_X64_NESTED_GUEST_MAPPING_FLUSH BIT(18)
> > #define HV_X64_NESTED_MSR_BITMAP BIT(19)
> >
> > +/* Hyper-V synthetic debugger platform capabilities */
> > +#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING BIT(1)
> > +
>
> hyperv-tlfs.h is not perfectly structured but still there is some
> structure there,
> e.g. HV_X64_NESTED_GUEST_MAPPING_FLUSH/HV_X64_NESTED_MSR_BITMAP/... are
> said to be HYPERV_CPUID_ENLIGHTMENT_INFO.EAX
> bits (see above HV_X64_AS_SWITCH_RECOMMENDED).
>
> To make it clear that HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING doesn't
> belong to these bits I'd suggest you add a comment like
>
> /*
> * Hyper-V synthetic debugger platform capabilities.
> * These are HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES.EAX bits.
> *
> */
>
> to make it clear.
>
> > /* Hyper-V specific model specific registers (MSRs) */
> >
> > /* MSR used to identify the guest OS. */
> > @@ -267,6 +275,14 @@
> > /* Hyper-V guest idle MSR */
> > #define HV_X64_MSR_GUEST_IDLE 0x400000F0
> >
> > +/* 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 guest crash notification MSR's */
> > #define HV_X64_MSR_CRASH_P0 0x40000100
> > #define HV_X64_MSR_CRASH_P1 0x40000101
^ permalink raw reply
* Re: [PATCH v2 2/4] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Vitaly Kuznetsov @ 2020-03-06 15:26 UTC (permalink / raw)
To: Jon Doron; +Cc: kvm, linux-hyperv
In-Reply-To: <20200305140142.413220-3-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.
>
I would suggest you split TLFS changes into it's own patch so Hyper-V
folks can ACK (or they can ack the whole patch with KVM changes of
course :-)
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
> arch/x86/include/asm/hyperv-tlfs.h | 16 ++++
> arch/x86/include/asm/kvm_host.h | 13 ++++
> arch/x86/kvm/hyperv.c | 114 ++++++++++++++++++++++++++++-
> arch/x86/kvm/hyperv.h | 5 ++
> arch/x86/kvm/trace.h | 25 +++++++
> arch/x86/kvm/x86.c | 9 +++
> include/uapi/linux/kvm.h | 10 +++
> 7 files changed, 191 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index 92abc1e42bfc..8efdf974c23f 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -33,6 +33,9 @@
> #define HYPERV_CPUID_ENLIGHTMENT_INFO 0x40000004
> #define HYPERV_CPUID_IMPLEMENT_LIMITS 0x40000005
> #define HYPERV_CPUID_NESTED_FEATURES 0x4000000A
> +#define HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
> +#define HYPERV_CPUID_SYNDBG_INTERFACE 0x40000081
> +#define HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
>
> #define HYPERV_HYPERVISOR_PRESENT_BIT 0x80000000
> #define HYPERV_CPUID_MIN 0x40000005
> @@ -131,6 +134,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)
>
> @@ -194,6 +199,9 @@
> #define HV_X64_NESTED_GUEST_MAPPING_FLUSH BIT(18)
> #define HV_X64_NESTED_MSR_BITMAP BIT(19)
>
> +/* Hyper-V synthetic debugger platform capabilities */
> +#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING BIT(1)
> +
hyperv-tlfs.h is not perfectly structured but still there is some
structure there,
e.g. HV_X64_NESTED_GUEST_MAPPING_FLUSH/HV_X64_NESTED_MSR_BITMAP/... are
said to be HYPERV_CPUID_ENLIGHTMENT_INFO.EAX
bits (see above HV_X64_AS_SWITCH_RECOMMENDED).
To make it clear that HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING doesn't
belong to these bits I'd suggest you add a comment like
/*
* Hyper-V synthetic debugger platform capabilities.
* These are HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES.EAX bits.
*
*/
to make it clear.
> /* Hyper-V specific model specific registers (MSRs) */
>
> /* MSR used to identify the guest OS. */
> @@ -267,6 +275,14 @@
> /* Hyper-V guest idle MSR */
> #define HV_X64_MSR_GUEST_IDLE 0x400000F0
>
> +/* 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 guest crash notification MSR's */
> #define HV_X64_MSR_CRASH_P0 0x40000100
> #define HV_X64_MSR_CRASH_P1 0x40000101
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 98959e8cd448..f8e58e8866bb 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -854,6 +854,18 @@ 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;
> +};
> +
> /* Hyper-V emulation context */
> struct kvm_hv {
> struct mutex hv_lock;
> @@ -877,6 +889,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 a86fda7a1d03..7cbc4afe9d07 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -266,6 +266,71 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
> return ret;
> }
>
> +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)
> +{
> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
> + int ret;
> +
> + trace_kvm_hv_syndbg_set_msr(vcpu->vcpu_id,
> + vcpu_to_hv_vcpu(vcpu)->vp_index, msr, data);
> + ret = 0;
> + switch (msr) {
> + case HV_X64_MSR_SYNDBG_CONTROL:
> + syndbg->control.control = data;
> + 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;
> + syndbg_exit(vcpu, msr);
> + break;
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> + syndbg->options = data;
> + break;
> + default:
> + ret = 1;
> + break;
> + }
> +
> + return ret;
> +}
> +
> static int synic_get_msr(struct kvm_vcpu_hv_synic *synic, u32 msr, u64 *pdata,
> bool host)
> {
> @@ -800,6 +865,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 +1128,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);
> default:
> vcpu_unimpl(vcpu, "Hyper-V unhandled wrmsr: 0x%x data 0x%llx\n",
> msr, data);
> @@ -1227,6 +1297,24 @@ 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:
> + data = hv->hv_syndbg.options;
> + break;
> + case HV_X64_MSR_SYNDBG_CONTROL:
> + data = hv->hv_syndbg.control.control;
> + break;
> + case HV_X64_MSR_SYNDBG_STATUS:
> + data = hv->hv_syndbg.control.status;
> + break;
> + case HV_X64_MSR_SYNDBG_SEND_BUFFER:
> + data = hv->hv_syndbg.control.send_page;
> + break;
> + case HV_X64_MSR_SYNDBG_RECV_BUFFER:
> + data = hv->hv_syndbg.control.recv_page;
> + break;
> + case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> + data = hv->hv_syndbg.control.pending_page;
> + break;
> default:
> vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
> return 1;
> @@ -1797,6 +1885,9 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
> { .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
> { .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
> { .function = HYPERV_CPUID_NESTED_FEATURES },
> + { .function = HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS },
> + { .function = HYPERV_CPUID_SYNDBG_INTERFACE },
> + { .function = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES },
> };
> int i, nent = ARRAY_SIZE(cpuid_entries);
>
> @@ -1821,7 +1912,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 = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES;
> ent->ebx = signature[0];
> ent->ecx = signature[1];
> ent->edx = signature[2];
> @@ -1856,9 +1947,12 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>
> ent->ebx |= HV_X64_POST_MESSAGES;
> ent->ebx |= HV_X64_SIGNAL_EVENTS;
> + ent->ebx |= HV_X64_DEBUGGING;
>
> ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
> ent->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
> + ent->edx |= HV_X64_GUEST_DEBUGGING_AVAILABLE;
> + ent->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
>
> /*
> * Direct Synthetic timers only make sense with in-kernel
> @@ -1903,6 +1997,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];
or maybe just copy HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS.EBX if it's not
any different?
> + 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 757cb578101c..6a86151fac53 100644
> --- a/arch/x86/kvm/hyperv.h
> +++ b/arch/x86/kvm/hyperv.h
> @@ -46,6 +46,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);
>
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index f194dd058470..5fd600916fd4 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -1515,6 +1515,31 @@ 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)
> +);
To be consistent, do we also want a kvm_hv_syndbg_get_msr tracepoint?
> #endif /* _TRACE_KVM_H */
>
> #undef TRACE_INCLUDE_PATH
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5de200663f51..619c24bac79e 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_DEBUGGING:
> case KVM_CAP_PCI_SEGMENT:
> case KVM_CAP_DEBUGREGS:
> case KVM_CAP_X86_ROBUST_SINGLESTEP:
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 9b4d449f4d20..ca28ea04d1d5 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;
> union {
> struct {
> @@ -202,6 +203,14 @@ struct kvm_hyperv_exit {
> __u64 params[2];
> __u32 pad;
> } hcall;
> + struct {
> + __u32 msr;
> + __u64 control;
> + __u64 status;
> + __u64 send_page;
> + __u64 recv_page;
> + __u64 pending_page;
> + } syndbg;
> } u;
> };
>
> @@ -1011,6 +1020,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_DEBUGGING 180
>
> #ifdef KVM_CAP_IRQ_ROUTING
--
Vitaly
^ permalink raw reply
* Re: [PATCH v2 1/4] x86/kvm/hyper-v: Align the hcall param for kvm_hyperv_exit
From: Jon Doron @ 2020-03-06 14:42 UTC (permalink / raw)
To: Vitaly Kuznetsov; +Cc: kvm, linux-hyperv, Paolo Bonzini
In-Reply-To: <87ftelepwz.fsf@vitty.brq.redhat.com>
Thanks Vitaly and Paoloo I'll fix the 1st patch and wait for the final
review on the other 3 and submit v3, I'll also look into adding
the proper test for the Hypercall patch to
https://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git, and submit
a separate patch
to that repository.
Thanks,
-- Jon.
On Fri, Mar 6, 2020 at 12:30 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > On 05/03/20 15:53, Jon Doron wrote:
> >> Vitaly recommended we will align the struct to 64bit...
> >
> > Oh, then I think you actually should add a padding after "__u32 type;"
> > and "__u32 msr;" if you want to make it explicit. The patch, as is, is
> > not aligning anything, hence my confusion.
> >
>
> Right,
>
> the problem I tried to highlight is that without propper padding ABI may
> change, e.g.
>
> #include <stdio.h>
> #include <stdint.h>
> #include <stddef.h>
>
> #define __u32 uint32_t
> #define __u64 uint64_t
>
> struct kvm_hyperv_exit {
> __u32 type;
> union {
> struct {
> __u32 msr;
> __u64 control;
> __u64 evt_page;
> __u64 msg_page;
> } synic;
> struct {
> __u64 input;
> __u64 result;
> __u64 params[2];
> } hcall;
> } u;
> };
>
> int main() {
> printf("%d\n", offsetof(struct kvm_hyperv_exit, u.synic.control));
> printf("%d\n", offsetof(struct kvm_hyperv_exit, u.hcall.input));
>
> return 0;
> }
>
> $ gcc -m32 1.c -o 1
> $ ./1
> 8
> 4
>
> $ gcc 1.c -o 1
> $ ./1
> 16
> 8
>
> if we add a padding after 'type' and 'msr' we'll get
> $ gcc -m32 1.c -o 1
> $ ./1
> 16
> 8
>
> $ gcc 1.c -o 1
> $ ./1
> 16
> 8
>
> which is much better. Technically, this is an ABI change on 32 bit but
> I'm pretty sure noone cares (famous last words!).
>
> --
> Vitaly
>
^ permalink raw reply
* Re: [GIT PULL] Hyper-V commits for 5.6-rc
From: Wei Liu @ 2020-03-06 10:29 UTC (permalink / raw)
To: Linus Torvalds
Cc: Wei Liu, kys, Stephen Hemminger, Linux Kernel Mailing List,
linux-hyperv, mikelley, Sasha Levin, haiyangz
In-Reply-To: <CAHk-=wisKNsaOOCO8TfigKwmuqX83+KvntYeGq-KbOcSPunQbg@mail.gmail.com>
On Thu, Mar 05, 2020 at 12:28:48PM -0600, Linus Torvalds wrote:
> On Thu, Mar 5, 2020 at 9:59 AM Wei Liu <wei.liu@kernel.org> wrote:
> >
> > This is mostly a "dry-run" attempt to sort out any wrinkles on my end.
> > If I have done something stupid, let me know.
>
> Looks fine. I generally like seeing the commits being a bit older than
> these are - you seem to have applied or rebased them not all that long
> before sending the pull request. I prefer seeing that they've been in
> linux-next etc, but for soemthing small like this I guess it doesn't
> much matter. Next time?
Ack.
Those patches were applied not very long before sending the pull request
because I thought simple patches should be fine.
In the future I will let patches go in linux-next first.
Wei.
^ permalink raw reply
* Re: [GIT PULL] Hyper-V commits for 5.6-rc
From: pr-tracker-bot @ 2020-03-05 18:45 UTC (permalink / raw)
To: Wei Liu
Cc: Linus Torvalds, Wei Liu, kys, sthemmin, linux-kernel,
linux-hyperv, mikelley, sashal, haiyangz
In-Reply-To: <20200305155901.xmstcqwutrb6s7pi@debian>
The pull request you sent on Thu, 5 Mar 2020 15:59:02 +0000:
> git://git.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/9f65ed5fe41ce08ed1cb1f6a950f9ec694c142ad
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [GIT PULL] Hyper-V commits for 5.6-rc
From: Linus Torvalds @ 2020-03-05 18:28 UTC (permalink / raw)
To: Wei Liu
Cc: kys, Stephen Hemminger, Linux Kernel Mailing List, linux-hyperv,
mikelley, Sasha Levin, haiyangz
In-Reply-To: <20200305155901.xmstcqwutrb6s7pi@debian>
On Thu, Mar 5, 2020 at 9:59 AM Wei Liu <wei.liu@kernel.org> wrote:
>
> This is mostly a "dry-run" attempt to sort out any wrinkles on my end.
> If I have done something stupid, let me know.
Looks fine. I generally like seeing the commits being a bit older than
these are - you seem to have applied or rebased them not all that long
before sending the pull request. I prefer seeing that they've been in
linux-next etc, but for soemthing small like this I guess it doesn't
much matter. Next time?
Also, it would generally be lovely to see signatures of the old
maintainer on the new maintainer's pgp key when transitions like this
happen, but it's not like we've really required that in the past
either. Literally just a "that would be nice"
Linus
^ permalink raw reply
* [PATCH AUTOSEL 5.5 32/67] hv_netvsc: Fix unwanted wakeup in netvsc_attach()
From: Sasha Levin @ 2020-03-05 17:12 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Haiyang Zhang, David S . Miller, Sasha Levin, linux-hyperv,
netdev
In-Reply-To: <20200305171309.29118-1-sashal@kernel.org>
From: Haiyang Zhang <haiyangz@microsoft.com>
[ Upstream commit f6f13c125e05603f68f5bf31f045b95e6d493598 ]
When netvsc_attach() is called by operations like changing MTU, etc.,
an extra wakeup may happen while netvsc_attach() calling
rndis_filter_device_add() which sends rndis messages when queue is
stopped in netvsc_detach(). The completion message will wake up queue 0.
We can reproduce the issue by changing MTU etc., then the wake_queue
counter from "ethtool -S" will increase beyond stop_queue counter:
stop_queue: 0
wake_queue: 1
The issue causes queue wake up, and counter increment, no other ill
effects in current code. So we didn't see any network problem for now.
To fix this, initialize tx_disable to true, and set it to false when
the NIC is ready to be attached or registered.
Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/hyperv/netvsc.c | 2 +-
drivers/net/hyperv/netvsc_drv.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index eab83e71567a9..6c0732fc8c250 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -99,7 +99,7 @@ static struct netvsc_device *alloc_net_device(void)
init_waitqueue_head(&net_device->wait_drain);
net_device->destroy = false;
- net_device->tx_disable = false;
+ net_device->tx_disable = true;
net_device->max_pkt = RNDIS_MAX_PKT_DEFAULT;
net_device->pkt_align = RNDIS_PKT_ALIGN_DEFAULT;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f3f9eb8a402a2..ee1ad7ae75550 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -977,6 +977,7 @@ static int netvsc_attach(struct net_device *ndev,
}
/* In any case device is now ready */
+ nvdev->tx_disable = false;
netif_device_attach(ndev);
/* Note: enable and attach happen when sub-channels setup */
@@ -2354,6 +2355,8 @@ static int netvsc_probe(struct hv_device *dev,
else
net->max_mtu = ETH_DATA_LEN;
+ nvdev->tx_disable = false;
+
ret = register_netdevice(net);
if (ret != 0) {
pr_err("Unable to register netdev.\n");
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 28/58] hv_netvsc: Fix unwanted wakeup in netvsc_attach()
From: Sasha Levin @ 2020-03-05 17:13 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Haiyang Zhang, David S . Miller, Sasha Levin, linux-hyperv,
netdev
In-Reply-To: <20200305171420.29595-1-sashal@kernel.org>
From: Haiyang Zhang <haiyangz@microsoft.com>
[ Upstream commit f6f13c125e05603f68f5bf31f045b95e6d493598 ]
When netvsc_attach() is called by operations like changing MTU, etc.,
an extra wakeup may happen while netvsc_attach() calling
rndis_filter_device_add() which sends rndis messages when queue is
stopped in netvsc_detach(). The completion message will wake up queue 0.
We can reproduce the issue by changing MTU etc., then the wake_queue
counter from "ethtool -S" will increase beyond stop_queue counter:
stop_queue: 0
wake_queue: 1
The issue causes queue wake up, and counter increment, no other ill
effects in current code. So we didn't see any network problem for now.
To fix this, initialize tx_disable to true, and set it to false when
the NIC is ready to be attached or registered.
Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/hyperv/netvsc.c | 2 +-
drivers/net/hyperv/netvsc_drv.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index eab83e71567a9..6c0732fc8c250 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -99,7 +99,7 @@ static struct netvsc_device *alloc_net_device(void)
init_waitqueue_head(&net_device->wait_drain);
net_device->destroy = false;
- net_device->tx_disable = false;
+ net_device->tx_disable = true;
net_device->max_pkt = RNDIS_MAX_PKT_DEFAULT;
net_device->pkt_align = RNDIS_PKT_ALIGN_DEFAULT;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 0dee358864f30..ca16ae8c83328 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -973,6 +973,7 @@ static int netvsc_attach(struct net_device *ndev,
}
/* In any case device is now ready */
+ nvdev->tx_disable = false;
netif_device_attach(ndev);
/* Note: enable and attach happen when sub-channels setup */
@@ -2350,6 +2351,8 @@ static int netvsc_probe(struct hv_device *dev,
else
net->max_mtu = ETH_DATA_LEN;
+ nvdev->tx_disable = false;
+
ret = register_netdevice(net);
if (ret != 0) {
pr_err("Unable to register netdev.\n");
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 14/31] hv_netvsc: Fix unwanted wakeup in netvsc_attach()
From: Sasha Levin @ 2020-03-05 17:14 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Haiyang Zhang, David S . Miller, Sasha Levin, linux-hyperv,
netdev
In-Reply-To: <20200305171516.30028-1-sashal@kernel.org>
From: Haiyang Zhang <haiyangz@microsoft.com>
[ Upstream commit f6f13c125e05603f68f5bf31f045b95e6d493598 ]
When netvsc_attach() is called by operations like changing MTU, etc.,
an extra wakeup may happen while netvsc_attach() calling
rndis_filter_device_add() which sends rndis messages when queue is
stopped in netvsc_detach(). The completion message will wake up queue 0.
We can reproduce the issue by changing MTU etc., then the wake_queue
counter from "ethtool -S" will increase beyond stop_queue counter:
stop_queue: 0
wake_queue: 1
The issue causes queue wake up, and counter increment, no other ill
effects in current code. So we didn't see any network problem for now.
To fix this, initialize tx_disable to true, and set it to false when
the NIC is ready to be attached or registered.
Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/hyperv/netvsc.c | 2 +-
drivers/net/hyperv/netvsc_drv.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index dbfd3a0c97d3b..77a9a753d9794 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -110,7 +110,7 @@ static struct netvsc_device *alloc_net_device(void)
init_waitqueue_head(&net_device->wait_drain);
net_device->destroy = false;
- net_device->tx_disable = false;
+ net_device->tx_disable = true;
net_device->max_pkt = RNDIS_MAX_PKT_DEFAULT;
net_device->pkt_align = RNDIS_PKT_ALIGN_DEFAULT;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 7ab576d8b6227..bdb55db4523b1 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -984,6 +984,7 @@ static int netvsc_attach(struct net_device *ndev,
}
/* In any case device is now ready */
+ nvdev->tx_disable = false;
netif_device_attach(ndev);
/* Note: enable and attach happen when sub-channels setup */
@@ -2336,6 +2337,8 @@ static int netvsc_probe(struct hv_device *dev,
else
net->max_mtu = ETH_DATA_LEN;
+ nvdev->tx_disable = false;
+
ret = register_netdevice(net);
if (ret != 0) {
pr_err("Unable to register netdev.\n");
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 1/4] x86/kvm/hyper-v: Align the hcall param for kvm_hyperv_exit
From: Paolo Bonzini @ 2020-03-05 16:45 UTC (permalink / raw)
To: Jon Doron; +Cc: kvm, linux-hyperv, Vitaly Kuznetsov
In-Reply-To: <CAP7QCogGkC_wOPuuz2cZDb0aRv0GzMGDR2Y0voU8w4hdtO39BQ@mail.gmail.com>
On 05/03/20 16:52, Jon Doron wrote:
> bah you are right sorry :( but if ill do that ill break userspace no?
No, you'd just be making the padding explicit.
Paolo
>
> On Thu, Mar 5, 2020 at 5:30 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 05/03/20 15:53, Jon Doron wrote:
>>> Vitaly recommended we will align the struct to 64bit...
>> Oh, then I think you actually should add a padding after "__u32 type;"
>> and "__u32 msr;" if you want to make it explicit. The patch, as is, is
>> not aligning anything, hence my confusion.
^ permalink raw reply
* [GIT PULL] Hyper-V commits for 5.6-rc
From: Wei Liu @ 2020-03-05 15:59 UTC (permalink / raw)
To: Linus Torvalds
Cc: Wei Liu, kys, sthemmin, linux-kernel, linux-hyperv, mikelley,
sashal, haiyangz
Hi Linus,
Sasha Levin recently stepped down as the Hyper-V tree maintainer and I
(along with Haiyang Zhang) will be responsible for sending Hyper-V
patches to you.
This is mostly a "dry-run" attempt to sort out any wrinkles on my end.
If I have done something stupid, let me know.
Please pull from the signed tag below.
The following changes since commit 98d54f81e36ba3bf92172791eba5ca5bd813989b:
Linux 5.6-rc4 (2020-03-01 16:38:46 -0600)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git
tags/hyperv-fixes-signed
for you to fetch changes up to 5313b2a58ef02e2b077d7ae8088043609e3155b0:
HID: hyperv: NULL check before some freeing functions is not needed.
(2020-03-05 14:17:11 +0000)
----------------------------------------------------------------
- Update MAINTAINERS file for Hyper-V.
- One cleanup patch for Hyper-V HID driver.
----------------------------------------------------------------
Lucas Tanure (1):
HID: hyperv: NULL check before some freeing functions is not needed.
Sasha Levin (1):
Hyper-V: Drop Sasha Levin from the Hyper-V maintainers
Wei Liu (1):
Hyper-V: add myself as a maintainer
MAINTAINERS | 2 +-
drivers/hid/hid-hyperv.c | 6 ++----
2 files changed, 3 insertions(+), 5 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 1/4] x86/kvm/hyper-v: Align the hcall param for kvm_hyperv_exit
From: Jon Doron @ 2020-03-05 15:52 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, linux-hyperv, Vitaly Kuznetsov
In-Reply-To: <0edfee0e-01ee-bb62-5fc5-67d7d45ec192@redhat.com>
bah you are right sorry :( but if ill do that ill break userspace no?
-- Jon..
On Thu, Mar 5, 2020 at 5:30 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 05/03/20 15:53, Jon Doron wrote:
> > Vitaly recommended we will align the struct to 64bit...
>
> Oh, then I think you actually should add a padding after "__u32 type;"
> and "__u32 msr;" if you want to make it explicit. The patch, as is, is
> not aligning anything, hence my confusion.
>
> Thanks,
>
> Paolo
>
> > On Thu, Mar 5, 2020 at 4:24 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>
> >> On 05/03/20 15:01, Jon Doron wrote:
> >>> Signed-off-by: Jon Doron <arilou@gmail.com>
> >>> ---
> >>> include/uapi/linux/kvm.h | 1 +
> >>> 1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> >>> index 4b95f9a31a2f..9b4d449f4d20 100644
> >>> --- a/include/uapi/linux/kvm.h
> >>> +++ b/include/uapi/linux/kvm.h
> >>> @@ -200,6 +200,7 @@ struct kvm_hyperv_exit {
> >>> __u64 input;
> >>> __u64 result;
> >>> __u64 params[2];
> >>> + __u32 pad;
> >>> } hcall;
> >>> } u;
> >>> };
> >>>
> >>
> >> Can you explain the purpose of this patch?
> >>
> >> Paolo
> >>
> >
>
^ permalink raw reply
* Re: [PATCH v2 1/4] x86/kvm/hyper-v: Align the hcall param for kvm_hyperv_exit
From: Paolo Bonzini @ 2020-03-05 15:29 UTC (permalink / raw)
To: Jon Doron; +Cc: kvm, linux-hyperv, Vitaly Kuznetsov
In-Reply-To: <CAP7QCoj9=mZCWdiOa92QP9Fjb=p3DfKTs0xHKZYQ+yRiMabmLA@mail.gmail.com>
On 05/03/20 15:53, Jon Doron wrote:
> Vitaly recommended we will align the struct to 64bit...
Oh, then I think you actually should add a padding after "__u32 type;"
and "__u32 msr;" if you want to make it explicit. The patch, as is, is
not aligning anything, hence my confusion.
Thanks,
Paolo
> On Thu, Mar 5, 2020 at 4:24 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 05/03/20 15:01, Jon Doron wrote:
>>> Signed-off-by: Jon Doron <arilou@gmail.com>
>>> ---
>>> include/uapi/linux/kvm.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>> index 4b95f9a31a2f..9b4d449f4d20 100644
>>> --- a/include/uapi/linux/kvm.h
>>> +++ b/include/uapi/linux/kvm.h
>>> @@ -200,6 +200,7 @@ struct kvm_hyperv_exit {
>>> __u64 input;
>>> __u64 result;
>>> __u64 params[2];
>>> + __u32 pad;
>>> } hcall;
>>> } u;
>>> };
>>>
>>
>> Can you explain the purpose of this patch?
>>
>> Paolo
>>
>
^ permalink raw reply
* Re: [PATCH v2 1/4] x86/kvm/hyper-v: Align the hcall param for kvm_hyperv_exit
From: Jon Doron @ 2020-03-05 14:53 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, linux-hyperv, Vitaly Kuznetsov
In-Reply-To: <09762184-9913-d334-0a33-b76d153bc371@redhat.com>
Vitaly recommended we will align the struct to 64bit...
On Thu, Mar 5, 2020 at 4:24 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 05/03/20 15:01, Jon Doron wrote:
> > Signed-off-by: Jon Doron <arilou@gmail.com>
> > ---
> > include/uapi/linux/kvm.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 4b95f9a31a2f..9b4d449f4d20 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -200,6 +200,7 @@ struct kvm_hyperv_exit {
> > __u64 input;
> > __u64 result;
> > __u64 params[2];
> > + __u32 pad;
> > } hcall;
> > } u;
> > };
> >
>
> Can you explain the purpose of this patch?
>
> Paolo
>
^ permalink raw reply
* Re: [PATCH v2 1/4] x86/kvm/hyper-v: Align the hcall param for kvm_hyperv_exit
From: Paolo Bonzini @ 2020-03-05 14:23 UTC (permalink / raw)
To: Jon Doron, kvm, linux-hyperv; +Cc: vkuznets
In-Reply-To: <20200305140142.413220-2-arilou@gmail.com>
On 05/03/20 15:01, Jon Doron wrote:
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
> include/uapi/linux/kvm.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 4b95f9a31a2f..9b4d449f4d20 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -200,6 +200,7 @@ struct kvm_hyperv_exit {
> __u64 input;
> __u64 result;
> __u64 params[2];
> + __u32 pad;
> } hcall;
> } u;
> };
>
Can you explain the purpose of this patch?
Paolo
^ permalink raw reply
* [PATCH v2 4/4] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
From: Jon Doron @ 2020-03-05 14:01 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron
In-Reply-To: <20200305140142.413220-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.
Signed-off-by: Jon Doron <arilou@gmail.com>
---
arch/x86/include/asm/hyperv-tlfs.h | 5 +++++
arch/x86/kvm/hyperv.c | 17 +++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 8efdf974c23f..4fa6bf3732a6 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -283,6 +283,8 @@
#define HV_X64_MSR_SYNDBG_PENDING_BUFFER 0x400000F5
#define HV_X64_MSR_SYNDBG_OPTIONS 0x400000FF
+#define HV_X64_SYNDBG_OPTION_USE_HCALLS BIT(2)
+
/* Hyper-V guest crash notification MSR's */
#define HV_X64_MSR_CRASH_P0 0x40000100
#define HV_X64_MSR_CRASH_P1 0x40000101
@@ -392,6 +394,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_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
#define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index d657a312004a..52517e11e643 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1800,6 +1800,23 @@ 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:
+ case HVCALL_RESET_DEBUG_SESSION: {
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+ if (!(syndbg->options & HV_X64_SYNDBG_OPTION_USE_HCALLS)) {
+ ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
+ 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 v2 3/4] x86/kvm/hyper-v: enable hypercalls regardless of hypercall page
From: Jon Doron @ 2020-03-05 14:01 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron
In-Reply-To: <20200305140142.413220-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.
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 7cbc4afe9d07..d657a312004a 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1618,7 +1618,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 ||
+ 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 v2 1/4] x86/kvm/hyper-v: Align the hcall param for kvm_hyperv_exit
From: Jon Doron @ 2020-03-05 14:01 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron
In-Reply-To: <20200305140142.413220-1-arilou@gmail.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
include/uapi/linux/kvm.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 4b95f9a31a2f..9b4d449f4d20 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -200,6 +200,7 @@ struct kvm_hyperv_exit {
__u64 input;
__u64 result;
__u64 params[2];
+ __u32 pad;
} hcall;
} u;
};
--
2.24.1
^ permalink raw reply related
* [PATCH v2 2/4] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Jon Doron @ 2020-03-05 14:01 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron
In-Reply-To: <20200305140142.413220-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.
Signed-off-by: Jon Doron <arilou@gmail.com>
---
arch/x86/include/asm/hyperv-tlfs.h | 16 ++++
arch/x86/include/asm/kvm_host.h | 13 ++++
arch/x86/kvm/hyperv.c | 114 ++++++++++++++++++++++++++++-
arch/x86/kvm/hyperv.h | 5 ++
arch/x86/kvm/trace.h | 25 +++++++
arch/x86/kvm/x86.c | 9 +++
include/uapi/linux/kvm.h | 10 +++
7 files changed, 191 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 92abc1e42bfc..8efdf974c23f 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -33,6 +33,9 @@
#define HYPERV_CPUID_ENLIGHTMENT_INFO 0x40000004
#define HYPERV_CPUID_IMPLEMENT_LIMITS 0x40000005
#define HYPERV_CPUID_NESTED_FEATURES 0x4000000A
+#define HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
+#define HYPERV_CPUID_SYNDBG_INTERFACE 0x40000081
+#define HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
#define HYPERV_HYPERVISOR_PRESENT_BIT 0x80000000
#define HYPERV_CPUID_MIN 0x40000005
@@ -131,6 +134,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)
@@ -194,6 +199,9 @@
#define HV_X64_NESTED_GUEST_MAPPING_FLUSH BIT(18)
#define HV_X64_NESTED_MSR_BITMAP BIT(19)
+/* Hyper-V synthetic debugger platform capabilities */
+#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING BIT(1)
+
/* Hyper-V specific model specific registers (MSRs) */
/* MSR used to identify the guest OS. */
@@ -267,6 +275,14 @@
/* Hyper-V guest idle MSR */
#define HV_X64_MSR_GUEST_IDLE 0x400000F0
+/* 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 guest crash notification MSR's */
#define HV_X64_MSR_CRASH_P0 0x40000100
#define HV_X64_MSR_CRASH_P1 0x40000101
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 98959e8cd448..f8e58e8866bb 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -854,6 +854,18 @@ 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;
+};
+
/* Hyper-V emulation context */
struct kvm_hv {
struct mutex hv_lock;
@@ -877,6 +889,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 a86fda7a1d03..7cbc4afe9d07 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -266,6 +266,71 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
return ret;
}
+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)
+{
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+ int ret;
+
+ trace_kvm_hv_syndbg_set_msr(vcpu->vcpu_id,
+ vcpu_to_hv_vcpu(vcpu)->vp_index, msr, data);
+ ret = 0;
+ switch (msr) {
+ case HV_X64_MSR_SYNDBG_CONTROL:
+ syndbg->control.control = data;
+ 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;
+ syndbg_exit(vcpu, msr);
+ break;
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ syndbg->options = data;
+ break;
+ default:
+ ret = 1;
+ break;
+ }
+
+ return ret;
+}
+
static int synic_get_msr(struct kvm_vcpu_hv_synic *synic, u32 msr, u64 *pdata,
bool host)
{
@@ -800,6 +865,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 +1128,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);
default:
vcpu_unimpl(vcpu, "Hyper-V unhandled wrmsr: 0x%x data 0x%llx\n",
msr, data);
@@ -1227,6 +1297,24 @@ 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:
+ data = hv->hv_syndbg.options;
+ break;
+ case HV_X64_MSR_SYNDBG_CONTROL:
+ data = hv->hv_syndbg.control.control;
+ break;
+ case HV_X64_MSR_SYNDBG_STATUS:
+ data = hv->hv_syndbg.control.status;
+ break;
+ case HV_X64_MSR_SYNDBG_SEND_BUFFER:
+ data = hv->hv_syndbg.control.send_page;
+ break;
+ case HV_X64_MSR_SYNDBG_RECV_BUFFER:
+ data = hv->hv_syndbg.control.recv_page;
+ break;
+ case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ data = hv->hv_syndbg.control.pending_page;
+ break;
default:
vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
return 1;
@@ -1797,6 +1885,9 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
{ .function = HYPERV_CPUID_NESTED_FEATURES },
+ { .function = HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS },
+ { .function = HYPERV_CPUID_SYNDBG_INTERFACE },
+ { .function = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES },
};
int i, nent = ARRAY_SIZE(cpuid_entries);
@@ -1821,7 +1912,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 = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES;
ent->ebx = signature[0];
ent->ecx = signature[1];
ent->edx = signature[2];
@@ -1856,9 +1947,12 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
ent->ebx |= HV_X64_POST_MESSAGES;
ent->ebx |= HV_X64_SIGNAL_EVENTS;
+ ent->ebx |= HV_X64_DEBUGGING;
ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
ent->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
+ ent->edx |= HV_X64_GUEST_DEBUGGING_AVAILABLE;
+ ent->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
/*
* Direct Synthetic timers only make sense with in-kernel
@@ -1903,6 +1997,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 757cb578101c..6a86151fac53 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -46,6 +46,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);
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index f194dd058470..5fd600916fd4 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -1515,6 +1515,31 @@ 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)
+);
#endif /* _TRACE_KVM_H */
#undef TRACE_INCLUDE_PATH
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5de200663f51..619c24bac79e 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_DEBUGGING:
case KVM_CAP_PCI_SEGMENT:
case KVM_CAP_DEBUGREGS:
case KVM_CAP_X86_ROBUST_SINGLESTEP:
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 9b4d449f4d20..ca28ea04d1d5 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;
union {
struct {
@@ -202,6 +203,14 @@ struct kvm_hyperv_exit {
__u64 params[2];
__u32 pad;
} hcall;
+ struct {
+ __u32 msr;
+ __u64 control;
+ __u64 status;
+ __u64 send_page;
+ __u64 recv_page;
+ __u64 pending_page;
+ } syndbg;
} u;
};
@@ -1011,6 +1020,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_DEBUGGING 180
#ifdef KVM_CAP_IRQ_ROUTING
--
2.24.1
^ permalink raw reply related
* [PATCH v2 0/4] x86/kvm/hyper-v: add support for synthetic debugger
From: Jon Doron @ 2020-03-05 14:01 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron
Add support for the synthetic debugger interface of hyper-v, the
synthetic debugger has 2 modes.
1. Use a set of MSRs to send/recv information
2. Use hypercalls
The first mode is based the following MSRs:
1. Control/Status MSRs which either asks for a send/recv .
2. Send/Recv MSRs each holds GPA where the send/recv buffers are.
3. Pending MSR, holds a GPA to a PAGE that simply has a boolean that
indicates if there is data pending to issue a recv VMEXIT.
In the first patch the first mode is being implemented in the sense that
it simply exits to user-space when a control MSR is being written and
when the pending MSR is being set, then it's up-to userspace to
implement the rest of the logic of sending/recving.
In the second mode instead of using MSRs KNet will simply issue
Hypercalls with the information to send/recv, in this mode the data
being transferred is UDP encapsulated, unlike in the previous mode in
which you get just the data to send.
The new hypercalls will exit to userspace which will be incharge of
re-encapsulating if needed the UDP packets to be sent.
There is an issue though in which KDNet does not respect the hypercall
page and simply issues vmcall/vmmcall instructions depending on the cpu
type expecting them to be handled as it a real hypercall was issued.
Jon Doron (4):
x86/kvm/hyper-v: Align the hcall param for kvm_hyperv_exit
x86/kvm/hyper-v: Add support for synthetic debugger capability
x86/kvm/hyper-v: enable hypercalls regardless of hypercall page
x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
arch/x86/include/asm/hyperv-tlfs.h | 21 +++++
arch/x86/include/asm/kvm_host.h | 13 +++
arch/x86/kvm/hyperv.c | 136 ++++++++++++++++++++++++++++-
arch/x86/kvm/hyperv.h | 5 ++
arch/x86/kvm/trace.h | 25 ++++++
arch/x86/kvm/x86.c | 9 ++
include/uapi/linux/kvm.h | 11 +++
7 files changed, 218 insertions(+), 2 deletions(-)
--
2.24.1
^ permalink raw reply
* Re: [PATCH v1 3/3] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
From: Jon Doron @ 2020-03-05 13:58 UTC (permalink / raw)
To: Vitaly Kuznetsov; +Cc: kvm, linux-hyperv
In-Reply-To: <87mu8wdxtt.fsf@vitty.brq.redhat.com>
On Wed, Mar 4, 2020 at 4:00 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> 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/include/asm/hyperv-tlfs.h | 5 +++++
> > arch/x86/kvm/hyperv.c | 11 +++++++++++
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> > index 8efdf974c23f..4fa6bf3732a6 100644
> > --- a/arch/x86/include/asm/hyperv-tlfs.h
> > +++ b/arch/x86/include/asm/hyperv-tlfs.h
> > @@ -283,6 +283,8 @@
> > #define HV_X64_MSR_SYNDBG_PENDING_BUFFER 0x400000F5
> > #define HV_X64_MSR_SYNDBG_OPTIONS 0x400000FF
> >
> > +#define HV_X64_SYNDBG_OPTION_USE_HCALLS BIT(2)
> > +
>
> BIT(2) of what? :-) Also, you don't seem to use this define anywhere.
>
Will use it now :) and it's a syndbg MSR option bit
> > /* Hyper-V guest crash notification MSR's */
> > #define HV_X64_MSR_CRASH_P0 0x40000100
> > #define HV_X64_MSR_CRASH_P1 0x40000101
> > @@ -392,6 +394,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_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
> > #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
> >
> > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> > index 7ec962d433af..593e0f3f4dba 100644
> > --- a/arch/x86/kvm/hyperv.c
> > +++ b/arch/x86/kvm/hyperv.c
> > @@ -1794,6 +1794,17 @@ 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:
> > + case HVCALL_RESET_DEBUG_SESSION:
> > + 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;
>
> --
> Vitaly
>
^ permalink raw reply
* Re: [PATCH v1 2/3] x86/kvm/hyper-v: enable hypercalls regardless of hypercall page
From: Jon Doron @ 2020-03-05 13:57 UTC (permalink / raw)
To: Vitaly Kuznetsov; +Cc: kvm, linux-hyperv
In-Reply-To: <87pndsdxxh.fsf@vitty.brq.redhat.com>
On Wed, Mar 4, 2020 at 3:58 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> 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 if the guest_os_id is
> > not 0.
> >
> > Signed-off-by: Jon Doron <arilou@gmail.com>
> > ---
> > arch/x86/kvm/hyperv.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> > index 13176ec23496..7ec962d433af 100644
> > --- a/arch/x86/kvm/hyperv.c
> > +++ b/arch/x86/kvm/hyperv.c
> > @@ -1615,7 +1615,7 @@ 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;
> > + return READ_ONCE(kvm->arch.hyperv.hv_guest_os_id) != 0;
> > }
> >
> > static void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
>
> I would've enabled it in both cases,
>
> return (READ_ONCE(kvm->arch.hyperv.hv_hypercall) &
> HV_X64_MSR_HYPERCALL_ENABLE) || (READ_ONCE(kvm->arch.hyperv.hv_guest_os_id) != 0);
>
> to be safe. We can also check what genuine Hyper-V does but I bet it has
> hypercalls always enabled. Also, the function can be made inline,
> there's a single caller.
I dont have any Hyper-V setup at the moment to validate this, i
believe your hunch is correct but ill do the
implementation you have suggested.
>
> --
> Vitaly
>
^ permalink raw reply
* Re: [PATCH v1 1/3] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Jon Doron @ 2020-03-05 13:56 UTC (permalink / raw)
To: Vitaly Kuznetsov; +Cc: kvm, linux-hyperv
In-Reply-To: <87sgiody8f.fsf@vitty.brq.redhat.com>
On Wed, Mar 4, 2020 at 3:51 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> 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.
> >
>
> Cc: linux-hyperv@ list where Hyper-V folks live :-) They're in charge of
> 'hyperv-tlfs.h' so an ACK from them will be needed.
>
Thanks will do in next version of this patchset.
> > Signed-off-by: Jon Doron <arilou@gmail.com>
> > ---
> > arch/x86/include/asm/hyperv-tlfs.h | 16 +++++
> > arch/x86/include/asm/kvm_host.h | 11 +++
> > arch/x86/kvm/hyperv.c | 109 +++++++++++++++++++++++++++++
> > arch/x86/kvm/hyperv.h | 5 ++
> > arch/x86/kvm/trace.h | 22 ++++++
> > arch/x86/kvm/x86.c | 8 +++
> > include/uapi/linux/kvm.h | 9 +++
> > 7 files changed, 180 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> > index 92abc1e42bfc..8efdf974c23f 100644
> > --- a/arch/x86/include/asm/hyperv-tlfs.h
> > +++ b/arch/x86/include/asm/hyperv-tlfs.h
> > @@ -33,6 +33,9 @@
> > #define HYPERV_CPUID_ENLIGHTMENT_INFO 0x40000004
> > #define HYPERV_CPUID_IMPLEMENT_LIMITS 0x40000005
> > #define HYPERV_CPUID_NESTED_FEATURES 0x4000000A
> > +#define HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
> > +#define HYPERV_CPUID_SYNDBG_INTERFACE 0x40000081
> > +#define HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
>
> Out of pure curiosity, are these CPUIDs and MSRs documented somewhere?
> I'm looking at TLFS v6.0 and failing to see them...
>
Basically a lot of playing around with KDVM/KDNet by setting a kernel
debugger, there are
also few hints about how the transport for KD works in the DDK (there
are samples that
show how to code a new kdtransport)
> >
> > #define HYPERV_HYPERVISOR_PRESENT_BIT 0x80000000
> > #define HYPERV_CPUID_MIN 0x40000005
> > @@ -131,6 +134,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)
> >
> > @@ -194,6 +199,9 @@
> > #define HV_X64_NESTED_GUEST_MAPPING_FLUSH BIT(18)
> > #define HV_X64_NESTED_MSR_BITMAP BIT(19)
> >
> > +/* Hyper-V synthetic debugger platform capabilities */
> > +#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING BIT(1)
> > +
> > /* Hyper-V specific model specific registers (MSRs) */
> >
> > /* MSR used to identify the guest OS. */
> > @@ -267,6 +275,14 @@
> > /* Hyper-V guest idle MSR */
> > #define HV_X64_MSR_GUEST_IDLE 0x400000F0
> >
> > +/* 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 guest crash notification MSR's */
> > #define HV_X64_MSR_CRASH_P0 0x40000100
> > #define HV_X64_MSR_CRASH_P1 0x40000101
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 98959e8cd448..2b755174d683 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -854,6 +854,15 @@ struct kvm_apic_map {
> > struct kvm_lapic *phys_map[];
> > };
> >
> > +/* Hyper-V synthetic debugger (SynDbg)*/
> > +struct kvm_hv_syndbg {
> > + u64 control;
> > + u64 status;
> > + u64 send_page;
> > + u64 recv_page;
> > + u64 pending_page;
> > +};
> > +
> > /* Hyper-V emulation context */
> > struct kvm_hv {
> > struct mutex hv_lock;
> > @@ -877,6 +886,8 @@ struct kvm_hv {
> > atomic_t num_mismatched_vp_indexes;
> >
> > struct hv_partition_assist_pg *hv_pa_pg;
> > + u64 hv_syndbg_options;
> > + struct kvm_hv_syndbg hv_syndbg;
>
> I would've encapsulated both to struct kvm_hv_syndbg, e.g.
>
> struct kvm_hv_syndbg {
> struct {
> u64 control;
> u64 status;
> u64 send_page;
> u64 recv_page;
> u64 pending_page;
> } control;
> u64 options;
> }
>
> To make it clear they're part of the same thing (are they?) I see you
> handle HV_X64_MSR_SYNDBG_OPTIONS differently (outside of
> syndbg_set_msr()).
>
Done.
> > };
> >
> > enum kvm_irqchip_mode {
> > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> > index a86fda7a1d03..13176ec23496 100644
> > --- a/arch/x86/kvm/hyperv.c
> > +++ b/arch/x86/kvm/hyperv.c
> > @@ -266,6 +266,66 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
> > return ret;
> > }
> >
> > +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.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;
> > + hv_vcpu->exit.u.syndbg.send_page = syndbg->send_page;
> > + hv_vcpu->exit.u.syndbg.recv_page = syndbg->recv_page;
> > + hv_vcpu->exit.u.syndbg.pending_page = syndbg->pending_page;
> > + vcpu->arch.complete_userspace_io =
> > + kvm_hv_syndbg_complete_userspace;
> > +
> > + kvm_make_request(KVM_REQ_HV_EXIT, vcpu);
>
> This new interface requires userspace support apparently so we can't
> enable it unconditionally (userspaces which don't support it will be
> very confused). You need to introduce a capability
> (KVM_CAP_HYPERV_DEBUGGING?)
>
Done.
> > +}
> > +
> > +static int syndbg_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > +{
> > + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
> > + int ret;
> > +
> > + trace_kvm_hv_syndbg_set_msr(vcpu->vcpu_id, msr, data);
> > + ret = 0;
> > + switch (msr) {
> > + case HV_X64_MSR_SYNDBG_CONTROL:
> > + syndbg->control = data;
> > + syndbg_exit(vcpu, msr);
> > + break;
> > + case HV_X64_MSR_SYNDBG_STATUS:
> > + syndbg->status = data;
> > + break;
> > + case HV_X64_MSR_SYNDBG_SEND_BUFFER:
> > + syndbg->send_page = data;
> > + break;
> > + case HV_X64_MSR_SYNDBG_RECV_BUFFER:
> > + syndbg->recv_page = data;
> > + break;
> > + case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> > + syndbg->pending_page = data;
> > + syndbg_exit(vcpu, msr);
> > + break;
> > + default:
> > + ret = 1;
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > static int synic_get_msr(struct kvm_vcpu_hv_synic *synic, u32 msr, u64 *pdata,
> > bool host)
> > {
> > @@ -800,6 +860,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 +1123,11 @@ 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:
> > + hv->hv_syndbg_options = data;
> > + break;
> > + case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> > + return syndbg_set_msr(vcpu, msr, data);
> > default:
> > vcpu_unimpl(vcpu, "Hyper-V unhandled wrmsr: 0x%x data 0x%llx\n",
> > msr, data);
> > @@ -1227,6 +1294,24 @@ 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:
> > + data = hv->hv_syndbg_options;
> > + break;
> > + case HV_X64_MSR_SYNDBG_CONTROL:
> > + data = hv->hv_syndbg.control;
> > + break;
> > + case HV_X64_MSR_SYNDBG_STATUS:
> > + data = hv->hv_syndbg.status;
> > + break;
> > + case HV_X64_MSR_SYNDBG_SEND_BUFFER:
> > + data = hv->hv_syndbg.send_page;
> > + break;
> > + case HV_X64_MSR_SYNDBG_RECV_BUFFER:
> > + data = hv->hv_syndbg.recv_page;
> > + break;
> > + case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> > + data = hv->hv_syndbg.pending_page;
> > + break;
> > default:
> > vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
> > return 1;
> > @@ -1797,6 +1882,9 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
> > { .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
> > { .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
> > { .function = HYPERV_CPUID_NESTED_FEATURES },
> > + { .function = HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS },
> > + { .function = HYPERV_CPUID_SYNDBG_INTERFACE },
> > + { .function = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES },
>
> HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS now returns
> 'HYPERV_CPUID_NESTED_FEATURES' as the last available leaf and I don't
> see you adjusting it - is this expected?
>
Good catch my bad :)
> > };
> > int i, nent = ARRAY_SIZE(cpuid_entries);
> >
> > @@ -1856,9 +1944,12 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
> >
> > ent->ebx |= HV_X64_POST_MESSAGES;
> > ent->ebx |= HV_X64_SIGNAL_EVENTS;
> > + ent->ebx |= HV_X64_DEBUGGING;
> >
> > ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
> > ent->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
> > + ent->edx |= HV_X64_GUEST_DEBUGGING_AVAILABLE;
> > + ent->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
> >
> > /*
> > * Direct Synthetic timers only make sense with in-kernel
> > @@ -1903,6 +1994,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, "Microsoft VS", 12);
>
> Does this string matter? E.g. for HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS
> we call ourselves "Linux KVM Hv", I think we should do the same here.
>
Nope does not really matter since Userspace will override this anyway
> > +
> > + 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 757cb578101c..6a86151fac53 100644
> > --- a/arch/x86/kvm/hyperv.h
> > +++ b/arch/x86/kvm/hyperv.h
> > @@ -46,6 +46,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);
> >
> > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> > index f194dd058470..235b9ab673a2 100644
> > --- a/arch/x86/kvm/trace.h
> > +++ b/arch/x86/kvm/trace.h
> > @@ -1515,6 +1515,28 @@ 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 msr, u64 data),
> > + TP_ARGS(vcpu_id, msr, data),
> > +
> > + TP_STRUCT__entry(
> > + __field(int, vcpu_id)
> > + __field(u32, msr)
> > + __field(u64, data)
> > + ),
> > +
> > + TP_fast_assign(
> > + __entry->vcpu_id = vcpu_id;
> > + __entry->msr = msr;
> > + __entry->data = data;
> > + ),
> > +
> > + TP_printk("vcpu_id %d msr 0x%x data 0x%llx",
> > + __entry->vcpu_id, __entry->msr, __entry->data)
>
> This doesn't give us any additional data trace_kvm_msr_* points are more
> or less the same. I think we can do better, e.g. for Hyper-V specific
> things log the processor's VP index.
>
Done.
> > +);
> > #endif /* _TRACE_KVM_H */
> >
> > #undef TRACE_INCLUDE_PATH
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 5de200663f51..9d4d72a88572 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:
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 4b95f9a31a2f..fae8cf608976 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;
> > union {
> > struct {
> > @@ -201,6 +202,14 @@ struct kvm_hyperv_exit {
> > __u64 result;
> > __u64 params[2];
> > } hcall;
> > + struct {
> > + __u32 msr;
> > + __u64 control;
> > + __u64 status;
> > + __u64 send_page;
> > + __u64 recv_page;
> > + __u64 pending_page;
> > + } syndbg;
> > } u;
> > };
>
> Not your fault but I just noticed that 'struct kvm_hyperv_exit' is not
> properly padded. 'synic' struct is OK, however, 'hcall' is not as
> there's gonna be a gap between '__u32 type' and it. Your 'struct syndbg'
> is also OK as it starts with '__u32 msr' but we should do something
> about hcall.
>
Created another patch to address this.
> --
> Vitaly
>
Cheers,
-- Jon.
^ permalink raw reply
* Re: [PATCH] PCI: hv: Replace zero-length array with flexible-array member
From: Gustavo A. R. Silva @ 2020-03-04 18:24 UTC (permalink / raw)
To: Wei Liu, Lorenzo Pieralisi
Cc: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
Sasha Levin, Andrew Murray, Bjorn Helgaas,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20200304181017.epqvhmtegefb4eba@debian>
On 3/4/20 12:10, Wei Liu wrote:
>>>>
>>>> Looks good to me. Thanks, Gustavo!
>>>>
>>>> Reviewed-by: Dexuan Cui <decui@microsoft.com>
>>>>
>>>
>>> Lorenzo, will you be picking up this patch? It seems to me you've been
>>> handling patches to pci-hyperv.c. This patch is not yet in pci/hv branch
>>> in your repository.
>>>
>>> Let me know what you think.
>>
>> I shall pick it up, I checked patchwork and it was erroneously
>> assigned to Bjorn, that's why I have not taken it yet.
>>
>> Fixed now, apologies, I will merge it shortly.
>
> Thanks for picking it up.
>
Thank you all, guys. :)
--
Gustavo
^ 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