Re: [PATCH 2/2] KVM: xen: (re-)initialize shared_info if guest (32/64-bit) mode is set

[Paul Durrant]

On 01/12/2023 10:45, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@xxxxxxxxxx>
>
> If the shared_info PFN cache has already been initialized then the content
> of the shared_info page needs to be (re-)initialized if the guest mode is
> set. It is no lnger done when the PFN cache is activated.
> Setting the guest mode is either done explicitly by the VMM via the
> KVM_XEN_ATTR_TYPE_LONG_MODE attribute, or implicitly when the guest writes
> the MSR to set up the hypercall page.
>
> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> ---
>   arch/x86/kvm/xen.c | 20 +++++++++++++++-----
>   1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 7bead3f65e55..bfc8f6698cbc 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -624,8 +624,15 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
>   		} else {
>   			mutex_lock(&kvm->arch.xen.xen_lock);
>   			kvm->arch.xen.long_mode = !!data->u.long_mode;
> +
> +			/*
> +			 * If shared_info has already been initialized
> +			 * then re-initialize it with the new width.
> +			 */
> +			r = kvm->arch.xen.shinfo_cache.active ?
> +				kvm_xen_shared_info_init(kvm) : 0;
> +
>   			mutex_unlock(&kvm->arch.xen.xen_lock);
> -			r = 0;
>   		}
>   		break;
>   
> @@ -657,9 +664,6 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
>   		}
>   		srcu_read_unlock(&kvm->srcu, idx);
>   
> -		if (!r && kvm->arch.xen.shinfo_cache.active)
> -			r = kvm_xen_shared_info_init(kvm);
> -
>   		mutex_unlock(&kvm->arch.xen.xen_lock);
>   		break;
>   	}
> @@ -1144,7 +1148,13 @@ int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data)
>   	bool lm = is_long_mode(vcpu);
>   
>   	/* Latch long_mode for shared_info pages etc. */
> -	vcpu->kvm->arch.xen.long_mode = lm;
> +	kvm->arch.xen.long_mode = lm;
> +
> +	if (kvm->arch.xen.shinfo_cache.active &&
> +	    kvm_xen_shared_info_init(kvm)) {
> +		mutex_unlock(&kvm->arch.xen.xen_lock);

This unlock is bogus; it should have been removed. I'll send a v2.

  Paul

> +		return 1;
> +	}
>   
>   	/*
>   	 * If Xen hypercall intercept is enabled, fill the hypercall