Re: [PATCH v5 11/15] x86/mtrr: construct a memory map with cache modes

[Borislav Petkov]

On Thu, Apr 20, 2023 at 03:57:43PM +0200, Juergen Gross wrote:
> So you are suggesting that prefetching can happen across one wrong speculated
> branch, but not across two of them? And you are not worrying about prefetches
> past the end of a copy with size > 0?

Maybe it will, maybe it won't.

I am worried about calling a function unnecessarily. I'm worried about
calling the asm version __memmove() with zero length unnecessarily. I'm
worried about executing the return thunk unnecessarily:

ffffffff8104c749:       48 83 c4 28             add    $0x28,%rsp
ffffffff8104c74d:       e9 72 2a 9b 00          jmp    ffffffff819ff1c4 <__x86_return_thunk>
ffffffff8104c752:       31 c0                   xor    %eax,%eax
ffffffff8104c754:       e9 6b 2a 9b 00          jmp    ffffffff819ff1c4 <__x86_return_thunk>
ffffffff8104c759:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)

Just say that you don't want to do this simple check and I will do it
myself because I'm tired of debating.

> "If two or more variable memory ranges match and one of the memory types is UC,
> the UC memory type used."
> 
> So technically no problem, apart from lower performance.

How do you come from "Write-combining to UC memory is not allowed" to
"lower performance"?

Not allowed is not allowed. Geez.

> Would you be fine with adding that as an additional patch?
>
> I believe if we really want that, then we should be able to disable such a
> cleanup. So it should be an add-on anyway.

Sure, whatever.

> I'm not against adding such additional checks. I wouldn't like to force them
> into this series right now, because we need this series for Hyper-V isolated
> guest support.

We will add this series when they're ready. If Hyper-V needs them
immediately they can take whatever they want and do whatever they want.

Or you can do a simpler fix for Hyper-V that goes before this, if you
want to address Hyper-V.

> Just to say it explicitly: you are concerned for the case that a complete
> MTRR is hidden beneath another one (e.g. a large UC MTRR hiding a smaller
> MTRR with another type, or a variable MTRR being hidden by fixed MTRRs)?

I am concerned about catching any and all inconsistencies with the MTRRs
and catching them right. If we're going to spend all this time on this,
then let's do it right, once and for all and do it in a manner that can
be improved in the future.

Thanks!

-- 
Regards/Gruss,
    Boris.

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