[PATCH v6 01/10] arm64: hyperv: Add core Hyper-V include files

[Michael Kelley]

On Sat, 14 Mar 2020 15:35:10 +0000,
Hi Michael,

hyperv-tlfs.h defines Hyper-V interfaces from the Hyper-V Top Level
Functional Spec (TLFS). The TLFS is distinctly oriented to x86/x64,
and Hyper-V has not separated out the architecture-dependent parts into
x86/x64 vs. ARM64. So hyperv-tlfs.h includes information for ARM64
docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/tlfs
mshyperv.h defines Linux-specific structures and routines for
interacting with Hyper-V on ARM64, and #includes the architecture-
independent part of mshyperv.h in include/asm-generic.
---
MAINTAINERS | 2 +
arch/arm64/include/asm/hyperv-tlfs.h | 413

+++++++++++++++++++++++++++++++++++

arch/arm64/include/asm/mshyperv.h | 115 ++++++++++
3 files changed, 530 insertions(+)
create mode 100644 arch/arm64/include/asm/hyperv-tlfs.h
create mode 100644 arch/arm64/include/asm/mshyperv.h

So this is a pretty large patch, mostly containing constants and other
data structures that don't necessarily make sense immediately (or at
least, I can't make sense of it without reading all the other 9
patches and going back to patch #1).
Could you please consider splitting this into more discreet bits that
get added as required by the supporting code?

Yes, I'll do this in the next version.

diff --git a/MAINTAINERS b/MAINTAINERS
index 58bb5c4..398cfdb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7809,6 +7809,8 @@ F: arch/x86/include/asm/trace/hyperv.h
F: arch/x86/include/asm/hyperv-tlfs.h
F: arch/x86/kernel/cpu/mshyperv.c
F: arch/x86/hyperv
+F: arch/arm64/include/asm/hyperv-tlfs.h
+F: arch/arm64/include/asm/mshyperv.h
F: drivers/clocksource/hyperv_timer.c
F: drivers/hid/hid-hyperv.c
F: drivers/hv/
diff --git a/arch/arm64/include/asm/hyperv-tlfs.h b/arch/arm64/include/asm/hyperv-

tlfs.h

new file mode 100644
index 0000000..5e6a087
--- /dev/null
+++ b/arch/arm64/include/asm/hyperv-tlfs.h
@@ -0,0 +1,413 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This file contains definitions from the Hyper-V Hypervisor Top-Level
+ * https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/tlfs
+ *
+ * Copyright (C) 2019, Microsoft, Inc.
+ *
+ */
+
+#ifndef _ASM_HYPERV_TLFS_H
+#define _ASM_HYPERV_TLFS_H
+
+#include <linux/types.h>
+
+/*
+ * All data structures defined in the TLFS that are shared between Hyper-V
+ * and a guest VM use Little Endian byte ordering. This matches the default
+ * byte ordering of Linux running on ARM64, so no special handling is required.
+ */
+
+
+/*
+ * While not explicitly listed in the TLFS, Hyper-V always runs with a page
+ * size of 4096. These definitions are used when communicating with Hyper-V
+ * using guest physical pages and guest physical page addresses, since the
+ * guest page size may not be 4096 on ARM64.
+ */
+#define HV_HYP_PAGE_SHIFT 12
+#define HV_HYP_PAGE_SIZE (1 << HV_HYP_PAGE_SHIFT)

Probably worth writing this as 1UL to be on the safe side.

Agreed.

+#define HV_HYP_PAGE_MASK (~(HV_HYP_PAGE_SIZE - 1))
+
+/*
+ * These Hyper-V registers provide information equivalent to the CPUID
+ * instruction on x86/x64.
+ */
+#define HV_REGISTER_HYPERVISOR_VERSION 0x00000100 /*CPUID

0x40000002 */

+#define HV_REGISTER_PRIVILEGES_AND_FEATURES 0x00000200 /*CPUID

0x40000003 */

+#define HV_REGISTER_FEATURES 0x00000201 /*CPUID

0x40000004 */

+#define HV_REGISTER_IMPLEMENTATION_LIMITS 0x00000202 /*CPUID

0x40000005 */

+#define HV_ARM64_REGISTER_INTERFACE_VERSION 0x00090006 /*CPUID

0x40000001 */

+
+/*
+ * Feature identification. HvRegisterPrivilegesAndFeaturesInfo returns a
+ * 128-bit value with flags indicating which features are available to the
+ * partition based upon the current partition privileges. The 128-bit
+ * value is broken up with different portions stored in different 32-bit
+ * fields in the ms_hyperv structure.
+ */
+
+/* Partition Reference Counter available*/
+#define HV_MSR_TIME_REF_COUNT_AVAILABLE BIT(1)
+
+/* Synthetic Timers available */
+#define HV_MSR_SYNTIMER_AVAILABLE BIT(3)
+
+/* Reference TSC available */
+#define HV_MSR_REFERENCE_TSC_AVAILABLE BIT(9)
+
+
+/*
+ * This group of flags is in the high order 64-bits of the returned
+ * 128-bit value. Note that this set of bit positions differs from what
+ * is used on x86/x64 architecture.
+ */
+
+/* Crash MSRs available */
+#define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(8)

It is confusing that you don't have a single bit space for all these
flags. It'd probably help if you had a structure describing this
128bit value in multiple 32bit or 64bit words, and indicating which
field the bit position is relevant to.

I'll add this in the next version.

[...]

+/* Define the hypercall status result */
+
+union hv_hypercall_status {
+ u64 as_uint64;

nit: it'd be more consistent if as_uint64 was actually a uint64 type.

Agreed.

+ struct {
+ u16 status;
+ u16 reserved;
+ u16 reps_completed; /* Low 12 bits */
+ u16 reserved2;
+ };
+};
+
+/* hypercall status code */
+#define HV_STATUS_SUCCESS 0
+#define HV_STATUS_INVALID_HYPERCALL_CODE 2
+#define HV_STATUS_INVALID_HYPERCALL_INPUT 3
+#define HV_STATUS_INVALID_ALIGNMENT 4
+#define HV_STATUS_INSUFFICIENT_MEMORY 11
+#define HV_STATUS_INVALID_CONNECTION_ID 18
+#define HV_STATUS_INSUFFICIENT_BUFFERS 19
+
+/* 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;
+
+struct hv_get_vp_register_output {
+ u64 registervaluelow;
+ u64 registervaluehigh;
+} __packed;
+
+#define HV_FLUSH_ALL_PROCESSORS BIT(0)
+#define HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES BIT(1)
+#define HV_FLUSH_NON_GLOBAL_MAPPINGS_ONLY BIT(2)

I"m curious: Are these supposed to be PV'd TLB invalidation
operations?

Yes, they are. Hyper-V provides PV TLB flush hypercalls on ARM64, but
this patch set doesn't use those hypercalls. I'll remove the definitions.

+#define HV_FLUSH_USE_EXTENDED_RANGE_FORMAT BIT(3)
+
+enum HV_GENERIC_SET_FORMAT {
+ HV_GENERIC_SET_SPARSE_4K,
+ HV_GENERIC_SET_ALL,
+};
+
+/*
+ * The Hyper-V TimeRefCount register and the TSC
+ * page provide a guest VM clock with 100ns tick rate
+ */
+#define HV_CLOCK_HZ (NSEC_PER_SEC/100)
+
+/*
+ * The fields in this structure are set by Hyper-V and read
+ * by the Linux guest. They should be accessed with READ_ONCE()
+ * so the compiler doesn't optimize in a way that will cause
+ * problems. The union pads the size out to the page size
+ * used to communicate with Hyper-V.
+ */
+struct ms_hyperv_tsc_page {
+ union {
+ struct {
+ u32 tsc_sequence;
+ u32 reserved1;
+ u64 tsc_scale;
+ s64 tsc_offset;
+ } __packed;
+ u8 reserved2[HV_HYP_PAGE_SIZE];
+ };
+};
+
+/* Define the number of synthetic interrupt sources. */
+#define HV_SYNIC_SINT_COUNT (16)
+/* Define the expected SynIC version. */
+#define HV_SYNIC_VERSION_1 (0x1)
+
+#define HV_SYNIC_CONTROL_ENABLE (1ULL << 0)
+#define HV_SYNIC_SIMP_ENABLE (1ULL << 0)
+#define HV_SYNIC_SIEFP_ENABLE (1ULL << 0)
+#define HV_SYNIC_SINT_MASKED (1ULL << 16)
+#define HV_SYNIC_SINT_AUTO_EOI (1ULL << 17)
+#define HV_SYNIC_SINT_VECTOR_MASK (0xFF)

Let's me guess: a PV interrupt controller? Do you really need this?
Specially as I don't see any PV irqchip driver in this submission...

No, the above definitions aren't needed. I'll remove them.

Hyper-V does provide a limited synthetic interrupt controller that's
implemented entirely in architecture independent code and has been
used on the x86 side since the beginning of Hyper-V support. It
pre-dates me by a lot of years, and I've never considered whether it
should be modeled as an irqchip.

[...]

diff --git a/arch/arm64/include/asm/mshyperv.h b/arch/arm64/include/asm/mshyperv.h
new file mode 100644
index 0000000..60b3f68
--- /dev/null
+++ b/arch/arm64/include/asm/mshyperv.h
@@ -0,0 +1,115 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Linux-specific definitions for managing interactions with Microsoft's
+ * Hyper-V hypervisor. The definitions in this file are specific to
+ * the ARM64 architecture. See include/asm-generic/mshyperv.h for
+ * definitions are that architecture independent.
+ *
+ * Definitions that are specified in the Hyper-V Top Level Functional
+ * Spec (TLFS) should not go in this file, but should instead go in
+ * hyperv-tlfs.h.
+ *
+ * Copyright (C) 2019, Microsoft, Inc.
+ *
+ */
+
+#ifndef _ASM_MSHYPERV_H
+#define _ASM_MSHYPERV_H
+
+#include <linux/types.h>
+#include <linux/interrupt.h>
+#include <linux/clocksource.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
+#include <linux/arm-smccc.h>
+#include <asm/hyperv-tlfs.h>
+
+/*
+ * Define the IRQ numbers/vectors used by Hyper-V VMbus interrupts
+ * and by STIMER0 Direct Mode interrupts. Hyper-V should be supplying
+ * these values through ACPI, but there are no other interrupting
+ * devices in a Hyper-V VM on ARM64, so it's OK to hard code for now.

I'm not convinced it is OK. If you don't try to do the right thing
now, what is the incentive to do it later? Starting to hard code
things is akin to going back to the ARM board files of old. Been
there, managed to fix it, not going back to it again anytime soon.

I'll check back with the Hyper-V guys on getting appropriate values
assigned in ACPI.

+ * The "CALLBACK_VECTOR" terminology is a left-over from the x86/x64
+ * world that is used in architecture independent Hyper-V code.
+ */
+#define HYPERVISOR_CALLBACK_VECTOR 16
+#define HV_STIMER0_IRQNR 17
+
+extern u64 hv_do_hvc(u64 control, ...);
+extern u64 hv_do_hvc_fast_get(u64 control, u64 input1, u64 input2, u64 input3,
+ struct hv_get_vp_register_output *output);
+
+/*
+ * Declare calls to get and set Hyper-V VP register values on ARM64, which
+ * requires a hypercall.
+ */
+extern void hv_set_vpreg(u32 reg, u64 value);
+extern u64 hv_get_vpreg(u32 reg);
+extern void hv_get_vpreg_128(u32 reg, struct hv_get_vp_register_output *result);
+
+/*
+ * 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))
+
+#define hv_get_simp(val) (val = hv_get_vpreg(HV_REGISTER_SIPP))
+#define hv_set_simp(val) hv_set_vpreg(HV_REGISTER_SIPP, val)
+
+#define hv_get_siefp(val) (val = hv_get_vpreg(HV_REGISTER_SIFP))
+#define hv_set_siefp(val) hv_set_vpreg(HV_REGISTER_SIFP, val)
+
+#define hv_get_synic_state(val) (val = hv_get_vpreg(HV_REGISTER_SCONTROL))
+#define hv_set_synic_state(val) hv_set_vpreg(HV_REGISTER_SCONTROL, val)
+
+#define hv_get_vp_index(index) (index = hv_get_vpreg(HV_REGISTER_VPINDEX))
+
+#define hv_signal_eom() hv_set_vpreg(HV_REGISTER_EOM, 0)
+
+/*
+ * Hyper-V SINT registers are numbered sequentially, so we can just
+ * add the SINT number to the register number of SINT0
+ */
+#define hv_get_synint_state(sint_num, val) \
+ (val = hv_get_vpreg(HV_REGISTER_SINT0 + sint_num))
+#define hv_set_synint_state(sint_num, val) \
+ hv_set_vpreg(HV_REGISTER_SINT0 + sint_num, val)
+
+#define hv_get_crash_ctl(val) \
+ (val = hv_get_vpreg(HV_REGISTER_CRASH_CTL))
+#define hv_get_time_ref_count(val) \
+ (val = hv_get_vpreg(HV_REGISTER_TIME_REFCOUNT))
+#define hv_get_reference_tsc(val) \
+ (val = hv_get_vpreg(HV_REGISTER_REFERENCE_TSC))
+#define hv_set_reference_tsc(val) \
+ hv_set_vpreg(HV_REGISTER_REFERENCE_TSC, val)
+#define hv_enable_vdso_clocksource()
+#define hv_set_clocksource_vdso(val) \
+ ((val).vdso_clock_mode = VDSO_CLOCKMODE_NONE)
+
+#if IS_ENABLED(CONFIG_HYPERV)

I don't think this guards anything useful.

You are probably right. I'll double-check.

+#define hv_enable_stimer0_percpu_irq(irq) enable_percpu_irq(irq, 0)
+#define hv_disable_stimer0_percpu_irq(irq) disable_percpu_irq(irq)

and this looks pretty premature.

I'm not sure I understand your comment about "premature". Could
you clarify?

+#endif
+
+/* ARM64 specific code to read the hardware clock */
+#define hv_get_raw_timer() arch_timer_read_counter()
+
+/* SMCCC hypercall parameters */
+#define HV_SMCCC_FUNC_NUMBER 1
+#define HV_FUNC_ID ARM_SMCCC_CALL_VAL( \
+ ARM_SMCCC_STD_CALL, \
+ ARM_SMCCC_SMC_64, \
+ ARM_SMCCC_OWNER_VENDOR_HYP, \

This is only defined in patch #2...

Indeed. :-( I'll fix as part of breaking up this patch into smaller
pieces.

Michael

+ HV_SMCCC_FUNC_NUMBER)
+
+#include <asm-generic/mshyperv.h>
+
+#endif

Thanks,
M.
--
Jazz is not dead, it just smells funny.

[Michael Kelley]

+
+/* 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.

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?

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.

+/* 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.

The __packed annotation here is not meaningful,
as the total size is already only a single byte.

Agreed.

+/* Define port identifier type. */
+union hv_port_id {
+ __u32 asu32;
+ struct {
+ __u32 id:24;
+ __u32 reserved:8;
+ } __packed u;
+};

Here, the __packed annotation is inconsistent with the use
in the rest of the file: marking only one member of the union
as __packed means that the union itself is still expected to
be 4 byte aligned. I would expect that either all of these
structures have a sensible alignment, or they are all
completely unaligned.

Agreed. Looks like it is wrong on the x86 side too.

+ * 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.

+#if IS_ENABLED(CONFIG_HYPERV)
+#define hv_enable_stimer0_percpu_irq(irq) enable_percpu_irq(irq, 0)
+#define hv_disable_stimer0_percpu_irq(irq) disable_percpu_irq(irq)
+#endif

Should there be an #else definition here? It helps readability
to have the two versions (with and without hyperv support) close
together rather than in different files. If there is no other
definition, just drop the #if.

Agreed. I'll figure out a way to

[Michael Kelley]

/*
+ * 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.
+ *
+ * These functions are used by arm64 specific code as well as
+ * arch independent Hyper-V drivers.
+ */
+
+void *hv_alloc_hyperv_page(void)
+{
+ BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
+ return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
+}
+EXPORT_SYMBOL_GPL(hv_alloc_hyperv_page);

I don't think there is any guarantee that kmalloc() returns
page-aligned
allocations in general.

guarantee
natural alignment for kmalloc(power-of-two)").

How about using get_free_pages() to implement this?

This would certainly work, at the expense of a lot of wasted memory when
PAGE_SIZE isn't 4k.

I'm sure this is the least of your problems when the guest runs with
a large base page size, you've already wasted most of your memory
otherwise then.

I think there's value in keeping these functions. There are 8 uses in
architecture independent code at the moment, which admittedly saves
only ~1/2 Mbyte of memory with a 64K page size, but we will have
additional uses with more memory savings as we get all of the
Hyper-V synthetic drivers to work with 64K page size. Furthermore,
there's coming work that will require additional steps to share a page
between a guest and the Hyper-V host. These functions are the right
place to put the code for the additional sharing steps. Removing them
now in favor of a bare kmalloc() and then adding them back doesn't
seem worthwhile.

Mic

[Michael Kelley]

Add ARM64-specific code to set up and handle the interrupts
generated by Hyper-V for VMbus messages and for stimer expiration.
This code is architecture dependent and is mostly driven by
architecture independent code in the VMbus driver and the
Hyper-V timer clocksource driver.
This code is built only when CONFIG_HYPERV is enabled.

This looks like it should be a nested irqchip driver instead, so your
device drivers can use the normal request_irq() functions etc.
Is anything preventing you from doing that? If so, please describe
that in the changelog and in a comment in the driver.

As mentioned in my reply on Patch 1, Hyper-V offers a limited synthetic
interrupt controller managed by Linux code that's been around the last
10 years on the x86 side. For reasons that pre-date me, it was not written
as an irqchip driver.

Modulo the small routines you see in this patch, the code is architecture
independent, and it seems we ought to keep the high degree of commonality.
Re-architecting the arch independent code to model as an irqchip driver seems
to carry some risk to the x86 side that has a lot of real-world usage today, but
I'll take a look and see what the risks look like and if it adds any clarit

[Michael Kelley]

On Sat, 14 Mar 2020 15:35:15 +0000,

Add functions to set up and remove kexec and panic
handlers, and to inform Hyper-V about a guest panic.
These functions are called from architecture independent
code in the VMbus driver.
This code is built only when CONFIG_HYPERV is enabled.
---
arch/arm64/hyperv/hv_core.c | 61

++++++++++++++++++++++++++++++++++++++++++++

arch/arm64/hyperv/mshyperv.c | 26 +++++++++++++++++++
2 files changed, 87 insertions(+)
diff --git a/arch/arm64/hyperv/hv_core.c b/arch/arm64/hyperv/hv_core.c
index 4aa6b8f..8d6de9f 100644
--- a/arch/arm64/hyperv/hv_core.c
+++ b/arch/arm64/hyperv/hv_core.c
@@ -199,3 +199,64 @@ void hv_get_vpreg_128(u32 msr, struct

hv_get_vp_register_output *res)

kfree(output);
}
EXPORT_SYMBOL_GPL(hv_get_vpreg_128);
+
+void hyperv_report_panic(struct pt_regs *regs, long err)
+{
+ static bool panic_reported;
+ u64 guest_id;
+
+ /*
+ * We prefer to report panic on 'die' chain as we have proper
+ * registers to report, but if we miss it (e.g. on BUG()) we need
+ * to report it on 'panic'.
+ */
+ if (panic_reported)
+ return;
+ panic_reported = true;

How does this work when multiple vcpus are crashing at once? Are you
guaranteed to be single-threaded at this point?

I'll need to go research. If not, the above should be an atomic
test-and-set.

+
+ guest_id = hv_get_vpreg(HV_REGISTER_GUEST_OSID);
+
+ /*
+ * Hyper-V provides the ability to store only 5 values.
+ * Pick the passed in error value, the guest_id, and the PC.
+ * The first two general registers are added arbitrarily.
+ */
+ hv_set_vpreg(HV_REGISTER_CRASH_P0, err);
+ hv_set_vpreg(HV_REGISTER_CRASH_P1, guest_id);
+ hv_set_vpreg(HV_REGISTER_CRASH_P2, regs->pc);
+ hv_set_vpreg(HV_REGISTER_CRASH_P3, regs->regs[0]);
+ hv_set_vpreg(HV_REGISTER_CRASH_P4, regs->regs[1]);

How about reporting useful information, a pointer to some data
structure describing the fault? As it is, the usefulness of this is
pretty dubious.

Yes, it is dubious. The version with more data describing the fault is
hyperv_report_panic_msg() below, which is provided by newer versions
of Hyper-V, including all versions for ARM64. So the above function
should never get called on ARM64. While we could stub it out, I'd
like to keep the notification to Hyper-V just in case
hyperv_report_panic_msg() is not available for some reason I can't
currently anticipate. The important thing is the notification, and
the register values aren't really important. I'll add a comment to
that effect.

+
+ /*
+ * Let Hyper-V know there is crash data available
+ */
+ hv_set_vpreg(HV_REGISTER_CRASH_CTL, HV_CRASH_CTL_CRASH_NOTIFY);
+}
+EXPORT_SYMBOL_GPL(hyperv_report_panic);
+
+/*
+ * hyperv_report_panic_msg - report panic message to Hyper-V
+ */
+void hyperv_report_panic_msg(phys_addr_t pa, size_t size)
+{
+ /*
+ * P3 to contain the physical address of the panic page & P4 to
+ * contain the size of the panic data in that page. Rest of the
+ * registers are no-op when the NOTIFY_MSG flag is set.
+ */
+ hv_set_vpreg(HV_REGISTER_CRASH_P0, 0);
+ hv_set_vpreg(HV_REGISTER_CRASH_P1, 0);
+ hv_set_vpreg(HV_REGISTER_CRASH_P2, 0);
+ hv_set_vpreg(HV_REGISTER_CRASH_P3, pa);
+ hv_set_vpreg(HV_REGISTER_CRASH_P4, size);
+
+ /*
+ * Let Hyper-V know there is crash data available along with
+ * the panic message.
+ */
+ hv_set_vpreg(HV_REGISTER_CRASH_CTL,
+ (HV_CRASH_CTL_CRASH_NOTIFY | HV_CRASH_CTL_CRASH_NOTIFY_MSG));
+}
+EXPORT_SYMBOL_GPL(hyperv_report_panic_msg);
diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
index ae6ece6..c58940d 100644
--- a/arch/arm64/hyperv/mshyperv.c
+++ b/arch/arm64/hyperv/mshyperv.c
@@ -23,6 +23,8 @@
static void (*vmbus_handler)(void);
static void (*hv_stimer0_handler)(void);
+static void (*hv_kexec_handler)(void);
+static void (*hv_crash_handler)(struct pt_regs *regs);

Why is this in the arch-specific code? Yes, it lives in the x86 arch
code too, but I don't see what prevents it from being moved to the
vmbus_drv.c code.

OK -- I'll see about moving it to arch independent code.

static int vmbus_irq;
static long __percpu *vmbus_evt;
@@ -137,3 +139,27 @@ void hv_remove_stimer0_irq(int irq)
}
}
EXPORT_SYMBOL_GPL(hv_remove_stimer0_irq);
+
+void hv_setup_kexec_handler(void (*handler)(void))
+{
+ hv_kexec_handler = handler;
+}
+EXPORT_SYMBOL_GPL(hv_setup_kexec_handler);
+
+void hv_remove_kexec_handler(void)
+{
+ hv_kexec_handler = NULL;
+}
+EXPORT_SYMBOL_GPL(hv_remove_kexec_handler);
+
+void hv_setup_crash_handler(void (*handler)(struct pt_regs *regs))
+{
+ hv_crash_handler = handler;
+}
+EXPORT_SYMBOL_GPL(hv_setup_crash_handler);
+
+void hv_remove_crash_handler(void)
+{
+ hv_crash_handler = NULL;
+}
+EXPORT_SYMBOL_GPL(hv_remove_crash_handler);
--
1.8.3.1

Thanks,
M.
--
Jazz is not dead, it just smells funny.

[Michael Kelley]

Add ARM64-specific code to initialize the Hyper-V
hypervisor when booting as a guest VM. Provide functions
and data structures indicating hypervisor status that
are needed by VMbus driver.
This code is built only when CONFIG_HYPERV is enabled.
---
arch/arm64/hyperv/hv_core.c | 156

++++++++++++++++++++++++++++++++++++++++++++
As you are effectively adding a new clocksource driver here, please move the
code to drivers/clocksource and send the patch to the respective maintainers
(added to Cc here), splitting it out from the rest of the patch.
You should also describe why your platform doesn't just use the normal
architected timer interface.

+TIMER_ACPI_DECLARE(hyperv, ACPI_SIG_GTDT, hyperv_init);

This looks like it registers a driver for the same device as the normal
arch timer. Won't that clash?
Arnd

There is a Hyper-V clocksource driver in drivers/clocksource/hyperv_timer.c.
It is architecture independent and works for both x86 and ARM64.

The requirement here is really for a place to hang the general Hyper-V
initialization code. On the x86 side, there's infrastructure already in place
to do hypervisor initialization, but nothing corresponding on the ARM64 side.
The TIMER_ACPI_DECLARE hook is admittedly a temporary approach, and I'm
happy to hear if someone has a better way to handle this.

FWIW, Hyper-V doesn't currently virtualize the ARM arch counter/timer for
guest VMs. The Hyper-V synthetic counter/timer in the Hyper-V clocksource
driver is used on both ARM64 and x86. But this Hyper-V init code doesn't actually
touch the GTDT device, so it won't interfere with the ARM arch counter/timer
when a future Hyper-V version

[Michael Kelley]

The Hyper-V frame buffer driver may be built as a module, and
it needs access to screen_info. So export screen_info.

Is there any chance of using a more modern KMS based driver for the screen
than the old fbdev subsystem? I had hoped to one day completely remove
support for the old CONFIG_VIDEO_FBDEV and screen_info from modern
architectures.

The current hyperv_fb.c driver is all we have today for the synthetic Hyper-V
frame buffer device. That driver builds and runs on both ARM64 and x86.

I'm not knowledgeable about video/graphics drivers, but when you
say "a more modern KMS based driver", are you meaning one based on
DRM & KMS? Does DRM make sense for a "dumb" frame buffer device?
Are there any drivers that would be a good