From: Roman Kisel <romank@xxxxxxxxxxxxxxxxxxx> Sent: Monday, July 14, 2025 3:16 PM
>
> 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.
>
> Provide functions that allow manipulating the SynIC registers
> through the paravisor.
This last paragraph seems to be a leftover from a previous
version of the commit message. I think it can be deleted since
the second paragraph now covers the topic.
>
> Signed-off-by: Roman Kisel <romank@xxxxxxxxxxxxxxxxxxx>
> Reviewed-by: Alok Tiwari <alok.a.tiwari@xxxxxxxxxx>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 39 ++++++++++++++++++
> drivers/hv/hv_common.c | 13 ++++++
> include/asm-generic/mshyperv.h | 75 ++++++++++++++++++----------------
> 3 files changed, 92 insertions(+), 35 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index c78f860419d6..07c60231d0d8 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -90,6 +90,45 @@ 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 communucations).
> + */
> +int hv_para_get_synic_register(unsigned int reg, u64 *val)
> +{
> + if (!hv_is_synic_msr(reg))
> + return -ENODEV;
> + return native_read_msr_safe(reg, val);
> +}
> +
> +/*
> + * Attempt to set the SynIC register value with the paravisor.
> + *
> + * Not all paravisors support setting SynIC registers, so this function
> + * 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 communucations).
> + */
> +int hv_para_set_synic_register(unsigned int reg, u64 val)
> +{
> + if (!hv_is_synic_msr(reg))
> + return -ENODEV;
> + return native_write_msr_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..9447558f425b 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -162,41 +162,6 @@ static inline u64 hv_generate_guest_id(u64 kernel_version)
> return guest_id;
> }
>
> -/* Free the message slot and signal end-of-message if required */
> -static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
> -{
> - /*
> - * On crash we're reading some other CPU's message page and we need
> - * to be careful: this other CPU may already had cleared the header
> - * and the host may already had delivered some other message there.
> - * In case we blindly write msg->header.message_type we're going
> - * to lose it. We can still lose a message of the same type but
> - * we count on the fact that there can only be one
> - * CHANNELMSG_UNLOAD_RESPONSE and we don't care about other messages
> - * on crash.
> - */
> - if (cmpxchg(&msg->header.message_type, old_msg_type,
> - HVMSG_NONE) != old_msg_type)
> - return;
> -
> - /*
> - * The cmxchg() above does an implicit memory barrier to
> - * ensure the write to MessageType (ie set to
> - * HVMSG_NONE) happens before we read the
> - * MessagePending and EOMing. Otherwise, the EOMing
> - * will not deliver any more messages since there is
> - * no empty slot
> - */
> - if (msg->header.message_flags.msg_pending) {
> - /*
> - * This will cause message queue rescan to
> - * possibly deliver another msg from the
> - * hypervisor
> - */
> - hv_set_msr(HV_MSR_EOM, 0);
> - }
> -}
> -
> int hv_get_hypervisor_version(union hv_hypervisor_version_info *info);
>
> void hv_setup_vmbus_handler(void (*handler)(void));
> @@ -333,6 +298,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);
> @@ -375,6 +342,44 @@ static inline int hv_call_create_vp(int node, u64 partition_id,
> u32 vp_index, u3
> #endif /* CONFIG_MSHV_ROOT */
> bool vmbus_is_confidential(void);
>
> +/* Free the message slot and signal end-of-message if required */
> +static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
> +{
> + /*
> + * On crash we're reading some other CPU's message page and we need
> + * to be careful: this other CPU may already had cleared the header
> + * and the host may already had delivered some other message there.
> + * In case we blindly write msg->header.message_type we're going
> + * to lose it. We can still lose a message of the same type but
> + * we count on the fact that there can only be one
> + * CHANNELMSG_UNLOAD_RESPONSE and we don't care about other messages
> + * on crash.
> + */
> + if (cmpxchg(&msg->header.message_type, old_msg_type,
> + HVMSG_NONE) != old_msg_type)
> + return;
> +
> + /*
> + * The cmxchg() above does an implicit memory barrier to
> + * ensure the write to MessageType (ie set to
> + * HVMSG_NONE) happens before we read the
> + * MessagePending and EOMing. Otherwise, the EOMing
> + * will not deliver any more messages since there is
> + * no empty slot
> + */
> + if (msg->header.message_flags.msg_pending) {
> + /*
> + * This will cause message queue rescan to
> + * possibly deliver another msg from the
> + * hypervisor
> + */
> + if (vmbus_is_confidential())
> + hv_para_set_synic_register(HV_MSR_EOM, 0);
As reported by the kernel test robot, this function call is generating an
error because there's no declaration for hv_para_set_synic_register()
when built with !CONFIG_HYPERV.
One solution is to add a stub higher up in this source code file in
the existing !CONFIG_HYPERV section.
But a better solution might be move vmbus_signal_eom() out of this
file entirely and put it in drivers/hv/hyperv_vmbus.h. Then no stub
for !CONFIG_HYPERV is needed. vmbus_signal_eom() is very much
a VMBus function, and it's only called from vmbus_wait_for_unload(),
__vmbus_on_msg_dpc(), and vmbus_message_sched().
The file asm-generic/mshyperv.h should be focused on the Hyper-V
interface, not on VMBus, so it's not clear to me why vmbus_signal_eom()
is in asm-generic/mshyperv.h in the first place. There's no reason to expose
vmbus_signal_eom() to code that needs definitions related to Hyper-V,
such as the x86 MTRR code or the Hyper-V emulation code in KVM
(which is where the kernel test robot detected the error). Note that
drivers/hv/hyperv_vmbus.h will need to add #include <asm/mshyperv.h>
in order for the move to work.
I've tested build with CONFIG_HYPERV and !CONFIG_HYPERV on
x86 and on arm64, and the proposed move seems to work OK.
Sorry my suggestion in v3 of your patch set to handle HV_MSR_EOM
here in vmbus_signal_eom() turned out to be more complicated than
I originally contemplated. :-(
> + else
> + hv_set_msr(HV_MSR_EOM, 0);
> + }
> +}
> +
> #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
> u8 __init get_vtl(void);
> #else
> --
> 2.43.0
