diff mbox series

[v2,2/2] x86/hyperv: fix invalid writes to MSRs during root partition kexec

Message ID 20221027095729.1676394-3-anrayabh@linux.microsoft.com
State New
Headers
Series Fix MSR access errors during kexec in root partition |

Commit Message

Anirudh Rayabharam Oct. 27, 2022, 9:57 a.m. UTC 
  hv_cleanup resets the hypercall page by setting the MSR to 0. However,
the root partition is not allowed to write to the GPA bits of the MSR.
Instead, it uses the hypercall page provided by the MSR. Similar is the
case with the reference TSC MSR.

Clear only the enable bit instead of zeroing the entire MSR to make
the code valid for root partition too.

Signed-off-by: Anirudh Rayabharam <anrayabh@linux.microsoft.com>
---
 arch/x86/hyperv/hv_init.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Michael Kelley (LINUX) Oct. 27, 2022, 1:44 p.m. UTC | #1 
From: Anirudh Rayabharam <anrayabh@linux.microsoft.com> Sent: Thursday, October 27, 2022 2:57 AM
> 
> hv_cleanup resets the hypercall page by setting the MSR to 0. However,

The function name is hyperv_cleanup(), not hv_cleanup().

> the root partition is not allowed to write to the GPA bits of the MSR.
> Instead, it uses the hypercall page provided by the MSR. Similar is the
> case with the reference TSC MSR.
> 
> Clear only the enable bit instead of zeroing the entire MSR to make
> the code valid for root partition too.
> 
> Signed-off-by: Anirudh Rayabharam <anrayabh@linux.microsoft.com>
> ---
>  arch/x86/hyperv/hv_init.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 29774126e931..80fdfff9266c 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -537,6 +537,7 @@ void __init hyperv_init(void)
>  void hyperv_cleanup(void)
>  {
>  	union hv_x64_msr_hypercall_contents hypercall_msr;
> +	union hv_reference_tsc_msr tsc_msr;
> 
>  	unregister_syscore_ops(&hv_syscore_ops);
> 
> @@ -552,12 +553,14 @@ void hyperv_cleanup(void)
>  	hv_hypercall_pg = NULL;
> 
>  	/* Reset the hypercall page */
> -	hypercall_msr.as_uint64 = 0;
> -	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +	hypercall_msr.as_uint64 = hv_get_register(HV_X64_MSR_HYPERCALL);
> +	hypercall_msr.enable = 0;
> +	hv_set_register(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> 
>  	/* Reset the TSC page */
> -	hypercall_msr.as_uint64 = 0;
> -	wrmsrl(HV_X64_MSR_REFERENCE_TSC, hypercall_msr.as_uint64);
> +	tsc_msr.as_uint64 = hv_get_register(HV_X64_MSR_REFERENCE_TSC);
> +	tsc_msr.enable = 0;
> +	hv_set_register(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
>  }
> 
>  void hyperv_report_panic(struct pt_regs *regs, long err, bool in_die)
> --
> 2.34.1

Modulo the nit in the commit message,

Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Wei Liu Oct. 27, 2022, 7:16 p.m. UTC | #2 
On Thu, Oct 27, 2022 at 01:44:40PM +0000, Michael Kelley (LINUX) wrote:
> From: Anirudh Rayabharam <anrayabh@linux.microsoft.com> Sent: Thursday, October 27, 2022 2:57 AM
> > 
> > hv_cleanup resets the hypercall page by setting the MSR to 0. However,
> 
> The function name is hyperv_cleanup(), not hv_cleanup().

I fixed this and applied both patches to hyperv-fixes. Thank you both.
Anirudh Rayabharam Oct. 28, 2022, 12:34 p.m. UTC | #3 
On Thu, Oct 27, 2022 at 07:16:49PM +0000, Wei Liu wrote:
> On Thu, Oct 27, 2022 at 01:44:40PM +0000, Michael Kelley (LINUX) wrote:
> > From: Anirudh Rayabharam <anrayabh@linux.microsoft.com> Sent: Thursday, October 27, 2022 2:57 AM
> > > 
> > > hv_cleanup resets the hypercall page by setting the MSR to 0. However,
> > 
> > The function name is hyperv_cleanup(), not hv_cleanup().
> 
> I fixed this and applied both patches to hyperv-fixes. Thank you both.

Thank you!

Anirudh.
diff mbox series

Patch

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 29774126e931..80fdfff9266c 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -537,6 +537,7 @@  void __init hyperv_init(void)
 void hyperv_cleanup(void)
 {
 	union hv_x64_msr_hypercall_contents hypercall_msr;
+	union hv_reference_tsc_msr tsc_msr;
 
 	unregister_syscore_ops(&hv_syscore_ops);
 
@@ -552,12 +553,14 @@  void hyperv_cleanup(void)
 	hv_hypercall_pg = NULL;
 
 	/* Reset the hypercall page */
-	hypercall_msr.as_uint64 = 0;
-	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+	hypercall_msr.as_uint64 = hv_get_register(HV_X64_MSR_HYPERCALL);
+	hypercall_msr.enable = 0;
+	hv_set_register(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
 
 	/* Reset the TSC page */
-	hypercall_msr.as_uint64 = 0;
-	wrmsrl(HV_X64_MSR_REFERENCE_TSC, hypercall_msr.as_uint64);
+	tsc_msr.as_uint64 = hv_get_register(HV_X64_MSR_REFERENCE_TSC);
+	tsc_msr.enable = 0;
+	hv_set_register(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
 }
 
 void hyperv_report_panic(struct pt_regs *regs, long err, bool in_die)