Re: [PATCH kernel 4/4] KVM: PPC: Propagate errors to the guest when failed instead of ignoring

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3672 bytes --]

On Thu, Aug 30, 2018 at 01:16:47PM +1000, Alexey Kardashevskiy wrote:
> At the moment if the PUT_TCE{_INDIRECT} handlers fail to update
> the hardware tables, we print a warning once, clear the entry and
> continue. This is so as at the time the assumption was that if
> a VFIO device is hotplugged into the guest, and the userspace replays
> virtual DMA mappings (i.e. TCEs) to the hardware tables and if this fails,
> then there is nothing useful we can do about it.
> 
> However the assumption is not valid as these handlers are not called for
> TCE replay (VFIO ioctl interface is used for that) and these handlers
> are for new TCEs.
> 
> This returns an error to the guest if there is a request which cannot be
> processed. By now the only possible failure must be H_TOO_HARD.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  arch/powerpc/kvm/book3s_64_vio.c    | 20 ++++++--------------
>  arch/powerpc/kvm/book3s_64_vio_hv.c | 20 ++++++--------------
>  2 files changed, 12 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
> index 5cd2a66..5e3151b 100644
> --- a/arch/powerpc/kvm/book3s_64_vio.c
> +++ b/arch/powerpc/kvm/book3s_64_vio.c
> @@ -568,14 +568,10 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
>  			ret = kvmppc_tce_iommu_map(vcpu->kvm, stt, stit->tbl,
>  					entry, ua, dir);
>  
> -		if (ret == H_SUCCESS)
> -			continue;
> -
> -		if (ret == H_TOO_HARD)
> +		if (ret != H_SUCCESS) {
> +			kvmppc_clear_tce(stit->tbl, entry);
>  			goto unlock_exit;
> -
> -		WARN_ON_ONCE(1);
> -		kvmppc_clear_tce(stit->tbl, entry);
> +		}
>  	}
>  
>  	kvmppc_tce_put(stt, entry, tce);
> @@ -660,14 +656,10 @@ long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
>  					stit->tbl, entry + i, ua,
>  					iommu_tce_direction(tce));
>  
> -			if (ret == H_SUCCESS)
> -				continue;
> -
> -			if (ret == H_TOO_HARD)
> +			if (ret != H_SUCCESS) {
> +				kvmppc_clear_tce(stit->tbl, entry);
>  				goto unlock_exit;
> -
> -			WARN_ON_ONCE(1);
> -			kvmppc_clear_tce(stit->tbl, entry);
> +			}
>  		}
>  
>  		kvmppc_tce_put(stt, entry + i, tce);
> diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
> index e79ffbb..8d82133 100644
> --- a/arch/powerpc/kvm/book3s_64_vio_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
> @@ -380,14 +380,10 @@ long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
>  			ret = kvmppc_rm_tce_iommu_map(vcpu->kvm, stt,
>  					stit->tbl, entry, ua, dir);
>  
> -		if (ret == H_SUCCESS)
> -			continue;
> -
> -		if (ret == H_TOO_HARD)
> +		if (ret != H_SUCCESS) {
> +			kvmppc_rm_clear_tce(stit->tbl, entry);
>  			return ret;
> -
> -		WARN_ON_ONCE_RM(1);
> -		kvmppc_rm_clear_tce(stit->tbl, entry);
> +		}
>  	}
>  
>  	kvmppc_tce_put(stt, entry, tce);
> @@ -533,14 +529,10 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
>  					stit->tbl, entry + i, ua,
>  					iommu_tce_direction(tce));
>  
> -			if (ret == H_SUCCESS)
> -				continue;
> -
> -			if (ret == H_TOO_HARD)
> +			if (ret != H_SUCCESS) {
> +				kvmppc_rm_clear_tce(stit->tbl, entry);
>  				goto unlock_exit;
> -
> -			WARN_ON_ONCE_RM(1);
> -			kvmppc_rm_clear_tce(stit->tbl, entry);
> +			}
>  		}
>  
>  		kvmppc_tce_put(stt, entry + i, tce);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]