diff mbox series

[v1,7/8] crypto: ccp - Skip DMA coherency check for platform psp

Message ID 20230123152250.26413-8-jpiotrowski@linux.microsoft.com
State New
Headers
Series Support ACPI PSP on Hyper-V |

Commit Message

Jeremi Piotrowski Jan. 23, 2023, 3:22 p.m. UTC 
  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(-)

Comments

Tom Lendacky Jan. 31, 2023, 8:42 p.m. UTC | #1 
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;
>   	}
Jeremi Piotrowski Feb. 8, 2023, 12:56 p.m. UTC | #2 
On 31/01/2023 21:42, Tom Lendacky wrote:
> 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/
> 

ok

>>   };
>>     #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.
>

ok (i had done that before i got to this comment for consistency)

>> +    }
>>       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.

makes sense

> 
> Thanks,
> Tom
> 
>>           dev_err(dev, "DMA is not supported");
>>           goto e_err;
>>       }
diff mbox series

Patch

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;
 };
 
 #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;
 		sp->dev_vdata = sp_get_plat_version(pdev);
+	}
 	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) {
 		dev_err(dev, "DMA is not supported");
 		goto e_err;
 	}