Re: [PATCH v5 21/31] powernv/fadump: process architected register state data provided by firmware

On 09/09/19 9:03 PM, Oliver O'Halloran wrote:

On Mon, Sep 9, 2019 at 11:23 PM Hari Bathini [off-list ref] wrote:

quoted

On 04/09/19 5:50 PM, Michael Ellerman wrote:

quoted

Hari Bathini [off-list ref] writes:

[...]

quoted

quoted

+/*
+ * 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.

quoted

+#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.

At least, the understanding is that this numbers not change across processor
generations. If something changes, it is supposed to be handled in SBE. Also,
I am told this numbers would be listed in the HDAT Spec. Not sure if that
happened yet though. Vasant, you have anything to add?

quoted

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.

I think, the changes if any, would have to be backward compatible for the sake
of sanity. Even if they are not, we are better off exporting the /proc/vmcore
with a warning and some crazy CPU register data (if parsing goes alright) than
no dump at all? 

- Hari