From: Roman Kisel <[email protected]> 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 <[email protected]> > Reviewed-by: Alok Tiwari <[email protected]> > --- > 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
