linux-kernel - Re: [PATCH v1 7/8] crypto: ccp - Skip DMA coherency check for platform psp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9a6a152e-1c60-6300-88ce-85b835d3d198@amd.com>
Date:   Tue, 31 Jan 2023 14:42:35 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Jeremi Piotrowski <jpiotrowski@...ux.microsoft.com>,
        linux-kernel@...r.kernel.org
Cc:     Brijesh Singh <brijesh.singh@....com>,
        "Kalra, Ashish" <ashish.kalra@....com>,
        linux-crypto@...r.kernel.org
Subject: Re: [PATCH v1 7/8] crypto: ccp - Skip DMA coherency check for
 platform psp

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@...ux.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;
>   	}