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

Roman Kisel posted 2 patches 1 year ago
There is a newer version of this series
arch/x86/hyperv/hv_vtl.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
[PATCH hyperv-next 0/2] x86/hyperv: VTL mode reboot fixes
Posted by Roman Kisel 1 year ago
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
Re: [PATCH hyperv-next 0/2] x86/hyperv: VTL mode reboot fixes
Posted by Wei Liu 12 months ago
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
Posted by Saurabh Singh Sengar 12 months ago
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
Posted by Roman Kisel 11 months, 4 weeks ago

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
Posted by Saurabh Singh Sengar 11 months, 4 weeks ago
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
>