Shortlog needs to have a verb somewhere.
On Wed, Dec 22, 2021, Jing Liu wrote:
From: Guang Zeng guang.zeng@intel.com
When AMX is enabled it requires a larger xstate buffer than the legacy hardcoded 4KB one. Exising kvm ioctls
Existing
(KVM_[G|S]ET_XSAVE under KVM_CAP_XSAVE) are not suitable for this purpose.
...
Reuse KVM_SET_XSAVE for both old/new formats by reimplementing it to do properly-sized memdup_user() based on the guest fpu container.
I'm confused, the first sentence says KVM_SET_XSAVE isn't suitable, the second says it can be reused with minimal effort.
Also, update the api doc with the new KVM_GET_XSAVE2 ioctl.
...
@@ -5367,7 +5382,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp, break; } case KVM_SET_XSAVE: {
u.xsave = memdup_user(argp, sizeof(*u.xsave));
int size = vcpu->arch.guest_fpu.uabi_size;
IIUC, reusing KVM_SET_XSAVE works by requiring that userspace use KVM_GET_XSAVE2 if userspace has expanded the guest FPU size by exposing relevant features to the guest via guest CPUID. If so, then that needs to be enforced in KVM_GET_XSAVE, otherwise userspace will get subtle corruption by invoking the wrong ioctl, e.g.
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2c9606380bca..5d2acbd52df5 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5386,6 +5386,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp, break; } case KVM_GET_XSAVE: { + r -EINVAL; + if (vcpu->arch.guest_fpu.uabi_size > sizeof(struct kvm_xsave)) + break; + u.xsave = kzalloc(sizeof(struct kvm_xsave), GFP_KERNEL_ACCOUNT); r = -ENOMEM; if (!u.xsave)
u.xsave = memdup_user(argp, size);@@ -5376,6 +5393,26 @@ long kvm_arch_vcpu_ioctl(struct file *filp, r = kvm_vcpu_ioctl_x86_set_xsave(vcpu, u.xsave); break; }
- case KVM_GET_XSAVE2: {
int size = vcpu->arch.guest_fpu.uabi_size;u.xsave = kzalloc(size, GFP_KERNEL_ACCOUNT);if (!u.xsave) {r = -ENOMEM;
I hate the odd patterns in this code as much as anyone, but for better or worse the style throughout is:
r = -ENOMEM; u.xsave = kzalloc(size, GFP_KERNEL_ACCOUNT); if (u.xsave) break;
break;}kvm_vcpu_ioctl_x86_get_xsave2(vcpu, u.buffer, size);if (copy_to_user(argp, u.xsave, size)) {r = -EFAULT;break;
Same style thing here.
}r = 0;break;- }
- case KVM_GET_XCRS: { u.xcrs = kzalloc(sizeof(struct kvm_xcrs), GFP_KERNEL_ACCOUNT); r = -ENOMEM;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 1daa45268de2..9d1c01669560 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1131,6 +1131,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_EXIT_ON_EMULATION_FAILURE 204 #define KVM_CAP_ARM_MTE 205 #define KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM 206 +#define KVM_CAP_XSAVE2 207 #ifdef KVM_CAP_IRQ_ROUTING @@ -1610,6 +1611,9 @@ struct kvm_enc_region { #define KVM_S390_NORMAL_RESET _IO(KVMIO, 0xc3) #define KVM_S390_CLEAR_RESET _IO(KVMIO, 0xc4) +/* Available with KVM_CAP_XSAVE2 */ +#define KVM_GET_XSAVE2 _IOR(KVMIO, 0xcf, struct kvm_xsave)
struct kvm_s390_pv_sec_parm { __u64 origin; __u64 length; -- 2.27.0