2023-12-04 15:12:56

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v3 08/10] x86/hyperv: Use TDX GHCI to access some MSRs in a TDX VM with the paravisor

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.

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_.


2024-01-17 02:22:55

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v3 08/10] x86/hyperv: Use TDX GHCI to access some MSRs in a TDX VM with the paravisor

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.