Re: [PATCH kernel v2 1/3] x86/amd: Cache values in percpu variables

[Borislav Petkov]

On Thu, Jan 12, 2023 at 04:21:28PM +1100, Alexey Kardashevskiy wrote:
> "that function" is set_dr_addr_mask() (btw should I rename it to start with
> amd_?

If you really wanna... I mean the code is already doing AMD-specific handling
but sure, it'll be more obvious then that arch_install_hw_breakpoint() does only
AMD-specific masking there under the info->mask test.

> If it is out of bounds, it won't neither set or get. And these 2 helpers are
> used only by the kernel and the callers pass 0..3 and nothing else. BUG_ON()
> would do too, for example.

Yeah, we don't do BUG_ON - look for Linus' colorful explanations why. :)

In any case, I think we should always aim for proper recovery from errors but
this case is not that important so let's leave it at the WARN_ON_ONCE.

> imho having 1,2,3 in the code eliminates the need in a comment and produces
> the exact same end result. But since nobody cares, I'll do it the shorter
> way with just +1 and +2.

Yeah, the more important goal is simplicity. And that pays off when you have to
revisit that code and figure out what it does, later.

> Sure. Out of curiosity - why?^w^w^w^w^  it reduced the vmlinux size by 48
> bytes, nice.

The same answer - simplicity and speed when reading it.

That

	if (per_cpu(amd_dr_addr_mask, smp_processor_id())[dr] == mask)

is a bit harder to parse than

	if (per_cpu(amd_dr_addr_mask, cpu)[dr] == mask)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette