Re: [PATCH kernel] x86/compressed/64: reduce #VC nesting for intercepted CPUID for SEV-SNP guest

[Ingo Molnar]

* Alexey Kardashevskiy <aik@xxxxxxx> wrote:

> > That makes this workaround to reduce nesting a lot easier to read & maintain in the 
> > longer run.
> 
> Currently it is:
> 
> #define rdmsr(msr, low, high)
> 
> Will the below signature do?
> 
> #define rdmsr_GHCB(msr, low, high, ghcb, ctxt) {( \
> 	int __ret; \
> 	ghcb_set_rcx((ghcb), (msr)); \
> 	__ret = sev_es_ghcb_hv_call((ghcb), (ctxt), SVM_EXIT_MSR, 0, 0); \
> 	if (__ret = ES_OK) { \
> 		low = (ghcb)->save.rax; \
> 		high = (ghcb)->save.rdx; \
> 	} \
> 	__ret; )}

Sounds about right. Small nit: please prettify the macro a bit for readability,
ie. something like:

#define rdmsr_safe_GHCB(msr, low, high, ghcb, ctxt) {(				\
	int __ret;								\
										\
	ghcb_set_rcx((ghcb), (msr));						\
	__ret = sev_es_ghcb_hv_call((ghcb), (ctxt), SVM_EXIT_MSR, 0, 0);	\
	if (__ret = ES_OK) {							\
		low  = (ghcb)->save.rax;					\
		high = (ghcb)->save.rdx;					\
	}									\
	__ret; )}


> rdmsr() does not return a value but rdmsr_GHCB() has to, can even make it
> static inline function, should I?

It's a bit like rdmsr_safe()? My above naming reflects this.

I'd keep it a CPP macro, it's simple enough, the other MSR accessors are doing
it too, and to make sure this isn't instrumented the wrong way...

Thanks,

	Ingo