On Mon, Feb 7, 2022 at 7:59 PM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > On 2/7/22 23:45, Leonardo Bras Soares Passos wrote: > > On Mon, Feb 7, 2022 at 6:00 PM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > >> > >> On 2/7/22 21:24, Leonardo Bras Soares Passos wrote: > >>>> With this patch, > >>>> we have to reason about the effect of calling KVM_SET_CPUID2 twice calls > >>>> back to back. I think an "&=" would be wrong in that case. > >>> > >>> So, you suggest something like this ? > >>> > >>> vcpu->arch.guest_fpu.fpstate->xfeatures = > >>> fpu_user_cfg.default_features & vcpu->arch.guest_supported_xcr0; > >>> > >> > >> Yes, but you need to change user_xfeatures instead of xfeatures. > >> KVM_GET_XSAVE and KVM_SET_XSAVE will take it into account automatically: > >> > >> - KVM_GET_XSAVE: fpu_copy_guest_fpstate_to_uabi -> __copy_xstate_to_uabi_buf > >> > >> - KVM_SET_XSAVE: fpu_copy_uabi_to_guest_fpstate -> > >> copy_uabi_from_kernel_to_xstate -> copy_uabi_to_xstate -> > >> validate_user_xstate_buffer > > > > > > Ok, I understand how this replaces patch 2/2, so no issue on that. > > > > About patch 1/2, you suggest that instead of fixing what we save in > > the regs buffer, we fix only what we want to return to the user when > > they call KVM_GET_XSAVE, is that correct? > > Yes, exactly. Thanks! I will update my patch and send a v2 shortly. I got really curious while I was debugging this issue: - Is it ok that the cpu has other features enabled (like PKRU), while our vcpu does not have them? - Should guest OS always use the cpuid for checking features available? - Would it be better if we could have exactly the same fpu features enabled in the cpu, as we have in the vcpu? - Why do we xsave with a mask different from what we xrstor ? > > Paolo >
