When the paravisor is present, a SNP VM must use GHCB to access some
special MSRs, including HV_X64_MSR_GUEST_OS_ID and some SynIC MSRs.
Similarly, when the paravisor is present, a TDX VM must use TDX GHCI
to access the same MSRs.
Implement hv_tdx_msr_write() and hv_tdx_msr_read(), and use the helper
functions hv_ivm_msr_read() and hv_ivm_msr_write() to access the MSRs
in a unified way for SNP/TDX VMs with the paravisor.
Do not export hv_tdx_msr_write() and hv_tdx_msr_read(), because we never
really used hv_ghcb_msr_write() and hv_ghcb_msr_read() in any module.
Update arch/x86/include/asm/mshyperv.h so that the kernel can still build
if CONFIG_AMD_MEM_ENCRYPT or CONFIG_INTEL_TDX_GUEST is not set, or
neither is set.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
Changes in v2: None
Changes in v3:
hv_tdx_read_msr -> hv_tdx_msr_read
hv_tdx_write_msr -> hv_tdx_msr_write
Do not export hv_tdx_msr_write() and hv_tdx_msr_read().
included <uapi/asm/vmx.h>
Updated arch/x86/include/asm/mshyperv.h so that the kernel
can still build if CONFIG_AMD_MEM_ENCRYPT and/or
CONFIG_INTEL_TDX_GUEST are not set.
arch/x86/hyperv/hv_init.c | 8 ++--
arch/x86/hyperv/ivm.c | 69 +++++++++++++++++++++++++++++++--
arch/x86/include/asm/mshyperv.h | 8 ++--
arch/x86/kernel/cpu/mshyperv.c | 8 ++--
4 files changed, 77 insertions(+), 16 deletions(-)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 3729eee21e47..c4cffa3b1c3c 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -500,8 +500,8 @@ void __init hyperv_init(void)
guest_id = hv_generate_guest_id(LINUX_VERSION_CODE);
wrmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id);
- /* Hyper-V requires to write guest os id via ghcb in SNP IVM. */
- hv_ghcb_msr_write(HV_X64_MSR_GUEST_OS_ID, guest_id);
+ /* With the paravisor, the VM must also write the ID via GHCB/GHCI */
+ hv_ivm_msr_write(HV_X64_MSR_GUEST_OS_ID, guest_id);
/* A TDX VM with no paravisor only uses TDX GHCI rather than hv_hypercall_pg */
if (hv_isolation_type_tdx() && !ms_hyperv.paravisor_present)
@@ -590,7 +590,7 @@ void __init hyperv_init(void)
clean_guest_os_id:
wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
- hv_ghcb_msr_write(HV_X64_MSR_GUEST_OS_ID, 0);
+ hv_ivm_msr_write(HV_X64_MSR_GUEST_OS_ID, 0);
cpuhp_remove_state(cpuhp);
free_ghcb_page:
free_percpu(hv_ghcb_pg);
@@ -611,7 +611,7 @@ void hyperv_cleanup(void)
/* Reset our OS id */
wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
- hv_ghcb_msr_write(HV_X64_MSR_GUEST_OS_ID, 0);
+ hv_ivm_msr_write(HV_X64_MSR_GUEST_OS_ID, 0);
/*
* Reset hypercall page reference before reset the page,
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index 7bd0359d5e38..fbc07493fcb4 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -24,6 +24,7 @@
#include <asm/realmode.h>
#include <asm/e820/api.h>
#include <asm/desc.h>
+#include <uapi/asm/vmx.h>
#ifdef CONFIG_AMD_MEM_ENCRYPT
@@ -186,7 +187,7 @@ bool hv_ghcb_negotiate_protocol(void)
return true;
}
-void hv_ghcb_msr_write(u64 msr, u64 value)
+static void hv_ghcb_msr_write(u64 msr, u64 value)
{
union hv_ghcb *hv_ghcb;
void **ghcb_base;
@@ -214,9 +215,8 @@ void hv_ghcb_msr_write(u64 msr, u64 value)
local_irq_restore(flags);
}
-EXPORT_SYMBOL_GPL(hv_ghcb_msr_write);
-void hv_ghcb_msr_read(u64 msr, u64 *value)
+static void hv_ghcb_msr_read(u64 msr, u64 *value)
{
union hv_ghcb *hv_ghcb;
void **ghcb_base;
@@ -246,10 +246,71 @@ void hv_ghcb_msr_read(u64 msr, u64 *value)
| ((u64)lower_32_bits(hv_ghcb->ghcb.save.rdx) << 32);
local_irq_restore(flags);
}
-EXPORT_SYMBOL_GPL(hv_ghcb_msr_read);
+#else
+static inline void hv_ghcb_msr_write(u64 msr, u64 value) {}
+static inline void hv_ghcb_msr_read(u64 msr, u64 *value) {}
#endif /* CONFIG_AMD_MEM_ENCRYPT */
+#ifdef CONFIG_INTEL_TDX_GUEST
+static void hv_tdx_msr_write(u64 msr, u64 val)
+{
+ struct tdx_hypercall_args args = {
+ .r10 = TDX_HYPERCALL_STANDARD,
+ .r11 = EXIT_REASON_MSR_WRITE,
+ .r12 = msr,
+ .r13 = val,
+ };
+
+ u64 ret = __tdx_hypercall(&args);
+
+ WARN_ONCE(ret, "Failed to emulate MSR write: %lld\n", ret);
+}
+
+static void hv_tdx_msr_read(u64 msr, u64 *val)
+{
+ struct tdx_hypercall_args args = {
+ .r10 = TDX_HYPERCALL_STANDARD,
+ .r11 = EXIT_REASON_MSR_READ,
+ .r12 = msr,
+ };
+
+ u64 ret = __tdx_hypercall_ret(&args);
+
+ if (WARN_ONCE(ret, "Failed to emulate MSR read: %lld\n", ret))
+ *val = 0;
+ else
+ *val = args.r11;
+}
+#else
+static inline void hv_tdx_msr_write(u64 msr, u64 value) {}
+static inline void hv_tdx_msr_read(u64 msr, u64 *value) {}
+#endif /* CONFIG_INTEL_TDX_GUEST */
+
+#if defined(CONFIG_AMD_MEM_ENCRYPT) || defined(CONFIG_INTEL_TDX_GUEST)
+void hv_ivm_msr_write(u64 msr, u64 value)
+{
+ if (!ms_hyperv.paravisor_present)
+ return;
+
+ if (hv_isolation_type_tdx())
+ hv_tdx_msr_write(msr, value);
+ else if (hv_isolation_type_snp())
+ hv_ghcb_msr_write(msr, value);
+}
+
+void hv_ivm_msr_read(u64 msr, u64 *value)
+{
+ if (!ms_hyperv.paravisor_present)
+ return;
+
+ if (hv_isolation_type_tdx())
+ hv_tdx_msr_read(msr, value);
+ else if (hv_isolation_type_snp())
+ hv_ghcb_msr_read(msr, value);
+}
+#endif
+
#if defined(CONFIG_AMD_MEM_ENCRYPT) || defined(CONFIG_INTEL_TDX_GUEST)
/*
* hv_mark_gpa_visibility - Set pages visible to host via hvcall.
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index a9f453c39371..101f71b85cfd 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -275,14 +275,10 @@ int hv_map_ioapic_interrupt(int ioapic_id, bool level, int vcpu, int vector,
int hv_unmap_ioapic_interrupt(int ioapic_id, struct hv_interrupt_entry *entry);
#ifdef CONFIG_AMD_MEM_ENCRYPT
-void hv_ghcb_msr_write(u64 msr, u64 value);
-void hv_ghcb_msr_read(u64 msr, u64 *value);
bool hv_ghcb_negotiate_protocol(void);
void __noreturn hv_ghcb_terminate(unsigned int set, unsigned int reason);
int hv_snp_boot_ap(int cpu, unsigned long start_ip);
#else
-static inline void hv_ghcb_msr_write(u64 msr, u64 value) {}
-static inline void hv_ghcb_msr_read(u64 msr, u64 *value) {}
static inline bool hv_ghcb_negotiate_protocol(void) { return false; }
static inline void hv_ghcb_terminate(unsigned int set, unsigned int reason) {}
static inline int hv_snp_boot_ap(int cpu, unsigned long start_ip) { return 0; }
@@ -292,8 +288,12 @@ extern bool hv_isolation_type_snp(void);
#if defined(CONFIG_AMD_MEM_ENCRYPT) || defined(CONFIG_INTEL_TDX_GUEST)
void hv_vtom_init(void);
+void hv_ivm_msr_write(u64 msr, u64 value);
+void hv_ivm_msr_read(u64 msr, u64 *value);
#else
static inline void hv_vtom_init(void) {}
+static inline void hv_ivm_msr_write(u64 msr, u64 value) {}
+static inline void hv_ivm_msr_read(u64 msr, u64 *value) {}
#endif
static inline bool hv_is_synic_reg(unsigned int reg)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 4c5a174935ca..4f51dac9eeb2 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -70,8 +70,8 @@ u64 hv_get_non_nested_register(unsigned int reg)
{
u64 value;
- if (hv_is_synic_reg(reg) && hv_isolation_type_snp())
- hv_ghcb_msr_read(reg, &value);
+ if (hv_is_synic_reg(reg) && ms_hyperv.paravisor_present)
+ hv_ivm_msr_read(reg, &value);
else
rdmsrl(reg, value);
return value;
@@ -80,8 +80,8 @@ EXPORT_SYMBOL_GPL(hv_get_non_nested_register);
void hv_set_non_nested_register(unsigned int reg, u64 value)
{
- if (hv_is_synic_reg(reg) && hv_isolation_type_snp()) {
- hv_ghcb_msr_write(reg, value);
+ if (hv_is_synic_reg(reg) && ms_hyperv.paravisor_present) {
+ hv_ivm_msr_write(reg, value);
/* Write proxy bit via wrmsl instruction */
if (hv_is_sint_reg(reg))
--
2.25.1On 8/24/23 01:07, Dexuan Cui wrote:
> +#ifdef CONFIG_INTEL_TDX_GUEST
> +static void hv_tdx_msr_write(u64 msr, u64 val)
> +{
> + struct tdx_hypercall_args args = {
> + .r10 = TDX_HYPERCALL_STANDARD,
> + .r11 = EXIT_REASON_MSR_WRITE,
> + .r12 = msr,
> + .r13 = val,
> + };
> +
> + u64 ret = __tdx_hypercall(&args);
> +
> + WARN_ONCE(ret, "Failed to emulate MSR write: %lld\n", ret);
> +}
First of all, I'd really appreciate if you could seek out explicit acks
for this kind of stuff before merging it. This surprised me.
Can you please merge these generic things back into the main TDX code?
There's nothing Hyper-V specific about any of this code. Basically, you
can make a hv_tdx_whatever() variant, but make _that_ in the generic TDX
code and then export only _that_.
Hi Dave
I was away and only saw your email now. Sorry for the late reply.
On Mon, Dec 04, 2023 at 07:10:29AM -0800, Dave Hansen wrote:
> On 8/24/23 01:07, Dexuan Cui wrote:
> > +#ifdef CONFIG_INTEL_TDX_GUEST
> > +static void hv_tdx_msr_write(u64 msr, u64 val)
> > +{
> > + struct tdx_hypercall_args args = {
> > + .r10 = TDX_HYPERCALL_STANDARD,
> > + .r11 = EXIT_REASON_MSR_WRITE,
> > + .r12 = msr,
> > + .r13 = val,
> > + };
> > +
> > + u64 ret = __tdx_hypercall(&args);
> > +
> > + WARN_ONCE(ret, "Failed to emulate MSR write: %lld\n", ret);
> > +}
>
> First of all, I'd really appreciate if you could seek out explicit acks
> for this kind of stuff before merging it. This surprised me.
>
I eyeballed the code and thought it only touched only the Hyper-V files,
so I merged this series without waiting.
> Can you please merge these generic things back into the main TDX code?
> There's nothing Hyper-V specific about any of this code. Basically, you
> can make a hv_tdx_whatever() variant, but make _that_ in the generic TDX
> code and then export only _that_.
The code is still there. Dexuan, can you send a patch to do the
refactoring?
Wei.
On 8/24/2023 4:07 PM, Dexuan Cui wrote: > When the paravisor is present, a SNP VM must use GHCB to access some > special MSRs, including HV_X64_MSR_GUEST_OS_ID and some SynIC MSRs. > > Similarly, when the paravisor is present, a TDX VM must use TDX GHCI > to access the same MSRs. > > Implement hv_tdx_msr_write() and hv_tdx_msr_read(), and use the helper > functions hv_ivm_msr_read() and hv_ivm_msr_write() to access the MSRs > in a unified way for SNP/TDX VMs with the paravisor. > > Do not export hv_tdx_msr_write() and hv_tdx_msr_read(), because we never > really used hv_ghcb_msr_write() and hv_ghcb_msr_read() in any module. > > Update arch/x86/include/asm/mshyperv.h so that the kernel can still build > if CONFIG_AMD_MEM_ENCRYPT or CONFIG_INTEL_TDX_GUEST is not set, or > neither is set. > > Signed-off-by: Dexuan Cui <decui@microsoft.com> Reviewed-by: Tianyu Lan <tiala@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Thursday, August 24, 2023 1:07 AM > > When the paravisor is present, a SNP VM must use GHCB to access some > special MSRs, including HV_X64_MSR_GUEST_OS_ID and some SynIC MSRs. > > Similarly, when the paravisor is present, a TDX VM must use TDX GHCI > to access the same MSRs. > > Implement hv_tdx_msr_write() and hv_tdx_msr_read(), and use the helper > functions hv_ivm_msr_read() and hv_ivm_msr_write() to access the MSRs > in a unified way for SNP/TDX VMs with the paravisor. > > Do not export hv_tdx_msr_write() and hv_tdx_msr_read(), because we never > really used hv_ghcb_msr_write() and hv_ghcb_msr_read() in any module. > > Update arch/x86/include/asm/mshyperv.h so that the kernel can still build > if CONFIG_AMD_MEM_ENCRYPT or CONFIG_INTEL_TDX_GUEST is not set, or > neither is set. > > Signed-off-by: Dexuan Cui <decui@microsoft.com> > --- > > Changes in v2: None > > Changes in v3: > hv_tdx_read_msr -> hv_tdx_msr_read > hv_tdx_write_msr -> hv_tdx_msr_write > Do not export hv_tdx_msr_write() and hv_tdx_msr_read(). > included <uapi/asm/vmx.h> > Updated arch/x86/include/asm/mshyperv.h so that the kernel > can still build if CONFIG_AMD_MEM_ENCRYPT and/or > CONFIG_INTEL_TDX_GUEST are not set. > > arch/x86/hyperv/hv_init.c | 8 ++-- > arch/x86/hyperv/ivm.c | 69 +++++++++++++++++++++++++++++++-- > arch/x86/include/asm/mshyperv.h | 8 ++-- > arch/x86/kernel/cpu/mshyperv.c | 8 ++-- > 4 files changed, 77 insertions(+), 16 deletions(-) Reviewed-by: Michael Kelley <mikelley@microsoft.com>
© 2016 - 2025 Red Hat, Inc.