Re: [PATCH V3 1/4] crypto: ccp - Fix SEV_INIT error logging on init

On Tue, Nov 9, 2021 at 12:25 PM Tom Lendacky [off-list ref] wrote:

On 11/9/21 10:46 AM, Peter Gonda wrote:

quoted

On Tue, Nov 9, 2021 at 9:27 AM Sean Christopherson [off-list ref] wrote:

quoted

On Tue, Nov 02, 2021, Peter Gonda wrote:

...

quoted

quoted

         SEV: failed to INIT error 0, rc -16

which a bit head scratching without looking at the code.  AFAICT, the PSP return
codes aren't intrinsically hex, so printing error as a signed demical and thus

         SEV: failed to INIT error -1, rc -16

would be less confusing.

And IMO requiring the caller to initialize error is will be neverending game of
whack-a-mole.  E.g. sev_ioctl() fails to set "error" in the userspace structure,
and literally every function exposed via include/linux/psp-sev.h has this same
issue.  Case in point, the retry path fails to re-initialize "error" and will
display stale information if the second sev_platform_init() fails without reaching
the PSP.

OK I can update __sev_platform_init_locked() to set error to -1. That
seems pretty reasonable. Tom, is that OK with you?

Yup, I'm ok with using -1.

quoted

...

quoted

These comments seem fine to me. But I'll refrain from updating
anything here since this seems out-of-scope of this series. Happy to
discuss further and work on that if Tom is interested in those
refactors too.

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
above seems reasonable I'll do those error refactors. Are people
envisioning something else for the error fixups?

Thanks,
Tom

quoted