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

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] [day] [month] [year] [list]

Message-ID: <ZRFa267Aa9Xs6fEF@gmail.com>
Date:   Mon, 25 Sep 2023 12:03:07 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Alexey Kardashevskiy <aik@....com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Tom Lendacky <thomas.lendacky@....com>
Subject: Re: [PATCH kernel] x86/compressed/64: reduce #VC nesting for
 intercepted CPUID for SEV-SNP guest


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