Re: [PATCH 0/4] Attempt to cleanup the HSW offcore bits

[Peter Zijlstra]

On Mon, Oct 27, 2014 at 01:23:40PM +0100, Andi Kleen wrote:
> On Thu, Oct 23, 2014 at 12:51:19PM +0200, Peter Zijlstra wrote:
> > So Don asked about offcore and because I forgot I looked at the code and found
> > the terrible mess Andi created with the HSW/BDW bits.
> > 
> > This series attempts to clean some of that up but seeing how it was all magic
> > numbers 
> 
> All the bits are documented. The actual definitions are available
> in the JSON offcore definitions at https://download.01.org/perfmon/

Yeah, no. That's not how we write code. Also, there's no actual JSON
offcore file for HSW only some TSV file, and I've no mind to go decode
and reverse engineer that stuff.

Also, semi unreadable files on a weird web location is not
documentation, the SDM is and the SDM does not explain why you didn't
put the PF request bits in the OP_PREFETCH events, nor why you added the
IFETCH bits to the OP_READ where all the other uarchs didn't.

Nor does it explain why you set bits 7,9 in OP_READ even though they're
listed as 'reserved'.

It also doesn't explain why you don't program a Supplier Info for
RESULT_ACCESS.

Nor why you set 0x78 in bits 23:30, which even for SNB are still listed
as reserved, even though its official offcore events have that set to
0xFF for all remote events.

And lacking SDM you should have explained all that in your Changelog,
but that's also entirely devoid of any usable information.

> > and no reasons provided for the differences with existing uarchs this
> > might just break things horribly.
> > That said, if this does break things, fixes will have to explain things, so
> > that's good.
> 
> I trust you won't submit or merge untested patches.

You trust wrong.. I should never have merged your patches, but seeing
they're in it needed cleaning up.

I can revert your patches if you'd rather have that?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/