[Skiboot] [PATCH 1/2] opal-prd: Add get_prd_flags to host interfaces

[Daniel M Crowell]

> Are you okay with an unknown set returning 0?
Yes, I think that is the only way it can work.  We assume the bugs are 
un-fixed and the features are not present unless the bits are explicitly 
set.

Do we need to plumb in the reverse (set_capabilities) as well?  e.g. If 
HBRT is still using a version of a function that the host (still haven't 
found a good generic name for opal-prd/phyp-adjunct) has deprecated, how 
would you guys know (assuming the ABI isn't smart enough to hide it)? 
Perhaps we save this for a different day (or even the P9 interface update) 
regardless though...

--
Dan Crowell
Senior Software Engineer - Power Systems Enablement Firmware
IBM Rochester: t/l 553-2987
dcrowell at us.ibm.com



From:   Jeremy Kerr <jk at ozlabs.org>
To:     Daniel M Crowell/Rochester/IBM at IBMUS
Cc:     Benjamin Herrenschmidt <benh at au1.ibm.com>, Corey 
Swenson/Rochester/IBM at IBMUS, Patrick Barrett/Rochester/IBM at IBMUS, skiboot 
list <skiboot at lists.ozlabs.org>
Date:   09/13/2016 10:56 PM
Subject:        Re: [PATCH 1/2] opal-prd: Add get_prd_flags to host 
interfaces



Hi Dan,

> My intention was that the flags for OPAL and PHYP would be distinct.
>  That would handle the specific case we're dealing with right now.  I
> don't want to force a change into PHYP to set a bit to indicate they
> don't have a bug we found in OPAL/opal-prd.  Adding features into the
> mix does make the idea of a comon set of flags more enticing though.
>  I'll have to search around for the notes I sent out months ago related
> to a 'get_capabilities' function to see if there are more ideas we 
forgot.
> 
> I still like the idea of multiple sets, so here's attempt #3.
> 
>      /* Common features */
>     #define HBRT_CAPABILITIES_SET0_COMMON  0
>      #define HBRT_CAPABILITIES_COMMON_AWESOME_FEATURE     (1ul << 0)
>     #define HBRT_CAPABILITIES_COMMON_NEW_INTERFACE_PARMS (2ul << 0)
> 
>      /* OPAL fixes */
>     #define HBRT_CAPABILITIES_SET1_OPAL  1
>     #define HBRT_CAPABILITIES_OPAL_HAS_XSCOM_RC  (1ul << 0)
> 
>     /* PHYP fixes */
>     #define HBRT_CAPABILITIES_SET2_PHYP  2
>     #define HBRT_CAPABILITIES_PHYP_FIXED_BROKEN_THING  (1ul << 0)

OK, that almost exactly lines up with the new revision that I'm working
on :)

>> Or should we make the sets based on functionality, rather than
>> implementation?
> Not completely sure what you are suggesting here.

This was more grouping the sets along the lines of the functionality
that they affect, rather than the implementer of the interface (ie,
OPAL vs PHYP).

However - since HBRT knows whether it's running on OPAL vs. PHYP, it
does makes sense to have fixes grouped into OPAL / PHYP categories. As
long as we do use the COMMON set(s) when appropriate, and coordinate on
changes to that, we should be fine.

Are you okay with an unknown set returning 0?

Cheers,


Jeremy





-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/skiboot/attachments/20160913/91b9a68c/attachment.html>