Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Sender: Ingo Molnar <mingo.kernel.org@gmail.com>
Date:   Mon, 25 Sep 2023 12:03:07 +0200
From:   Ingo Molnar <mingo@kernel.org>
To:     Alexey Kardashevskiy <aik@amd.com>
Cc:     x86@kernel.org, linux-kernel@vger.kernel.org,
        Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [PATCH kernel] x86/compressed/64: reduce #VC nesting for
 intercepted CPUID for SEV-SNP guest
Message-ID: <ZRFa267Aa9Xs6fEF@gmail.com>
References: <20230925042302.593317-1-aik@amd.com>
 <ZRFHtCCsL4kKajKF@gmail.com>
 <53e60186-674f-449b-a99f-01cd759c65de@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <53e60186-674f-449b-a99f-01cd759c65de@amd.com>
Precedence: bulk


* Alexey Kardashevskiy <aik@amd.com> 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