Messages in this thread | | | Subject | Re: [PATCH v3 11/13] KVM: selftests / xen: map shared_info using HVA rather than GFN | From | David Woodhouse <> | Date | Mon, 18 Sep 2023 17:16:08 +0100 |
| |
On Mon, 2023-09-18 at 14:41 +0000, Paul Durrant wrote: > > for (;;) { > - __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_activate); > - __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_deactivate); > + __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_activate_gfn); > pthread_testcancel(); > + __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_deactivate_gfn); > + > + if (xen_caps & KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA) { > + __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_activate_hva); > + pthread_testcancel(); > + __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_deactivate_hva); > + } > } >
So now the loop starts by activating it in GFN mode even if it was already activated in HVA mode. Is that something we should even allow? I suppose it doesn't hurt.
And it *may* leave it activated in either HVA or GFN mode.
Are both deactivate modes equivalent? I think they are, aren't they?
So it could be...
for (;;) { __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_deactivate); __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_activate);
if (xen_caps & KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA) { __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_deactivate_hva); __vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &cache_activate_hva); } pthread_testcancel(); }
But that's just nitpicking, I suppose.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> [unhandled content-type:application/pkcs7-signature] | |