From: Michael Kelley <hidden> Date: 2021-03-01 01:17:06
To support Linux guests on Hyper-V on multiple architectures, the original
approach factored out all differences between Hyper-V on x86/x64 and
Hyper-V on ARM64 into functions or #defines under arch/x86 and
arch/arm64. Some of these differences are truly related to the
architecture, but others are more properly treated as Linux OS
differences or just quirks in Hyper-V. Feedback from Arnd Bergmann[1]
recommended that differences other than architecture should be
incorporated into the architecture independent Hyper-V code. Each
difference can be handled with conditions specific to the difference
instead of tying it to the broader x86/x64 vs. ARM64. This approach
reduces the amount of code under arch/x86 and arch/arm64 and keeps
the non-architectural differences localized and more easily understood.
This patch set implements the new approach by changing the interface
between the architecture independent code and the architecture dependent
code for x86/x64. The patches move code from arch/x86 to the
architecture independent Hyper-V code whenever possible, and add
architecture independent support needed by other architectures like
ARM64. No functionality is changed for x86/x64. A subsequent patch
set will provide the Hyper-V support code under arch/arm64.
This patch set results in an increase in lines of code (though some
of the increase is additional comments). But the lines needed under
arch/arm64 in the upcoming patch set is significantly reduced, resulting
in a net decrease of about 125 lines.
[1] https://lore.kernel.org/lkml/CAK8P3a1hDBVembCd+6=ENUWYFz=72JBTFMrKYZ2aFd+_Q04F+g@mail.gmail.com/
Changes in v2:
* In patch 9/10, for consistency change the rating on hyperv_cs_msr
along with the rating on hyperv_cs_tsc [Boqun Feng]
* In patch 10/10, add missing call to hv_remove_stimer0_handler()
[Boqun Feng]
* In patch 10/10, add clarifying comment that hv_setup_stimer0_irq()
is not used on x86/x64 [Wei Liu]
Michael Kelley (10):
Drivers: hv: vmbus: Move Hyper-V page allocator to arch neutral code
x86/hyper-v: Move hv_message_type to architecture neutral module
Drivers: hv: Redo Hyper-V synthetic MSR get/set functions
Drivers: hv: vmbus: Move hyperv_report_panic_msg to arch neutral code
Drivers: hv: vmbus: Handle auto EOI quirk inline
Drivers: hv: vmbus: Move handling of VMbus interrupts
clocksource/drivers/hyper-v: Handle vDSO differences inline
clocksource/drivers/hyper-v: Handle sched_clock differences inline
clocksource/drivers/hyper-v: Set clocksource rating based on Hyper-V
feature
clocksource/drivers/hyper-v: Move handling of STIMER0 interrupts
arch/x86/hyperv/hv_init.c | 53 +-------
arch/x86/include/asm/hyperv-tlfs.h | 131 +++++++++----------
arch/x86/include/asm/mshyperv.h | 67 ++--------
arch/x86/kernel/cpu/mshyperv.c | 23 +---
drivers/clocksource/hyperv_timer.c | 258 ++++++++++++++++++++++++++-----------
drivers/hv/hv.c | 93 ++++++++++---
drivers/hv/vmbus_drv.c | 89 +++++++++++--
include/asm-generic/hyperv-tlfs.h | 35 +++++
include/asm-generic/mshyperv.h | 19 ++-
include/clocksource/hyperv_timer.h | 3 +-
10 files changed, 458 insertions(+), 313 deletions(-)
--
1.8.3.1
From: Michael Kelley <hidden> Date: 2021-03-01 01:17:07
The Hyper-V page allocator functions are implemented in an architecture
neutral way. Move them into the architecture neutral VMbus module so
a separate implementation for ARM64 is not needed.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
arch/x86/hyperv/hv_init.c | 22 ----------------------
arch/x86/include/asm/mshyperv.h | 5 -----
drivers/hv/hv.c | 36 ++++++++++++++++++++++++++++++++++++
include/asm-generic/mshyperv.h | 4 ++++
4 files changed, 40 insertions(+), 27 deletions(-)
From: Michael Kelley <hidden> Date: 2021-03-01 01:17:55
Current code defines a separate get and set macro for each Hyper-V
synthetic MSR used by the VMbus driver. Furthermore, the get macro
can't be converted to a standard function because the second argument
is modified in place, which is somewhat bad form.
Redo this by providing a single get and a single set function that
take a parameter specifying the MSR to be operated on. Fixup usage
of the get function. Calling locations are no more complex than before,
but the code under arch/x86 and the upcoming code under arch/arm64
is significantly simplified.
Also standardize the names of Hyper-V synthetic MSRs that are
architecture neutral. But keep the old x86-specific names as aliases
that can be removed later when all references (particularly in KVM
code) have been cleaned up in a separate patch series.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
arch/x86/hyperv/hv_init.c | 2 +-
arch/x86/include/asm/hyperv-tlfs.h | 102 +++++++++++++++++++++++--------------
arch/x86/include/asm/mshyperv.h | 39 ++++----------
drivers/clocksource/hyperv_timer.c | 26 +++++-----
drivers/hv/hv.c | 37 ++++++++------
drivers/hv/vmbus_drv.c | 2 +-
include/asm-generic/mshyperv.h | 2 +-
7 files changed, 110 insertions(+), 100 deletions(-)
@@ -75,7 +75,7 @@ static int hv_cpu_init(unsigned int cpu)*output_arg=page_address(pg+1);}-hv_get_vp_index(msr_vp_index);+msr_vp_index=hv_get_register(HV_REGISTER_VP_INDEX);hv_vp_index[smp_processor_id()]=msr_vp_index;
@@ -156,7 +156,7 @@ enum hv_isolation_type {#define HV_X64_MSR_HYPERCALL 0x40000001/* MSR used to provide vcpu index */-#define HV_X64_MSR_VP_INDEX 0x40000002+#define HV_REGISTER_VP_INDEX 0x40000002/* MSR used to reset the guest OS. */#define HV_X64_MSR_RESET 0x40000003
@@ -165,10 +165,10 @@ enum hv_isolation_type {#define HV_X64_MSR_VP_RUNTIME 0x40000010/* MSR used to read the per-partition time reference counter */-#define HV_X64_MSR_TIME_REF_COUNT 0x40000020+#define HV_REGISTER_TIME_REF_COUNT 0x40000020/* A partition's reference time stamp counter (TSC) page */-#define HV_X64_MSR_REFERENCE_TSC 0x40000021+#define HV_REGISTER_REFERENCE_TSC 0x40000021/* MSR used to retrieve the TSC frequency */#define HV_X64_MSR_TSC_FREQUENCY 0x40000022
@@ -68,14 +68,14 @@ static int hv_ce_set_next_event(unsigned long delta,current_tick=hv_read_reference_counter();current_tick+=delta;-hv_init_timer(0,current_tick);+hv_set_register(HV_REGISTER_STIMER0_COUNT,current_tick);return0;}staticinthv_ce_shutdown(structclock_event_device*evt){-hv_init_timer(0,0);-hv_init_timer_config(0,0);+hv_set_register(HV_REGISTER_STIMER0_COUNT,0);+hv_set_register(HV_REGISTER_STIMER0_CONFIG,0);if(direct_mode_enabled)hv_disable_stimer0_percpu_irq(stimer0_irq);
@@ -105,7 +105,7 @@ static int hv_ce_set_oneshot(struct clock_event_device *evt)timer_cfg.direct_mode=0;timer_cfg.sintx=stimer0_message_sint;}-hv_init_timer_config(0,timer_cfg.as_uint64);+hv_set_register(HV_REGISTER_STIMER0_CONFIG,timer_cfg.as_uint64);return0;}
@@ -198,34 +198,36 @@ void hv_synic_enable_regs(unsigned int cpu)unionhv_synic_scontrolsctrl;/* Setup the Synic's message page */-hv_get_simp(simp.as_uint64);+simp.as_uint64=hv_get_register(HV_REGISTER_SIMP);simp.simp_enabled=1;simp.base_simp_gpa=virt_to_phys(hv_cpu->synic_message_page)>>HV_HYP_PAGE_SHIFT;-hv_set_simp(simp.as_uint64);+hv_set_register(HV_REGISTER_SIMP,simp.as_uint64);/* Setup the Synic's event page */-hv_get_siefp(siefp.as_uint64);+siefp.as_uint64=hv_get_register(HV_REGISTER_SIEFP);siefp.siefp_enabled=1;siefp.base_siefp_gpa=virt_to_phys(hv_cpu->synic_event_page)>>HV_HYP_PAGE_SHIFT;-hv_set_siefp(siefp.as_uint64);+hv_set_register(HV_REGISTER_SIEFP,siefp.as_uint64);/* Setup the shared SINT. */-hv_get_synint_state(VMBUS_MESSAGE_SINT,shared_sint.as_uint64);+shared_sint.as_uint64=hv_get_register(HV_REGISTER_SINT0++VMBUS_MESSAGE_SINT);shared_sint.vector=hv_get_vector();shared_sint.masked=false;shared_sint.auto_eoi=hv_recommend_using_aeoi();-hv_set_synint_state(VMBUS_MESSAGE_SINT,shared_sint.as_uint64);+hv_set_register(HV_REGISTER_SINT0+VMBUS_MESSAGE_SINT,+shared_sint.as_uint64);/* Enable the global synic bit */-hv_get_synic_state(sctrl.as_uint64);+sctrl.as_uint64=hv_get_register(HV_REGISTER_SCONTROL);sctrl.enable=1;-hv_set_synic_state(sctrl.as_uint64);+hv_set_register(HV_REGISTER_SCONTROL,sctrl.as_uint64);}inthv_synic_init(unsignedintcpu)
@@ -247,32 +249,35 @@ void hv_synic_disable_regs(unsigned int cpu)unionhv_synic_siefpsiefp;unionhv_synic_scontrolsctrl;-hv_get_synint_state(VMBUS_MESSAGE_SINT,shared_sint.as_uint64);+shared_sint.as_uint64=hv_get_register(HV_REGISTER_SINT0++VMBUS_MESSAGE_SINT);shared_sint.masked=1;/* Need to correctly cleanup in the case of SMP!!! *//* Disable the interrupt */-hv_set_synint_state(VMBUS_MESSAGE_SINT,shared_sint.as_uint64);+hv_set_register(HV_REGISTER_SINT0+VMBUS_MESSAGE_SINT,+shared_sint.as_uint64);-hv_get_simp(simp.as_uint64);+simp.as_uint64=hv_get_register(HV_REGISTER_SIMP);simp.simp_enabled=0;simp.base_simp_gpa=0;-hv_set_simp(simp.as_uint64);+hv_set_register(HV_REGISTER_SIMP,simp.as_uint64);-hv_get_siefp(siefp.as_uint64);+siefp.as_uint64=hv_get_register(HV_REGISTER_SIEFP);siefp.siefp_enabled=0;siefp.base_siefp_gpa=0;-hv_set_siefp(siefp.as_uint64);+hv_set_register(HV_REGISTER_SIEFP,siefp.as_uint64);/* Disable the global synic bit */-hv_get_synic_state(sctrl.as_uint64);+sctrl.as_uint64=hv_get_register(HV_REGISTER_SCONTROL);sctrl.enable=0;-hv_set_synic_state(sctrl.as_uint64);+hv_set_register(HV_REGISTER_SCONTROL,sctrl.as_uint64);}+inthv_synic_cleanup(unsignedintcpu){structvmbus_channel*channel,*sc;
From: Michael Kelley <hidden> Date: 2021-03-01 01:18:28
With the new Hyper-V MSR set function, hyperv_report_panic_msg() can be
architecture neutral, so move it out from under arch/x86 and merge into
hv_kmsg_dump(). This move also avoids needing a separate implementation
under arch/arm64.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
arch/x86/hyperv/hv_init.c | 27 ---------------------------
drivers/hv/vmbus_drv.c | 24 +++++++++++++++++++-----
include/asm-generic/mshyperv.h | 1 -
3 files changed, 19 insertions(+), 33 deletions(-)
@@ -1392,22 +1392,36 @@ static void hv_kmsg_dump(struct kmsg_dumper *dumper,enumkmsg_dump_reasonreason){size_tbytes_written;-phys_addr_tpanic_pa;/* We are only interested in panics. */if((reason!=KMSG_DUMP_PANIC)||(!sysctl_record_panic_msg))return;-panic_pa=virt_to_phys(hv_panic_page);-/**Writedumpcontentstothepage.Noneedtosynchronize;panicshould*besingle-threaded.*/kmsg_dump_get_buffer(dumper,false,hv_panic_page,HV_HYP_PAGE_SIZE,&bytes_written);-if(bytes_written)-hyperv_report_panic_msg(panic_pa,bytes_written);+if(!bytes_written)+return;+/*+*P3tocontainthephysicaladdressofthepanicpage&P4to+*containthesizeofthepanicdatainthatpage.Restofthe+*registersareno-opwhentheNOTIFY_MSGflagisset.+*/+hv_set_register(HV_REGISTER_CRASH_P0,0);+hv_set_register(HV_REGISTER_CRASH_P1,0);+hv_set_register(HV_REGISTER_CRASH_P2,0);+hv_set_register(HV_REGISTER_CRASH_P3,virt_to_phys(hv_panic_page));+hv_set_register(HV_REGISTER_CRASH_P4,bytes_written);++/*+*LetHyper-Vknowthereiscrashdataavailablealongwith+*thepanicmessage.+*/+hv_set_register(HV_REGISTER_CRASH_CTL,+(HV_CRASH_CTL_CRASH_NOTIFY|HV_CRASH_CTL_CRASH_NOTIFY_MSG));}staticstructkmsg_dumperhv_kmsg_dumper={
From: Michael Kelley <hidden> Date: 2021-03-01 01:18:45
The definition of enum hv_message_type includes arch neutral and
x86/x64-specific values. Ideally there would be a way to put the
arch neutral values in an arch neutral module, and the arch
specific values in an arch specific module. But C doesn't provide
a way to extend enum types. As a compromise, move the entire
definition into an arch neutral module, to avoid duplicating the
arch neutral values for x86/x64 and for ARM64.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
arch/x86/include/asm/hyperv-tlfs.h | 29 -----------------------------
include/asm-generic/hyperv-tlfs.h | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+), 29 deletions(-)
From: Michael Kelley <hidden> Date: 2021-03-01 01:18:58
On x86/x64, Hyper-V provides a flag to indicate auto EOI functionality,
but it doesn't on ARM64. Handle this quirk inline instead of calling
into code under arch/x86 (and coming, under arch/arm64).
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
arch/x86/include/asm/mshyperv.h | 3 ---
drivers/hv/hv.c | 12 +++++++++++-
2 files changed, 11 insertions(+), 4 deletions(-)
From: Michael Kelley <hidden> Date: 2021-03-01 01:19:45
VMbus interrupts are most naturally modelled as per-cpu IRQs. But
because x86/x64 doesn't have per-cpu IRQs, the core VMbus interrupt
handling machinery is done in code under arch/x86 and Linux IRQs are
not used. Adding support for ARM64 means adding equivalent code
using per-cpu IRQs under arch/arm64.
A better model is to treat per-cpu IRQs as the normal path (which it is
for modern architectures), and the x86/x64 path as the exception. Do this
by incorporating standard Linux per-cpu IRQ allocation into the main VMbus
driver, and bypassing it in the x86/x64 exception case. For x86/x64,
special case code is retained under arch/x86, but no VMbus interrupt
handling code is needed under arch/arm64.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
arch/x86/include/asm/mshyperv.h | 1 -
arch/x86/kernel/cpu/mshyperv.c | 13 +++------
drivers/hv/hv.c | 8 +++++-
drivers/hv/vmbus_drv.c | 63 ++++++++++++++++++++++++++++++++++++-----
include/asm-generic/mshyperv.h | 7 ++---
5 files changed, 70 insertions(+), 22 deletions(-)
@@ -60,23 +60,18 @@set_irq_regs(old_regs);}-inthv_setup_vmbus_irq(intirq,void(*handler)(void))+voidhv_setup_vmbus_handler(void(*handler)(void)){-/*-*The'irq'argumentisignoredonx86/x64becauseahard-coded-*interruptvectorisusedforHyper-Vinterrupts.-*/vmbus_handler=handler;-return0;}+EXPORT_SYMBOL_GPL(hv_setup_vmbus_handler);-voidhv_remove_vmbus_irq(void)+voidhv_remove_vmbus_handler(void){/* We have no way to deallocate the interrupt gate */vmbus_handler=NULL;}-EXPORT_SYMBOL_GPL(hv_setup_vmbus_irq);-EXPORT_SYMBOL_GPL(hv_remove_vmbus_irq);+EXPORT_SYMBOL_GPL(hv_remove_vmbus_handler);/**Routinestodoper-architecturehandlingofstimer0
From: Michael Kelley <hidden> Date: 2021-03-01 01:19:48
While the Hyper-V Reference TSC code is architecture neutral, the
pv_ops.time.sched_clock() function is implemented for x86/x64, but not
for ARM64. Current code calls a utility function under arch/x86 (and
coming, under arch/arm64) to handle the difference.
Change this approach to handle the difference inline based on whether
GENERIC_SCHED_CLOCK is present. The new approach removes code under
arch/* since the difference is tied more to the specifics of the Linux
implementation than to the architecture.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
arch/x86/include/asm/mshyperv.h | 11 -----------
drivers/clocksource/hyperv_timer.c | 21 +++++++++++++++++++++
2 files changed, 21 insertions(+), 11 deletions(-)
From: Michael Kelley <hidden> Date: 2021-03-01 01:19:58
While the driver for the Hyper-V Reference TSC and STIMERs is architecture
neutral, vDSO is implemented for x86/x64, but not for ARM64. Current code
calls into utility functions under arch/x86 (and coming, under arch/arm64)
to handle the difference.
Change this approach to handle the difference inline based on whether
VDSO_CLOCK_MODE_HVCLOCK is present. The new approach removes code under
arch/* since the difference is tied more to the specifics of the Linux
implementation than to the architecture.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
arch/x86/include/asm/mshyperv.h | 4 ----
drivers/clocksource/hyperv_timer.c | 10 ++++++++--
2 files changed, 8 insertions(+), 6 deletions(-)
From: Michael Kelley <hidden> Date: 2021-03-01 01:20:34
STIMER0 interrupts are most naturally modeled as per-cpu IRQs. But
because x86/x64 doesn't have per-cpu IRQs, the core STIMER0 interrupt
handling machinery is done in code under arch/x86 and Linux IRQs are
not used. Adding support for ARM64 means adding equivalent code
using per-cpu IRQs under arch/arm64.
A better model is to treat per-cpu IRQs as the normal path (which it is
for modern architectures), and the x86/x64 path as the exception. Do this
by incorporating standard Linux per-cpu IRQ allocation into the main
SITMER0 driver code, and bypass it in the x86/x64 exception case. For
x86/x64, special case code is retained under arch/x86, but no STIMER0
interrupt handling code is needed under arch/arm64.
No functional change.
Signed-off-by: Michael Kelley <redacted>
---
arch/x86/hyperv/hv_init.c | 2 +-
arch/x86/include/asm/mshyperv.h | 4 -
arch/x86/kernel/cpu/mshyperv.c | 10 +--
drivers/clocksource/hyperv_timer.c | 180 ++++++++++++++++++++++++++-----------
include/asm-generic/mshyperv.h | 5 --
include/clocksource/hyperv_timer.h | 3 +-
6 files changed, 132 insertions(+), 72 deletions(-)
@@ -90,21 +90,17 @@ void hv_remove_vmbus_handler(void)set_irq_regs(old_regs);}-inthv_setup_stimer0_irq(int*irq,int*vector,void(*handler)(void))+/* For x86/x64, override weak placeholders in hyperv_timer.c */+voidhv_setup_stimer0_handler(void(*handler)(void)){-*vector=HYPERV_STIMER0_VECTOR;-*irq=-1;/* Unused on x86/x64 */hv_stimer0_handler=handler;-return0;}-EXPORT_SYMBOL_GPL(hv_setup_stimer0_irq);-voidhv_remove_stimer0_irq(intirq)+voidhv_remove_stimer0_handler(void){/* We have no way to deallocate the interrupt gate */hv_stimer0_handler=NULL;}-EXPORT_SYMBOL_GPL(hv_remove_stimer0_irq);voidhv_setup_kexec_handler(void(*handler)(void)){
@@ -76,8 +88,8 @@ static int hv_ce_shutdown(struct clock_event_device *evt){hv_set_register(HV_REGISTER_STIMER0_COUNT,0);hv_set_register(HV_REGISTER_STIMER0_CONFIG,0);-if(direct_mode_enabled)-hv_disable_stimer0_percpu_irq(stimer0_irq);+if(direct_mode_enabled&&stimer0_irq>=0)+disable_percpu_irq(stimer0_irq);return0;}
@@ -95,8 +107,9 @@ static int hv_ce_set_oneshot(struct clock_event_device *evt)*onthespecifiedhardwarevector/IRQ.*/timer_cfg.direct_mode=1;-timer_cfg.apic_vector=stimer0_vector;-hv_enable_stimer0_percpu_irq(stimer0_irq);+timer_cfg.apic_vector=HYPERV_STIMER0_VECTOR;+if(stimer0_irq>=0)+enable_percpu_irq(stimer0_irq,IRQ_TYPE_NONE);}else{/**Whenitexpires,thetimerwillgenerateaVMbusmessage,
@@ -169,10 +182,70 @@ int hv_stimer_cleanup(unsigned int cpu)}EXPORT_SYMBOL_GPL(hv_stimer_cleanup);+/*+*Theseplaceholdersareoverriddenbyarchspecificcodeon+*architecturesthatneedspecialsetupofthestimer0IRQbecause+*theydon'tsupportper-cpuIRQs(suchasx86/x64).+*/+void__weakhv_setup_stimer0_handler(void(*handler)(void))+{+};++void__weakhv_remove_stimer0_handler(void)+{+};++/* Called only on architectures with per-cpu IRQs (i.e., not x86/x64) */+staticinthv_setup_stimer0_irq(void)+{+intret;++ret=acpi_register_gsi(NULL,HYPERV_STIMER0_VECTOR,+ACPI_EDGE_SENSITIVE,ACPI_ACTIVE_HIGH);+if(ret<0){+pr_err("Can't register Hyper-V stimer0 GSI. Error %d",ret);+returnret;+}+stimer0_irq=ret;++stimer0_evt=alloc_percpu(long);+if(!stimer0_evt){+ret=-ENOMEM;+gotounregister_gsi;+}+ret=request_percpu_irq(stimer0_irq,hv_stimer0_percpu_isr,+"Hyper-V stimer0",stimer0_evt);+if(ret){+pr_err("Can't request Hyper-V stimer0 IRQ %d. Error %d",+stimer0_irq,ret);+gotofree_stimer0_evt;+}+returnret;++free_stimer0_evt:+free_percpu(stimer0_evt);+unregister_gsi:+acpi_unregister_gsi(stimer0_irq);+stimer0_irq=-1;+returnret;+}++staticvoidhv_remove_stimer0_irq(void)+{+if(stimer0_irq==-1){+hv_remove_stimer0_handler();+}else{+free_percpu_irq(stimer0_irq,stimer0_evt);+free_percpu(stimer0_evt);+acpi_unregister_gsi(stimer0_irq);+stimer0_irq=-1;+}+}+/* hv_stimer_alloc - Global initialization of the clockevent and stimer0 */-inthv_stimer_alloc(void)+inthv_stimer_alloc(boolhave_percpu_irqs){-intret=0;+intret;/**Synthetictimersarealwaysavailableexceptonoldversionsof
@@ -188,29 +261,37 @@ int hv_stimer_alloc(void)direct_mode_enabled=ms_hyperv.misc_features&HV_STIMER_DIRECT_MODE_AVAILABLE;-if(direct_mode_enabled){-ret=hv_setup_stimer0_irq(&stimer0_irq,&stimer0_vector,-hv_stimer0_isr);++/*+*IfDirectModeisn'tenabled,theremainderoftheinitialization+*isdonelaterbyhv_stimer_legacy_init()+*/+if(!direct_mode_enabled)+return0;++if(have_percpu_irqs){+ret=hv_setup_stimer0_irq();if(ret)-gotofree_percpu;+gotofree_clock_event;+}else{+hv_setup_stimer0_handler(hv_stimer0_isr);+}-/*-*SinceweareinDirectMode,stimerinitialization-*canbedonenowwithaCPUHPvalueinthesamerange-*asotherclockeventdevices.-*/-ret=cpuhp_setup_state(CPUHP_AP_HYPERV_TIMER_STARTING,-"clockevents/hyperv/stimer:starting",-hv_stimer_init,hv_stimer_cleanup);-if(ret<0)-gotofree_stimer0_irq;+/*+*SinceweareinDirectMode,stimerinitialization+*canbedonenowwithaCPUHPvalueinthesamerange+*asotherclockeventdevices.+*/+ret=cpuhp_setup_state(CPUHP_AP_HYPERV_TIMER_STARTING,+"clockevents/hyperv/stimer:starting",+hv_stimer_init,hv_stimer_cleanup);+if(ret<0){+hv_remove_stimer0_irq();+gotofree_clock_event;}returnret;-free_stimer0_irq:-hv_remove_stimer0_irq(stimer0_irq);-stimer0_irq=0;-free_percpu:+free_clock_event:free_percpu(hv_clock_event);hv_clock_event=NULL;returnret;
@@ -254,23 +335,6 @@ void hv_stimer_legacy_cleanup(unsigned int cpu)}EXPORT_SYMBOL_GPL(hv_stimer_legacy_cleanup);--/* hv_stimer_free - Free global resources allocated by hv_stimer_alloc() */-voidhv_stimer_free(void)-{-if(!hv_clock_event)-return;--if(direct_mode_enabled){-cpuhp_remove_state(CPUHP_AP_HYPERV_TIMER_STARTING);-hv_remove_stimer0_irq(stimer0_irq);-stimer0_irq=0;-}-free_percpu(hv_clock_event);-hv_clock_event=NULL;-}-EXPORT_SYMBOL_GPL(hv_stimer_free);-/**Doaglobalcleanupofclockeventsforthecasesofkexecand*vmbusexit
@@ -21,8 +21,7 @@#define HV_MIN_DELTA_TICKS 1/* Routines called by the VMbus driver */-externinthv_stimer_alloc(void);-externvoidhv_stimer_free(void);+externinthv_stimer_alloc(boolhave_percpu_irqs);externinthv_stimer_cleanup(unsignedintcpu);externvoidhv_stimer_legacy_init(unsignedintcpu,intsint);externvoidhv_stimer_legacy_cleanup(unsignedintcpu);
From: Michael Kelley <hidden> Date: 2021-03-01 01:20:58
On x86/x64, the TSC clocksource is available in a Hyper-V VM only if
Hyper-V provides the TSC_INVARIANT flag. The rating on the Hyper-V
Reference TSC page clocksource is currently set so that it will not
override the TSC clocksource in this case. Alternatively, if the TSC
clocksource is not available, then the Hyper-V clocksource is used.
But on ARM64, the Hyper-V Reference TSC page clocksource should
override the ARM arch counter, since the Hyper-V clocksource provides
scaling and offsetting during live migrations that is not provided
for the ARM arch counter.
To get the needed behavior for both x86/x64 and ARM64, tweak the
logic by defaulting the Hyper-V Reference TSC page clocksource
rating to a large value that will always override. If the Hyper-V
TSC_INVARIANT flag is set, then reduce the rating so that it will not
override the TSC.
While the logic for getting there is slightly different, the net
result in the normal cases is no functional change.
Signed-off-by: Michael Kelley <redacted>
---
drivers/clocksource/hyperv_timer.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
From: Wei Liu <wei.liu@kernel.org> Date: 2021-03-01 11:32:12
On Sun, Feb 28, 2021 at 05:15:22PM -0800, Michael Kelley wrote:
To support Linux guests on Hyper-V on multiple architectures, the original
approach factored out all differences between Hyper-V on x86/x64 and
Hyper-V on ARM64 into functions or #defines under arch/x86 and
arch/arm64. Some of these differences are truly related to the
architecture, but others are more properly treated as Linux OS
differences or just quirks in Hyper-V. Feedback from Arnd Bergmann[1]
recommended that differences other than architecture should be
incorporated into the architecture independent Hyper-V code. Each
difference can be handled with conditions specific to the difference
instead of tying it to the broader x86/x64 vs. ARM64. This approach
reduces the amount of code under arch/x86 and arch/arm64 and keeps
the non-architectural differences localized and more easily understood.
This patch set implements the new approach by changing the interface
between the architecture independent code and the architecture dependent
code for x86/x64. The patches move code from arch/x86 to the
architecture independent Hyper-V code whenever possible, and add
architecture independent support needed by other architectures like
ARM64. No functionality is changed for x86/x64. A subsequent patch
set will provide the Hyper-V support code under arch/arm64.
This patch set results in an increase in lines of code (though some
of the increase is additional comments). But the lines needed under
arch/arm64 in the upcoming patch set is significantly reduced, resulting
in a net decrease of about 125 lines.
[1] https://lore.kernel.org/lkml/CAK8P3a1hDBVembCd+6=ENUWYFz=72JBTFMrKYZ2aFd+_Q04F+g@mail.gmail.com/
This series looks good to me.
Given this series touches mostly Hyper-V code. I will be taking this it
via hyperv-next once the last two patches are reviewed.
Wei.
From: Daniel Lezcano <hidden> Date: 2021-03-01 12:22:55
On 01/03/2021 02:15, Michael Kelley wrote:
While the driver for the Hyper-V Reference TSC and STIMERs is architecture
neutral, vDSO is implemented for x86/x64, but not for ARM64. Current code
calls into utility functions under arch/x86 (and coming, under arch/arm64)
to handle the difference.
Change this approach to handle the difference inline based on whether
VDSO_CLOCK_MODE_HVCLOCK is present. The new approach removes code under
arch/* since the difference is tied more to the specifics of the Linux
implementation than to the architecture.
No functional change.
From: Daniel Lezcano <hidden> Date: 2021-03-01 14:27:57
On 01/03/2021 02:15, Michael Kelley wrote:
While the Hyper-V Reference TSC code is architecture neutral, the
pv_ops.time.sched_clock() function is implemented for x86/x64, but not
for ARM64. Current code calls a utility function under arch/x86 (and
coming, under arch/arm64) to handle the difference.
Change this approach to handle the difference inline based on whether
GENERIC_SCHED_CLOCK is present. The new approach removes code under
arch/* since the difference is tied more to the specifics of the Linux
implementation than to the architecture.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
[ ... ]
+/*
+ * Reference to pv_ops must be inline so objtool
+ * detection of noinstr violations can work correctly.
+ */
+static __always_inline void hv_setup_sched_clock(void *sched_clock)
+{
+#ifdef CONFIG_GENERIC_SCHED_CLOCK
+ /*
+ * We're on an architecture with generic sched clock (not x86/x64).
+ * The Hyper-V sched clock read function returns nanoseconds, not
+ * the normal 100ns units of the Hyper-V synthetic clock.
+ */
+ sched_clock_register(sched_clock, 64, NSEC_PER_SEC);
+#else
+#ifdef CONFIG_PARAVIRT
+ /* We're on x86/x64 *and* using PV ops */
+ pv_ops.time.sched_clock = sched_clock;
+#endif
+#endif
+}
From: Daniel Lezcano <hidden> Date: 2021-03-01 15:43:20
On 01/03/2021 02:15, Michael Kelley wrote:
On x86/x64, the TSC clocksource is available in a Hyper-V VM only if
Hyper-V provides the TSC_INVARIANT flag. The rating on the Hyper-V
Reference TSC page clocksource is currently set so that it will not
override the TSC clocksource in this case. Alternatively, if the TSC
clocksource is not available, then the Hyper-V clocksource is used.
But on ARM64, the Hyper-V Reference TSC page clocksource should
override the ARM arch counter, since the Hyper-V clocksource provides
scaling and offsetting during live migrations that is not provided
for the ARM arch counter.
To get the needed behavior for both x86/x64 and ARM64, tweak the
logic by defaulting the Hyper-V Reference TSC page clocksource
rating to a large value that will always override. If the Hyper-V
TSC_INVARIANT flag is set, then reduce the rating so that it will not
override the TSC.
While the logic for getting there is slightly different, the net
result in the normal cases is no functional change.
Signed-off-by: Michael Kelley <redacted>
From: Daniel Lezcano <hidden> Date: 2021-03-02 01:20:06
On 01/03/2021 02:15, Michael Kelley wrote:
quoted hunk
STIMER0 interrupts are most naturally modeled as per-cpu IRQs. But
because x86/x64 doesn't have per-cpu IRQs, the core STIMER0 interrupt
handling machinery is done in code under arch/x86 and Linux IRQs are
not used. Adding support for ARM64 means adding equivalent code
using per-cpu IRQs under arch/arm64.
A better model is to treat per-cpu IRQs as the normal path (which it is
for modern architectures), and the x86/x64 path as the exception. Do this
by incorporating standard Linux per-cpu IRQ allocation into the main
SITMER0 driver code, and bypass it in the x86/x64 exception case. For
x86/x64, special case code is retained under arch/x86, but no STIMER0
interrupt handling code is needed under arch/arm64.
No functional change.
Signed-off-by: Michael Kelley <redacted>
---
arch/x86/hyperv/hv_init.c | 2 +-
arch/x86/include/asm/mshyperv.h | 4 -
arch/x86/kernel/cpu/mshyperv.c | 10 +--
drivers/clocksource/hyperv_timer.c | 180 ++++++++++++++++++++++++++-----------
include/asm-generic/mshyperv.h | 5 --
include/clocksource/hyperv_timer.h | 3 +-
6 files changed, 132 insertions(+), 72 deletions(-)
@@ -90,21 +90,17 @@ void hv_remove_vmbus_handler(void)set_irq_regs(old_regs);}-inthv_setup_stimer0_irq(int*irq,int*vector,void(*handler)(void))+/* For x86/x64, override weak placeholders in hyperv_timer.c */+voidhv_setup_stimer0_handler(void(*handler)(void)){-*vector=HYPERV_STIMER0_VECTOR;-*irq=-1;/* Unused on x86/x64 */hv_stimer0_handler=handler;-return0;}-EXPORT_SYMBOL_GPL(hv_setup_stimer0_irq);-voidhv_remove_stimer0_irq(intirq)+voidhv_remove_stimer0_handler(void){/* We have no way to deallocate the interrupt gate */hv_stimer0_handler=NULL;}-EXPORT_SYMBOL_GPL(hv_remove_stimer0_irq);voidhv_setup_kexec_handler(void(*handler)(void)){
From: Michael Kelley <hidden> Date: 2021-03-02 07:47:25
From: Daniel Lezcano <redacted> Sent: Monday, March 1, 2021 4:22 AM
On 01/03/2021 02:15, Michael Kelley wrote:
quoted
While the driver for the Hyper-V Reference TSC and STIMERs is architecture
neutral, vDSO is implemented for x86/x64, but not for ARM64. Current code
calls into utility functions under arch/x86 (and coming, under arch/arm64)
to handle the difference.
Change this approach to handle the difference inline based on whether
VDSO_CLOCK_MODE_HVCLOCK is present. The new approach removes code under
arch/* since the difference is tied more to the specifics of the Linux
implementation than to the architecture.
No functional change.
Is there any particular benefit (that I might not be recognizing)
to having the .enable function be NULL vs. a function that
does nothing? I can see the handful of places where the
.enable function is invoked, and there doesn't seem to be
much difference.
In any case, I have no problem with making the change in
a v3 of the patch set.
Michael
From: Michael Kelley <hidden> Date: 2021-03-02 07:47:27
From: Daniel Lezcano <redacted> Sent: Monday, March 1, 2021 6:25 AM
On 01/03/2021 02:15, Michael Kelley wrote:
quoted
While the Hyper-V Reference TSC code is architecture neutral, the
pv_ops.time.sched_clock() function is implemented for x86/x64, but not
for ARM64. Current code calls a utility function under arch/x86 (and
coming, under arch/arm64) to handle the difference.
Change this approach to handle the difference inline based on whether
GENERIC_SCHED_CLOCK is present. The new approach removes code under
arch/* since the difference is tied more to the specifics of the Linux
implementation than to the architecture.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
[ ... ]
quoted
+/*
+ * Reference to pv_ops must be inline so objtool
+ * detection of noinstr violations can work correctly.
+ */
+static __always_inline void hv_setup_sched_clock(void *sched_clock)
+{
+#ifdef CONFIG_GENERIC_SCHED_CLOCK
+ /*
+ * We're on an architecture with generic sched clock (not x86/x64).
+ * The Hyper-V sched clock read function returns nanoseconds, not
+ * the normal 100ns units of the Hyper-V synthetic clock.
+ */
+ sched_clock_register(sched_clock, 64, NSEC_PER_SEC);
+#else
+#ifdef CONFIG_PARAVIRT
+ /* We're on x86/x64 *and* using PV ops */
+ pv_ops.time.sched_clock = sched_clock;
+#endif
+#endif
+}
Please refer to:
Documentation/process/coding-style.rst
Section 21)
[ ... ]
Prefer to compile out entire functions, rather than portions of
functions or portions of expressions.
[ ... ]
OK. I'll rework the #ifdef in v3 of the patch set. Is the following
the preferred approach?
#ifdef CONFIG_GENERIC_SCHED_CLOCK
static __always_inline void hv_setup_sched_clock(void *sched_clock)
{
sched_clock_register(sched_clock, 64 NSEC_PER_SEC);
}
#else
#ifdef CONFIG_PARAVIRT
static __always_inline void hv_setup_sched_clock(void *sched_clock)
{
pv_ops.time.sched_clock = sched_clock:
}
#else
static __always_inline void hv_setup_sched_clock(void *sched_clock) {}
#endif
#endif
Michael
From: Michael Kelley <hidden> Date: 2021-03-02 07:47:28
From: Daniel Lezcano <redacted> Sent: Monday, March 1, 2021 10:45 AM
On 01/03/2021 02:15, Michael Kelley wrote:
quoted
STIMER0 interrupts are most naturally modeled as per-cpu IRQs. But
because x86/x64 doesn't have per-cpu IRQs, the core STIMER0 interrupt
handling machinery is done in code under arch/x86 and Linux IRQs are
not used. Adding support for ARM64 means adding equivalent code
using per-cpu IRQs under arch/arm64.
A better model is to treat per-cpu IRQs as the normal path (which it is
for modern architectures), and the x86/x64 path as the exception. Do this
by incorporating standard Linux per-cpu IRQ allocation into the main
SITMER0 driver code, and bypass it in the x86/x64 exception case. For
x86/x64, special case code is retained under arch/x86, but no STIMER0
interrupt handling code is needed under arch/arm64.
No functional change.
Signed-off-by: Michael Kelley <redacted>
---
arch/x86/hyperv/hv_init.c | 2 +-
arch/x86/include/asm/mshyperv.h | 4 -
arch/x86/kernel/cpu/mshyperv.c | 10 +--
drivers/clocksource/hyperv_timer.c | 180 ++++++++++++++++++++++++++-----------
include/asm-generic/mshyperv.h | 5 --
include/clocksource/hyperv_timer.h | 3 +-
6 files changed, 132 insertions(+), 72 deletions(-)
@@ -90,21 +90,17 @@ void hv_remove_vmbus_handler(void)set_irq_regs(old_regs);}-inthv_setup_stimer0_irq(int*irq,int*vector,void(*handler)(void))+/* For x86/x64, override weak placeholders in hyperv_timer.c */+voidhv_setup_stimer0_handler(void(*handler)(void)){-*vector=HYPERV_STIMER0_VECTOR;-*irq=-1;/* Unused on x86/x64 */hv_stimer0_handler=handler;-return0;}-EXPORT_SYMBOL_GPL(hv_setup_stimer0_irq);-voidhv_remove_stimer0_irq(intirq)+voidhv_remove_stimer0_handler(void){/* We have no way to deallocate the interrupt gate */hv_stimer0_handler=NULL;}-EXPORT_SYMBOL_GPL(hv_remove_stimer0_irq);voidhv_setup_kexec_handler(void(*handler)(void)){
From: Daniel Lezcano <hidden> Date: 2021-03-02 15:28:37
On 02/03/2021 02:29, Michael Kelley wrote:
From: Daniel Lezcano <redacted> Sent: Monday, March 1, 2021 4:22 AM
quoted
On 01/03/2021 02:15, Michael Kelley wrote:
quoted
While the driver for the Hyper-V Reference TSC and STIMERs is architecture
neutral, vDSO is implemented for x86/x64, but not for ARM64. Current code
calls into utility functions under arch/x86 (and coming, under arch/arm64)
to handle the difference.
Change this approach to handle the difference inline based on whether
VDSO_CLOCK_MODE_HVCLOCK is present. The new approach removes code under
arch/* since the difference is tied more to the specifics of the Linux
implementation than to the architecture.
No functional change.
Is there any particular benefit (that I might not be recognizing)
to having the .enable function be NULL vs. a function that
does nothing? I can see the handful of places where the
.enable function is invoked, and there doesn't seem to be
much difference.
In any case, I have no problem with making the change in
a v3 of the patch set.
The Hyper-V page allocator functions are implemented in an architecture
neutral way. Move them into the architecture neutral VMbus module so
a separate implementation for ARM64 is not needed.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
arch/x86/hyperv/hv_init.c | 22 ----------------------
arch/x86/include/asm/mshyperv.h | 5 -----
drivers/hv/hv.c | 36 ++++++++++++++++++++++++++++++++++++
include/asm-generic/mshyperv.h | 4 ++++
4 files changed, 40 insertions(+), 27 deletions(-)
@@ -37,6 +37,42 @@ int hv_init(void)}/*+*Functionsforallocatingandfreeingmemorywithsizeand+*alignmentHV_HYP_PAGE_SIZE.Thesefunctionsareneededbecause+*theguestpagesizemaynotbethesameastheHyper-Vpage+*size.Wedependuponkmalloc()aligningpower-of-twosize+*allocationstotheallocationsizeboundary,sothatthe+*allocatedmemoryappearstoHyper-Vasapageofthesize+*itexpects.+*/++void*hv_alloc_hyperv_page(void)+{+BUILD_BUG_ON(PAGE_SIZE<HV_HYP_PAGE_SIZE);++if(PAGE_SIZE==HV_HYP_PAGE_SIZE)+return(void*)__get_free_page(GFP_KERNEL);+else+returnkmalloc(HV_HYP_PAGE_SIZE,GFP_KERNEL);
PAGE_SIZE and HV_HYP_PAGE_SIZE are known compile-time and in case this
won't change in the future we can probably write this as
#if PAGE_SIZE == HV_HYP_PAGE_SIZE
return (void *)__get_free_page(GFP_KERNEL);
#else
return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
#endif
(not sure if the output is going to be any different with e.g. gcc's '-O2')
quoted hunk
+}
+
+void *hv_alloc_hyperv_zeroed_page(void)
+{
+ if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
+ return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
+ else
+ return kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
+}
+
+void hv_free_hyperv_page(unsigned long addr)
+{
+ if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
+ free_page(addr);
+ else
+ kfree((void *)addr);
+}
+
+/*
* hv_post_message - Post a message using the hypervisor message IPC.
*
* This involves a hypercall.
From: Daniel Lezcano <hidden> Date: 2021-03-02 15:57:30
On 02/03/2021 02:38, Michael Kelley wrote:
From: Daniel Lezcano <redacted> Sent: Monday, March 1, 2021 6:25 AM
quoted
On 01/03/2021 02:15, Michael Kelley wrote:
quoted
While the Hyper-V Reference TSC code is architecture neutral, the
pv_ops.time.sched_clock() function is implemented for x86/x64, but not
for ARM64. Current code calls a utility function under arch/x86 (and
coming, under arch/arm64) to handle the difference.
Change this approach to handle the difference inline based on whether
GENERIC_SCHED_CLOCK is present. The new approach removes code under
arch/* since the difference is tied more to the specifics of the Linux
implementation than to the architecture.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
[ ... ]
quoted
+/*
+ * Reference to pv_ops must be inline so objtool
+ * detection of noinstr violations can work correctly.
+ */
+static __always_inline void hv_setup_sched_clock(void *sched_clock)
+{
+#ifdef CONFIG_GENERIC_SCHED_CLOCK
+ /*
+ * We're on an architecture with generic sched clock (not x86/x64).
+ * The Hyper-V sched clock read function returns nanoseconds, not
+ * the normal 100ns units of the Hyper-V synthetic clock.
+ */
+ sched_clock_register(sched_clock, 64, NSEC_PER_SEC);
+#else
+#ifdef CONFIG_PARAVIRT
+ /* We're on x86/x64 *and* using PV ops */
+ pv_ops.time.sched_clock = sched_clock;
+#endif
+#endif
+}
Please refer to:
Documentation/process/coding-style.rst
Section 21)
[ ... ]
Prefer to compile out entire functions, rather than portions of
functions or portions of expressions.
[ ... ]
OK. I'll rework the #ifdef in v3 of the patch set. Is the following
the preferred approach?
Yes but with an indentation and comment to describe the section end.
eg.
#ifdef A
#else
# ifdef B
...
# else
# endif /* B */
#endif /* A */
From: Michael Kelley <hidden> Date: 2021-03-02 20:58:51
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Tuesday, March 2, 2021 4:57 AM
Michael Kelley [off-list ref] writes:
quoted
The Hyper-V page allocator functions are implemented in an architecture
neutral way. Move them into the architecture neutral VMbus module so
a separate implementation for ARM64 is not needed.
No functional change.
Signed-off-by: Michael Kelley <redacted>
Reviewed-by: Boqun Feng <redacted>
---
arch/x86/hyperv/hv_init.c | 22 ----------------------
arch/x86/include/asm/mshyperv.h | 5 -----
drivers/hv/hv.c | 36 ++++++++++++++++++++++++++++++++++++
include/asm-generic/mshyperv.h | 4 ++++
4 files changed, 40 insertions(+), 27 deletions(-)
[snip]
quoted
/*
+ * Functions for allocating and freeing memory with size and
+ * alignment HV_HYP_PAGE_SIZE. These functions are needed because
+ * the guest page size may not be the same as the Hyper-V page
+ * size. We depend upon kmalloc() aligning power-of-two size
+ * allocations to the allocation size boundary, so that the
+ * allocated memory appears to Hyper-V as a page of the size
+ * it expects.
+ */
+
+void *hv_alloc_hyperv_page(void)
+{
+ BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
+
+ if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
+ return (void *)__get_free_page(GFP_KERNEL);
+ else
+ return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
PAGE_SIZE and HV_HYP_PAGE_SIZE are known compile-time and in case this
won't change in the future we can probably write this as
#if PAGE_SIZE == HV_HYP_PAGE_SIZE
return (void *)__get_free_page(GFP_KERNEL);
#else
return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
#endif
(not sure if the output is going to be any different with e.g. gcc's '-O2')
I looked at the generated code, and the compiler does the right
thing on both x86/x64 and on ARM64. I'd rather leave the code
as is so that both legs of the 'if' statement get checked by the
compiler regardless of whether PAGE_SIZE == HV_HYP_PAGE_SIZE.
Michael