* Re: [PATCH 0/4] x86/Hyper-V: Unload vmbus channel in hv panic callback
From: Tianyu Lan @ 2020-03-19 8:24 UTC (permalink / raw)
To: Wei Liu
Cc: kys, haiyangz, sthemmin, liuwe, tglx, mingo, bp, hpa, x86,
michael.h.kelley, Tianyu Lan, linux-hyperv, linux-kernel,
vkuznets
In-Reply-To: <20200317173553.jerf6gjtaotqjbac@debian>
On 3/18/2020 1:35 AM, Wei Liu wrote:
> On Tue, Mar 17, 2020 at 06:25:20AM -0700, ltykernel@gmail.com wrote:
>> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>>
>> Customer reported Hyper-V VM still responded network traffic
>> ack packets after kernel panic with kernel parameter "panic=0”.
>> This becauses vmbus driver interrupt handler still works
>> on the panic cpu after kernel panic. Panic cpu falls into
>> infinite loop of panic() with interrupt enabled at that point.
>> Vmbus driver can still handle network traffic.
>>
>> This confuses remote service that the panic system is still
>> alive when it gets ack packets. Unload vmbus channel in hv panic
>> callback and fix it.
>>
>> vmbus_initiate_unload() maybe double called during panic process
>> (e.g, hyperv_panic_event() and hv_crash_handler()). So check
>> and set connection state in vmbus_initiate_unload() to resolve
>> reenter issue.
>>
>> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
>> ---
>> drivers/hv/channel_mgmt.c | 5 +++++
>> drivers/hv/vmbus_drv.c | 17 +++++++++--------
>> 2 files changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
>> index 0370364169c4..893493f2b420 100644
>> --- a/drivers/hv/channel_mgmt.c
>> +++ b/drivers/hv/channel_mgmt.c
>> @@ -839,6 +839,9 @@ void vmbus_initiate_unload(bool crash)
>> {
>> struct vmbus_channel_message_header hdr;
>>
>> + if (vmbus_connection.conn_state == DISCONNECTED)
>> + return;
>> +
>> /* Pre-Win2012R2 hosts don't support reconnect */
>> if (vmbus_proto_version < VERSION_WIN8_1)
>> return;
>> @@ -857,6 +860,8 @@ void vmbus_initiate_unload(bool crash)
>> wait_for_completion(&vmbus_connection.unload_event);
>> else
>> vmbus_wait_for_unload();
>> +
>> + vmbus_connection.conn_state = DISCONNECTED;
>
> This is only set at the end of the function. I don't see how this solve
> the re-entrant issue with the check at the beginning. Do I miss anything
> here?
>
For this issue, vmbus_initiate_unload() maybe called on the panic vcpu
twice and so just split check and set conn_state.
> Maybe this function should check and set the state to
> DISCONNECTING/DISCONNECTED at the beginning of this function?
>
Yes, Vitaly also gave suggestion to use "xchg" to check and set
conn_state. Will update in the next version.
^ permalink raw reply
* Re: [PATCH v8 3/5] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Vitaly Kuznetsov @ 2020-03-19 8:30 UTC (permalink / raw)
To: Jon Doron; +Cc: kvm, linux-hyperv
In-Reply-To: <20200319063836.678562-4-arilou@gmail.com>
Jon Doron <arilou@gmail.com> writes:
> Add support for Hyper-V synthetic debugger (syndbg) interface.
> The syndbg interface is using MSRs to emulate a way to send/recv packets
> data.
>
> The debug transport dll (kdvm/kdnet) will identify if Hyper-V is enabled
> and if it supports the synthetic debugger interface it will attempt to
> use it, instead of trying to initialize a network adapter.
>
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
> Documentation/virt/kvm/api.rst | 16 ++++
> arch/x86/include/asm/kvm_host.h | 14 +++
> arch/x86/kvm/hyperv.c | 146 +++++++++++++++++++++++++++++++-
> arch/x86/kvm/hyperv.h | 6 ++
> arch/x86/kvm/trace.h | 51 +++++++++++
> arch/x86/kvm/x86.c | 13 +++
> include/uapi/linux/kvm.h | 11 +++
> 7 files changed, 256 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 4872c47bbcff..fe992dcf4f93 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -5024,6 +5024,7 @@ EOI was received.
> struct kvm_hyperv_exit {
> #define KVM_EXIT_HYPERV_SYNIC 1
> #define KVM_EXIT_HYPERV_HCALL 2
> + #define KVM_EXIT_HYPERV_SYNDBG 3
> __u32 type;
> __u32 pad1;
> union {
> @@ -5039,6 +5040,15 @@ EOI was received.
> __u64 result;
> __u64 params[2];
> } hcall;
> + struct {
> + __u32 msr;
> + __u32 pad2;
> + __u64 control;
> + __u64 status;
> + __u64 send_page;
> + __u64 recv_page;
> + __u64 pending_page;
> + } syndbg;
> } u;
> };
> /* KVM_EXIT_HYPERV */
> @@ -5055,6 +5065,12 @@ Hyper-V SynIC state change. Notification is used to remap SynIC
> event/message pages and to enable/disable SynIC messages/events processing
> in userspace.
>
> + - KVM_EXIT_HYPERV_SYNDBG -- synchronously notify user-space about
> +
> +Hyper-V Synthetic debugger state change. Notification is used to either update
> +the pending_page location or to send a control command (send the buffer located
> +in send_page or recv a buffer to recv_page).
> +
> ::
>
> /* KVM_EXIT_ARM_NISV */
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 98959e8cd448..c09fa7401b13 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -854,6 +854,19 @@ struct kvm_apic_map {
> struct kvm_lapic *phys_map[];
> };
>
> +/* Hyper-V synthetic debugger (SynDbg)*/
> +struct kvm_hv_syndbg {
> + struct {
> + u64 control;
> + u64 status;
> + u64 send_page;
> + u64 recv_page;
> + u64 pending_page;
> + } control;
> + u64 options;
> + bool active;
> +};
> +
> /* Hyper-V emulation context */
> struct kvm_hv {
> struct mutex hv_lock;
> @@ -877,6 +890,7 @@ struct kvm_hv {
> atomic_t num_mismatched_vp_indexes;
>
> struct hv_partition_assist_pg *hv_pa_pg;
> + struct kvm_hv_syndbg hv_syndbg;
> };
>
> enum kvm_irqchip_mode {
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index a86fda7a1d03..b2b50c67badd 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -266,6 +266,115 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
> return ret;
> }
>
> +void kvm_hv_activate_syndbg(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
> +
> + syndbg->active = true;
> +}
> +
> +static int kvm_hv_syndbg_complete_userspace(struct kvm_vcpu *vcpu)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + struct kvm_hv *hv = &kvm->arch.hyperv;
> +
> + if (vcpu->run->hyperv.u.syndbg.msr == HV_X64_MSR_SYNDBG_CONTROL)
> + hv->hv_syndbg.control.status =
> + vcpu->run->hyperv.u.syndbg.status;
> + return 1;
> +}
> +
> +static void syndbg_exit(struct kvm_vcpu *vcpu, u32 msr)
> +{
> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
> + struct kvm_vcpu_hv *hv_vcpu = &vcpu->arch.hyperv;
> +
> + hv_vcpu->exit.type = KVM_EXIT_HYPERV_SYNDBG;
> + hv_vcpu->exit.u.syndbg.msr = msr;
> + hv_vcpu->exit.u.syndbg.control = syndbg->control.control;
> + hv_vcpu->exit.u.syndbg.send_page = syndbg->control.send_page;
> + hv_vcpu->exit.u.syndbg.recv_page = syndbg->control.recv_page;
> + hv_vcpu->exit.u.syndbg.pending_page = syndbg->control.pending_page;
> + vcpu->arch.complete_userspace_io =
> + kvm_hv_syndbg_complete_userspace;
> +
> + kvm_make_request(KVM_REQ_HV_EXIT, vcpu);
> +}
> +
> +static int syndbg_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
> +
> + if (!syndbg->active)
> + return 1;
> +
> + trace_kvm_hv_syndbg_set_msr(vcpu->vcpu_id,
> + vcpu_to_hv_vcpu(vcpu)->vp_index, msr, data);
> + switch (msr) {
> + case HV_X64_MSR_SYNDBG_CONTROL:
> + syndbg->control.control = data;
> + 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:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int syndbg_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> +{
> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
> +
> + if (!syndbg->active)
> + return 1;
> +
> + switch (msr) {
> + case HV_X64_MSR_SYNDBG_CONTROL:
> + *pdata = syndbg->control.control;
> + break;
> + case HV_X64_MSR_SYNDBG_STATUS:
> + *pdata = syndbg->control.status;
> + break;
> + case HV_X64_MSR_SYNDBG_SEND_BUFFER:
> + *pdata = syndbg->control.send_page;
> + break;
> + case HV_X64_MSR_SYNDBG_RECV_BUFFER:
> + *pdata = syndbg->control.recv_page;
> + break;
> + case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> + *pdata = syndbg->control.pending_page;
> + break;
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> + *pdata = syndbg->options;
> + break;
> + default:
> + break;
> + }
> +
> + trace_kvm_hv_syndbg_get_msr(vcpu->vcpu_id,
> + vcpu_to_hv_vcpu(vcpu)->vp_index, msr,
> + *pdata);
> +
> + return 0;
> +}
> +
> static int synic_get_msr(struct kvm_vcpu_hv_synic *synic, u32 msr, u64 *pdata,
> bool host)
> {
> @@ -800,6 +909,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 +1172,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 +1341,9 @@ static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> case HV_X64_MSR_TSC_EMULATION_STATUS:
> data = hv->hv_tsc_emulation_status;
> break;
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> + case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> + return syndbg_get_msr(vcpu, msr, pdata);
> default:
> vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
> return 1;
> @@ -1797,8 +1914,12 @@ 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);
> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
>
> if (kvm_x86_ops->nested_get_evmcs_version)
> evmcs_ver = kvm_x86_ops->nested_get_evmcs_version(vcpu);
Right below this we do
/* Skip NESTED_FEATURES if eVMCS is not supported */
if (!evmcs_ver)
--nent;
which becomes incorrect: we'll skip
HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES instead of
HYPERV_CPUID_NESTED_FEATURES. And we can't get away
with something like
if (!syndbg->active)
nent -= 3;
before just because we may have evmcs disabled and syndbg enabled: and
we only need to skip HYPERV_CPUID_NESTED_FEATURES - which is in the
middle and the code we have doesn't allow that.
We now need to support all 4 possible options: evmcs & syndbg, !evmcs &&
syndbg, evmcs & !syndbg, !evmcs && !syndbg and I think you'll have to
dinamically allocate an array for cpuid_entries, move feature enablement
(evmcs & syndbg) checks down to 'for' loop and fill in the newly
allocated array one by one (or do something similar).
Also, when you're done, could you please modify the test in
tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c to make sure we're not
breaking things? It would be great to check all possible combinations.
> @@ -1821,7 +1942,9 @@ 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 = syndbg->active ?
> + HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES :
> + HYPERV_CPUID_NESTED_FEATURES;
> ent->ebx = signature[0];
> ent->ecx = signature[1];
> ent->edx = signature[2];
> @@ -1856,9 +1979,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;
With Enlightened VMCS we advertise the interface when it is enabled
if (evmcs_ver)
ent->eax |= HV_X64_ENLIGHTENED_VMCS_RECOMMENDED;
I'd suggest we do the same for debug MSRs:
if (syndbg->active) {
ent->edx |= HV_X64_GUEST_DEBUGGING_AVAILABLE;
ent->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
}
Similar to eVMCS, the userspace will have to enable
KVM_CAP_HYPERV_SYNDBG first.
>
> /*
> * Direct Synthetic timers only make sense with in-kernel
> @@ -1903,6 +2029,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 5e4780bf6dd7..45aaaf08bf15 100644
> --- a/arch/x86/kvm/hyperv.h
> +++ b/arch/x86/kvm/hyperv.h
> @@ -73,6 +73,11 @@ static inline struct kvm_vcpu *synic_to_vcpu(struct kvm_vcpu_hv_synic *synic)
> return hv_vcpu_to_vcpu(container_of(synic, struct kvm_vcpu_hv, synic));
> }
>
> +static inline struct kvm_hv_syndbg *vcpu_to_hv_syndbg(struct kvm_vcpu *vcpu)
> +{
> + return &vcpu->kvm->arch.hyperv.hv_syndbg;
> +}
> +
> int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
> int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host);
>
> @@ -83,6 +88,7 @@ void kvm_hv_irq_routing_update(struct kvm *kvm);
> int kvm_hv_synic_set_irq(struct kvm *kvm, u32 vcpu_id, u32 sint);
> void kvm_hv_synic_send_eoi(struct kvm_vcpu *vcpu, int vector);
> int kvm_hv_activate_synic(struct kvm_vcpu *vcpu, bool dont_zero_synic_pages);
> +void kvm_hv_activate_syndbg(struct kvm_vcpu *vcpu);
>
> void kvm_hv_vcpu_init(struct kvm_vcpu *vcpu);
> void kvm_hv_vcpu_postcreate(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index f194dd058470..bf6c3852868d 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -1515,6 +1515,57 @@ TRACE_EVENT(kvm_nested_vmenter_failed,
> __print_symbolic(__entry->err, VMX_VMENTER_INSTRUCTION_ERRORS))
> );
>
> +/*
> + * Tracepoint for syndbg_set_msr.
> + */
> +TRACE_EVENT(kvm_hv_syndbg_set_msr,
> + TP_PROTO(int vcpu_id, u32 vp_index, u32 msr, u64 data),
> + TP_ARGS(vcpu_id, vp_index, msr, data),
> +
> + TP_STRUCT__entry(
> + __field(int, vcpu_id)
> + __field(u32, vp_index)
> + __field(u32, msr)
> + __field(u64, data)
> + ),
> +
> + TP_fast_assign(
> + __entry->vcpu_id = vcpu_id;
> + __entry->vp_index = vp_index;
> + __entry->msr = msr;
> + __entry->data = data;
> + ),
> +
> + TP_printk("vcpu_id %d vp_index %u msr 0x%x data 0x%llx",
> + __entry->vcpu_id, __entry->vp_index, __entry->msr,
> + __entry->data)
> +);
> +
> +/*
> + * Tracepoint for syndbg_get_msr.
> + */
> +TRACE_EVENT(kvm_hv_syndbg_get_msr,
> + TP_PROTO(int vcpu_id, u32 vp_index, u32 msr, u64 data),
> + TP_ARGS(vcpu_id, vp_index, msr, data),
> +
> + TP_STRUCT__entry(
> + __field(int, vcpu_id)
> + __field(u32, vp_index)
> + __field(u32, msr)
> + __field(u64, data)
> + ),
> +
> + TP_fast_assign(
> + __entry->vcpu_id = vcpu_id;
> + __entry->vp_index = vp_index;
> + __entry->msr = msr;
> + __entry->data = data;
> + ),
> +
> + TP_printk("vcpu_id %d vp_index %u msr 0x%x data 0x%llx",
> + __entry->vcpu_id, __entry->vp_index, __entry->msr,
> + __entry->data)
> +);
> #endif /* _TRACE_KVM_H */
>
> #undef TRACE_INCLUDE_PATH
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 3156e25b0774..7bfa30d7a525 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1214,6 +1214,10 @@ static const u32 emulated_msrs_all[] = {
> HV_X64_MSR_VP_ASSIST_PAGE,
> HV_X64_MSR_REENLIGHTENMENT_CONTROL, HV_X64_MSR_TSC_EMULATION_CONTROL,
> HV_X64_MSR_TSC_EMULATION_STATUS,
> + HV_X64_MSR_SYNDBG_OPTIONS,
> + HV_X64_MSR_SYNDBG_CONTROL, HV_X64_MSR_SYNDBG_STATUS,
> + HV_X64_MSR_SYNDBG_SEND_BUFFER, HV_X64_MSR_SYNDBG_RECV_BUFFER,
> + HV_X64_MSR_SYNDBG_PENDING_BUFFER,
>
> MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
> MSR_KVM_PV_EOI_EN,
> @@ -2906,6 +2910,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> */
> break;
> case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
> + case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> case HV_X64_MSR_CRASH_CTL:
> case HV_X64_MSR_STIMER0_CONFIG ... HV_X64_MSR_STIMER3_COUNT:
> @@ -3151,6 +3157,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> msr_info->data = 0x20000000;
> break;
> case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
> + case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> case HV_X64_MSR_CRASH_CTL:
> case HV_X64_MSR_STIMER0_CONFIG ... HV_X64_MSR_STIMER3_COUNT:
> @@ -3323,6 +3331,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> case KVM_CAP_HYPERV_TLBFLUSH:
> case KVM_CAP_HYPERV_SEND_IPI:
> case KVM_CAP_HYPERV_CPUID:
> + case KVM_CAP_HYPERV_SYNDBG:
> case KVM_CAP_PCI_SEGMENT:
> case KVM_CAP_DEBUGREGS:
> case KVM_CAP_X86_ROBUST_SINGLESTEP:
> @@ -4178,6 +4187,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
> return -EINVAL;
>
> switch (cap->cap) {
> + case KVM_CAP_HYPERV_SYNDBG:
> + kvm_hv_activate_syndbg(vcpu);
> + return 0;
> +
> case KVM_CAP_HYPERV_SYNIC2:
> if (cap->args[0])
> return -EINVAL;
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 7ee0ddc4c457..0b573e1155c8 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -188,6 +188,7 @@ struct kvm_s390_cmma_log {
> struct kvm_hyperv_exit {
> #define KVM_EXIT_HYPERV_SYNIC 1
> #define KVM_EXIT_HYPERV_HCALL 2
> +#define KVM_EXIT_HYPERV_SYNDBG 3
> __u32 type;
> __u32 pad1;
> union {
> @@ -203,6 +204,15 @@ struct kvm_hyperv_exit {
> __u64 result;
> __u64 params[2];
> } hcall;
> + struct {
> + __u32 msr;
> + __u32 pad2;
> + __u64 control;
> + __u64 status;
> + __u64 send_page;
> + __u64 recv_page;
> + __u64 pending_page;
> + } syndbg;
> } u;
> };
>
> @@ -1012,6 +1022,7 @@ struct kvm_ppc_resize_hpt {
> #define KVM_CAP_ARM_NISV_TO_USER 177
> #define KVM_CAP_ARM_INJECT_EXT_DABT 178
> #define KVM_CAP_S390_VCPU_RESETS 179
> +#define KVM_CAP_HYPERV_SYNDBG 180
>
> #ifdef KVM_CAP_IRQ_ROUTING
--
Vitaly
^ permalink raw reply
* Re: [PATCH v2 4/8] powernv/memtrace: always online added memory blocks
From: Michael Ellerman @ 2020-03-19 9:49 UTC (permalink / raw)
To: David Hildenbrand, linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand,
Benjamin Herrenschmidt, Paul Mackerras, Andrew Morton,
Greg Kroah-Hartman, Michal Hocko, Oscar Salvador,
Rafael J. Wysocki, Baoquan He, Wei Yang
In-Reply-To: <20200317104942.11178-5-david@redhat.com>
David Hildenbrand <david@redhat.com> writes:
> Let's always try to online the re-added memory blocks. In case add_memory()
> already onlined the added memory blocks, the first device_online() call
> will fail and stop processing the remaining memory blocks.
>
> This avoids manually having to check memhp_auto_online.
>
> Note: PPC always onlines all hotplugged memory directly from the kernel
> as well - something that is handled by user space on other
> architectures.
>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> arch/powerpc/platforms/powernv/memtrace.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
Fine by me.
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
> diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
> index d6d64f8718e6..13b369d2cc45 100644
> --- a/arch/powerpc/platforms/powernv/memtrace.c
> +++ b/arch/powerpc/platforms/powernv/memtrace.c
> @@ -231,16 +231,10 @@ static int memtrace_online(void)
> continue;
> }
>
> - /*
> - * If kernel isn't compiled with the auto online option
> - * we need to online the memory ourselves.
> - */
> - if (!memhp_auto_online) {
> - lock_device_hotplug();
> - walk_memory_blocks(ent->start, ent->size, NULL,
> - online_mem_block);
> - unlock_device_hotplug();
> - }
> + lock_device_hotplug();
> + walk_memory_blocks(ent->start, ent->size, NULL,
> + online_mem_block);
> + unlock_device_hotplug();
>
> /*
> * Memory was added successfully so clean up references to it
> --
> 2.24.1
^ permalink raw reply
* [PATCH v3 8/8] mm/memory_hotplug: allow to specify a default online_type
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand, Wei Yang,
Michal Hocko, Greg Kroah-Hartman, Andrew Morton, Michal Hocko,
Oscar Salvador, Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
For now, distributions implement advanced udev rules to essentially
- Don't online any hotplugged memory (s390x)
- Online all memory to ZONE_NORMAL (e.g., most virt environments like
hyperv)
- Online all memory to ZONE_MOVABLE in case the zone imbalance is taken
care of (e.g., bare metal, special virt environments)
In summary: All memory is usually onlined the same way, however, the
kernel always has to ask user space to come up with the same answer.
E.g., Hyper-V always waits for a memory block to get onlined before
continuing, otherwise it might end up adding memory faster than
onlining it, which can result in strange OOM situations. This waiting
slows down adding of a bigger amount of memory.
Let's allow to specify a default online_type, not just "online" and
"offline". This allows distributions to configure the default online_type
when booting up and be done with it.
We can now specify "offline", "online", "online_movable" and
"online_kernel" via
- "memhp_default_state=" on the kernel cmdline
- /sys/devices/system/memory/auto_online_blocks
just like we are able to specify for a single memory block via
/sys/devices/system/memory/memoryX/state
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 11 +++++------
include/linux/memory_hotplug.h | 2 ++
mm/memory_hotplug.c | 8 ++++----
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 8d3e16dab69f..2b09b68b9f78 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -35,7 +35,7 @@ static const char *const online_type_to_str[] = {
[MMOP_ONLINE_MOVABLE] = "online_movable",
};
-static int memhp_online_type_from_str(const char *str)
+int memhp_online_type_from_str(const char *str)
{
int i;
@@ -394,13 +394,12 @@ static ssize_t auto_online_blocks_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- if (sysfs_streq(buf, "online"))
- memhp_default_online_type = MMOP_ONLINE;
- else if (sysfs_streq(buf, "offline"))
- memhp_default_online_type = MMOP_OFFLINE;
- else
+ const int online_type = memhp_online_type_from_str(buf);
+
+ if (online_type < 0)
return -EINVAL;
+ memhp_default_online_type = online_type;
return count;
}
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 6d6f85bb66e9..93d9ada74ddd 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -118,6 +118,8 @@ extern int arch_add_memory(int nid, u64 start, u64 size,
struct mhp_params *params);
extern u64 max_mem_size;
+extern int memhp_online_type_from_str(const char *str);
+
/* Default online_type (MMOP_*) when new memory blocks are added. */
extern int memhp_default_online_type;
/* If movable_node boot option specified */
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 4efcf8cb9ac5..89197163d138 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -74,10 +74,10 @@ int memhp_default_online_type = MMOP_ONLINE;
static int __init setup_memhp_default_state(char *str)
{
- if (!strcmp(str, "online"))
- memhp_default_online_type = MMOP_ONLINE;
- else if (!strcmp(str, "offline"))
- memhp_default_online_type = MMOP_OFFLINE;
+ const int online_type = memhp_online_type_from_str(str);
+
+ if (online_type >= 0)
+ memhp_default_online_type = online_type;
return 1;
}
--
2.24.1
^ permalink raw reply related
* [PATCH v3 7/8] mm/memory_hotplug: convert memhp_auto_online to store an online_type
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand, Wei Yang,
Michal Hocko, Greg Kroah-Hartman, Andrew Morton, Michal Hocko,
Oscar Salvador, Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
... and rename it to memhp_default_online_type. This is a preparation
for more detailed default online behavior.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 10 ++++------
include/linux/memory_hotplug.h | 3 ++-
mm/memory_hotplug.c | 11 ++++++-----
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 8a7f29c0bf97..8d3e16dab69f 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -386,10 +386,8 @@ static DEVICE_ATTR_RO(block_size_bytes);
static ssize_t auto_online_blocks_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- if (memhp_auto_online)
- return sprintf(buf, "online\n");
- else
- return sprintf(buf, "offline\n");
+ return sprintf(buf, "%s\n",
+ online_type_to_str[memhp_default_online_type]);
}
static ssize_t auto_online_blocks_store(struct device *dev,
@@ -397,9 +395,9 @@ static ssize_t auto_online_blocks_store(struct device *dev,
const char *buf, size_t count)
{
if (sysfs_streq(buf, "online"))
- memhp_auto_online = true;
+ memhp_default_online_type = MMOP_ONLINE;
else if (sysfs_streq(buf, "offline"))
- memhp_auto_online = false;
+ memhp_default_online_type = MMOP_OFFLINE;
else
return -EINVAL;
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 76f3c617a8ab..6d6f85bb66e9 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -118,7 +118,8 @@ extern int arch_add_memory(int nid, u64 start, u64 size,
struct mhp_params *params);
extern u64 max_mem_size;
-extern bool memhp_auto_online;
+/* Default online_type (MMOP_*) when new memory blocks are added. */
+extern int memhp_default_online_type;
/* If movable_node boot option specified */
extern bool movable_node_enabled;
static inline bool movable_node_is_enabled(void)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e21a7d53ade5..4efcf8cb9ac5 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -67,17 +67,17 @@ void put_online_mems(void)
bool movable_node_enabled = false;
#ifndef CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE
-bool memhp_auto_online;
+int memhp_default_online_type = MMOP_OFFLINE;
#else
-bool memhp_auto_online = true;
+int memhp_default_online_type = MMOP_ONLINE;
#endif
static int __init setup_memhp_default_state(char *str)
{
if (!strcmp(str, "online"))
- memhp_auto_online = true;
+ memhp_default_online_type = MMOP_ONLINE;
else if (!strcmp(str, "offline"))
- memhp_auto_online = false;
+ memhp_default_online_type = MMOP_OFFLINE;
return 1;
}
@@ -993,6 +993,7 @@ static int check_hotplug_memory_range(u64 start, u64 size)
static int online_memory_block(struct memory_block *mem, void *arg)
{
+ mem->online_type = memhp_default_online_type;
return device_online(&mem->dev);
}
@@ -1065,7 +1066,7 @@ int __ref add_memory_resource(int nid, struct resource *res)
mem_hotplug_done();
/* online pages if requested */
- if (memhp_auto_online)
+ if (memhp_default_online_type != MMOP_OFFLINE)
walk_memory_blocks(start, size, NULL, online_memory_block);
return ret;
--
2.24.1
^ permalink raw reply related
* [PATCH v3 6/8] mm/memory_hotplug: unexport memhp_auto_online
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand, Wei Yang,
Michal Hocko, Andrew Morton, Michal Hocko, Oscar Salvador,
Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
All in-tree users except the mm-core are gone. Let's drop the export.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
mm/memory_hotplug.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index da6aab272c9b..e21a7d53ade5 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -71,7 +71,6 @@ bool memhp_auto_online;
#else
bool memhp_auto_online = true;
#endif
-EXPORT_SYMBOL_GPL(memhp_auto_online);
static int __init setup_memhp_default_state(char *str)
{
--
2.24.1
^ permalink raw reply related
* [PATCH v3 5/8] hv_balloon: don't check for memhp_auto_online manually
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand,
Vitaly Kuznetsov, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, Andrew Morton, Michal Hocko,
Oscar Salvador, Rafael J. Wysocki, Baoquan He, Wei Yang
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
We get the MEM_ONLINE notifier call if memory is added right from the
kernel via add_memory() or later from user space.
Let's get rid of the "ha_waiting" flag - the wait event has an inbuilt
mechanism (->done) for that. Initialize the wait event only once and
reinitialize before adding memory. Unconditionally call complete() and
wait_for_completion_timeout().
If there are no waiters, complete() will only increment ->done - which
will be reset by reinit_completion(). If complete() has already been
called, wait_for_completion_timeout() will not wait.
There is still the chance for a small race between concurrent
reinit_completion() and complete(). If complete() wins, we would not
wait - which is tolerable (and the race exists in current code as well).
Note: We only wait for "some" memory to get onlined, which seems to be
good enough for now.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Wei Liu <wei.liu@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: linux-hyperv@vger.kernel.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/hv/hv_balloon.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index a02ce43d778d..32e3bc0aa665 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -533,7 +533,6 @@ struct hv_dynmem_device {
* State to synchronize hot-add.
*/
struct completion ol_waitevent;
- bool ha_waiting;
/*
* This thread handles hot-add
* requests from the host as well as notifying
@@ -634,10 +633,7 @@ static int hv_memory_notifier(struct notifier_block *nb, unsigned long val,
switch (val) {
case MEM_ONLINE:
case MEM_CANCEL_ONLINE:
- if (dm_device.ha_waiting) {
- dm_device.ha_waiting = false;
- complete(&dm_device.ol_waitevent);
- }
+ complete(&dm_device.ol_waitevent);
break;
case MEM_OFFLINE:
@@ -726,8 +722,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
has->covered_end_pfn += processed_pfn;
spin_unlock_irqrestore(&dm_device.ha_lock, flags);
- init_completion(&dm_device.ol_waitevent);
- dm_device.ha_waiting = !memhp_auto_online;
+ reinit_completion(&dm_device.ol_waitevent);
nid = memory_add_physaddr_to_nid(PFN_PHYS(start_pfn));
ret = add_memory(nid, PFN_PHYS((start_pfn)),
@@ -753,15 +748,14 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
}
/*
- * Wait for the memory block to be onlined when memory onlining
- * is done outside of kernel (memhp_auto_online). Since the hot
- * add has succeeded, it is ok to proceed even if the pages in
- * the hot added region have not been "onlined" within the
- * allowed time.
+ * Wait for memory to get onlined. If the kernel onlined the
+ * memory when adding it, this will return directly. Otherwise,
+ * it will wait for user space to online the memory. This helps
+ * to avoid adding memory faster than it is getting onlined. As
+ * adding succeeded, it is ok to proceed even if the memory was
+ * not onlined in time.
*/
- if (dm_device.ha_waiting)
- wait_for_completion_timeout(&dm_device.ol_waitevent,
- 5*HZ);
+ wait_for_completion_timeout(&dm_device.ol_waitevent, 5 * HZ);
post_status(&dm_device);
}
}
@@ -1706,6 +1700,7 @@ static int balloon_probe(struct hv_device *dev,
#ifdef CONFIG_MEMORY_HOTPLUG
set_online_page_callback(&hv_online_page);
+ init_completion(&dm_device.ol_waitevent);
register_memory_notifier(&hv_memory_nb);
#endif
--
2.24.1
^ permalink raw reply related
* [PATCH v3 4/8] powernv/memtrace: always online added memory blocks
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand, Wei Yang,
Michal Hocko, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Andrew Morton, Greg Kroah-Hartman, Michal Hocko,
Oscar Salvador, Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
Let's always try to online the re-added memory blocks. In case add_memory()
already onlined the added memory blocks, the first device_online() call
will fail and stop processing the remaining memory blocks.
This avoids manually having to check memhp_auto_online.
Note: PPC always onlines all hotplugged memory directly from the kernel
as well - something that is handled by user space on other
architectures.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/powerpc/platforms/powernv/memtrace.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index d6d64f8718e6..13b369d2cc45 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -231,16 +231,10 @@ static int memtrace_online(void)
continue;
}
- /*
- * If kernel isn't compiled with the auto online option
- * we need to online the memory ourselves.
- */
- if (!memhp_auto_online) {
- lock_device_hotplug();
- walk_memory_blocks(ent->start, ent->size, NULL,
- online_mem_block);
- unlock_device_hotplug();
- }
+ lock_device_hotplug();
+ walk_memory_blocks(ent->start, ent->size, NULL,
+ online_mem_block);
+ unlock_device_hotplug();
/*
* Memory was added successfully so clean up references to it
--
2.24.1
^ permalink raw reply related
* [PATCH v3 3/8] drivers/base/memory: store mapping between MMOP_* and string in an array
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand, Wei Yang,
Michal Hocko, Greg Kroah-Hartman, Andrew Morton, Michal Hocko,
Oscar Salvador, Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
Let's use a simple array which we can reuse soon. While at it, move the
string->mmop conversion out of the device hotplug lock.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index e7e77cafef80..8a7f29c0bf97 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -28,6 +28,24 @@
#define MEMORY_CLASS_NAME "memory"
+static const char *const online_type_to_str[] = {
+ [MMOP_OFFLINE] = "offline",
+ [MMOP_ONLINE] = "online",
+ [MMOP_ONLINE_KERNEL] = "online_kernel",
+ [MMOP_ONLINE_MOVABLE] = "online_movable",
+};
+
+static int memhp_online_type_from_str(const char *str)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(online_type_to_str); i++) {
+ if (sysfs_streq(str, online_type_to_str[i]))
+ return i;
+ }
+ return -EINVAL;
+}
+
#define to_memory_block(dev) container_of(dev, struct memory_block, dev)
static int sections_per_block;
@@ -236,26 +254,17 @@ static int memory_subsys_offline(struct device *dev)
static ssize_t state_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
+ const int online_type = memhp_online_type_from_str(buf);
struct memory_block *mem = to_memory_block(dev);
- int ret, online_type;
+ int ret;
+
+ if (online_type < 0)
+ return -EINVAL;
ret = lock_device_hotplug_sysfs();
if (ret)
return ret;
- if (sysfs_streq(buf, "online_kernel"))
- online_type = MMOP_ONLINE_KERNEL;
- else if (sysfs_streq(buf, "online_movable"))
- online_type = MMOP_ONLINE_MOVABLE;
- else if (sysfs_streq(buf, "online"))
- online_type = MMOP_ONLINE;
- else if (sysfs_streq(buf, "offline"))
- online_type = MMOP_OFFLINE;
- else {
- ret = -EINVAL;
- goto err;
- }
-
switch (online_type) {
case MMOP_ONLINE_KERNEL:
case MMOP_ONLINE_MOVABLE:
@@ -271,7 +280,6 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
ret = -EINVAL; /* should never happen */
}
-err:
unlock_device_hotplug();
if (ret < 0)
--
2.24.1
^ permalink raw reply related
* [PATCH v3 2/8] drivers/base/memory: map MMOP_OFFLINE to 0
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand, Wei Yang,
Michal Hocko, Greg Kroah-Hartman, Andrew Morton, Michal Hocko,
Oscar Salvador, Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
Historically, we used the value -1. Just treat 0 as the special
case now. Clarify a comment (which was wrong, when we come via
device_online() the first time, the online_type would have been 0 /
MEM_ONLINE). The default is now always MMOP_OFFLINE. This removes the
last user of the manual "-1", which didn't use the enum value.
This is a preparation to use the online_type as an array index.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 11 ++++-------
include/linux/memory_hotplug.h | 2 +-
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 8c5ce42c0fc3..e7e77cafef80 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -211,17 +211,14 @@ static int memory_subsys_online(struct device *dev)
return 0;
/*
- * If we are called from state_store(), online_type will be
- * set >= 0 Otherwise we were called from the device online
- * attribute and need to set the online_type.
+ * When called via device_online() without configuring the online_type,
+ * we want to default to MMOP_ONLINE.
*/
- if (mem->online_type < 0)
+ if (mem->online_type == MMOP_OFFLINE)
mem->online_type = MMOP_ONLINE;
ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
-
- /* clear online_type */
- mem->online_type = -1;
+ mem->online_type = MMOP_OFFLINE;
return ret;
}
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 3aaf00db224c..76f3c617a8ab 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -48,7 +48,7 @@ enum {
/* Types for control the zone type of onlined and offlined memory */
enum {
/* Offline the memory. */
- MMOP_OFFLINE = -1,
+ MMOP_OFFLINE = 0,
/* Online the memory. Zone depends, see default_zone_for_pfn(). */
MMOP_ONLINE,
/* Online the memory to ZONE_NORMAL. */
--
2.24.1
^ permalink raw reply related
* [PATCH v3 0/8] mm/memory_hotplug: allow to specify a default online_type
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand,
Vitaly Kuznetsov, Yumei Huang, Igor Mammedov, Baoquan He,
Eduardo Habkost, Milan Zamazal, Andrew Morton,
Benjamin Herrenschmidt, Greg Kroah-Hartman, Haiyang Zhang,
K. Y. Srinivasan, Michael Ellerman, Michal Hocko, Michal Hocko,
Oscar Salvador, Paul Mackerras, Rafael J. Wysocki,
Stephen Hemminger, Wei Liu, Wei Yang
Distributions nowadays use udev rules ([1] [2]) to specify if and
how to online hotplugged memory. The rules seem to get more complex with
many special cases. Due to the various special cases,
CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE cannot be used. All memory hotplug
is handled via udev rules.
Everytime we hotplug memory, the udev rule will come to the same
conclusion. Especially Hyper-V (but also soon virtio-mem) add a lot of
memory in separate memory blocks and wait for memory to get onlined by user
space before continuing to add more memory blocks (to not add memory faster
than it is getting onlined). This of course slows down the whole memory
hotplug process.
To make the job of distributions easier and to avoid udev rules that get
more and more complicated, let's extend the mechanism provided by
- /sys/devices/system/memory/auto_online_blocks
- "memhp_default_state=" on the kernel cmdline
to be able to specify also "online_movable" as well as "online_kernel"
v2 -> v3:
- "hv_balloon: don't check for memhp_auto_online manually"
-- init_completion() before register_memory_notifier()
- Minor typo fix
v1 -> v2:
- Tweaked some patch descriptions
- Added
-- "powernv/memtrace: always online added memory blocks"
-- "hv_balloon: don't check for memhp_auto_online manually"
-- "mm/memory_hotplug: unexport memhp_auto_online"
- "mm/memory_hotplug: convert memhp_auto_online to store an online_type"
-- No longer touches hv/memtrace code
=== Example /usr/libexec/config-memhotplug ===
#!/bin/bash
VIRT=`systemd-detect-virt --vm`
ARCH=`uname -p`
sense_virtio_mem() {
if [ -d "/sys/bus/virtio/drivers/virtio_mem/" ]; then
DEVICES=`find /sys/bus/virtio/drivers/virtio_mem/ -maxdepth 1 -type l | wc -l`
if [ $DEVICES != "0" ]; then
return 0
fi
fi
return 1
}
if [ ! -e "/sys/devices/system/memory/auto_online_blocks" ]; then
echo "Memory hotplug configuration support missing in the kernel"
exit 1
fi
if grep "memhp_default_state=" /proc/cmdline > /dev/null; then
echo "Memory hotplug configuration overridden in kernel cmdline (memhp_default_state=)"
exit 1
fi
if [ $VIRT == "microsoft" ]; then
echo "Detected Hyper-V on $ARCH"
# Hyper-V wants all memory in ZONE_NORMAL
ONLINE_TYPE="online_kernel"
elif sense_virtio_mem; then
echo "Detected virtio-mem on $ARCH"
# virtio-mem wants all memory in ZONE_NORMAL
ONLINE_TYPE="online_kernel"
elif [ $ARCH == "s390x" ] || [ $ARCH == "s390" ]; then
echo "Detected $ARCH"
# standby memory should not be onlined automatically
ONLINE_TYPE="offline"
elif [ $ARCH == "ppc64" ] || [ $ARCH == "ppc64le" ]; then
echo "Detected" $ARCH
# PPC64 onlines all hotplugged memory right from the kernel
ONLINE_TYPE="offline"
elif [ $VIRT == "none" ]; then
echo "Detected bare-metal on $ARCH"
# Bare metal users expect hotplugged memory to be unpluggable. We assume
# that ZONE imbalances on such enterpise servers cannot happen and is
# properly documented
ONLINE_TYPE="online_movable"
else
# TODO: Hypervisors that want to unplug DIMMs and can guarantee that ZONE
# imbalances won't happen
echo "Detected $VIRT on $ARCH"
# Usually, ballooning is used in virtual environments, so memory should go to
# ZONE_NORMAL. However, sometimes "movable_node" is relevant.
ONLINE_TYPE="online"
fi
echo "Selected online_type:" $ONLINE_TYPE
# Configure what to do with memory that will be hotplugged in the future
echo $ONLINE_TYPE 2>/dev/null > /sys/devices/system/memory/auto_online_blocks
if [ $? != "0" ]; then
echo "Memory hotplug cannot be configured (e.g., old kernel or missing permissions)"
# A backup udev rule should handle old kernels if necessary
exit 1
fi
# Process all already pluggedd blocks (e.g., DIMMs, but also Hyper-V or virtio-mem)
if [ $ONLINE_TYPE != "offline" ]; then
for MEMORY in /sys/devices/system/memory/memory*; do
STATE=`cat $MEMORY/state`
if [ $STATE == "offline" ]; then
echo $ONLINE_TYPE > $MEMORY/state
fi
done
fi
=== Example /usr/lib/systemd/system/config-memhotplug.service ===
[Unit]
Description=Configure memory hotplug behavior
DefaultDependencies=no
Conflicts=shutdown.target
Before=sysinit.target shutdown.target
After=systemd-modules-load.service
ConditionPathExists=|/sys/devices/system/memory/auto_online_blocks
[Service]
ExecStart=/usr/libexec/config-memhotplug
Type=oneshot
TimeoutSec=0
RemainAfterExit=yes
[Install]
WantedBy=sysinit.target
=== Example modification to the 40-redhat.rules [2] ===
diff --git a/40-redhat.rules b/40-redhat.rules-new
index 2c690e5..168fd03 100644
--- a/40-redhat.rules
+++ b/40-redhat.rules-new
@@ -6,6 +6,9 @@ SUBSYSTEM=="cpu", ACTION=="add", TEST=="online", ATTR{online}=="0", ATTR{online}
# Memory hotadd request
SUBSYSTEM!="memory", GOTO="memory_hotplug_end"
ACTION!="add", GOTO="memory_hotplug_end"
+# memory hotplug behavior configured
+PROGRAM=="grep online /sys/devices/system/memory/auto_online_blocks", GOTO="memory_hotplug_end"
+
PROGRAM="/bin/uname -p", RESULT=="s390*", GOTO="memory_hotplug_end"
ENV{.state}="online"
===
[1] https://github.com/lnykryn/systemd-rhel/pull/281
[2] https://github.com/lnykryn/systemd-rhel/blob/staging/rules/40-redhat.rules
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Yumei Huang <yuhuang@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Milan Zamazal <mzamazal@redhat.com>
David Hildenbrand (8):
drivers/base/memory: rename MMOP_ONLINE_KEEP to MMOP_ONLINE
drivers/base/memory: map MMOP_OFFLINE to 0
drivers/base/memory: store mapping between MMOP_* and string in an
array
powernv/memtrace: always online added memory blocks
hv_balloon: don't check for memhp_auto_online manually
mm/memory_hotplug: unexport memhp_auto_online
mm/memory_hotplug: convert memhp_auto_online to store an online_type
mm/memory_hotplug: allow to specify a default online_type
arch/powerpc/platforms/powernv/memtrace.c | 14 ++---
drivers/base/memory.c | 71 ++++++++++++-----------
drivers/hv/hv_balloon.c | 25 ++++----
include/linux/memory_hotplug.h | 13 ++++-
mm/memory_hotplug.c | 16 ++---
5 files changed, 69 insertions(+), 70 deletions(-)
--
2.24.1
^ permalink raw reply related
* [PATCH v3 1/8] drivers/base/memory: rename MMOP_ONLINE_KEEP to MMOP_ONLINE
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linuxppc-dev, linux-hyperv, David Hildenbrand, Wei Yang,
Greg Kroah-Hartman, Andrew Morton, Michal Hocko, Oscar Salvador,
Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
The name is misleading and it's not really clear what is "kept". Let's just
name it like the online_type name we expose to user space ("online").
Add some documentation to the types.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 9 +++++----
include/linux/memory_hotplug.h | 6 +++++-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 6448c9ece2cb..8c5ce42c0fc3 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -216,7 +216,7 @@ static int memory_subsys_online(struct device *dev)
* attribute and need to set the online_type.
*/
if (mem->online_type < 0)
- mem->online_type = MMOP_ONLINE_KEEP;
+ mem->online_type = MMOP_ONLINE;
ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
@@ -251,7 +251,7 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
else if (sysfs_streq(buf, "online_movable"))
online_type = MMOP_ONLINE_MOVABLE;
else if (sysfs_streq(buf, "online"))
- online_type = MMOP_ONLINE_KEEP;
+ online_type = MMOP_ONLINE;
else if (sysfs_streq(buf, "offline"))
online_type = MMOP_OFFLINE;
else {
@@ -262,7 +262,7 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
switch (online_type) {
case MMOP_ONLINE_KERNEL:
case MMOP_ONLINE_MOVABLE:
- case MMOP_ONLINE_KEEP:
+ case MMOP_ONLINE:
/* mem->online_type is protected by device_hotplug_lock */
mem->online_type = online_type;
ret = device_online(&mem->dev);
@@ -342,7 +342,8 @@ static ssize_t valid_zones_show(struct device *dev,
}
nid = mem->nid;
- default_zone = zone_for_pfn_range(MMOP_ONLINE_KEEP, nid, start_pfn, nr_pages);
+ default_zone = zone_for_pfn_range(MMOP_ONLINE, nid, start_pfn,
+ nr_pages);
strcat(buf, default_zone->name);
print_allowed_zone(buf, nid, start_pfn, nr_pages, MMOP_ONLINE_KERNEL,
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 3195d11876ea..3aaf00db224c 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -47,9 +47,13 @@ enum {
/* Types for control the zone type of onlined and offlined memory */
enum {
+ /* Offline the memory. */
MMOP_OFFLINE = -1,
- MMOP_ONLINE_KEEP,
+ /* Online the memory. Zone depends, see default_zone_for_pfn(). */
+ MMOP_ONLINE,
+ /* Online the memory to ZONE_NORMAL. */
MMOP_ONLINE_KERNEL,
+ /* Online the memory to ZONE_MOVABLE. */
MMOP_ONLINE_MOVABLE,
};
--
2.24.1
^ permalink raw reply related
* Re: [PATCH 0/4] x86/Hyper-V: Panic code path fixes
From: Tianyu Lan @ 2020-03-19 14:08 UTC (permalink / raw)
To: Michael Kelley, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
hpa@zytor.com, x86@kernel.org
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <MW2PR2101MB1052F185AF4134EB2BB9ECBFD7F40@MW2PR2101MB1052.namprd21.prod.outlook.com>
Hi Michael:
Thanks for your review.
On 3/19/2020 8:57 AM, Michael Kelley wrote:
> From: ltykernel@gmail.com <ltykernel@gmail.com> Sent: Tuesday, March 17, 2020 6:25 AM
>>
>> This patchset fixes some issues in the Hyper-V panic code path.
>> Patch 1 resolves issue that panic system still responses network
>> packets.
>> Patch 2-3 resolves crash enlightenment issues.
>> Patch 4 is to set crash_kexec_post_notifiers to true for Hyper-V
>> VM in order to report crash data or kmsg to host before running
>> kdump kernel.
>
> I still see an issue that isn't addressed by these patches. The VMbus
> driver registers a "die notifier" and a "panic notifier". But die() will
> eventually call panic() if panic_on_oops is set (which I think it typically
> is). If the CRASH_NOTIFY_MSG option is *not* enabled, then
> hyperv_report_panic() could get called by the die notifier, and then
> again by the panic notifier.
>
> Do we even need the "die notifier"? If it was removed, there would
> not be any notification to Hyper-V via the die() path unless panic_on_oops
> is set, which I think is actually the correct behavior. I'm not
> completely clear on what is supposed to happen in general to the
> Linux kernel if panic_on_oops is not set. Does it try to continue to run?
> If so, then we should not be notifying Hyper-V if panic_on_oops is not
> set, and removing the die notifier is the right thing to do.
>
hyperv_report_panic() has re-enter check inside and so kernel only
reports crash register data once during die(). From comment in the
hyperv_report_panic(), register value reported in die chain is more
exact than value in panic chain. The register value in die chain is
passed by die() caller. Register value reported in panic chain
is collected in the hyperv_panic_event().
If panic_on_oops is not set, the task should be killed and kernel
still runs. In this case, we may not trigger crash enlightenment.
> Michael
>
>>
>> Tianyu Lan (4):
>> x86/Hyper-V: Unload vmbus channel in hv panic callback
>> x86/Hyper-V: Free hv_panic_page when fail to register kmsg dump
>> x86/Hyper-V: Trigger crash enlightenment only once during system
>> crash.
>> x86/Hyper-V: Report crash register data or ksmg before running crash
>> kernel
>>
>> arch/x86/kernel/cpu/mshyperv.c | 10 ++++++++++
>> drivers/hv/channel_mgmt.c | 5 +++++
>> drivers/hv/vmbus_drv.c | 35 +++++++++++++++++++++++++----------
>> 3 files changed, 40 insertions(+), 10 deletions(-)
>>
>> --
>> 2.14.5
>
^ permalink raw reply
* RE: [PATCH 0/4] x86/Hyper-V: Unload vmbus channel in hv panic callback
From: Michael Kelley @ 2020-03-19 15:06 UTC (permalink / raw)
To: vkuznets, ltykernel@gmail.com
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, hpa@zytor.com, x86@kernel.org
In-Reply-To: <87mu8c22ky.fsf@vitty.brq.redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Thursday, March 19, 2020 1:04 AM
>
> Michael Kelley <mikelley@microsoft.com> writes:
>
> >> > --- a/drivers/hv/vmbus_drv.c
> >> > +++ b/drivers/hv/vmbus_drv.c
> >> > @@ -53,9 +53,12 @@ static int hyperv_panic_event(struct notifier_block *nb,
> unsigned
> >> long val,
> >> > {
> >> > struct pt_regs *regs;
> >> >
> >> > - regs = current_pt_regs();
> >> > + vmbus_initiate_unload(true);
> >> >
> >> > - hyperv_report_panic(regs, val);
> >> > + if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
> >>
> >> With Michael's effors to make code in drivers/hv arch agnostic, I think
> >> we need a better, arch-neutral way.
> >
> > Vitaly -- could you elaborate on what part is not arch-neutral? I don't see
> > a problem. ms_hyperv and the misc_features field exist for both the x86
> > and ARM64 code branches. It turns out the particular bit for
> > GUEST_CRASH_MSR_AVAILABLE is different on the two architectures, but
> > the compiler will do the right thing.
> >
>
> Ah, apologies, missed the fact that we also call it
> 'HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE' - I have to admit I was confused
> by the 'MSR' part. We can probably rename this to something like
> HV_FEATURE_GUEST_CRASH_REGS_AVAILABLE - but not as part of the series.
>
Good point. Agreed.
Michael
^ permalink raw reply
* RE: [PATCH 0/4] x86/Hyper-V: Panic code path fixes
From: Michael Kelley @ 2020-03-19 15:14 UTC (permalink / raw)
To: Tianyu Lan, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
hpa@zytor.com, x86@kernel.org
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <1d1bc90c-7fbe-6123-eeea-5f9a5aad77e4@gmail.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Thursday, March 19, 2020 7:08 AM
> >>
> >> This patchset fixes some issues in the Hyper-V panic code path.
> >> Patch 1 resolves issue that panic system still responses network
> >> packets.
> >> Patch 2-3 resolves crash enlightenment issues.
> >> Patch 4 is to set crash_kexec_post_notifiers to true for Hyper-V
> >> VM in order to report crash data or kmsg to host before running
> >> kdump kernel.
> >
> > I still see an issue that isn't addressed by these patches. The VMbus
> > driver registers a "die notifier" and a "panic notifier". But die() will
> > eventually call panic() if panic_on_oops is set (which I think it typically
> > is). If the CRASH_NOTIFY_MSG option is *not* enabled, then
> > hyperv_report_panic() could get called by the die notifier, and then
> > again by the panic notifier.
> >
> > Do we even need the "die notifier"? If it was removed, there would
> > not be any notification to Hyper-V via the die() path unless panic_on_oops
> > is set, which I think is actually the correct behavior. I'm not
> > completely clear on what is supposed to happen in general to the
> > Linux kernel if panic_on_oops is not set. Does it try to continue to run?
> > If so, then we should not be notifying Hyper-V if panic_on_oops is not
> > set, and removing the die notifier is the right thing to do.
> >
>
> hyperv_report_panic() has re-enter check inside and so kernel only
> reports crash register data once during die().
Ah, yes, you are right.
> From comment in the
> hyperv_report_panic(), register value reported in die chain is more
> exact than value in panic chain. The register value in die chain is
> passed by die() caller. Register value reported in panic chain
> is collected in the hyperv_panic_event().
>
> If panic_on_oops is not set, the task should be killed and kernel
> still runs. In this case, we may not trigger crash enlightenment.
I'm not completely clear on your last statement. It seems like there
is still a problem in that die() will call hyperv_report_panic() even if
panic_on_oops is not set. We will have reported a panic to Hyper-V
even though the VM did not stop running.
Michael
^ permalink raw reply
* RE: [PATCH 0/4] x86/Hyper-V: Panic code path fixes
From: Michael Kelley @ 2020-03-19 16:07 UTC (permalink / raw)
To: Michael Kelley, Tianyu Lan, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, hpa@zytor.com, x86@kernel.org
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <MW2PR2101MB10524F27F366005959A1007FD7F40@MW2PR2101MB1052.namprd21.prod.outlook.com>
From: Michael Kelley <mikelley@microsoft.com> Sent: Thursday, March 19, 2020 8:15 AM
>
> From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Thursday, March 19, 2020 7:08 AM
> > >>
> > >> This patchset fixes some issues in the Hyper-V panic code path.
> > >> Patch 1 resolves issue that panic system still responses network
> > >> packets.
> > >> Patch 2-3 resolves crash enlightenment issues.
> > >> Patch 4 is to set crash_kexec_post_notifiers to true for Hyper-V
> > >> VM in order to report crash data or kmsg to host before running
> > >> kdump kernel.
> > >
> > > I still see an issue that isn't addressed by these patches. The VMbus
> > > driver registers a "die notifier" and a "panic notifier". But die() will
> > > eventually call panic() if panic_on_oops is set (which I think it typically
> > > is). If the CRASH_NOTIFY_MSG option is *not* enabled, then
> > > hyperv_report_panic() could get called by the die notifier, and then
> > > again by the panic notifier.
> > >
> > > Do we even need the "die notifier"? If it was removed, there would
> > > not be any notification to Hyper-V via the die() path unless panic_on_oops
> > > is set, which I think is actually the correct behavior. I'm not
> > > completely clear on what is supposed to happen in general to the
> > > Linux kernel if panic_on_oops is not set. Does it try to continue to run?
> > > If so, then we should not be notifying Hyper-V if panic_on_oops is not
> > > set, and removing the die notifier is the right thing to do.
> > >
> >
> > hyperv_report_panic() has re-enter check inside and so kernel only
> > reports crash register data once during die().
>
> Ah, yes, you are right.
>
> > From comment in the
> > hyperv_report_panic(), register value reported in die chain is more
> > exact than value in panic chain. The register value in die chain is
> > passed by die() caller. Register value reported in panic chain
> > is collected in the hyperv_panic_event().
> >
> > If panic_on_oops is not set, the task should be killed and kernel
> > still runs. In this case, we may not trigger crash enlightenment.
>
> I'm not completely clear on your last statement. It seems like there
> is still a problem in that die() will call hyperv_report_panic() even if
> panic_on_oops is not set. We will have reported a panic to Hyper-V
> even though the VM did not stop running.
>
> Michael
There's one more issue to consider. hv_kmsg_dump() skips calling
hyperv_report_panic_msg() if sysctl_record_panic_msg has been cleared
by a sysctl command. (This sysctl option gives a customer the ability to
increase privacy by not having the VM's dmesg contents sent to Hyper-V.)
In this case, the earlier hyperv_report_panic() call should be used. Otherwise,
there would not be any notification to Hyper-V about the panic.
Michael
^ permalink raw reply
* Re: [PATCH v3 1/8] drivers/base/memory: rename MMOP_ONLINE_KEEP to MMOP_ONLINE
From: Pankaj Gupta @ 2020-03-19 16:28 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv, Wei Yang,
Greg Kroah-Hartman, Andrew Morton, Michal Hocko, Oscar Salvador,
Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-2-david@redhat.com>
> The name is misleading and it's not really clear what is "kept". Let's just
> name it like the online_type name we expose to user space ("online").
>
> Add some documentation to the types.
>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> drivers/base/memory.c | 9 +++++----
> include/linux/memory_hotplug.h | 6 +++++-
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 6448c9ece2cb..8c5ce42c0fc3 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -216,7 +216,7 @@ static int memory_subsys_online(struct device *dev)
> * attribute and need to set the online_type.
> */
> if (mem->online_type < 0)
> - mem->online_type = MMOP_ONLINE_KEEP;
> + mem->online_type = MMOP_ONLINE;
>
> ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
>
> @@ -251,7 +251,7 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
> else if (sysfs_streq(buf, "online_movable"))
> online_type = MMOP_ONLINE_MOVABLE;
> else if (sysfs_streq(buf, "online"))
> - online_type = MMOP_ONLINE_KEEP;
> + online_type = MMOP_ONLINE;
> else if (sysfs_streq(buf, "offline"))
> online_type = MMOP_OFFLINE;
> else {
> @@ -262,7 +262,7 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
> switch (online_type) {
> case MMOP_ONLINE_KERNEL:
> case MMOP_ONLINE_MOVABLE:
> - case MMOP_ONLINE_KEEP:
> + case MMOP_ONLINE:
> /* mem->online_type is protected by device_hotplug_lock */
> mem->online_type = online_type;
> ret = device_online(&mem->dev);
> @@ -342,7 +342,8 @@ static ssize_t valid_zones_show(struct device *dev,
> }
>
> nid = mem->nid;
> - default_zone = zone_for_pfn_range(MMOP_ONLINE_KEEP, nid, start_pfn, nr_pages);
> + default_zone = zone_for_pfn_range(MMOP_ONLINE, nid, start_pfn,
> + nr_pages);
> strcat(buf, default_zone->name);
>
> print_allowed_zone(buf, nid, start_pfn, nr_pages, MMOP_ONLINE_KERNEL,
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 3195d11876ea..3aaf00db224c 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -47,9 +47,13 @@ enum {
>
> /* Types for control the zone type of onlined and offlined memory */
> enum {
> + /* Offline the memory. */
> MMOP_OFFLINE = -1,
> - MMOP_ONLINE_KEEP,
> + /* Online the memory. Zone depends, see default_zone_for_pfn(). */
> + MMOP_ONLINE,
> + /* Online the memory to ZONE_NORMAL. */
> MMOP_ONLINE_KERNEL,
> + /* Online the memory to ZONE_MOVABLE. */
> MMOP_ONLINE_MOVABLE,
> };
>
> --
Looks good to me.
Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> 2.24.1
>
>
^ permalink raw reply
* Re: [PATCH v3 2/8] drivers/base/memory: map MMOP_OFFLINE to 0
From: Pankaj Gupta @ 2020-03-19 16:29 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv, Wei Yang,
Michal Hocko, Greg Kroah-Hartman, Andrew Morton, Michal Hocko,
Oscar Salvador, Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-3-david@redhat.com>
> Historically, we used the value -1. Just treat 0 as the special
> case now. Clarify a comment (which was wrong, when we come via
> device_online() the first time, the online_type would have been 0 /
> MEM_ONLINE). The default is now always MMOP_OFFLINE. This removes the
> last user of the manual "-1", which didn't use the enum value.
>
> This is a preparation to use the online_type as an array index.
>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> drivers/base/memory.c | 11 ++++-------
> include/linux/memory_hotplug.h | 2 +-
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 8c5ce42c0fc3..e7e77cafef80 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -211,17 +211,14 @@ static int memory_subsys_online(struct device *dev)
> return 0;
>
> /*
> - * If we are called from state_store(), online_type will be
> - * set >= 0 Otherwise we were called from the device online
> - * attribute and need to set the online_type.
> + * When called via device_online() without configuring the online_type,
> + * we want to default to MMOP_ONLINE.
> */
> - if (mem->online_type < 0)
> + if (mem->online_type == MMOP_OFFLINE)
> mem->online_type = MMOP_ONLINE;
>
> ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
> -
> - /* clear online_type */
> - mem->online_type = -1;
> + mem->online_type = MMOP_OFFLINE;
>
> return ret;
> }
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 3aaf00db224c..76f3c617a8ab 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -48,7 +48,7 @@ enum {
> /* Types for control the zone type of onlined and offlined memory */
> enum {
> /* Offline the memory. */
> - MMOP_OFFLINE = -1,
> + MMOP_OFFLINE = 0,
> /* Online the memory. Zone depends, see default_zone_for_pfn(). */
> MMOP_ONLINE,
> /* Online the memory to ZONE_NORMAL. */
> --
> 2.24.1
Looks good to me.
Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
>
>
^ permalink raw reply
* Re: [PATCH v3 3/8] drivers/base/memory: store mapping between MMOP_* and string in an array
From: Pankaj Gupta @ 2020-03-19 16:33 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv, Wei Yang,
Michal Hocko, Greg Kroah-Hartman, Andrew Morton, Michal Hocko,
Oscar Salvador, Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-4-david@redhat.com>
> Let's use a simple array which we can reuse soon. While at it, move the
> string->mmop conversion out of the device hotplug lock.
>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> drivers/base/memory.c | 38 +++++++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index e7e77cafef80..8a7f29c0bf97 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -28,6 +28,24 @@
>
> #define MEMORY_CLASS_NAME "memory"
>
> +static const char *const online_type_to_str[] = {
> + [MMOP_OFFLINE] = "offline",
> + [MMOP_ONLINE] = "online",
> + [MMOP_ONLINE_KERNEL] = "online_kernel",
> + [MMOP_ONLINE_MOVABLE] = "online_movable",
> +};
> +
> +static int memhp_online_type_from_str(const char *str)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(online_type_to_str); i++) {
> + if (sysfs_streq(str, online_type_to_str[i]))
> + return i;
> + }
> + return -EINVAL;
> +}
> +
> #define to_memory_block(dev) container_of(dev, struct memory_block, dev)
>
> static int sections_per_block;
> @@ -236,26 +254,17 @@ static int memory_subsys_offline(struct device *dev)
> static ssize_t state_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> + const int online_type = memhp_online_type_from_str(buf);
> struct memory_block *mem = to_memory_block(dev);
> - int ret, online_type;
> + int ret;
> +
> + if (online_type < 0)
> + return -EINVAL;
>
> ret = lock_device_hotplug_sysfs();
> if (ret)
> return ret;
>
> - if (sysfs_streq(buf, "online_kernel"))
> - online_type = MMOP_ONLINE_KERNEL;
> - else if (sysfs_streq(buf, "online_movable"))
> - online_type = MMOP_ONLINE_MOVABLE;
> - else if (sysfs_streq(buf, "online"))
> - online_type = MMOP_ONLINE;
> - else if (sysfs_streq(buf, "offline"))
> - online_type = MMOP_OFFLINE;
> - else {
> - ret = -EINVAL;
> - goto err;
> - }
> -
> switch (online_type) {
> case MMOP_ONLINE_KERNEL:
> case MMOP_ONLINE_MOVABLE:
> @@ -271,7 +280,6 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
> ret = -EINVAL; /* should never happen */
> }
>
> -err:
> unlock_device_hotplug();
>
> if (ret < 0)
> --
Nice cleanup patch.
Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> 2.24.1
>
>
^ permalink raw reply
* Re: [PATCH v3 6/8] mm/memory_hotplug: unexport memhp_auto_online
From: Pankaj Gupta @ 2020-03-19 16:52 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv, Wei Yang,
Michal Hocko, Andrew Morton, Michal Hocko, Oscar Salvador,
Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-7-david@redhat.com>
> All in-tree users except the mm-core are gone. Let's drop the export.
>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> mm/memory_hotplug.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index da6aab272c9b..e21a7d53ade5 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -71,7 +71,6 @@ bool memhp_auto_online;
> #else
> bool memhp_auto_online = true;
> #endif
> -EXPORT_SYMBOL_GPL(memhp_auto_online);
>
> static int __init setup_memhp_default_state(char *str)
> {
> --
> 2.24.1
Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
>
>
^ permalink raw reply
* Re: [PATCH v3 7/8] mm/memory_hotplug: convert memhp_auto_online to store an online_type
From: Pankaj Gupta @ 2020-03-19 17:19 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv, Wei Yang,
Michal Hocko, Greg Kroah-Hartman, Andrew Morton, Michal Hocko,
Oscar Salvador, Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-8-david@redhat.com>
> ... and rename it to memhp_default_online_type. This is a preparation
> for more detailed default online behavior.
>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> drivers/base/memory.c | 10 ++++------
> include/linux/memory_hotplug.h | 3 ++-
> mm/memory_hotplug.c | 11 ++++++-----
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 8a7f29c0bf97..8d3e16dab69f 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -386,10 +386,8 @@ static DEVICE_ATTR_RO(block_size_bytes);
> static ssize_t auto_online_blocks_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - if (memhp_auto_online)
> - return sprintf(buf, "online\n");
> - else
> - return sprintf(buf, "offline\n");
> + return sprintf(buf, "%s\n",
> + online_type_to_str[memhp_default_online_type]);
> }
>
> static ssize_t auto_online_blocks_store(struct device *dev,
> @@ -397,9 +395,9 @@ static ssize_t auto_online_blocks_store(struct device *dev,
> const char *buf, size_t count)
> {
> if (sysfs_streq(buf, "online"))
> - memhp_auto_online = true;
> + memhp_default_online_type = MMOP_ONLINE;
> else if (sysfs_streq(buf, "offline"))
> - memhp_auto_online = false;
> + memhp_default_online_type = MMOP_OFFLINE;
> else
> return -EINVAL;
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 76f3c617a8ab..6d6f85bb66e9 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -118,7 +118,8 @@ extern int arch_add_memory(int nid, u64 start, u64 size,
> struct mhp_params *params);
> extern u64 max_mem_size;
>
> -extern bool memhp_auto_online;
> +/* Default online_type (MMOP_*) when new memory blocks are added. */
> +extern int memhp_default_online_type;
> /* If movable_node boot option specified */
> extern bool movable_node_enabled;
> static inline bool movable_node_is_enabled(void)
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e21a7d53ade5..4efcf8cb9ac5 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -67,17 +67,17 @@ void put_online_mems(void)
> bool movable_node_enabled = false;
>
> #ifndef CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE
> -bool memhp_auto_online;
> +int memhp_default_online_type = MMOP_OFFLINE;
> #else
> -bool memhp_auto_online = true;
> +int memhp_default_online_type = MMOP_ONLINE;
> #endif
>
> static int __init setup_memhp_default_state(char *str)
> {
> if (!strcmp(str, "online"))
> - memhp_auto_online = true;
> + memhp_default_online_type = MMOP_ONLINE;
> else if (!strcmp(str, "offline"))
> - memhp_auto_online = false;
> + memhp_default_online_type = MMOP_OFFLINE;
>
> return 1;
> }
> @@ -993,6 +993,7 @@ static int check_hotplug_memory_range(u64 start, u64 size)
>
> static int online_memory_block(struct memory_block *mem, void *arg)
> {
> + mem->online_type = memhp_default_online_type;
> return device_online(&mem->dev);
> }
>
> @@ -1065,7 +1066,7 @@ int __ref add_memory_resource(int nid, struct resource *res)
> mem_hotplug_done();
>
> /* online pages if requested */
> - if (memhp_auto_online)
> + if (memhp_default_online_type != MMOP_OFFLINE)
> walk_memory_blocks(start, size, NULL, online_memory_block);
>
> return ret;
> --
Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> 2.24.1
>
>
^ permalink raw reply
* Re: [PATCH v3 8/8] mm/memory_hotplug: allow to specify a default online_type
From: Pankaj Gupta @ 2020-03-19 17:26 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linuxppc-dev, linux-hyperv, Wei Yang,
Michal Hocko, Greg Kroah-Hartman, Andrew Morton, Michal Hocko,
Oscar Salvador, Rafael J. Wysocki, Baoquan He
In-Reply-To: <20200319131221.14044-9-david@redhat.com>
> For now, distributions implement advanced udev rules to essentially
> - Don't online any hotplugged memory (s390x)
> - Online all memory to ZONE_NORMAL (e.g., most virt environments like
> hyperv)
> - Online all memory to ZONE_MOVABLE in case the zone imbalance is taken
> care of (e.g., bare metal, special virt environments)
>
> In summary: All memory is usually onlined the same way, however, the
> kernel always has to ask user space to come up with the same answer.
> E.g., Hyper-V always waits for a memory block to get onlined before
> continuing, otherwise it might end up adding memory faster than
> onlining it, which can result in strange OOM situations. This waiting
> slows down adding of a bigger amount of memory.
>
> Let's allow to specify a default online_type, not just "online" and
> "offline". This allows distributions to configure the default online_type
> when booting up and be done with it.
>
> We can now specify "offline", "online", "online_movable" and
> "online_kernel" via
> - "memhp_default_state=" on the kernel cmdline
> - /sys/devices/system/memory/auto_online_blocks
> just like we are able to specify for a single memory block via
> /sys/devices/system/memory/memoryX/state
>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> drivers/base/memory.c | 11 +++++------
> include/linux/memory_hotplug.h | 2 ++
> mm/memory_hotplug.c | 8 ++++----
> 3 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 8d3e16dab69f..2b09b68b9f78 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -35,7 +35,7 @@ static const char *const online_type_to_str[] = {
> [MMOP_ONLINE_MOVABLE] = "online_movable",
> };
>
> -static int memhp_online_type_from_str(const char *str)
> +int memhp_online_type_from_str(const char *str)
> {
> int i;
>
> @@ -394,13 +394,12 @@ static ssize_t auto_online_blocks_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - if (sysfs_streq(buf, "online"))
> - memhp_default_online_type = MMOP_ONLINE;
> - else if (sysfs_streq(buf, "offline"))
> - memhp_default_online_type = MMOP_OFFLINE;
> - else
> + const int online_type = memhp_online_type_from_str(buf);
> +
> + if (online_type < 0)
> return -EINVAL;
>
> + memhp_default_online_type = online_type;
> return count;
> }
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 6d6f85bb66e9..93d9ada74ddd 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -118,6 +118,8 @@ extern int arch_add_memory(int nid, u64 start, u64 size,
> struct mhp_params *params);
> extern u64 max_mem_size;
>
> +extern int memhp_online_type_from_str(const char *str);
> +
> /* Default online_type (MMOP_*) when new memory blocks are added. */
> extern int memhp_default_online_type;
> /* If movable_node boot option specified */
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 4efcf8cb9ac5..89197163d138 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -74,10 +74,10 @@ int memhp_default_online_type = MMOP_ONLINE;
>
> static int __init setup_memhp_default_state(char *str)
> {
> - if (!strcmp(str, "online"))
> - memhp_default_online_type = MMOP_ONLINE;
> - else if (!strcmp(str, "offline"))
> - memhp_default_online_type = MMOP_OFFLINE;
> + const int online_type = memhp_online_type_from_str(str);
> +
> + if (online_type >= 0)
> + memhp_default_online_type = online_type;
>
> return 1;
> }
> --
Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> 2.24.1
>
>
^ permalink raw reply
* RE: [PATCH v8 2/5] x86/hyper-v: Add synthetic debugger definitions
From: Michael Kelley @ 2020-03-19 20:49 UTC (permalink / raw)
To: Jon Doron, kvm@vger.kernel.org, linux-hyperv@vger.kernel.org; +Cc: vkuznets
In-Reply-To: <20200319063836.678562-3-arilou@gmail.com>
From: Jon Doron <arilou@gmail.com> Sent: Wednesday, March 18, 2020 11:39 PM
>
> Hyper-V synthetic debugger has two modes, one that uses MSRs and
> the other that use Hypercalls.
>
> Add all the required definitions to both types of synthetic debugger
> interface.
>
> Some of the required new CPUIDs and MSRs are not documented in the TLFS
> so they are in hyperv.h instead.
>
> The reason they are not documented is because they are subjected to be
> removed in future versions of Windows.
>
> Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> ---
> arch/x86/include/asm/hyperv-tlfs.h | 6 ++++++
> arch/x86/kvm/hyperv.h | 27 +++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index 92abc1e42bfc..671ce2a39d4b 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -131,6 +131,8 @@
> #define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE BIT(8)
> /* Crash MSR available */
> #define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(10)
> +/* Support for debug MSRs available */
> +#define HV_FEATURE_DEBUG_MSRS_AVAILABLE BIT(11)
> /* stimer Direct Mode is available */
> #define HV_STIMER_DIRECT_MODE_AVAILABLE BIT(19)
>
> @@ -376,6 +378,9 @@ struct hv_tsc_emulation_status {
> #define HVCALL_SEND_IPI_EX 0x0015
> #define HVCALL_POST_MESSAGE 0x005c
> #define HVCALL_SIGNAL_EVENT 0x005d
> +#define HVCALL_POST_DEBUG_DATA 0x0069
> +#define HVCALL_RETRIEVE_DEBUG_DATA 0x006a
> +#define HVCALL_RESET_DEBUG_SESSION 0x006b
> #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
> #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
>
> @@ -419,6 +424,7 @@ enum HV_GENERIC_SET_FORMAT {
> #define HV_STATUS_INVALID_HYPERCALL_INPUT 3
> #define HV_STATUS_INVALID_ALIGNMENT 4
> #define HV_STATUS_INVALID_PARAMETER 5
> +#define HV_STATUS_OPERATION_DENIED 8
> #define HV_STATUS_INSUFFICIENT_MEMORY 11
> #define HV_STATUS_INVALID_PORT_ID 17
> #define HV_STATUS_INVALID_CONNECTION_ID 18
> diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
> index 757cb578101c..5e4780bf6dd7 100644
> --- a/arch/x86/kvm/hyperv.h
> +++ b/arch/x86/kvm/hyperv.h
> @@ -23,6 +23,33 @@
>
> #include <linux/kvm_host.h>
>
> +/*
> + * The #defines related to the synthetic debugger are required by KDNet, but
> + * they are not documented in the Hyper-V TLFS because the synthetic debugger
> + * functionality has been deprecated and is subject to removal in future versions
> + * of Windows.
> + */
> +#define HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
> +#define HYPERV_CPUID_SYNDBG_INTERFACE 0x40000081
> +#define HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
> +
> +/*
> + * Hyper-V synthetic debugger platform capabilities
> + * These are HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES.EAX bits.
> + */
> +#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING BIT(1)
> +
> +/* Hyper-V Synthetic debug options MSR */
> +#define HV_X64_MSR_SYNDBG_CONTROL 0x400000F1
> +#define HV_X64_MSR_SYNDBG_STATUS 0x400000F2
> +#define HV_X64_MSR_SYNDBG_SEND_BUFFER 0x400000F3
> +#define HV_X64_MSR_SYNDBG_RECV_BUFFER 0x400000F4
> +#define HV_X64_MSR_SYNDBG_PENDING_BUFFER 0x400000F5
> +#define HV_X64_MSR_SYNDBG_OPTIONS 0x400000FF
> +
> +/* Hyper-V HV_X64_MSR_SYNDBG_OPTIONS bits */
> +#define HV_X64_SYNDBG_OPTION_USE_HCALLS BIT(2)
> +
> static inline struct kvm_vcpu_hv *vcpu_to_hv_vcpu(struct kvm_vcpu *vcpu)
> {
> return &vcpu->arch.hyperv;
> --
> 2.24.1
^ permalink raw reply
* RE: [PATCH v6 03/10] arm64: hyperv: Add hypercall and register access functions
From: Michael Kelley @ 2020-03-19 21:04 UTC (permalink / raw)
To: Hillf Danton
Cc: will@kernel.org, ardb@kernel.org, arnd@arndb.de,
catalin.marinas@arm.com, mark.rutland@arm.com, maz@kernel.org,
linux-arm-kernel@lists.infradead.org, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-efi@vger.kernel.org, linux-arch@vger.kernel.org,
olaf@aepfle.de, apw@canonical.com, vkuznets, jasowang@redhat.com,
marcelo.cerri@canonical.com, KY Srinivasan, Sunil Muthuswamy,
boqun.feng@gmail.com
In-Reply-To: <20200318031130.5476-1-hdanton@sina.com>
From: Hillf Danton <hdanton@sina.com> Sent: Tuesday, March 17, 2020 8:12 PM
>
> On Sat, 14 Mar 2020 08:35:12 -0700 Michael Kelley wrote:
> > +/*
> > + * Get the value of a single VP register. One version
> > + * returns just 64 bits and another returns the full 128 bits.
> > + * The two versions are separate to avoid complicating the
> > + * calling sequence for the more frequently used 64 bit version.
> > + */
> > +
> > +/*
> > + * Input and output memory allocation sizes are rounded up to a power
> > + * of 2 so kmalloc() will guarantee alignment. In turn, the alignment
> > + * ensures that the allocations don't cross a page boundary, which is
>
> Better to specify kmalloc's current alignment and why it fails to ensure
> (4 * sizeof(u64))-sized allocations wont cross page boundary.
>
Is your comment referring to ARCH_KMALLOC_MINALIGN? If so, I see
how that makes sense. BUILD_BUG_ON (sizeof (*input) >
ARCH_KMALLOC_MINALIGN) would be a cleaner solution.
Thanks,
Michael
> > + * required by the hypercall interface.
> > + */
> > +#define INPUTSIZE (4 * sizeof(u64))
> > +#define OUTPUTSIZE (2 * sizeof(u64))
> > +
> > +static void __hv_get_vpreg_128(u32 msr, struct hv_get_vp_register_output *res)
> > +{
> > + union hv_hypercall_status status;
> > + struct hv_get_vp_register_input *input;
> > +
> > + BUILD_BUG_ON(sizeof(*input) > INPUTSIZE);
> > +
> > + input = kzalloc(INPUTSIZE, GFP_ATOMIC);
> > +
> > + input->partitionid = HV_PARTITION_ID_SELF;
> > + input->vpindex = HV_VP_INDEX_SELF;
> > + input->inputvtl = 0;
> > + input->name0 = msr;
> > + input->name1 = 0;
> > +
> > +
> > + status.as_uint64 = hv_do_hypercall(
> > + HVCALL_GET_VP_REGISTERS | HV_HYPERCALL_REP_COUNT_1,
> > + input, res);
> > +
> > + /*
> > + * Something is fundamentally broken in the hypervisor if
> > + * getting a VP register fails. There's really no way to
> > + * continue as a guest VM, so panic.
> > + */
> > + BUG_ON(status.status != HV_STATUS_SUCCESS);
> > +
> > + kfree(input);
> > +}
^ permalink raw reply
* RE: [PATCH v6 01/10] arm64: hyperv: Add core Hyper-V include files
From: Michael Kelley @ 2020-03-19 21:31 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Will Deacon, Ard Biesheuvel, Catalin Marinas, Mark Rutland,
Marc Zyngier, Linux ARM, gregkh, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-efi, linux-arch,
olaf@aepfle.de, Andy Whitcroft, vkuznets, Jason Wang,
marcelo.cerri@canonical.com, KY Srinivasan, Sunil Muthuswamy,
Boqun Feng
In-Reply-To: <CAK8P3a02EULGxyuKFq8YnbG8BQ_m-RKciaNEc9ZbdP2yz9dt+Q@mail.gmail.com>
From: Arnd Bergmann <arnd@arndb.de> Sent: Wednesday, March 18, 2020 3:10 AM
>
> On Wed, Mar 18, 2020 at 1:12 AM Michael Kelley <mikelley@microsoft.com> wrote:
> > From: Arnd Bergmann <arnd@arndb.de> Sent: Monday, March 16, 2020 1:48 AM
> > > On Sat, Mar 14, 2020 at 4:36 PM Michael Kelley <mikelley@microsoft.com> wrote:
> > >
> > > > +
> > > > +/* Define input and output layout for Get VP Register hypercall */
> > > > +struct hv_get_vp_register_input {
> > > > + u64 partitionid;
> > > > + u32 vpindex;
> > > > + u8 inputvtl;
> > > > + u8 padding[3];
> > > > + u32 name0;
> > > > + u32 name1;
> > > > +} __packed;
> > >
> > > Are you sure these need to be made byte-aligned according to the
> > > specification? If the structure itself is aligned to 64 bit, better mark only
> > > the individual fields that are misaligned as __packed.
> > >
> > > If the structure is aligned to only 32-bit addresses instead of
> > > 64-bit, mark it as "__packed __aligned(4)" to let the compiler
> > > generate better code for accessing it.
> >
> > None of the fields are misaligned and it will always be aligned to 64-bit
> > addresses, so there should be no padding needed or added. There was
> > a discussion of __packed and the Hyper-V data structures in general on
> > LKML here: https://lkml.org/lkml/2018/11/30/848 Adding __packed was
> > done as a preventative measure, not because anything was actually
> > broken. Marking as __aligned(8) here would indicate the correct intent,
> > though the use of the structure always ensures 64-bit alignment.
>
> Just drop the __packed annotations then, they just confuse the compiler
> in this case. In particular, when the compiler thinks that a structure is
> misaligned, it tries to avoid using load/store instructions on it that are
> inefficient or trap with misaligned code, so having default alignment
> produces better object code.
So I'm confused a bit. Were the original concerns in the above LKML
discussion bogus? Is it legal for the compiler to reorder fields or add
padding, even if the layout of fields in the structure doesn't require it?
If the compiler *could* do such, then it seems like keeping the __packed
would be appropriate per the LKML discussion.
>
> > > Also, in order to write portable code, it would be helpful to mark
> > > all the fields as explicitly little-endian, and use __le32_to_cpu()
> > > etc for accessing them.
> >
> > There's an opening comment in this file stating that all data
> > structures shared between Hyper-V and a guest VM are little
> > endian. Is there some other marking to consider using?
>
> Yes, device drivers should generally define data structures using
> the __le32, __le64 etc types, and use the conversion functions
> to access them. Building with 'make C=1' usually tells you when
> you have mismatched annotations.
>
> > We have definitely not allowed for the case of Hyper-V running on
> > a big endian architecture. There are a *lot* of messages and data
> > structures passed between the guest and Hyper-V, and coding
> > to handle either endianness is a big project. I'm doubtful
> > of the value until and unless we actually have a need for it.
>
> In general, the use of big-endian software on Linux is declining, however
>
> - arm64 as an architecture is meant to support both endian types,
> and we still try to ensure it works either way as long as there are
> users that depend on it.
>
> - The remaining users of big-endian software are probably
> more likely to run on virtual machines than on real hardware
>
> - Any device driver should generally be written against portable
> interfaces, even if you think you know how it will be used. As
> driver writers tend to look at existing code for new drivers, it's
> better to have them all be portable. (This is a similar argument
> to the irqchip interface).
>
> Even if you don't convert any of the existing architecture independent
> code to run both ways, I see no reason to not do it for new drivers.
OK, let me look into this. Given how the major Linux distros on
ARM64 have all gone little-endian, I'm a bit skeptical of the value
for the big server environments in which Hyper-V would be used.
>
> > > > +/* Define synthetic interrupt controller message flags. */
> > > > +union hv_message_flags {
> > > > + __u8 asu8;
> > > > + struct {
> > > > + __u8 msg_pending:1;
> > > > + __u8 reserved:7;
> > > > + } __packed;
> > > > +};
> > >
> > > For similar reasons, please avoid bit fields and just use a
> > > bit mask on the first member of the union.
> >
> > Unfortunately, changing to a bit mask ripples into
> > architecture independent code and into the x86
> > implementation. I'd prefer not to drag that complexity
> > into this patch set.
>
> How so? If this file is arm64 specific, there should be no need to make
> x86 do the same change.
This file, hyperv-tlfs.h, is duplicating some definitions on the x86 and
ARM64 sides that are used by arch independent code, and this is one
of those definitions. I had held off on breaking the file into arch
independent and arch specific portions because the Hyper-V team has
left some gray areas for functionality that isn't yet used on the ARM64
side. So in some cases, it's hard to know what functionality to put
into the arch independent portion.
But I think I'll go ahead and make the separation with reasonably good
accuracy, and update the x86 side accordingly. That will reduce the size
of this patch set to contain only the things that we know are ARM64
specific and which are actually used by the ARM64 code. Things like the
hv_message_flags will go into the arch independent portion so that
they can be used by the arch independent code without cluttering up
the arch specific code. Making the change will help reduce any
confusion about what is ARM64-specific. The other core #include file,
mshyperv.h, has already been done this way.
Michael
>
> > > > + * Use the Hyper-V provided stimer0 as the timer that is made
> > > > + * available to the architecture independent Hyper-V drivers.
> > > > + */
> > > > +#define hv_init_timer(timer, tick) \
> > > > + hv_set_vpreg(HV_REGISTER_STIMER0_COUNT + (2*timer), tick)
> > > > +#define hv_init_timer_config(timer, val) \
> > > > + hv_set_vpreg(HV_REGISTER_STIMER0_CONFIG + (2*timer), val)
> > > > +#define hv_get_current_tick(tick) \
> > > > + (tick = hv_get_vpreg(HV_REGISTER_TIME_REFCOUNT))
> > >
> > > In general, we prefer inline functions over macros in header files.
> >
> > I can change the "set" calls to inline functions. As you can see, the "get"
> > functions are coded and used in architecture independent code and on
> > the x86 side in a way that won't convert to inline functions.
>
> Ok.
>
> Arnd
^ 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