| Message ID | 20240419161623.45842-8-vsntk18@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add #VC exception handling for AMD SEV-ES | expand |
On Fri, Apr 19, 2024, vsntk18@gmail.com wrote: > From: Vasant Karasulli <vkarasulli@suse.de> > > Processing CPUID #VC for AMD SEV-ES requires copying xcr0 into GHCB. > Move the xsave read/write helpers used by xsave testcase to lib/x86 > to share as common code. This doesn't make any sense, processor.h _is_ common code. And using get_supported_xcr0(), which does CPUID, in a #VC handler is even more nonsensical. Indeed, it's still used only by test_xsave() at the end of this series. > Signed-off-by: Varad Gautam <varad.gautam@suse.com> > Signed-off-by: Vasant Karasulli <vkarasulli@suse.de> > Reviewed-by: Marc Orr <marcorr@google.com> > --- > lib/x86/processor.h | 10 ---------- > lib/x86/xsave.c | 26 ++++++++++++++++++++++++++ > lib/x86/xsave.h | 15 +++++++++++++++ > x86/Makefile.common | 1 + > x86/xsave.c | 17 +---------------- > 5 files changed, 43 insertions(+), 26 deletions(-) > create mode 100644 lib/x86/xsave.c > create mode 100644 lib/x86/xsave.h
On Mi 05-06-24 09:04:04, Sean Christopherson wrote: > On Fri, Apr 19, 2024, vsntk18@gmail.com wrote: > > From: Vasant Karasulli <vkarasulli@suse.de> > > > > Processing CPUID #VC for AMD SEV-ES requires copying xcr0 into GHCB. > > Move the xsave read/write helpers used by xsave testcase to lib/x86 > > to share as common code. > > This doesn't make any sense, processor.h _is_ common code. And using > get_supported_xcr0(), which does CPUID, in a #VC handler is even more nonsensical. > Indeed, it's still used only by test_xsave() at the end of this series. > The idea was to have xcr0 related declarations and definitions in the same place which were distributed across files. Does that make sense to you? If not I will move back get_supported_xcr0() to where it was. > > Signed-off-by: Varad Gautam <varad.gautam@suse.com> > > Signed-off-by: Vasant Karasulli <vkarasulli@suse.de> > > Reviewed-by: Marc Orr <marcorr@google.com> > > --- > > lib/x86/processor.h | 10 ---------- > > lib/x86/xsave.c | 26 ++++++++++++++++++++++++++ > > lib/x86/xsave.h | 15 +++++++++++++++ > > x86/Makefile.common | 1 + > > x86/xsave.c | 17 +---------------- > > 5 files changed, 43 insertions(+), 26 deletions(-) > > create mode 100644 lib/x86/xsave.c > > create mode 100644 lib/x86/xsave.h -- Vasant Karasulli Kernel generalist www.suse.com<http://www.suse.com> [https://www.suse.com/assets/img/social-platforms-suse-logo.png]<http://www.suse.com/> SUSE - Open Source Solutions for Enterprise Servers & Cloud<http://www.suse.com/> Modernize your infrastructure with SUSE Linux Enterprise servers, cloud technology for IaaS, and SUSE's software-defined storage. www.suse.com
On Thu, Jun 06, 2024, Vasant Karasulli wrote: > On Mi 05-06-24 09:04:04, Sean Christopherson wrote: > > On Fri, Apr 19, 2024, vsntk18@gmail.com wrote: > > > From: Vasant Karasulli <vkarasulli@suse.de> > > > > > > Processing CPUID #VC for AMD SEV-ES requires copying xcr0 into GHCB. > > > Move the xsave read/write helpers used by xsave testcase to lib/x86 > > > to share as common code. > > > > This doesn't make any sense, processor.h _is_ common code. And using > > get_supported_xcr0(), which does CPUID, in a #VC handler is even more nonsensical. > > Indeed, it's still used only by test_xsave() at the end of this series. > > > > The idea was to have xcr0 related declarations and definitions in the same place > which were distributed across files. Does that make sense to you? If not > I will move back get_supported_xcr0() to where it was. I don't necessarily disagree with that approach, but I don't think we need a dedicated xsave.h, e.g. just move get_supported_xcr0() to processor.h.
diff --git a/lib/x86/processor.h b/lib/x86/processor.h index b324cbf0..d839308f 100644 --- a/lib/x86/processor.h +++ b/lib/x86/processor.h @@ -477,16 +477,6 @@ static inline uint64_t rdpmc(uint32_t index) return val; } -static inline int xgetbv_safe(u32 index, u64 *result) -{ - return rdreg64_safe(".byte 0x0f,0x01,0xd0", index, result); -} - -static inline int xsetbv_safe(u32 index, u64 value) -{ - return wrreg64_safe(".byte 0x0f,0x01,0xd1", index, value); -} - static inline int write_cr0_safe(ulong val) { return asm_safe("mov %0,%%cr0", "r" (val)); diff --git a/lib/x86/xsave.c b/lib/x86/xsave.c new file mode 100644 index 00000000..85aae78f --- /dev/null +++ b/lib/x86/xsave.c @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include "libcflat.h" +#include "xsave.h" +#include "processor.h" + +int xgetbv_safe(u32 index, u64 *result) +{ + return rdreg64_safe(".byte 0x0f,0x01,0xd0", index, result); +} + +int xsetbv_safe(u32 index, u64 value) +{ + return wrreg64_safe(".byte 0x0f,0x01,0xd1", index, value); +} + +uint64_t get_supported_xcr0(void) +{ + struct cpuid r; + r = cpuid_indexed(0xd, 0); + printf("eax %x, ebx %x, ecx %x, edx %x\n", + r.a, r.b, r.c, r.d); + return r.a + ((u64)r.d << 32); +} + + diff --git a/lib/x86/xsave.h b/lib/x86/xsave.h new file mode 100644 index 00000000..34c1c149 --- /dev/null +++ b/lib/x86/xsave.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _X86_XSAVE_H_ +#define _X86_XSAVE_H_ + +#define XCR_XFEATURE_ENABLED_MASK 0x00000000 +#define XCR_XFEATURE_ILLEGAL_MASK 0x00000010 + +#define XSTATE_FP 0x1 +#define XSTATE_SSE 0x2 +#define XSTATE_YMM 0x4 + +int xgetbv_safe(u32 index, u64 *result); +int xsetbv_safe(u32 index, u64 value); +uint64_t get_supported_xcr0(void); +#endif diff --git a/x86/Makefile.common b/x86/Makefile.common index 25ae6f78..c1e90b86 100644 --- a/x86/Makefile.common +++ b/x86/Makefile.common @@ -23,6 +23,7 @@ cflatobjs += lib/x86/stack.o cflatobjs += lib/x86/fault_test.o cflatobjs += lib/x86/delay.o cflatobjs += lib/x86/pmu.o +cflatobjs += lib/x86/xsave.o ifeq ($(CONFIG_EFI),y) cflatobjs += lib/x86/amd_sev.o cflatobjs += lib/x86/amd_sev_vc.o diff --git a/x86/xsave.c b/x86/xsave.c index 5d80f245..f3cbfca4 100644 --- a/x86/xsave.c +++ b/x86/xsave.c @@ -1,6 +1,7 @@ #include "libcflat.h" #include "desc.h" #include "processor.h" +#include "xsave.h" #ifdef __x86_64__ #define uint64_t unsigned long @@ -8,22 +9,6 @@ #define uint64_t unsigned long long #endif -static uint64_t get_supported_xcr0(void) -{ - struct cpuid r; - r = cpuid_indexed(0xd, 0); - printf("eax %x, ebx %x, ecx %x, edx %x\n", - r.a, r.b, r.c, r.d); - return r.a + ((u64)r.d << 32); -} - -#define XCR_XFEATURE_ENABLED_MASK 0x00000000 -#define XCR_XFEATURE_ILLEGAL_MASK 0x00000010 - -#define XSTATE_FP 0x1 -#define XSTATE_SSE 0x2 -#define XSTATE_YMM 0x4 - static void test_xsave(void) { unsigned long cr4;