Messages in this thread | | | Date | Wed, 2 Feb 2022 16:06:16 +0300 | From | "Kirill A. Shutemov" <> | Subject | Re: [PATCHv2 06/29] x86/tdx: Add MSR support for TDX guests |
| |
On Tue, Feb 01, 2022 at 10:38:05PM +0100, Thomas Gleixner wrote: > On Mon, Jan 24 2022 at 18:01, Kirill A. Shutemov wrote: > > +static bool tdx_read_msr(unsigned int msr, u64 *val) > > +{ > > + struct tdx_hypercall_output out; > > + > > + /* > > + * Emulate the MSR read via hypercall. More info about ABI > > + * can be found in TDX Guest-Host-Communication Interface > > + * (GHCI), sec titled "TDG.VP.VMCALL<Instruction.RDMSR>". > > + */ > > + if (_tdx_hypercall(EXIT_REASON_MSR_READ, msr, 0, 0, 0, &out)) > > + return false; > > + > > + *val = out.r11; > > + > > + return true; > > +} > > + > > +static bool tdx_write_msr(unsigned int msr, unsigned int low, > > + unsigned int high) > > +{ > > + u64 ret; > > + > > + /* > > + * Emulate the MSR write via hypercall. More info about ABI > > + * can be found in TDX Guest-Host-Communication Interface > > + * (GHCI) sec titled "TDG.VP.VMCALL<Instruction.WRMSR>". > > + */ > > + ret = _tdx_hypercall(EXIT_REASON_MSR_WRITE, msr, (u64)high << 32 | low, > > + 0, 0, NULL); > > + > > + return ret ? false : true; > > +} > > + > > bool tdx_get_ve_info(struct ve_info *ve) > > { > > struct tdx_module_output out; > > @@ -132,11 +165,22 @@ static bool tdx_virt_exception_user(struct pt_regs *regs, struct ve_info *ve) > > static bool tdx_virt_exception_kernel(struct pt_regs *regs, struct ve_info *ve) > > { > > bool ret = false; > > + u64 val; > > > > switch (ve->exit_reason) { > > case EXIT_REASON_HLT: > > ret = tdx_halt(); > > break; > > + case EXIT_REASON_MSR_READ: > > + ret = tdx_read_msr(regs->cx, &val); > > + if (ret) { > > + regs->ax = lower_32_bits(val); > > + regs->dx = upper_32_bits(val); > > + } > > + break; > > Why here? > > static bool tdx_read_msr(struct pt_regs *regs) > { > struct tdx_hypercall_output out; > > /* > * Emulate the MSR read via hypercall. More info about ABI > * can be found in TDX Guest-Host-Communication Interface > * (GHCI), sec titled "TDG.VP.VMCALL<Instruction.RDMSR>". > */ > if (_tdx_hypercall(EXIT_REASON_MSR_READ, regs->cx, 0, 0, 0, &out)) > return false; > > regs->ax = lower_32_bits(out.r11); > regs->dx = upper_32_bits(out.r11); > return true; > } > > and > > static bool tdx_read_msr(struct pt_regs *regs) > { > /* > * Emulate the MSR write via hypercall. More info about ABI > * can be found in TDX Guest-Host-Communication Interface > * (GHCI) sec titled "TDG.VP.VMCALL<Instruction.WRMSR>". > */ > return !!_tdx_hypercall(EXIT_REASON_MSR_WRITE, regs->cx, > (u64)regs->dx << 32 | regs->ax, > 0, 0, NULL); > } > > Also the switch case can be simplified as the only action after 'break;' > is 'return ret': > > switch (ve->exit_reason) { > case EXIT_REASON_HLT: > return tdx_halt(); > case EXIT_REASON_MSR_READ: > return tdx_read_msr(regs); > case EXIT_REASON_MSR_WRITE: > return tdx_write_msr(regs); > default: > .... > > Hmm?
No problem with this approach on read side.
On the write side there's one important optimization (outside of the initial TDX enabling):
https://github.com/intel/tdx/commit/2cea8becaa5a287c93266c01fc7f2a4ed53c509d
It will require rework, maybe use separate __tdx_hypercall() for the paravirt call implementation.
-- Kirill A. Shutemov
| |