[PATCH hyperv-next 0/2] x86/hyperv: VTL mode reboot fixes

[PATCH hyperv-next 0/2] x86/hyperv: VTL mode reboot fixes

[Roman Kisel]

The first patch defines a specialized machine emergency restart
callback not to write to the physical address of 0x472 which is
what the native_machine_emergency_restart() does unconditionally.

I first wanted to tweak that function[1], and in the course of
the discussion it looked as the risks of doing that would
outweigh the benefit: the bare-metal systems have likely adopted
that behavior as a standard although I could not find any mentions
of that magic address in the UEFI+ACPI specification.

The second patch removes the need to always supply "reboot=t"
to the kernel command line in the OpenHCL bootloader [2]. There is
no other option at the moment; when/if it appears the newly added
callback's code can be adjusted as required.

It would be great to apply this to the stable tree if no concerns,
should apply cleanly.

[1] https://lore.kernel.org/all/20250109204352.1720337-1-romank@linux.microsoft.com/
[2] https://github.com/microsoft/openvmm/blob/7a9d0e0a00461be6e5f3267af9ea54cc7157c900/openhcl/openhcl_boot/src/main.rs#L139

Roman Kisel (2):
  x86/hyperv: VTL mode emergency restart callback
  x86/hyperv: VTL mode callback for restarting the system

 arch/x86/hyperv/hv_vtl.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)


base-commit: 2e03358be78b65d28b66e17aca9e0c8700b0df78
-- 
2.34.1

[PATCH hyperv-next 1/2] x86/hyperv: VTL mode emergency restart callback

[Roman Kisel]

By default, X86(-64) systems use the emergecy restart routine
in the course of which the code unconditionally writes to
the physical address of 0x472 to indicate the boot mode
to the firmware (BIOS or UEFI).

When the kernel itself runs as a firmware in the VTL mode,
that write corrupts the memory of the guest upon emergency
restarting. Preserving the state intact in that situation
is important for debugging, at least.

Define the specialized machine callback to avoid that write
and use the triple fault to perform emergency restart.

Cc: stable@vger.kernel.org
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 arch/x86/hyperv/hv_vtl.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 4e1b1e3b5658..8931fc186a5f 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -12,6 +12,7 @@
 #include <asm/i8259.h>
 #include <asm/mshyperv.h>
 #include <asm/realmode.h>
+#include <asm/reboot.h>
 #include <../kernel/smpboot.h>
 
 extern struct boot_params boot_params;
@@ -22,6 +23,27 @@ static bool __init hv_vtl_msi_ext_dest_id(void)
 	return true;
 }
 
+/*
+ * The `native_machine_emergency_restart` function from `reboot.c` writes
+ * to the physical address 0x472 to indicate the type of reboot for the
+ * firmware. We cannot have that in VSM as the memory composition might
+ * be more generic, and such write effectively corrupts the memory thus
+ * making diagnostics harder at the very least.
+ */
+static void  __noreturn hv_vtl_emergency_restart(void)
+{
+	/*
+	 * Cause a triple fault and the immediate reset. Here the code does not run
+	 * on the top of any firmware, whereby cannot reach out to its services.
+	 * The inifinite loop is for the improbable case that the triple fault does
+	 * not work and have to preserve the state intact for debugging.
+	 */
+	for (;;) {
+		idt_invalidate();
+		__asm__ __volatile__("int3");
+	}
+}
+
 void __init hv_vtl_init_platform(void)
 {
 	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
@@ -246,5 +268,7 @@ int __init hv_vtl_early_init(void)
 	real_mode_header = &hv_vtl_real_mode_header;
 	apic_update_callback(wakeup_secondary_cpu_64, hv_vtl_wakeup_secondary_cpu);
 
+	machine_ops.emergency_restart = hv_vtl_emergency_restart;
+
 	return 0;
 }
-- 
2.34.1

[PATCH hyperv-next 2/2] x86/hyperv: VTL mode callback for restarting the system

[Roman Kisel]

The kernel runs as a firmware in the VTL mode, and the only way
to restart on x86 is to triple fault. Thus, one has to always
supply "reboot=t" on the kernel command line, and missing that
renders rebooting not working.

Define the machine restart callback to always use the triple
fault to provide the robust configuration by default.

Cc: stable@vger.kernel.org
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 arch/x86/hyperv/hv_vtl.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 8931fc186a5f..eb402362d738 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -44,6 +44,12 @@ static void  __noreturn hv_vtl_emergency_restart(void)
 	}
 }
 
+/* The only way to restart is triple fault */
+static void  __noreturn hv_vtl_restart(char *)
+{
+	hv_vtl_emergency_restart();
+}
+
 void __init hv_vtl_init_platform(void)
 {
 	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
@@ -269,6 +275,7 @@ int __init hv_vtl_early_init(void)
 	apic_update_callback(wakeup_secondary_cpu_64, hv_vtl_wakeup_secondary_cpu);
 
 	machine_ops.emergency_restart = hv_vtl_emergency_restart;
+	machine_ops.restart = hv_vtl_restart;
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH hyperv-next 2/2] x86/hyperv: VTL mode callback for restarting the system

[kernel test robot]

Hi Roman,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 2e03358be78b65d28b66e17aca9e0c8700b0df78]

url:    https://github.com/intel-lab-lkp/linux/commits/Roman-Kisel/x86-hyperv-VTL-mode-emergency-restart-callback/20250118-050923
base:   2e03358be78b65d28b66e17aca9e0c8700b0df78
patch link:    https://lore.kernel.org/r/20250117210702.1529580-3-romank%40linux.microsoft.com
patch subject: [PATCH hyperv-next 2/2] x86/hyperv: VTL mode callback for restarting the system
config: x86_64-buildonly-randconfig-003-20250118 (https://download.01.org/0day-ci/archive/20250118/202501182304.lZJpR7pL-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250118/202501182304.lZJpR7pL-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501182304.lZJpR7pL-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> arch/x86/hyperv/hv_vtl.c:48:46: warning: omitting the parameter name in a function definition is a C23 extension [-Wc23-extensions]
      48 | static void  __noreturn hv_vtl_restart(char *)
         |                                              ^
   1 warning generated.


vim +48 arch/x86/hyperv/hv_vtl.c

    46	
    47	/* The only way to restart is triple fault */
  > 48	static void  __noreturn hv_vtl_restart(char *)
    49	{
    50		hv_vtl_emergency_restart();
    51	}
    52	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH hyperv-next 0/2] x86/hyperv: VTL mode reboot fixes

[Wei Liu]

On Fri, Jan 17, 2025 at 01:07:00PM -0800, Roman Kisel wrote:
> The first patch defines a specialized machine emergency restart
> callback not to write to the physical address of 0x472 which is
> what the native_machine_emergency_restart() does unconditionally.
> 
> I first wanted to tweak that function[1], and in the course of
> the discussion it looked as the risks of doing that would
> outweigh the benefit: the bare-metal systems have likely adopted
> that behavior as a standard although I could not find any mentions
> of that magic address in the UEFI+ACPI specification.
> 
> The second patch removes the need to always supply "reboot=t"
> to the kernel command line in the OpenHCL bootloader [2]. There is
> no other option at the moment; when/if it appears the newly added
> callback's code can be adjusted as required.
> 
> It would be great to apply this to the stable tree if no concerns,
> should apply cleanly.
> 
> [1] https://lore.kernel.org/all/20250109204352.1720337-1-romank@linux.microsoft.com/
> [2] https://github.com/microsoft/openvmm/blob/7a9d0e0a00461be6e5f3267af9ea54cc7157c900/openhcl/openhcl_boot/src/main.rs#L139
> 
> Roman Kisel (2):
>   x86/hyperv: VTL mode emergency restart callback
>   x86/hyperv: VTL mode callback for restarting the system

Saurabh please review these patches. Thanks.

I don't have a strong opinion on them.

> 
>  arch/x86/hyperv/hv_vtl.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> 
> base-commit: 2e03358be78b65d28b66e17aca9e0c8700b0df78
> -- 
> 2.34.1
> 

Re: [PATCH hyperv-next 0/2] x86/hyperv: VTL mode reboot fixes

[Saurabh Singh Sengar]

On Wed, Feb 12, 2025 at 02:21:18AM +0000, Wei Liu wrote:
> On Fri, Jan 17, 2025 at 01:07:00PM -0800, Roman Kisel wrote:
> > The first patch defines a specialized machine emergency restart
> > callback not to write to the physical address of 0x472 which is
> > what the native_machine_emergency_restart() does unconditionally.
> > 
> > I first wanted to tweak that function[1], and in the course of
> > the discussion it looked as the risks of doing that would
> > outweigh the benefit: the bare-metal systems have likely adopted
> > that behavior as a standard although I could not find any mentions
> > of that magic address in the UEFI+ACPI specification.
> > 
> > The second patch removes the need to always supply "reboot=t"
> > to the kernel command line in the OpenHCL bootloader [2]. There is
> > no other option at the moment; when/if it appears the newly added
> > callback's code can be adjusted as required.
> > 
> > It would be great to apply this to the stable tree if no concerns,
> > should apply cleanly.
> > 
> > [1] https://lore.kernel.org/all/20250109204352.1720337-1-romank@linux.microsoft.com/
> > [2] https://github.com/microsoft/openvmm/blob/7a9d0e0a00461be6e5f3267af9ea54cc7157c900/openhcl/openhcl_boot/src/main.rs#L139
> > 
> > Roman Kisel (2):
> >   x86/hyperv: VTL mode emergency restart callback
> >   x86/hyperv: VTL mode callback for restarting the system
> 
> Saurabh please review these patches. Thanks.

Hi Roman,

Thanks for the patch, few suggestions and queries:

1. Please fix the kernel bot warning
2. Cc Stable tree is not enough, you need to mention the "Fixes" tag as well
   for the commit upto where you want this patch to be backported.
3. In your 2/2 commit, you mention 'triple fault' is the only way to reboot in x86.
   Is that accurate ? Do you mean to say OpenHCL/VTL here ?
   If this behaviour is specific to OpenHCl and not VTLs in general, is there a way
   we can make these changes only for OpenHCL.
   

- Saurabh

> 
> I don't have a strong opinion on them.
> 
> > 
> >  arch/x86/hyperv/hv_vtl.c | 31 +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> > 
> > 
> > base-commit: 2e03358be78b65d28b66e17aca9e0c8700b0df78
> > -- 
> > 2.34.1
> > 

Re: [PATCH hyperv-next 0/2] x86/hyperv: VTL mode reboot fixes

[Roman Kisel]

On 2/12/2025 9:54 AM, Saurabh Singh Sengar wrote:
> On Wed, Feb 12, 2025 at 02:21:18AM +0000, Wei Liu wrote:
[...]
>>
>> Saurabh please review these patches. Thanks.
> 
> Hi Roman,
Hi Saurabh,

> 
> Thanks for the patch, few suggestions and queries:
> 
> 1. Please fix the kernel bot warning
Will do!

> 2. Cc Stable tree is not enough, you need to mention the "Fixes" tag as well
>     for the commit upto where you want this patch to be backported.
Understood, thanks!

> 3. In your 2/2 commit, you mention 'triple fault' is the only way to reboot in x86.
>     Is that accurate ? Do you mean to say OpenHCL/VTL here ?
>     If this behaviour is specific to OpenHCl and not VTLs in general, is there a way
>     we can make these changes only for OpenHCL.
Right, I meant OpenHCL/VTL2, thank you very much! The changes are scoped
to running in VTLs in general at the moment. I can add a check for the
VTL == 2 if you'd like me to.

For VTL1 (aka LVBS), maybe folks would like to do something else,
do you happen to know? Maybe to report that to the VTL0 guest kernel
although seems dubious: the higher VTL failed so the lights must be out
for the lower VTLs? Or kexec?

>     
> 
> - Saurabh
> 
>>
>> I don't have a strong opinion on them.
>>
>>>
>>>   arch/x86/hyperv/hv_vtl.c | 31 +++++++++++++++++++++++++++++++
>>>   1 file changed, 31 insertions(+)
>>>
>>>
>>> base-commit: 2e03358be78b65d28b66e17aca9e0c8700b0df78
>>> -- 
>>> 2.34.1
>>>

-- 
Thank you,
Roman

Re: [PATCH hyperv-next 0/2] x86/hyperv: VTL mode reboot fixes

[Saurabh Singh Sengar]

On Wed, Feb 12, 2025 at 02:56:43PM -0800, Roman Kisel wrote:
> 
> 
> On 2/12/2025 9:54 AM, Saurabh Singh Sengar wrote:
> >On Wed, Feb 12, 2025 at 02:21:18AM +0000, Wei Liu wrote:
> [...]
> >>
> >>Saurabh please review these patches. Thanks.
> >
> >Hi Roman,
> Hi Saurabh,
> 
> >
> >Thanks for the patch, few suggestions and queries:
> >
> >1. Please fix the kernel bot warning
> Will do!
> 
> >2. Cc Stable tree is not enough, you need to mention the "Fixes" tag as well
> >    for the commit upto where you want this patch to be backported.
> Understood, thanks!
> 
> >3. In your 2/2 commit, you mention 'triple fault' is the only way to reboot in x86.
> >    Is that accurate ? Do you mean to say OpenHCL/VTL here ?
> >    If this behaviour is specific to OpenHCl and not VTLs in general, is there a way
> >    we can make these changes only for OpenHCL.
> Right, I meant OpenHCL/VTL2, thank you very much! The changes are scoped
> to running in VTLs in general at the moment. I can add a check for the
> VTL == 2 if you'd like me to.
> 
> For VTL1 (aka LVBS), maybe folks would like to do something else,
> do you happen to know? Maybe to report that to the VTL0 guest kernel
> although seems dubious: the higher VTL failed so the lights must be out
> for the lower VTLs? Or kexec?

I am not aware of LVBS plans, and as far as I consider OpenVMM the only current
user of this VTL code. I recommend keeping the code as simple as possible unless
there is a clear use case for additional complexity. It would be helpful to include
these details in a comment so that future users of this file can understand and
modify it as needed.

- Saurabh

> 
> >
> >- Saurabh
> >
> >>
> >>I don't have a strong opinion on them.
> >>
> >>>
> >>>  arch/x86/hyperv/hv_vtl.c | 31 +++++++++++++++++++++++++++++++
> >>>  1 file changed, 31 insertions(+)
> >>>
> >>>
> >>>base-commit: 2e03358be78b65d28b66e17aca9e0c8700b0df78
> >>>-- 
> >>>2.34.1
> >>>
> 
> -- 
> Thank you,
> Roman
>