~myrslint <myrsl...@git.sr.ht> writes: > From: myrslint <qemu.haziness...@passinbox.com> > > Addresses this issue: > https://gitlab.com/qemu-project/qemu/-/issues/2943
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2943 > Most Intel CPUs in current use have self-snoop. The few added lines of > code also check for availability of the quirk disablement option so if > some CPU does not have this feature no change of behavior will occur. > --- > accel/kvm/kvm-all.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index 890d5ea9f8..c3d06ae2f8 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -2682,6 +2682,32 @@ static int kvm_init(AccelState *as, MachineState *ms) > > s->vmfd = ret; > > +/* if target platform has no notion of this or kernel version does > + * not have it there is no use for compiling this in */ > +#ifdef KVM_X86_QUIRK_IGNORE_GUEST_PAT architecture specific changes should be in kvm_arch_init (in this case in target/i386/kvm/kvm.c) > + /* first check for modifiable quirks bitmask */ > + ret = kvm_check_extension(s, KVM_CAP_DISABLE_QUIRKS2); > + /* next make sure disabling it is allowed */ > + if (ret & KVM_X86_QUIRK_IGNORE_GUEST_PAT) { > + struct kvm_enable_cap *cap; > + cap = calloc(1, sizeof(struct kvm_enable_cap)); I few things, style says: Use of the ``malloc/free/realloc/calloc/valloc/memalign/posix_memalign`` APIs is not allowed in the QEMU codebase. Instead of these routines, use the GLib memory allocation routines ``g_malloc/g_malloc0/g_new/g_new0/g_realloc/g_free`` or QEMU's ``qemu_memalign/qemu_blockalign/qemu_vfree`` APIs. And to avoid manual free you would do: g_autofree kvm_enable_cap *cap = g_new0(kvm_enable_cap, 1); However why not just have a structure on the stack, e.g: struct kvm_enable_cap cap = { .cap = KVM_CAP_DISABLE_QUIRKS2, .args = { KVM_X86_QUIRK_IGNORE_GUEST_PAT }; In fact looking at kvm_vcpu_enable_cap() you should probably just use that. > + if (cap) { > + cap->cap = KVM_CAP_DISABLE_QUIRKS2; > + cap->args[0] = KVM_X86_QUIRK_IGNORE_GUEST_PAT; > + /* if intel cpu does not support self-snoop this is a nop */ > + ret = kvm_vm_ioctl(s, KVM_ENABLE_CAP, cap); > + if (ret < 0) { > + error_printf("KVM_X86_QUIRK_IGNORE_GUEST_PAT available and " > + "modifiable but we failed to disable > it\n"); I think this should be error_report > + } > + free(cap); > + } else { You don't need the else leg if you are dynamically allocating as g_new and friends will abort(). > + error_printf("KVM_X86_QUIRK_IGNORE_GUEST_PAT: could not " > + "allocate memory\n"); > + } > + } > +#endif > + > s->nr_as = kvm_vm_check_extension(s, KVM_CAP_MULTI_ADDRESS_SPACE); > if (s->nr_as <= 1) { > s->nr_as = 1; -- Alex Bennée Virtualisation Tech Lead @ Linaro