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] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 13 Jul 2023 03:46:52 +0000
From:   "Huang, Kai" <kai.huang@...el.com>
To:     "isaku.yamahata@...il.com" <isaku.yamahata@...il.com>
CC:     "Hansen, Dave" <dave.hansen@...el.com>,
        "Christopherson,, Sean" <seanjc@...gle.com>,
        "bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "Yamahata, Isaku" <isaku.yamahata@...el.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "sathyanarayanan.kuppuswamy@...ux.intel.com" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
Subject: Re: [PATCH 09/10] x86/virt/tdx: Wire up basic SEAMCALL functions

On Wed, 2023-07-12 at 15:15 -0700, Isaku Yamahata wrote:
> > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much
> > TDCALL infrastructure.  Wire up basic functions to make SEAMCALLs for
> > the basic TDX support: __seamcall(), __seamcall_ret() and
> > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function.
> 
> Hi.  __seamcall_saved_ret() uses struct tdx_module_arg as input and output.  For
> KVM TDH.VP.ENTER case, those arguments are already in unsigned long
> kvm_vcpu_arch::regs[].  It's silly to move those values twice.  From
> kvm_vcpu_arch::regs to tdx_module_args.  From tdx_module_args to real registers.
> 
> If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to
> take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]?  Maybe I can make the
> change with TDX KVM patch series.

The assembly code assumes the second argument is a pointer to 'struct
tdx_module_args'.  I don't know how can we change __seamcall_saved_ret() to
achieve what you said.  We might change the kvm_vcpu_argh::regs[NR_VCPU_REGS] to
match 'struct tdx_module_args''s layout and manually convert part of "regs" to
the structure and pass to __seamcall_saved_ret(), but it's too hacky I suppose.

This was one concern that I mentioned VP.ENTER can be implemented by KVM in its
own assembly in the TDX host v12 discussion.  I kinda agree we should leverage
KVM's existing kvm_vcpu_arch::regs[NR_CPU_REGS] infrastructure to minimize the
code change to the KVM's common infrastructure.  If so, I guess we have to carry
this memory copy burden between two structures.

Btw, I do find KVM's VP.ENTER code is a little bit redundant to the common
SEAMCALL assembly, which is a good reason for KVM to use __seamcall() variants
for TDH.VP.ENTER.

So it's a tradeoff I think.

On the other hand, given CoCo VMs normally don't expose all GPRs to VMM, it's
also debatable whether we should invent another infrastructure to the KVM code
to handle register access of CoCo VMs too, e.g., we can catch bugs easily when
KVM tries to access the registers that it shouldn't access.

It's better KVM maintainer can provide some input here. :)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ