On 11/19/22 10:46, David Woodhouse wrote:
> @@ -584,23 +705,57 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
> ...
> + if (IS_ENABLED(CONFIG_64BIT) && vcpu->kvm->arch.xen.long_mode)
> + sz = sizeof(struct vcpu_runstate_info);
> + else
> + sz = sizeof(struct compat_vcpu_runstate_info);
> +
> + /* How much fits in the (first) page? */
> + sz1 = PAGE_SIZE - (data->u.gpa & ~PAGE_MASK);
> r = kvm_gpc_activate(vcpu->kvm, &vcpu->arch.xen.runstate_cache,
> - NULL, KVM_HOST_USES_PFN, data->u.gpa,
> - sizeof(struct vcpu_runstate_info));
> - break;
> + NULL, KVM_HOST_USES_PFN, data->u.gpa, sz1);
> + if (r)
> + goto deactivate_out;
>
> + /* Either map the second page, or deactivate the second GPC */
> + if (sz1 > sz) {
Out of curiosity: is there a reason behind potentially
kvm_gpc_activate()ing a "len=0" cache? (sz1==sz leads to sz2=0)
I feel I may be missing something, but shouldn't the comparison be >=?
> + kvm_gpc_deactivate(vcpu->kvm,
> + &vcpu->arch.xen.runstate2_cache);
> + } else {
> + sz2 = sz - sz1;
> + BUG_ON((data->u.gpa + sz1) & ~PAGE_MASK);
> + r = kvm_gpc_activate(vcpu->kvm, &vcpu->arch.xen.runstate2_cache,
> + NULL, KVM_HOST_USES_PFN,
> + data->u.gpa + sz1, sz2);
> + if (r)
> + goto deactivate_out;
> + }
thanks,
Michal
