Re: [PATCH v10 05/11] x86/tdx: Add __tdx_module_call() and __tdx_hypercall() helper functions

[Sathyanarayanan Kuppuswamy]

On 10/14/21 12:28 AM, Thomas Gleixner wrote:
> On Fri, Oct 08 2021 at 22:37, Kuppuswamy Sathyanarayanan wrote:
> >   
> > +#ifdef CONFIG_INTEL_TDX_GUEST
> > +#include <asm/tdx.h>
> > +#endif
> Please get rid of the #ifdef and make sure that tdx.h can be included unconditionally.


It can be included unconditionally. I will remove it in next version.

> > +	/* Restore callee-saved GPRs as mandated by the x86_64 ABI */
> > +	pop %r12
> > +	pop %r13
> > +	pop %r14
> > +	pop %r15
> > +
> > +	jmp 2f
> > +1:
> ASM supports named labels.

I will use a meaningful label instead of 1 or 2. I will fix this in next 
version.

> > +       movq $(-EINVAL), %rax
> > +2:
> > +       FRAME_END
> > +
> > +       retq
> > +SYM_FUNC_END(__tdx_hypercall)
>
> > +/*
> > + * Wrapper for standard use of __tdx_hypercall with BUG_ON() check
> > + * for TDCALL error.
> > + */
> > +static inline u64 _tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14,
> > +				 u64 r15, struct tdx_hypercall_output *out)
> > +{
> > +	struct tdx_hypercall_output outl;
> > +	u64 err;
> > +
> > +	/* __tdx_hypercall() does not accept NULL output pointer */
> > +	if (!out)
> > +		out = &outl;
> > +
> > +	err = __tdx_hypercall(TDX_HYPERCALL_STANDARD, fn, r12, r13, r14,
> > +			      r15, out);
> > +
> > +	/* Non zero return value indicates buggy TDX module, so panic */
> > +	BUG_ON(err);
> BUG() does not necessarily panic. If you want to panic in then invoke
> the function which does that, i.e. panic().


Yes, we want to panic here. I will use panic() in next version.


> Thanks,
>
>          tglx

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer