linux-kernel - RE: [PATCH v2 8/9] x86/hyperv: Use TDX GHCI to access some MSRs in a TDX VM with the paravisor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <SA1PR21MB13354314F55F178602183650BF1CA@SA1PR21MB1335.namprd21.prod.outlook.com>
Date:   Wed, 23 Aug 2023 04:30:55 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     "Michael Kelley (LINUX)" <mikelley@...rosoft.com>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "arnd@...db.de" <arnd@...db.de>, "bp@...en8.de" <bp@...en8.de>,
        "brijesh.singh@....com" <brijesh.singh@....com>,
        "dan.j.williams@...el.com" <dan.j.williams@...el.com>,
        "dave.hansen@...el.com" <dave.hansen@...el.com>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "jane.chu@...cle.com" <jane.chu@...cle.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        KY Srinivasan <kys@...rosoft.com>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "luto@...nel.org" <luto@...nel.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "sathyanarayanan.kuppuswamy@...ux.intel.com" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "seanjc@...gle.com" <seanjc@...gle.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>, jason <jason@...c4.com>,
        "nik.borisov@...e.com" <nik.borisov@...e.com>
CC:     "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        Tianyu Lan <Tianyu.Lan@...rosoft.com>,
        "rick.p.edgecombe@...el.com" <rick.p.edgecombe@...el.com>,
        Anthony Davis <andavis@...hat.com>,
        Mark Heslin <mheslin@...hat.com>,
        vkuznets <vkuznets@...hat.com>,
        "xiaoyao.li@...el.com" <xiaoyao.li@...el.com>
Subject: RE: [PATCH v2 8/9] x86/hyperv: Use TDX GHCI to access some MSRs in a
 TDX VM with the paravisor

> From: Michael Kelley (LINUX) <mikelley@...rosoft.com>
> 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.