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]
Message-ID: <Yix4zDfZD8ZusAdO@zn.tnic>
Date:   Sat, 12 Mar 2022 11:41:16 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc:     tglx@...utronix.de, mingo@...hat.com, dave.hansen@...el.com,
        luto@...nel.org, peterz@...radead.org,
        sathyanarayanan.kuppuswamy@...ux.intel.com, aarcange@...hat.com,
        ak@...ux.intel.com, dan.j.williams@...el.com, david@...hat.com,
        hpa@...or.com, jgross@...e.com, jmattson@...gle.com,
        joro@...tes.org, jpoimboe@...hat.com, knsathya@...nel.org,
        pbonzini@...hat.com, sdeep@...are.com, seanjc@...gle.com,
        tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
        thomas.lendacky@....com, brijesh.singh@....com, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCHv5 03/30] x86/tdx: Add __tdx_module_call() and
 __tdx_hypercall() helper functions

On Fri, Mar 11, 2022 at 12:20:59AM +0300, Kirill A. Shutemov wrote:
> > > A guest uses TDCALL to communicate with both the TDX module and VMM.
> > > The value of the RAX register when executing the TDCALL instruction is
> > > used to determine the TDCALL type. A variant of TDCALL used to communicate
> > > with the VMM is called TDVMCALL.
> > > 
> > > Add generic interfaces to communicate with the TDX module and VMM
> > > (using the TDCALL instruction).
> > > 
> > > __tdx_hypercall()    - Used by the guest to request services from the
> > > 		       VMM (via TDVMCALL).
> > > __tdx_module_call()  - Used to communicate with the TDX module (via
> > > 		       TDCALL).
> > 
> > Ok, you need to fix this: this sounds to me like there are two insns:
> > TDCALL and TDVMCALL. But there's only TDCALL.
> > 
> > And I'm not even clear on how the differentiation is done - I guess
> > with %r11 which contains the VMCALL subfunction number in the
> > __tdx_hypercall() case but I'm not sure.
> 
> TDVMCALL is a leaf of TDCALL. The leaf number is encoded in RAX: RAX==0 is
> TDVMCALL.
> 
> I'm not completely sure what has to be fixed. Make it clear that TDVMCALL
> is leaf of TDCALL? Something like this:
> 
> 	__tdx_module_call()  - Used to communicate with the TDX module (via
> 			       TDCALL instruction).
> 	__tdx_hypercall()    - Used by the guest to request services from the
> 			       VMM (via TDVMCALL leaf of TDCALL).

Yes, it says above "via TDVMCALL" and "A variant of TDCALL used to
communicate with the VMM is called TDVMCALL." and that is ambiguous as
to whether this is about two instructions or one and a modification of
the same.

We write insn mnemonics with all caps so I see "TDCALL" and go, ah ok,
that's the insn but then when I see "TDVMCALL" I don't know what that
is. Another insn? Maybe, it is in all caps too...

> > And when explaining this, pls put it in the comment over the function so
> > that it is clear how the distinction is made.
> 
> But it's already there:

No not that - the explantion you wrote above that TDVMCALL is a leaf of
TDCALL. That needs to be there explicitly so that there's no confusion.

> Okay, will change. But it is not what we agreed about before:
> 
> https://lore.kernel.org/all/Yg5q742GsjCRHXZL@zn.tnic

I must've been confused from doing so many things at the same time,
sorry about that. :-\

> To me "invalid opcode" at "RIP: 0010:__tdx_hypercall+0x6d/0x70" is pretty
> clear where it comes from, no?

Probably to you and a couple more people who know how to read oops
messages. You have to think about our common users too.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ