On Tue, Jul 28, 2020 at 5:41 AM Alexander Graf <graf@xxxxxxxxxx> wrote: > > > > On 28.07.20 10:15, Vitaly Kuznetsov wrote: > > > > Alexander Graf <graf@xxxxxxxxxx> writes: > > > >> MSRs are weird. Some of them are normal control registers, such as EFER. > >> Some however are registers that really are model specific, not very > >> interesting to virtualization workloads, and not performance critical. > >> Others again are really just windows into package configuration. > >> > >> Out of these MSRs, only the first category is necessary to implement in > >> kernel space. Rarely accessed MSRs, MSRs that should be fine tunes against > >> certain CPU models and MSRs that contain information on the package level > >> are much better suited for user space to process. However, over time we have > >> accumulated a lot of MSRs that are not the first category, but still handled > >> by in-kernel KVM code. > >> > >> This patch adds a generic interface to handle WRMSR and RDMSR from user > >> space. With this, any future MSR that is part of the latter categories can > >> be handled in user space. This sounds similar to Peter Hornyack's RFC from 5 years ago: https://www.mail-archive.com/kvm@xxxxxxxxxxxxxxx/msg124448.html. > >> Furthermore, it allows us to replace the existing "ignore_msrs" logic with > >> something that applies per-VM rather than on the full system. That way you > >> can run productive VMs in parallel to experimental ones where you don't care > >> about proper MSR handling. > >> > > > > In theory, we can go further: userspace will give KVM the list of MSRs > > it is interested in. This list may even contain MSRs which are normally > > handled by KVM, in this case userspace gets an option to mangle KVM's > > reply (RDMSR) or do something extra (WRMSR). I'm not sure if there is a > > real need behind this, just an idea. > > > > The problem with this approach is: if currently some MSR is not > > implemented in KVM you will get an exit. When later someone comes with a > > patch to implement this MSR your userspace handling will immediately get > > broken so the list of not implemented MSRs effectively becomes an API :-) Indeed. This is a legitimate concern. At Google, we have experienced this problem already, using Peter Hornyack's approach. We ended up commenting out some MSRs from kvm, which is less than ideal. > Yeah, I'm not quite sure how to do this without bloating the kernel's > memory footprint too much though. > > One option would be to create a shared bitmap with user space. But that > would need to be sparse and quite big to be able to address all of > today's possible MSR indexes. From a quick glimpse at Linux's MSR > defines, there are: > > 0x00000000 - 0x00001000 (Intel) > 0x00001000 - 0x00002000 (VIA) > 0x40000000 - 0x50000000 (PV) > 0xc0000000 - 0xc0003000 (AMD) > 0xc0010000 - 0xc0012000 (AMD) > 0x80860000 - 0x80870000 (Transmeta) > > Another idea would be to turn the logic around and implement an > allowlist in KVM with all of the MSRs that KVM should handle. In that > API we could ask for an array of KVM supported MSRs into user space. > User space could then bounce that array back to KVM to have all in-KVM > supported MSRs handled. Or it could remove entries that it wants to > handle on its own. > > KVM internally could then save the list as a dense bitmap, translating > every list entry into its corresponding bit. > > While it does feel a bit overengineered, it would solve the problem that > we're turning in-KVM handled MSRs into an ABI. It seems unlikely that userspace is going to know what to do with a large number of MSRs. I suspect that a small enumerated list will suffice. In fact, +Aaron Lewis is working on upstreaming a local Google patch set that does just that.