linux-kernel - RE: [PATCH hyperv-next v3 04/15] arch/x86: mshyperv: Trap on access for some synthetic MSRs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <SN6PR02MB4157E84CE73DF70697A09DFCD472A@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Wed, 18 Jun 2025 16:18:01 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Roman Kisel <romank@...ux.microsoft.com>, "alok.a.tiwari@...cle.com"
	<alok.a.tiwari@...cle.com>, "arnd@...db.de" <arnd@...db.de>, "bp@...en8.de"
	<bp@...en8.de>, "corbet@....net" <corbet@....net>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"decui@...rosoft.com" <decui@...rosoft.com>, "haiyangz@...rosoft.com"
	<haiyangz@...rosoft.com>, "hpa@...or.com" <hpa@...or.com>,
	"kys@...rosoft.com" <kys@...rosoft.com>, "mingo@...hat.com"
	<mingo@...hat.com>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"wei.liu@...nel.org" <wei.liu@...nel.org>, "linux-arch@...r.kernel.org"
	<linux-arch@...r.kernel.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "x86@...nel.org" <x86@...nel.org>
CC: "apais@...rosoft.com" <apais@...rosoft.com>, "benhill@...rosoft.com"
	<benhill@...rosoft.com>, "bperkins@...rosoft.com" <bperkins@...rosoft.com>,
	"sunilmut@...rosoft.com" <sunilmut@...rosoft.com>
Subject: RE: [PATCH hyperv-next v3 04/15] arch/x86: mshyperv: Trap on access
 for some synthetic MSRs

From: Roman Kisel <romank@...ux.microsoft.com> Sent: Tuesday, June 3, 2025 5:44 PM
> 
> To set up and run the confidential VMBus, the guest needs the paravisor
> to intercept access to some synthetic MSRs. In the non-confidential case,
> the guest continues using the vendor-specific guest-host communication
> protocol.
> 
> Update the hv_set_non_nested_msr() function to trap access to some
> synthetic MSRs.

"trap access" is somewhat generic, and it's not clear what the intent is.
I'd suggest something like:

hv_set_non_nested_msr() has special handling for SINT MSRs
when a paravisor is present. In addition to updating the MSR on the
host, the mirror MSR in the paravisor is updated, including with the
proxy bit. But with Confidential VMBus, the proxy bit must not be
used, so add a special case to skip it.

> 
> Signed-off-by: Roman Kisel <romank@...ux.microsoft.com>
> Reviewed-by: Alok Tiwari <alok.a.tiwari@...cle.com>
> ---
>  arch/x86/kernel/cpu/mshyperv.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 83a85d94bcb3..db6f3e3db012 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -28,6 +28,7 @@
>  #include <asm/apic.h>
>  #include <asm/timer.h>
>  #include <asm/reboot.h>
> +#include <asm/msr.h>
>  #include <asm/nmi.h>
>  #include <clocksource/hyperv_timer.h>
>  #include <asm/numa.h>
> @@ -77,14 +78,28 @@ EXPORT_SYMBOL_GPL(hv_get_non_nested_msr);
> 
>  void hv_set_non_nested_msr(unsigned int reg, u64 value)
>  {
> +	if (reg == HV_X64_MSR_EOM && vmbus_is_confidential()) {
> +		/* Reach out to the paravisor. */
> +		native_wrmsrl(reg, value);
> +		return;
> +	}
> +

It seems a bit inconsistent to have this particular MSR treated as
a special case in the generic code path, when the new functions
hv_para_get/set_synic_register() have been introduced to handle
the unique requirements of Confidential VMBus. This MSR is set
only in vmbus_signal_eom(), so maybe vmbus_signal_eom()
should test for confidential VM, and call hv_para_set_synic_register()
instead?

>  	if (hv_is_synic_msr(reg) && ms_hyperv.paravisor_present) {
> +		/* The hypervisor will get the intercept. */
>  		hv_ivm_msr_write(reg, value);
> 
> -		/* Write proxy bit via wrmsl instruction */
> -		if (hv_is_sint_msr(reg))
> -			wrmsrl(reg, value | 1 << 20);
> +		if (hv_is_sint_msr(reg)) {
> +			/*
> +			 * Write proxy bit in the case of non-confidential VMBus.
> +			 * Using wrmsl instruction so the following goes to the paravisor.
> +			 */
> +			u32 proxy = vmbus_is_confidential() ? 0 : 1;
> +
> +			value |= (proxy << 20);
> +			native_wrmsrl(reg, value);
> +		}
>  	} else {
> -		wrmsrl(reg, value);
> +		native_wrmsrl(reg, value);
>  	}
>  }
>  EXPORT_SYMBOL_GPL(hv_set_non_nested_msr);
> --
> 2.43.0