| Message ID | 1584200119-18594-7-git-send-email-mikelley@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Subject: Enable Linux guests on Hyper-V on ARM64 | expand |
On Sat, 14 Mar 2020 15:35:15 +0000, Michael Kelley <mikelley@microsoft.com> wrote: > > 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. > > Signed-off-by: Michael Kelley <mikelley@microsoft.com> > --- > 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? > + > + 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. > + > + /* > + * 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 > + * @pa: physical address of the panic page containing the message > + * @size: size of the message in the page > + */ > +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. > > 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.
From: Marc Zyngier <maz@kernel.org> Sent: Sunday, March 15, 2020 11:16 AM > > On Sat, 14 Mar 2020 15:35:15 +0000, > Michael Kelley <mikelley@microsoft.com> wrote: > > > > 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. > > > > Signed-off-by: Michael Kelley <mikelley@microsoft.com> > > --- > > 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 > > + * @pa: physical address of the panic page containing the message > > + * @size: size of the message in the page > > + */ > > +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.
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; + + 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]); + + /* + * 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 + * @pa: physical address of the panic page containing the message + * @size: size of the message in the page + */ +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); 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);
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. Signed-off-by: Michael Kelley <mikelley@microsoft.com> --- arch/arm64/hyperv/hv_core.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ arch/arm64/hyperv/mshyperv.c | 26 +++++++++++++++++++ 2 files changed, 87 insertions(+)