Message-ID: <9a6a152e-1c60-6300-88ce-85b835d3d198@amd.com>
Date:   Tue, 31 Jan 2023 14:42:35 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v1 7/8] crypto: ccp - Skip DMA coherency check for
 platform psp
Content-Language: en-US
To:     Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>,
        linux-kernel@vger.kernel.org
Cc:     Brijesh Singh <brijesh.singh@amd.com>,
        "Kalra, Ashish" <ashish.kalra@amd.com>,
        linux-crypto@vger.kernel.org
References: <20230123152250.26413-1-jpiotrowski@linux.microsoft.com>
 <20230123152250.26413-8-jpiotrowski@linux.microsoft.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
In-Reply-To: <20230123152250.26413-8-jpiotrowski@linux.microsoft.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?aVJqQWpzRU9hWGF3TW9PQzlUWVRHVkRURUFUeTlwVlhZUHhRNXJEU0dKa1NE?=
 =?utf-8?B?NGxWMDVUa3RMaVhBRFhYRzlyMVlUN2RFd1ROai9zQ3F2SmtvQi9JNi80UmN5?=
 =?utf-8?B?d0trMjh1WnpIZWVwQTlnWnk5VmdPbE5wU0NFYWNqR1hPVU5tNWVvWVFFUmlZ?=
 =?utf-8?B?YnFjUkFJUktULy8yZVBQYnV3cjZwS0RlS1R3UHVhOUhudUdJU3JOaXRNbmd0?=
 =?utf-8?B?TDYvdTlMUzFleVpsTjRPQjFlQmJ2aXV2MnV1bkxWMmJzMzVCeW5zdkFjc2Zt?=
 =?utf-8?B?L2dpRW8ydzQ5MCsyTlBLYUxrMDdVWnBDakxEOElJOC8xK3BnYXJ5Vkl4cUVT?=
 =?utf-8?B?S3FkZzM5VERRTTRqTUVrckNkaWdJRStSRWtvQnVmNWEwcXNuaTQyYUNORmZM?=
 =?utf-8?B?MkdKaXBDMThuNmpFZ2d4KzlrUng0dnZ4RjErRFBLSTR1VE1zc1JnT2lSUVpL?=
 =?utf-8?B?NzJ0QWordTFaNUtQZlp5YWZSYkQrMmhwRy9oQlJGWlh1TXZsMnh5VXlvdnU3?=
 =?utf-8?B?Q1E0dmRuRnBBK0RBK2Y4Q3EzRU5SV2t0dmd2NTVvSjFhNWhuRXVmR2tlT3c5?=
 =?utf-8?B?bjhlcmsvclBGZVlnUXBNSHBMd0pEcFUxR21IdTBHTWR5bitDTEJLVW9tbEFk?=
 =?utf-8?B?SURQZnFNaFF2VE5YQzZMNENKOHdiVnZBSXBpSmJQYWp2bysxb2RLQ0xMdzd6?=
 =?utf-8?B?WnNxeG5ZT0JSRllwRy9WNnNYSFNGY0c2bHlxL2xHRWFWc3kxbUFZRkV4TWs0?=
 =?utf-8?B?OUZmZnM3cGNYMzZ4RHVmcCtwMGZGUWZwL1U5RXFWU29sbWRwOFIvd1RWMC9G?=
 =?utf-8?B?eFUzdEZSVkVTVWIxNXB1UGdGR0ZnME14aGdnbE1qYUJLd1lkbldJQmVhbjh5?=
 =?utf-8?B?N1M3dTJLVzkra3lNYnBNdGs1aStVVENRWXgxQjd4ZGZlWndtMVh3TmxaME1V?=
 =?utf-8?B?NnFMcFcrSERqdlJid0JVc3RESnpQSHNkMXJMandPQkVNd25OZnViV3VVcjV6?=
 =?utf-8?B?cnNaVnRzMlhuVU85NXIwbU4wSWU4elhWL0tRc1NveHhNQXVYemlOMXFVR1VS?=
 =?utf-8?B?U3BNVm9iOVQwTko2ZWlCbHVxSlpqVU5hUGZnYVFuU3Azb1lZeFdiK054OTB5?=
 =?utf-8?B?akZjeTFQeXNBQTI2YUY5QStvZzNOUTBlanhtcGtoUzEraGdKZHB3UmVWK3dM?=
 =?utf-8?B?bnVib0k0d1FJRmNEY1BMUHliK2laUFlydGF3U0xtUzhsbTRIRGNzdUtDclpy?=
 =?utf-8?B?N253Zk1HbHlYdnBHd0RjemRCbXphd0xWUTNzUHc1ZDViOEJ6RUlpd2tCeTZI?=
 =?utf-8?B?SGVzREk1b1ZjS1RJeW45NnZoM203RXYvK0JZN3V6eG9aKzB5U0VVbkhjWllh?=
 =?utf-8?B?TmN1ZnYwYU0yelNKN2RLTWdZbG9FSlZJNEFubURQU2FHanZUZzViSEVlbjdF?=
 =?utf-8?B?NTUyRzZWMGJMYUdiMVN1YmJuWXdSbmVpSjdMWkVQbUVNLzNQQng3Z09CR3lY?=
 =?utf-8?B?NTFBVDA0bVRxamVEbVhHRS9GSjE3RGVaMWNEeGRHSk5ZOE1rNHBEc0MyaDFm?=
 =?utf-8?B?REpsakljd1pVT1ZETGJsY2xCV1Q1UzlseHd3bGl3UGp3L2JTSFFHRVFRTHJs?=
 =?utf-8?B?NzVUR2M2bWJuSGVNamF1VU5pVi8yRjhvWmM3ZWVKQk00Z09GNUpWR3dsbUpC?=
 =?utf-8?B?ck11Q1ZNNnJUNXpyTFJ0dHZhVVZRbkQyVko3aFlSVytzQjNYSFVQV0t0OEkx?=
 =?utf-8?B?NHBhRkdlZzlkSkhqMW9GUnFhelprdkN2NU5WUUNEbTJZblo4SmVHcVFwakwz?=
 =?utf-8?B?RENXalJGMjBqVEY1VWRTMVhzZkZ5NU5EV1owUEoyYUdvdi8vRkU5Yyt0V2dP?=
 =?utf-8?B?N2ZjR1dJZG9WRDZ2TlExSkFjZlh4Y0d1QzQzbW85YzJQaVBla2k4eURVRVFq?=
 =?utf-8?B?ZThEem02T0tZd3c3OFR0YWUzLzBjSS9BQ3pOS3JEb0hpeTA1Q2s0VnNBUDdz?=
 =?utf-8?B?cDdQQ3hKQjFraXN2TzBodzhsMk1SM1J0UWs3VVRISXNvK3YyMGZRMkp5bFR0?=
 =?utf-8?B?NG95MG5mYWRQU25kb3NyVUdnTVFSV1Q2ejlZN1RxczRRYUFpRzAzQnhqTXJx?=
 =?utf-8?Q?JJvetzL1pVB78WQrW2nRa0nRR?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c009de12-a127-4e83-831a-08db03cbb050
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2023 20:42:37.6399
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: d2feargdDvTthAxxmErZh5Fz0gBAAMW6xSrGVC7oHdAg5NOpOTfJ2p9c0tYiDfMkYcycKNqhGhOGOAPSj1EdbA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV2PR12MB6016
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

On 1/23/23 09:22, Jeremi Piotrowski wrote:
> The value of device_get_dma_attr() is only relevenat for ARM64 and CCP
> devices to configure the value of the axcache attribute used to access
> memory by the coprocessor. None of this applies to the platform psp so
> skip it.
> 
> Signed-off-by: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
> ---
>   drivers/crypto/ccp/sp-platform.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
> index 281dbf6b150c..b74f16e0e963 100644
> --- a/drivers/crypto/ccp/sp-platform.c
> +++ b/drivers/crypto/ccp/sp-platform.c
> @@ -29,6 +29,7 @@
>   struct sp_platform {
>   	int coherent;
>   	unsigned int irq_count;
> +	bool is_platform;

s/is_platform/is_platform_device/

>   };
>   
>   #ifdef CONFIG_CRYPTO_DEV_SP_PSP
> @@ -190,8 +191,10 @@ static int sp_platform_probe(struct platform_device *pdev)
>   	sp->dev_specific = sp_platform;
>   	sp->dev_vdata = pdev->dev.of_node ? sp_get_of_version(pdev)
>   					 : sp_get_acpi_version(pdev);
> -	if (!sp->dev_vdata && pdev->id_entry)
> +	if (!sp->dev_vdata && pdev->id_entry) {
> +		sp_platform->is_platform = true;

Move this into the sp_get_plat_version() function.

>   		sp->dev_vdata = sp_get_plat_version(pdev);

And I probably should have made this comment in the previous patch, but 
you should probably spell out platform here.

> +	}
>   	if (!sp->dev_vdata) {
>   		ret = -ENODEV;
>   		dev_err(dev, "missing driver data\n");
> @@ -205,7 +208,7 @@ static int sp_platform_probe(struct platform_device *pdev)
>   	}
>   
>   	attr = device_get_dma_attr(dev);
> -	if (attr == DEV_DMA_NOT_SUPPORTED) {
> +	if (!sp_platform->is_platform && attr == DEV_DMA_NOT_SUPPORTED) {

Just a nit but I'd prefer to see this as:

	if (attr == DEV_DMA_NOT_SUPPORTED && !sp_platform->is_platform) {

The diff is easier to see that way.

Thanks,
Tom

>   		dev_err(dev, "DMA is not supported");
>   		goto e_err;
>   	}