On Mon, Sep 9, 2019 at 11:23 PM Hari Bathini <[email protected]> wrote: > > On 04/09/19 5:50 PM, Michael Ellerman wrote: > > Hari Bathini <[email protected]> writes: > > > > [...] > > >> +/* > >> + * CPU state data is provided by f/w. Below are the definitions > >> + * provided in HDAT spec. Refer to latest HDAT specification for > >> + * any update to this format. > >> + */ > > > > How is this meant to work? If HDAT ever changes the format they will > > break all existing kernels in the field. > > > >> +#define HDAT_FADUMP_CPU_DATA_VERSION 1 > > Changes are not expected here. But this is just to cover for such scenario, > if that ever happens.
The HDAT spec doesn't define the SPR numbers for NIA, MSR and the CR. As far as I can tell the values you've assumed here are chip-specific, non-architected SPR numbers that come from an array buried somewhere in the SBE codebase. I don't believe you for a second when you say that this will never change. > Also, I think it is a bit far-fetched to error out if versions mismatch. > Warning and proceeding sounds worthier because the changes are usually > backward compatible, if and when there are any. Will update accordingly... Literally the only reason I didn't drop the CPU DATA parts of the OPAL MPIPL series was because I assumed the kernel would do the sensible thing and reject or ignore the structure if it did not know how to parse the data. Oliver
