linux-kernel - Re: [PATCH 1/2] KVM: X86: Move ignore

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]

Message-ID: <610241d9-b2ab-8643-1ede-3f957573dff3@redhat.com>
Date:   Fri, 10 Jul 2020 00:11:54 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Peter Xu <peterx@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        Vitaly Kuznetsov <vkuznets@...hat.com>
Subject: Re: [PATCH 1/2] KVM: X86: Move ignore_msrs handling upper the stack

On 09/07/20 23:50, Peter Xu wrote:
>> Sean: Objection your honor.
>> Paolo: Overruled, you're wrong.
>> Sean: Phooey.
>>
>> My point is that even though I still object to this series, Paolo has final
>> say.
>
> I could be wrong, but I feel like Paolo was really respecting your input, as
> always.

I do respect Sean's input, but I also believe that in this case there's
three questions:

a) should KVM be allowed to use the equivalent of rdmsr*_safe() on guest
MSRs?  I say a mild yes, Sean says a strong no.

b) is it good to separate the "1" and "-EINVAL" results so that
ignore_msrs handling can be moved out of the MSR access functions?  I
say yes because KVM should never rely on ignore_msrs; Sean didn't say
anything (it's not too relevant if you answer no to the first question).

c) is it possible to reimplement TSX_CTRL_MSR to avoid using the
equivalent of rdmsr*_safe()?  Sean says yes and it's not really possible
to argue against that, but then it doesn't really matter if you answer
yes to the first two questions.

Sean sees your patch mostly as answering "yes" to the question (a), and
therefore disagrees with it.  I see your patch mostly as answering "yes"
to question (b), and therefore like it.  I would also accept a patch
that reimplements TSX_CTRL_MSR (question c), but I consider your patch
to be an improvement anyway (question b).

> It's just as simple as a 2:1 vote, isn't it? (I can still count myself
> in for the vote, right? :)

I do have the final say but I try to use that as little as possible (or
never).  And then it happens that ever so rare disagreements cluster in
the same week!

The important thing is to analyze the source of the disagreement.
Usually when that happens, it's because a change has multiple purposes
and people see it in a different way.

In this case, I'm happy to accept this patch (and overrule Sean) not
because he's wrong on question (a), but because in my opinion the actual
motivation of the patch is question (b).

To be fair, I would prefer it if ignore_msrs didn't apply to
host-initiated MSR accesses at all (only guest accesses).  That would
make this series much much simpler.  It wouldn't solve the disagremement
on question (a), but perhaps it would be a patch that Sean would agree on.

Thanks,

Paolo

> Btw, you didn't reply to my other email:
> 
>   https://lore.kernel.org/kvm/20200626191118.GC175520@xz-x1/
> 
> Let me change the question a bit - Do you think e.g. we should never use
> rdmsr*_safe() in the Linux kernel as long as the MSR has a bit somewhere
> telling whether the MSR exists (so we should never trigger #GP on these MSRs)?
> I think it's a similar question that we're discussing here..