Re: [PATCH 2/3] KVM: PPC: Book3S HV: XIVE: do not test the EQ flag validity when resetting

[Greg Kurz <groug@kaod.org>]

On Mon, 20 May 2019 09:15:13 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> When a CPU is hot-unplugged, the EQ is deconfigured using a zero size
> and a zero address. In this case, there is no need to check the flag
> and queue size validity. Move the checks after the queue reset code
> section to fix CPU hot-unplug.
> 
> Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
> Tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  arch/powerpc/kvm/book3s_xive_native.c | 36 +++++++++++++--------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index 796d86549cfe..3fdea6bf4e97 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -565,24 +565,6 @@ static int kvmppc_xive_native_set_queue_config(struct kvmppc_xive *xive,
>  		 __func__, server, priority, kvm_eq.flags,
>  		 kvm_eq.qshift, kvm_eq.qaddr, kvm_eq.qtoggle, kvm_eq.qindex);
>  
> -	/*
> -	 * sPAPR specifies a "Unconditional Notify (n) flag" for the
> -	 * H_INT_SET_QUEUE_CONFIG hcall which forces notification
> -	 * without using the coalescing mechanisms provided by the
> -	 * XIVE END ESBs. This is required on KVM as notification
> -	 * using the END ESBs is not supported.
> -	 */
> -	if (kvm_eq.flags != KVM_XIVE_EQ_ALWAYS_NOTIFY) {
> -		pr_err("invalid flags %d\n", kvm_eq.flags);
> -		return -EINVAL;
> -	}
> -
> -	rc = xive_native_validate_queue_size(kvm_eq.qshift);
> -	if (rc) {
> -		pr_err("invalid queue size %d\n", kvm_eq.qshift);
> -		return rc;
> -	}
> -
>  	/* reset queue and disable queueing */
>  	if (!kvm_eq.qshift) {
>  		q->guest_qaddr  = 0;
> @@ -604,6 +586,24 @@ static int kvmppc_xive_native_set_queue_config(struct kvmppc_xive *xive,
>  		return 0;
>  	}
>  
> +	/*
> +	 * sPAPR specifies a "Unconditional Notify (n) flag" for the
> +	 * H_INT_SET_QUEUE_CONFIG hcall which forces notification
> +	 * without using the coalescing mechanisms provided by the
> +	 * XIVE END ESBs. This is required on KVM as notification
> +	 * using the END ESBs is not supported.
> +	 */
> +	if (kvm_eq.flags != KVM_XIVE_EQ_ALWAYS_NOTIFY) {
> +		pr_err("invalid flags %d\n", kvm_eq.flags);
> +		return -EINVAL;
> +	}
> +
> +	rc = xive_native_validate_queue_size(kvm_eq.qshift);
> +	if (rc) {
> +		pr_err("invalid queue size %d\n", kvm_eq.qshift);
> +		return rc;
> +	}
> +
>  	if (kvm_eq.qaddr & ((1ull << kvm_eq.qshift) - 1)) {
>  		pr_err("queue page is not aligned %llx/%llx\n", kvm_eq.qaddr,
>  		       1ull << kvm_eq.qshift);