答复: [PATCH] iommu/amd: RCU-protect iommu_ga_log_notifier

[Li,Rongqing]

> From: Li RongQing <lirongqing@xxxxxxxxx>
> 
> iommu_poll_ga_log() accesses iommu_ga_log_notifier without an RCU
> read-side critical section, so synchronize_rcu() in the unregister path does not
> wait for an in-flight notifier callback.
> 
> Use rcu_assign_pointer() when updating the notifier and
> rcu_dereference() under scoped_guard(rcu) when invoking it. Load the
> pointer once to close the race with unregister.
> 
> Keep the RCU read-side critical section minimal: only the pointer load and
> the indirect notifier call stay inside scoped_guard(rcu). Move pr_debug and
> pr_err outside the section so that logging does not extend the window that
> synchronize_rcu() must wait for.
> 

Ping

thanks

[Li,Rongqing] 


> Signed-off-by: Li RongQing <lirongqing@xxxxxxxxx>
> ---
>  drivers/iommu/amd/iommu.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index a06cd61..a31464a 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -1054,11 +1054,11 @@ static void iommu_poll_events(struct
> amd_iommu *iommu)  }
> 
>  #ifdef CONFIG_IRQ_REMAP
> -static int (*iommu_ga_log_notifier)(u32);
> +static int (__rcu *iommu_ga_log_notifier)(u32 ga_tag);
> 
>  int amd_iommu_register_ga_log_notifier(int (*notifier)(u32))  {
> -	iommu_ga_log_notifier = notifier;
> +	rcu_assign_pointer(iommu_ga_log_notifier, notifier);
> 
>  	/*
>  	 * Ensure all in-flight IRQ handlers run to completion before returning
> @@ -1094,17 +1094,25 @@ static void iommu_poll_ga_log(struct
> amd_iommu *iommu)
> 
>  		/* Handle GA entry */
>  		switch (GA_REQ_TYPE(log_entry)) {
> -		case GA_GUEST_NR:
> -			if (!iommu_ga_log_notifier)
> -				break;
> -
> -			pr_debug("%s: devid=%#x, ga_tag=%#x\n",
> -				 __func__, GA_DEVID(log_entry),
> -				 GA_TAG(log_entry));
> +		case GA_GUEST_NR: {
> +			int (*notifier)(u32 ga_tag);
> +			int ret = 0;
> +
> +			scoped_guard(rcu) {
> +				notifier = rcu_dereference(iommu_ga_log_notifier);
> +				if (notifier)
> +					ret = notifier(GA_TAG(log_entry));
> +			}
> 
> -			if (iommu_ga_log_notifier(GA_TAG(log_entry)) != 0)
> -				pr_err("GA log notifier failed.\n");
> +			if (notifier) {
> +				pr_debug("%s: devid=%#x, ga_tag=%#x\n",
> +					 __func__, GA_DEVID(log_entry),
> +					 GA_TAG(log_entry));
> +				if (ret)
> +					pr_err("GA log notifier failed.\n");
> +			}
>  			break;
> +		}
>  		default:
>  			break;
>  		}
> --
> 2.9.4