On Friday, 2021-06-11 at 18:01:55 +02, Paolo Bonzini wrote: > First of all, sorry for the delayed review. > > On 20/05/21 16:56, David Edmondson wrote: >> AMD EPYC-Milan CPUs introduced support for protection keys, previously >> available only with Intel CPUs. >> >> AMD chose to place the XSAVE state component for the protection keys >> at a different offset in the XSAVE state area than that chosen by >> Intel. >> >> To accommodate this, modify QEMU to behave appropriately on AMD >> systems, allowing a VM to properly take advantage of the new feature. > > Uff, that sucks. :( > > If I understand correctly, the problem is that the layout of > KVM_GET_XSAVE/KVM_SET_XSAVE depends on the host CPUID, which in > retrospect would be obvious. Is that correct? Yes. > If so, it would make sense and might even be easier to drop all usage > of X86XSaveArea: > > * update ext_save_areas based on CPUID information in kvm_cpu_instance_init > > * make x86_cpu_xsave_all_areas and x86_cpu_xrstor_all_areas use the > ext_save_areas offsets to build pointers to XSaveAVX, XSaveBNDREG, etc. > > What do you think? I will produce a patch and send it out. > Paolo > >> Further, avoid manipulating XSAVE state components that are not >> present on AMD systems. >> >> The code in patch 6 that changes the CPUID 0x0d leaf is mostly dumped >> somewhere that seemed to work - I'm not sure where it really belongs. >> >> David Edmondson (7): >> target/i386: Declare constants for XSAVE offsets >> target/i386: Use constants for XSAVE offsets >> target/i386: Clarify the padding requirements of X86XSaveArea >> target/i386: Prepare for per-vendor X86XSaveArea layout >> target/i386: Introduce AMD X86XSaveArea sub-union >> target/i386: Adjust AMD XSAVE PKRU area offset in CPUID leaf 0xd >> target/i386: Manipulate only AMD XSAVE state on AMD >> >> target/i386/cpu.c | 19 +++++---- >> target/i386/cpu.h | 80 ++++++++++++++++++++++++++++-------- >> target/i386/kvm/kvm.c | 57 +++++++++---------------- >> target/i386/tcg/fpu_helper.c | 20 ++++++--- >> target/i386/xsave_helper.c | 70 +++++++++++++++++++------------ >> 5 files changed, 152 insertions(+), 94 deletions(-) >> dme. -- Oliver darling, call Mister Haney, I think our speakers are blown.
