On 24.08.20 19:23, Jim Mattson wrote:
On Sun, Aug 23, 2020 at 6:35 PM Alexander Graf <graf@xxxxxxxxxx> wrote:
On 21.08.20 19:58, Jim Mattson wrote:
On Thu, Aug 20, 2020 at 3:55 PM Jim Mattson <jmattson@xxxxxxxxxx> wrote:
On Thu, Aug 20, 2020 at 2:59 PM Alexander Graf <graf@xxxxxxxxxx> wrote:
Do we really need to do all of this dance of differentiating in kernel
space between an exit that's there because user space asked for the exit
and an MSR access that would just generate a #GP?
At the end of the day, user space *knows* which MSRs it asked to
receive. It can filter for them super easily.
If no one else has an opinion, I can let this go. :-)
However, to make the right decision in kvm_emulate_{rdmsr,wrmsr}
(without the unfortunate before and after checks that Aaron added),
kvm_{get,set}_msr should at least distinguish between "permission
denied" and "raise #GP," so I can provide a deny list without asking
for userspace exits on #GP.
Actually, I think this whole discussion is moot. You no longer need
the first ioctl (ask for a userspace exit on #GP). The allow/deny list
is sufficient. Moreover, the allow/deny list checks can be in
kvm_emulate_{rdmsr,wrmsr} before the call to kvm_{get,set}_msr, so we
needn't be concerned with distinguishable error values either.
I also care about cases where I allow in-kernel handling, but for
whatever reason there still would be a #GP injected into the guest. I
want to record those events and be able to later have data that tell me
why something went wrong.
So yes, for your use case you do not care about the distinction between
"deny MSR access" and "report invalid MSR access". However, I do care :).
In that case, I'm going to continue to hold a hard line on the
distinction between a #GP for an invalid MSR access and the #GP for an
unknown MSR. If, for instance, you wanted to implement ignore_msrs in
userspace, as you've proposed in the past, this would be extremely
helpful. Without it, userspace gets an exit because (1) the MSR access
isn't in the allow list, (2) the MSR access is invalid, or (3) the MSR
is unknown to kvm. As you've pointed out, it is easy for userspace to
distinguish (1) from the others, since it provided the allow/deny list
in the first place. But how do you distinguish (2) from (3) without
replicating the logic in the kernel?
My stance on this is again that it's trivial to handle a few invalid MSR
#GPs from user space and just not report anything. It should come at
almost negligible performance cost, no?
Yes, the performance cost should be negligible, but what is the point?
We're trying to design a good API here, aren't we?
As for your argumentation above, we have a second call chain into
kvm_{get,set}_msr from the x86 emulator which you'd also need to cover.
One thing we could do I guess is to add a parameter to ENABLE_CAP on
KVM_CAP_X86_USER_SPACE_MSR so that it only bounces on certain return
values, such as -ENOENT. I still fail to see cases where that's
genuinely beneficial though.
I'd like to see two completely independent APIs, so that I can just
request a bounce on -EPERM through a deny list. I think it's useful
Where would that bounce to? Which user space event does that trigger?
Yet another one? Wouldn't 4 exit reasons just for MSR traps be a bit
much? :)
to distinguish between -ENOENT and -EINVAL, but I have no issues wih
both causing an exit to userspace, if userspace has requested exits on
MSR #GPs.
So imagine we took the first argument to ENABLE_CAP as filter:
(1<<0) REPORT_ENOENT
(1<<1) REPORT_EINVAL
(1<<2) REPORT_EPERM
(1<<31) REPORT_ANY
Then we also add the reason to the kvm_run exit response and user space
can differentiate easily between the different events.
Alex
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
