linux-kernel - Re: [RFC v2-fix 1/1] x86/tdx: Handle in-kernel MMIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 18 May 2021 10:15:45 -0700
From:   Andi Kleen <ak@...ux.intel.com>
To:     Sean Christopherson <seanjc@...gle.com>,
        Dave Hansen <dave.hansen@...el.com>
Cc:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Andy Lutomirski <luto@...nel.org>,
        Tony Luck <tony.luck@...el.com>,
        Kirill Shutemov <kirill.shutemov@...ux.intel.com>,
        Kuppuswamy Sathyanarayanan <knsathya@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Raj Ashok <ashok.raj@...el.com>, linux-kernel@...r.kernel.org
Subject: Re: [RFC v2-fix 1/1] x86/tdx: Handle in-kernel MMIO

>>> * If we didn't annotate we would need to add an alternative to every
>>>    MMIO access in the kernel (even though 99.9% will never be used on
>>>    TDX) which would be a complete waste and incredible binary bloat
>>>    for nothing.
>> That sounds like something objective we can measure.  Does this cost 1
>> byte of extra text per readl/writel?  10?  100?
> Agreed.  And IMO, it's worth converting the common case (macros) if the overhead
> is acceptable, while leaving the #VE handling in place for non-standard code.

We have many millions of lines of MMIO using driver code in the kernel 
99.99% of which never runs in TDX. I don't see any point in impacting 
everything for this. That would be just against all good code change 
hygiene practices, and also just be bloated.

But we also don't don't want to touch every driver, for similar reasons.

What I think would make sense is to convert something to a direct TDCALL 
if we figure out the extra #VE is a real life performance problem. AFAIK 
the only candidate that I have in mind for this is the virtio doorbell 
write (and potentially later its VMBus equivalent). But we should really 
only do that if some measurements show it's needed.

> Why does this code exist at all?  TDX and SEV-ES absolutely must share code for
> handling MMIO reflection.  It will require a fair amount of refactoring to move
> the guts of vc_handle_mmio() to common code, but there is zero reason to maintain
> two separate versions of the opcode cracking.

While that's true on the high level, all the low level details are 
different. We looked at unifying at some point, but it would have been a 
callback hell. I don't think unifying would make anything cleaner.

Besides the bulk of the decoding work is already unified in the common 
x86 instruction decoder. The actual actions are different, and the code 
fetching is also different, so on the rest there isn't that much to unify.

> The existing SEV-ES #VC handlers appear to be missing page split checks, so that
> needs to be fixed.

Only if anyone in the kernel actually relies on it?

-Andi