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

[Peter Gonda]

On Tue, Nov 9, 2021 at 12:25 PM Tom Lendacky <thomas.lendacky@xxxxxxx> wrote:
>
> On 11/9/21 10:46 AM, Peter Gonda wrote:
> > On Tue, Nov 9, 2021 at 9:27 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> >> On Tue, Nov 02, 2021, Peter Gonda wrote:
>
> ...
>
> >>
> >>          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.
>
> >
>
> ...
>
> >
> > 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
>
> >