| 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
| ||
|
Message-ID:
<SN6PR02MB415750DBD8B57234F840BF12D472A@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Wed, 18 Jun 2025 16:17:47 +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 03/15] arch: hyperv: Get/set SynIC
synth.registers via paravisor
From: Roman Kisel <romank@...ux.microsoft.com> Sent: Tuesday, June 3, 2025 5:43 PM
>
> The confidential VMBus is built on the guest talking to the
> paravisor only.
>
> Provide functions that allow manipulating the SynIC registers
> via paravisor. No functional changes.
I'd like to see a little more detailed explanation for "why" the
new functions. Something like:
The existing Hyper-V wrappers for getting and setting MSRs are
hv_get/set_msr(). Via hv_get/set_non_nested_msr(), they detect
when running in a CoCo VM with a paravisor, and use the TDX or
SNP guest-host communication protocol to bypass the paravisor
and go directly to the host hypervisor for SynIC MSRs. The "set"
function also implements the required special handling for the
SINT MSRs.
But in some Confidential VMBus cases, the guest wants to talk only
with the paravisor. To accomplish this, provide new functions for
accessing SynICs that always do direct accesses (i.e., not via
TDX or SNP GHCB), which will go to the paravisor. The mirroring
of the existing "set" function is also not needed. These functions
should be used only in the specific Confidential VMBus cases that
require them.
And I'm not sure "No functional changes" is correct. This is adding
new functionality. It's not just a cosmetic change or code
refactoring.
>
> Signed-off-by: Roman Kisel <romank@...ux.microsoft.com>
> Reviewed-by: Alok Tiwari <alok.a.tiwari@...cle.com>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 44 ++++++++++++++++++++++++++++++++++
> drivers/hv/hv_common.c | 13 ++++++++++
> include/asm-generic/mshyperv.h | 2 ++
> 3 files changed, 59 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 3e2533954675..83a85d94bcb3 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -89,6 +89,50 @@ void hv_set_non_nested_msr(unsigned int reg, u64 value)
> }
> EXPORT_SYMBOL_GPL(hv_set_non_nested_msr);
>
> +/*
> + * Attempt to get the SynIC register value from the paravisor.
> + *
> + * Not all paravisors support reading SynIC registers, so this function
> + * may fail. The register for the SynIC of the running CPU is accessed.
> + *
> + * Writes the SynIC register value into the memory pointed by val,
> + * and ~0ULL is on failure.
> + *
> + * Returns -ENODEV if the MSR is not a SynIC register, or another error
> + * code if getting the MSR fails (meaning the paravisor doesn't support
> + * relaying VMBus comminucations).
s/comminucations/communications/
> + */
> +int hv_para_get_synic_register(unsigned int reg, u64 *val)
> +{
> + u64 reg_val = ~0ULL;
> + int err = -ENODEV;
> +
> + if (hv_is_synic_msr(reg))
> + reg_val = native_read_msr_safe(reg, &err);
> + *val = reg_val;
> +
> + return err;
> +}
> +
> +/*
> + * Attempt to set the SynIC register value with the paravisor.
> + *
> + * Not all paravisors support reading SynIC registers, so this function
s/reading/setting/
> + * may fail. The register for the SynIC of the running CPU is accessed.
> + *
> + * Sets the register to the value supplied.
> + *
> + * Returns: -ENODEV if the MSR is not a SynIC register, or another error
> + * code if writing to the MSR fails (meaning the paravisor doesn't support
> + * relaying VMBus comminucations).
s/comminucations/communications/
> + */
> +int hv_para_set_synic_register(unsigned int reg, u64 val)
> +{
> + if (!hv_is_synic_msr(reg))
> + return -ENODEV;
> + return wrmsrl_safe(reg, val);
> +}
> +
> u64 hv_get_msr(unsigned int reg)
> {
> if (hv_nested)
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 49898d10faff..a179ea482cb1 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -716,6 +716,19 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64
> param2)
> }
> EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
>
> +int __weak hv_para_get_synic_register(unsigned int reg, u64 *val)
> +{
> + *val = ~0ULL;
> + return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(hv_para_get_synic_register);
> +
> +int __weak hv_para_set_synic_register(unsigned int reg, u64 val)
> +{
> + return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(hv_para_set_synic_register);
> +
> void hv_identify_partition_type(void)
> {
> /* Assume guest role */
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 9722934a8332..f239b102fc53 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -333,6 +333,8 @@ bool hv_is_isolation_supported(void);
> bool hv_isolation_type_snp(void);
> u64 hv_ghcb_hypercall(u64 control, void *input, void *output, u32 input_size);
> u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
> +int hv_para_get_synic_register(unsigned int reg, u64 *val);
> +int hv_para_set_synic_register(unsigned int reg, u64 val);
> void hyperv_cleanup(void);
> bool hv_query_ext_cap(u64 cap_query);
> void hv_setup_dma_ops(struct device *dev, bool coherent);
> --
> 2.43.0
Powered by blists - more mailing lists