Subject: Re: [PATCH V3 1/4] crypto: ccp - Fix SEV_INIT error logging on init
To:     Peter Gonda <pgonda@google.com>
Cc:     Sean Christopherson <seanjc@google.com>,
        Marc Orr <marcorr@google.com>,
        David Rientjes <rientjes@google.com>,
        Brijesh Singh <brijesh.singh@amd.com>,
        Joerg Roedel <jroedel@suse.de>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        John Allen <john.allen@amd.com>,
        "David S. Miller" <davem@davemloft.net>,
        Paolo Bonzini <pbonzini@redhat.com>,
        linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20211102142331.3753798-1-pgonda@google.com>
 <20211102142331.3753798-2-pgonda@google.com> <YYqhT+Enba5xa4cO@google.com>
 <CAMkAt6oVySH-1g+EXKvxQ9vmBV8rwTLq=qfe2+VZC+c6vATL7w@mail.gmail.com>
 <45af7051-aab0-2b19-2904-64eceec93df5@amd.com>
 <CAMkAt6pivYG=nV_EhP9Uj2gB8kBQB4dAnTcSeGd9249KuWzsyg@mail.gmail.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
Message-ID: <a6823579-c4af-6506-ba13-d6391b42d4ec@amd.com>
Date:   Thu, 11 Nov 2021 08:10:36 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
In-Reply-To: <CAMkAt6pivYG=nV_EhP9Uj2gB8kBQB4dAnTcSeGd9249KuWzsyg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cENMcHIwenp0MFdMNnozZ1RBdkFIS25mbVFMbzlUVnd0QllJNzBUdGRuWmpj?=
 =?utf-8?B?UGRrRHlTeU9iTnYyL0RtamZ1T2R2YklIVEpZNUxLaHJ2WW8vQjA5KzVnSFly?=
 =?utf-8?B?RjdOUGdtbmkyUkFlUDE0RlFlbHVYSzdWRUl2UU52ZFlhRURoTlg5Nk9qdEts?=
 =?utf-8?B?MnkrcUJIV2hzSUFVdjhHeWlBWlVhZEtqTHIvN2xVdUdjbjhucXVXQnZsMDE2?=
 =?utf-8?B?TGVWK0Y0WDhsRjVjRVgwaTV2cmlacjlSSFpseklMQzNWb1BYcHpGMHd6U3dR?=
 =?utf-8?B?MHk0WllkQzdTWjJiV1FTOTJsRUF3dmpFQU1HMWxpbXhsZkllWmFZMzkxY3Yw?=
 =?utf-8?B?WU1saVpjUDI4ajk4U2FUMHErbnAxN1NWQnZqM3dzRmFMRFltVEtHU3pYQXFT?=
 =?utf-8?B?SytpWHVFZmJyS1lwVzFFL2JzZWc1a3FwL2FZTEtoNzI5WU54WUlPYzlvZSt1?=
 =?utf-8?B?WjV6Mk43c3BKSDFZVDczcEdoUHZ0UVhmbUhwNjluY3NMeDFOY3hMbTJCeUJB?=
 =?utf-8?B?ZTM1aGVIS0cyOXlHMmVRcUhoa1VaZmlieGR3RjZBbDU3eUxqNTFLbzJ6Slpa?=
 =?utf-8?B?dnZZY2RQSzVnVmwxdTFXSkFUQkhoS2l0a2hiaW9pNmVjaGZHdDM0U3JTRi9o?=
 =?utf-8?B?MnBBRkFpUGZKcVFXaDJHRDAzazU5MVd1Q0FhcVNNa0VUd1h3U0RvdXlhV0Qz?=
 =?utf-8?B?ZHBXb0FLb2FhQlBqUHhLMHh6ZGlwRk1WN1VVUHIzU2NsZkxpZ1FMSE16dTgr?=
 =?utf-8?B?R0VEa05HTjBXQnZoV1JPY2xxTy9aMk5HV2VSRGNjOGZoSklka3BzMWUwakVx?=
 =?utf-8?B?VVVNN0pWK09WNlgzUm5TSFhrYVBDaFhuTlFKZ2ZoS0xMdDNpVG45eEVJZW9t?=
 =?utf-8?B?ZTRxdXRTcVZ0TnR6dWVweWp3WkVQTHJrK3ByM0s4bW9iOWlibFYwSnVoeXZh?=
 =?utf-8?B?a1FxcVpnZFphcUk5WGpRaytXUXFqOTl5SE5jVkxIbnM5YTJTYW8zWmVyNGNi?=
 =?utf-8?B?NDdKM0c3ZlpoN3dFam9XcXowWWtLY0lTUU9ZNnkvSnBwZW56VFdOaXd4U2R5?=
 =?utf-8?B?TDE0b0Ywak1uT3BDYUgyRTBxdE9yeUZoQ1dMd1JXcTBkUVNIYTlkY0FJY2pW?=
 =?utf-8?B?TFNuSW1KMERyVHltVlgxU2VmUGVKM0UxY0tvZ0R5eC83bWV6WE9MRmg2a0JK?=
 =?utf-8?B?M0duOStVMWRJZ0FNVDJPWmtPc015UGh6L2R1UDFIbklYNDZOeUZrUVhiaWw5?=
 =?utf-8?B?N3plRXlBRXQzbDZPYkRIMU1ROUZkM2ZETlY4UGE1YTdUNjhiUy9mOFdOellG?=
 =?utf-8?B?UjlyZ25hQ3dhOTdTc2Z5bjA4YjZOQ2FaZkJTM3NndU9KanU2S2dCUUVwTDlM?=
 =?utf-8?B?cnRQQ1AwV3AwMXRXTEZXN3M3ekRHaC9oQ1hLTERLa2RpeTd6SUtFNG1iZkVy?=
 =?utf-8?B?eFdhV1NHSndyS1RlTk5UaVRhU2JSUG9yNVN3SlpvZDV6UThWMkY1WllLY3Ru?=
 =?utf-8?B?bitXVEl6ZXhGMWFTWTJjS1ZZSUd0RlR1QU5UTis4eGY5aFU0SjNoMzYvMi9O?=
 =?utf-8?B?Vko2K1RlbThrSmFZS2lIQ0Zya01uckZkSmFRQU8rSFhkYWpwblJPVng0WldK?=
 =?utf-8?B?ZGdHYmZ3OWMrQWk3a24xMEFrUjVuWXo1ZmsyZ1N6S05OTThBUmZQTkFSb3p0?=
 =?utf-8?B?Y2VnVS94OUZpQ0dGSHNtME9IaU5UWllPTTBzTUo1Vjh5SjNIamZyT0pCdzlZ?=
 =?utf-8?B?NGlzSi9oZUhaVGZQd2hZcnNENmlOYXNqdnJqWXRRQzNEeFB5UEJ1STFUR0Nh?=
 =?utf-8?B?K3RHNzFSR0JRUGJONU9xVUpFV050K2g4M3NwdUx2SEpVdFh0dmwrb0hUbTNS?=
 =?utf-8?B?ZzZwQjN4MkYxSGowTm1IaDlXclZlelJNRXpJYytpWkFIc1dITVd5akN3RzJM?=
 =?utf-8?B?a1RzQVZuYjBURDNCTEZOZnZTRm1QeGtQK05iaGFyU0hEcjdiaUFJSkxFazZq?=
 =?utf-8?B?VlhXdUcwNndYN1dLMy9kR1dLMlA1T3RaaEF5OGtUcVJqaG1ydzlWYU9LNk5B?=
 =?utf-8?B?R29HVnBTOGtSbXZnblpZcjUwMzVZK3NudXFpVzJwd3VKM0lUR1h0NXBqQ0t6?=
 =?utf-8?B?dEVWOGZVS2tKL0ZSMFNab0s2cjFXQ2pXK0NGaCtjbHEzUTFzU0FFejVFSXFJ?=
 =?utf-8?Q?2huj+C8BEEW3ngOvX8d8bvo=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1f63cdfe-5e7b-4966-4e58-08d9a51d09fe
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Nov 2021 14:10:39.1713
 (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: zuWEwq4dQFxDNt8h9Gc/unMOrqVpjDMftk0TMcSNIcarM7TS9AqrmERezRfI79bCsQtgx8QtwSfnptageDqLVg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR12MB5479
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

On 11/10/21 11:29 AM, Peter Gonda wrote:
> On Tue, Nov 9, 2021 at 12:25 PM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>> On 11/9/21 10:46 AM, Peter Gonda wrote:
>>> On Tue, Nov 9, 2021 at 9:27 AM Sean Christopherson <seanjc@google.com> wrote:
>>>> On Tue, Nov 02, 2021, Peter Gonda wrote:
>>

...

>>
>> That's one of those things we've wanted to get around to improving but
>> just haven't had the time. So, yes, if you wish to refactor the 'error'
>> related area, that would be great.
> 
> OK so when I actually sat down to work on this. I realized this is
> bigger than I thought. If we want to have error == -1 for all return
> from psp-sev.h functions where the PSP isn't called yet there are a
> lot of changes. For example if CONFIG_CRYPTO_DEV_SP_PSP is not defined
> all these stubs need to be to set `*errror == -`, basically all these
> functions need to be updated.
> 
> So to keep this series more targeted. I think I'll drop the error
> here. And just have this patch print the rc value. If what I said

In that case, I think you should keep the error value and initialize it to 
0. That is consistent with the other paths. Then, if you take on the 
fixups, it can be changed then.

> above seems reasonable I'll do those error refactors. Are people
> envisioning something else for the error fixups?

The main refactoring I wanted was to make sure the caller didn't have to 
initialize the error variable. Whether to initialize it to 0 or -1 wasn't 
part of my original thoughts. But I do like the -1 value because, 
theoretically, we shouldn't get such a value back from the PSP. So if the 
value printed is not -1, that is an indication that the PSP API was called 
no matter the value of rc.

Thanks,
Tom

> 
>>
>> Thanks,
>> Tom
>>
>>>