[Skiboot] [PATCH v4 04/11] doc: add opal secure variable documentation

[Oliver O'Halloran]

On Thu, Oct 31, 2019 at 2:07 PM Eric Richter <erichte at linux.ibm.com> wrote:
>
> On 10/30/19 9:46 PM, Michael Ellerman wrote:
> > Eric Richter <erichte at linux.ibm.com> writes:
> >> On 10/28/19 10:47 PM, Michael Ellerman wrote:
> >>> Eric Richter <erichte at linux.ibm.com> writes:
> > ...
> >>>> +    update-status:      contains the return code of the update queue
> >>>> +                        process run during initialization. Signifies if
> >>>> +                        updates were processed or not, and if there was
> >>>> +                        an error. See table below.
> >>>
> >>> Encoded how? I assume you mean as a u32, but please say so.
> >>>
> >>> This is a bit weird, encoding an OPAL return value in a property. I
> >>> don't understand how the kernel is going to use it. I don't see any
> >>> patches posted yet that use it?
> >>>
> >>
> >> It is a u64, will update to mention that.
> >>
> >> This isn't a property intended for the kernel to consume, meant for exposing
> >> to userspace the status of update. Update processing is handled at boot, so
> >> outside of writing to a log, it seemed like the best way to signal status
> >> to a user.
> >
> > But are we expecting a user to look at the device tree to find the
> > status? That seems suboptimal.
> >
> We don't currently have any tools that check for this, but this information
> should be available. Ideally, a script, or petitboot should report the status
> of the update to the user.

Sounds kind of janky, but whatever. I'm fine with this provided we do
enough pre-validation of updates to ensure that we should never see an
update failure unless the PNOR has been tampered with.

> > Shouldn't the status be exported via sysfs along with the other secvar
> > stuff?
> >
>
> ...and so maybe it should belong in sysfs as well, but the information still needs
> to be handed to the kernel in some form.

Yes, something about the expected format for updates should be in sysfs.

> >> As for the use of OPAL codes: they were convenient to use as they matched the
> >> error cases that likely all backends will share. Would it be more clear to
> >> define a new set of return codes, or represent this information in a
> >> different way?
> >
> > Using the OPAL codes is fine, if the kernel is going to consume them.
>
> If the value is exposed in sysfs, should the kernel translate the value to
> something else? A Linux/POSIX error code or a user-friendly string?

I don't have any strong opinions about what the encoding should be,
but but don't use Linux or POSIX errno values. We try to maintain the
pretense of OPAL being OS-independent so baking in Linux return codes
isn't acceptable. I'm also fairly sure POSIX doesn't actually specify
the values of the error codes, just their symbolic names. Either stick
with what you're currently doing and use the OPAL codes, defines a new
enum for secvar specific errors or put in a string.

> A sysadmin would likely inspect the OPAL msglog for more detail if there were
> an error, so I would suspect keeping this as a programmatic value would make
> more sense.

I've had people ask me where to find the OPAL msglog even after they'd
worked with OPAL systems for multiple years. Don't bet on people
knowing that they should be looking in the OPAL log.

Oliver