> From: Michael Kelley (LINUX) <mikelley@xxxxxxxxxxxxx>
> Sent: Monday, August 21, 2023 12:33 PM
> > [...]
> > @@ -186,7 +186,49 @@ bool hv_ghcb_negotiate_protocol(void)
> > return true;
> > }
> >
> > -void hv_ghcb_msr_write(u64 msr, u64 value)
> > +#define EXIT_REASON_MSR_READ 31
> > +#define EXIT_REASON_MSR_WRITE 32
>
> These exit reasons are defined in arch/x86/include/uapi/asm/vmx.h.
> Are they conceptually the same thing and should be reused?
There is no VM Exit here, but I think we can use the header file.
I'll add
#include <uapi/asm/vmx.h>
and remove the 2 defines I added in this file.
> > +static void hv_tdx_read_msr(u64 msr, u64 *val)
>
> Could you make the function name be
> hv_tdx_msr_read() so it matches hv_ghcb_msr_read()
> and hv_ivm_msr_read()? :-)
Will do.
I'll also move the new functions around so that the new functions
won't get interleaved among the hv_ghcb_* functions.
I'll remove
EXPORT_SYMBOL_GPL(hv_ivm_msr_write);
EXPORT_SYMBOL_GPL(hv_ivm_msr_read);
because we never really used hv_ghcb_msr_write() and
hv_ghcb_msr_read() in any module.
The changelog is updated accordingly.
> > +{
> > + struct tdx_hypercall_args args = {
> > + .r10 = TDX_HYPERCALL_STANDARD,
> > + .r11 = EXIT_REASON_MSR_READ,
> > + .r12 = msr,
> > + };
> > +
> > +#ifdef CONFIG_INTEL_TDX_GUEST
> > + u64 ret = __tdx_hypercall_ret(&args);
> > +#else
> > + u64 ret = HV_STATUS_INVALID_PARAMETER;
> > +#endif
> > +
> > + if (WARN_ONCE(ret, "Failed to emulate MSR read: %lld\n", ret))
> > + *val = 0;
> > + else
> > + *val = args.r11;
> > +}
> > +
> > +static void hv_tdx_write_msr(u64 msr, u64 val)
>
> Same here on the function name.
Will fix.
> > #ifdef CONFIG_AMD_MEM_ENCRYPT
> > -void hv_ghcb_msr_write(u64 msr, u64 value);
> > -void hv_ghcb_msr_read(u64 msr, u64 *value);
> > +void hv_ivm_msr_write(u64 msr, u64 value);
> > +void hv_ivm_msr_read(u64 msr, u64 *value);
>
> These declarations are under CONFIG_AMD_MEM_ENCRYPT, which
> is problematic for TDX if the kernel is built with CONFIG_INTEL_TDX_GUEST
> but not CONFIG_AMD_MEM_ENCRYPT. Presumably we want to make
> sure that combination builds and works correctly.
>
> I think there's a bigger problem in that arch/x86/hyperv/ivm.c has
> a big #ifdef CONFIG_AMD_MEM_ENCRYPT in it, and TDX with paravisor
> wants to use the "vtom" functions that are under that #ifdef.
I worked out a new version of this patch:
https://github.com/dcui/tdx/commit/17b065e175082497907563297083b13dc86d84a9
I updated arch/x86/include/asm/mshyperv.h and arch/x86/hyperv/ivm.c
so that the kernel can still buid if CONFIG_AMD_MEM_ENCRYPT or
CONFIG_INTEL_TDX_GUEST is not set, or neither is set.
