On Fri, Jul 10, 2020 at 12:11:54AM +0200, Paolo Bonzini wrote: > 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 Ya, my comments were in jest. Sorry if I implied I was grumpy about Paolo taking this patch, because I'm not. Just stubborn :-) > 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. It's more that I don't think host_initiated=true is the equivalent of rdmsr_safe(). It kind of holds true for rdmsr, but that's most definitely not the case for wrmsr where host_initiated=true completely changes what is/isn't allow. And if using host_initiated=true for rdmsr is allowed, then logically using it for wrmsr is also allowed. > 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. I think I could get behind that. It shoudn't interfere with my crusade to vanquish host_initiated :-)