On Wed, Feb 22, 2023 at 11:41:59AM -0800, KP Singh wrote:
Sure, I think the docs do already cover it,
I mean *our docs*. The stuff you're adding in your patch 2.
but I sort of disagree with your statement around the commit message. I feel the more context you can add in the commit message, the better it is.
That's ofc wrong. And you'll find that out when you do git archeology and you come across a huuuge wall of text explaining the world and some more.
No, commit messages should be to the point with a structure similar to something like this:
1. Prepare the context for the explanation briefly.
2. Explain the problem at hand.
3. "It happens because of <...>"
4. "Fix it by doing X"
5. "(Potentially do Y)."
concentrating on *why* the fix is being done.
When I am looking at the change log, it would be helpful to have the information that I mentioned in the Q&A. Small things like, "eIBRS needs the IBRS bit set which also enables cross-thread protections" is a very important context for this patch IMHO. Without this one is just left head scratching and scrambling to read lengthy docs and processor manuals.
Yes, that's why you say in the commit message: "For more details, see Documentation/admin-guide/hw-vuln/spectre.rst." where:
1. you can explain in a lot more detail
2. put it in place where people can find it *easily*
This sort of loosely implies that the IBRS bit also enables cross-thread protections. Can you atleast add this one explicitly?
"Setting the IBRS bit also enables cross thread protections"
Ok.
Not at the stage when the kernel decides to drop the STIBP protection when eIBRS is enabled.
We can't dump every possible interaction between the mitigations. It is a huge mess already. But I'm open to looking at improvements of the situation *and* documenting stuff as we go.
Thx.