Just a quick review on the API:
On 01/04/21 16:18, Maxim Levitsky wrote:
+struct kvm_sregs2 {
+ /* out (KVM_GET_SREGS2) / in (KVM_SET_SREGS2) */
+ struct kvm_segment cs, ds, es, fs, gs, ss;
+ struct kvm_segment tr, ldt;
+ struct kvm_dtable gdt, idt;
+ __u64 cr0, cr2, cr3, cr4, cr8;
+ __u64 efer;
+ __u64 apic_base;
+ __u64 flags; /* must be zero*/
I think it would make sense to define a flag bit for the PDPTRs, so that
userspace can use KVM_SET_SREGS2 unconditionally (e.g. even when
migrating from a source that uses KVM_GET_SREGS and therefore doesn't
provide the PDPTRs).
+ __u64 pdptrs[4];
+ __u64 padding;
No need to add padding; if we add more fields in the future we can use
the flags to determine the length of the userspace data, similar to
KVM_GET/SET_NESTED_STATE.
+ idx = srcu_read_lock(&vcpu->kvm->srcu);
+ if (is_pae_paging(vcpu)) {
+ for (i = 0 ; i < 4 ; i++)
+ kvm_pdptr_write(vcpu, i, sregs2->pdptrs[i]);
+ kvm_register_mark_dirty(vcpu, VCPU_EXREG_PDPTR);
+ mmu_reset_needed = 1;
+ }
+ srcu_read_unlock(&vcpu->kvm->srcu, idx);
+
SRCU should not be needed here?
+ case KVM_GET_SREGS2: {
+ u.sregs2 = kzalloc(sizeof(struct kvm_sregs2),
GFP_KERNEL_ACCOUNT);
+ r = -ENOMEM;
+ if (!u.sregs2)
+ goto out;
No need to account, I think it's a little slower and this allocation is
very short lived.
#define KVM_CAP_PPC_DAWR1 194
+#define KVM_CAP_SREGS2 196
195, not 196.
#define KVM_XEN_VCPU_GET_ATTR _IOWR(KVMIO, 0xca, struct kvm_xen_vcpu_attr)
#define KVM_XEN_VCPU_SET_ATTR _IOW(KVMIO, 0xcb, struct kvm_xen_vcpu_attr)
+
+#define KVM_GET_SREGS2 _IOR(KVMIO, 0xca, struct kvm_sregs2)
+#define KVM_SET_SREGS2 _IOW(KVMIO, 0xcb, struct kvm_sregs2)
+
It's not exactly overlapping, but please bump the ioctls to 0xcc/0xcd.
Paolo
