On Tue, Apr 6, 2021 at 7:00 AM Ashish Kalra <ashish.kalra@xxxxxxx> wrote: > > Hello Paolo, > > On Tue, Apr 06, 2021 at 03:47:59PM +0200, Paolo Bonzini wrote: > > On 06/04/21 15:26, Ashish Kalra wrote: > > > > It's a little unintuitive to see KVM_MSR_RET_FILTERED here, since > > > > userspace can make this happen on its own without having an entry in > > > > this switch statement (by setting it in the msr filter bitmaps). When > > > > using MSR filters, I would only expect to get MSR filter exits for > > > > MSRs I specifically asked for. > > > > > > > > Not a huge deal, just a little unintuitive. I'm not sure other options > > > > are much better (you could put KVM_MSR_RET_INVALID, or you could just > > > > not have these entries in svm_{get,set}_msr). > > > > > > > Actually KVM_MSR_RET_FILTERED seems more logical to use, especially in > > > comparison with KVM_MSR_RET_INVALID. > > > > > > Also, hooking this msr in svm_{get,set}_msr allows some in-kernel error > > > pre-processsing before doing the pass-through to userspace. > > > > I agree that it should be up to userspace to set up the filter since we now > > have that functionality. > > > > The userspace is still setting up the filter and handling this MSR, it > is only some basic error pre-processing being done in-kernel here. The bit that is unintuitive is that userspace will still get the kvm_exit from an msr filter for KVM_MSR_RET_FILTERED even if they did not add it to the filters. I don't think this is a huge deal: userspace asked for it indirectly (through cpuid+sev enablement). > > Thanks, > Ashish > > > Let me read the whole threads for the past versions to see what the > > objections were... > > > > Paolo > >