Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Message-ID: <5dca6853-cd14-a8fc-ce1b-b64b1b8b3412@intel.com>
Date:   Tue, 31 Oct 2023 10:57:46 -0500
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.15.1
Subject: Re: [PATCH v4 1/2] dmaengine: idxd: Protect int_handle field in hw
 descriptor
To:     'Guanjun' <guanjun@linux.alibaba.com>, <dave.jiang@intel.com>,
        <dmaengine@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <vkoul@kernel.org>, <tony.luck@intel.com>, <fenghua.yu@intel.com>
CC:     <jing.lin@intel.com>, <ashok.raj@intel.com>,
        <sanjay.k.kumar@intel.com>, <megha.dey@intel.com>,
        <jacob.jun.pan@intel.com>, <yi.l.liu@intel.com>,
        <tglx@linutronix.de>
References: <20231031025511.1516342-1-guanjun@linux.alibaba.com>
 <20231031025511.1516342-2-guanjun@linux.alibaba.com>
Content-Language: en-US
From:   Lijun Pan <lijun.pan@intel.com>
In-Reply-To: <20231031025511.1516342-2-guanjun@linux.alibaba.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WkoxYUM1LzJqcE4vazRKVmFwd1NjTnZZZGZQNmRaK2dCNWhRVVZYNUE2U0xq?=
 =?utf-8?B?dTkzWVFZTUJ3dS92eVZkbG5idlRhYWZFaldFOC9lSi9NV1QxaDZWaG9JZ1Er?=
 =?utf-8?B?T01LbG41amt1bEdacnZOeUdoVnVlU1lmZVQ3ZVFvWU1tOWFpRGhLY2xtVDBG?=
 =?utf-8?B?a1ZiTGlSZzRuQ1RZR1FackJGZDMzM2gra0pOTUZLSTQrb2RIL1NLbHlXNk5v?=
 =?utf-8?B?VnNlb3MreGV6TzBjZXFkM29zbCtwaGlLWWIvazRZTUVNS3NVTjNrMGEydkpL?=
 =?utf-8?B?TXR2U3VTdFFxL3BVWUNaNFpWZTdyUGtHV2RjaUNNTXlsSWpmcjZ6Mmw4ZEV3?=
 =?utf-8?B?MEl5UE9WN3c4ZU92bStrSWcyR3VGTzVudi9JSURzUi9TaFRzYzM3VHFhVnJs?=
 =?utf-8?B?UjBJU2V4TWhic3FkVlFRczJjR3Q4cGpwd0JRNlp6RkVyR0M1ZzhISDZOVmJu?=
 =?utf-8?B?UnlCVXFBY1gvVk5sb0MyaTdYVkEyOE5CdUh3bC8yTC85RXZBNWUvVllNdTZF?=
 =?utf-8?B?UmZrZ2dEYzNVVmRyOEVKRm45OGZYSnJmVzBHMkp2VEJ5TStkaEVacUlVQXNr?=
 =?utf-8?B?UnI0OVNXRFpmbmk0UEFCR3FzNnR0b1d6NGFvd2Z3TXlZcmdEeWJKMUNLNUFM?=
 =?utf-8?B?TnZVRmxkVEF4cDZNeG1BblZ3REhqTXY5TmJpN3YvVlZ2ZHJ1VXZmZi8wbE1S?=
 =?utf-8?B?d1lEV1ZBOWVQWlJQWk9SeG9CaUhyc0FJSk5WM0d2eGxaWnJNMHE0OHNSYjZ1?=
 =?utf-8?B?aUJ0UUpMRGNCQWtDM3FmeSswMzFLd3I5RFh3YmhsQjR6c05DeTlFb2pKU09H?=
 =?utf-8?B?R0dPa1JwS2ZzKzFrOGw2OE1hbEgxdTlobThlY1lHeDY0RkxpYStvSTFVS2dF?=
 =?utf-8?B?QTNDaXQzRk1DT0Y5V3I4RUFpY2IxNXJRb05Scy9MTmhnNGRCN2pDbUp4SDFu?=
 =?utf-8?B?ZGRkcHdvU0dJRERPSzRMek5sa3Z2NSs2REtqdVd0TFg5WDg0b2d0Rk51cU95?=
 =?utf-8?B?Y2dVeWF4Wjk4QVdKcDlSc1VGNXZsU2VVWGtRcExvODJJWDhtZkdlTEFCVnlO?=
 =?utf-8?B?Ym9QUzMycFVlTlN4Z0VFbXJyN2VQZWdFSXdEbXpaUHBlLzJ1SytKQkRoa1FD?=
 =?utf-8?B?cWYxVGRPOWozODVlSzZ0UlZDbWozeWFzRlREdEJVY1cwRytBNmUzN2trcnBP?=
 =?utf-8?B?V1RDZWZ4azhNK0JqK3Zhc2YzOURLUVRVRklkKzNyVXVQMmR1dEE3NEtIdWJD?=
 =?utf-8?B?V001L3hncGJWdFZEL05CM3RWOW5tT3FtTnV6WGlzWTdpYjg4VmZROTZwbWg0?=
 =?utf-8?B?R2RaR1dtZHlOOVN1NFJ4TzZpMHNCTDEvYjZyZWVVZ01rdHZlelVpRGdabXlT?=
 =?utf-8?B?bG42Wms2NEdNem5BTTFUb0FXQk1LR0pGVE93RVFHOVE2MGNXK0p6MWNtN0ZT?=
 =?utf-8?B?MWRtWjRzeUl2cnhlaFI2T2kyTEpsV3lhZHhueWRzV2ZXNE1WTXYxaEZXTFFy?=
 =?utf-8?B?QmExWkVaRmFHN0ZuMzVLa1FDbjcveVM5NDRwaVg0cVZ1S2pueUd0dE9SRDVs?=
 =?utf-8?B?U2RBd1JIaHkyai9SYmJSa3hmUlpQMlE5Sk16N0tGRjd4d05xaGU5bnpVcGZO?=
 =?utf-8?B?SC9UaVluYzF5alFleTgwdWJ3Y0kvZFpkL2o4cm53cGFrcW1vaVlsNFAzYWRS?=
 =?utf-8?B?bXBMM205RnlLcmZ1amZ1dG5Gd2taWWhEY0FnaTBXbC9vOGU3T1ZTMzQyOUU1?=
 =?utf-8?B?QXNYRHlDbEZUU003aTA2bkN4MFUvNTMwdUIwU1RvWkxVeUxzNWViRGVUQjVE?=
 =?utf-8?B?TXA0WlFScGxqM1VGRlhEbkk4a1VVNmQ5MFNSbVJCM1kxaWlZSWhKN0Z4dHcr?=
 =?utf-8?B?QTN1b0VTczNyTWVKM3JNVi93QXRVZmt1Sk1lczFLRUErZEcwYVhHa2JVWjBE?=
 =?utf-8?B?MjFvV2pJWEtKRkhUWW5IdlFib3ptc0Z3OHk2ampROHVXSWI5dlJmZWExMjZ1?=
 =?utf-8?B?K2RaNU9XZHc0OEYxcURhRHhTNUlrMXcvQS9vQUZSeDlVUXRTcER3cDV5aXp0?=
 =?utf-8?B?eEZwc0J0NEpFZUZwVDJvZ0hIbzVvRlNKVVN5RmRRbHYxc2ZBblNYdVFXQWtG?=
 =?utf-8?Q?G7z9RrLXxD+iBgkfiiWlq2oRd?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8cc6880b-c23a-4991-0e03-08dbda2a2304
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB7141.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Oct 2023 15:57:51.5725
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: oAKDGkE7efkGO/jQxpkLBvhchubwuoFlYkEordBFsAy5vhkzzgBRWAdB2NmBBvpwXOMLV3Gc6Pyg5a6ear80IA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5066
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Tue, 31 Oct 2023 08:58:14 -0700 (PDT)


On 10/30/2023 9:55 PM, 'Guanjun' wrote:
> From: Guanjun <guanjun@linux.alibaba.com>
> 
> The int_handle field in hw descriptor should also be protected
> by wmb() before possibly triggering a DMA read.
> 
> Fixes: ec0d64231615 (dmaengine: idxd: embed irq_entry in idxd_wq struct)

I think the direct fix is to eb0cf33a91b4 which moves the interrupt 
handle assignment to idxd_submit_desc() and has a write to 
desc->hw->int_handle before submission of desc->hw.

Fixes: eb0cf33a91b4 ("dmaengine: idxd: move interrupt handle assignment")

Other than that,
Reviewed-by: Lijun Pan <lijun.pan@intel.com>


> Signed-off-by: Guanjun <guanjun@linux.alibaba.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>   drivers/dma/idxd/submit.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/idxd/submit.c b/drivers/dma/idxd/submit.c
> index c01db23e3333..3f922518e3a5 100644
> --- a/drivers/dma/idxd/submit.c
> +++ b/drivers/dma/idxd/submit.c
> @@ -182,13 +182,6 @@ int idxd_submit_desc(struct idxd_wq *wq, struct idxd_desc *desc)
>   
>   	portal = idxd_wq_portal_addr(wq);
>   
> -	/*
> -	 * The wmb() flushes writes to coherent DMA data before
> -	 * possibly triggering a DMA read. The wmb() is necessary
> -	 * even on UP because the recipient is a device.
> -	 */
> -	wmb();
> -
>   	/*
>   	 * Pending the descriptor to the lockless list for the irq_entry
>   	 * that we designated the descriptor to.
> @@ -199,6 +192,13 @@ int idxd_submit_desc(struct idxd_wq *wq, struct idxd_desc *desc)
>   		llist_add(&desc->llnode, &ie->pending_llist);
>   	}
>   
> +	/*
> +	 * The wmb() flushes writes to coherent DMA data before
> +	 * possibly triggering a DMA read. The wmb() is necessary
> +	 * even on UP because the recipient is a device.
> +	 */
> +	wmb();
> +
>   	if (wq_dedicated(wq)) {
>   		iosubmit_cmds512(portal, desc->hw, 1);
>   	} else {