From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liran Alon Subject: Re: [PATCH v2 3/8] KVM: x86: set/get_events ioctl should consider only injected exceptions Date: Thu, 23 Nov 2017 20:20:04 +0200 Message-ID: <5A171154.9080005@ORACLE.COM> References: <1511278211-12257-1-git-send-email-liran.alon@oracle.com> <1511278211-12257-4-git-send-email-liran.alon@oracle.com> <20171122202503.GB21279@flask> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Cc: pbonzini@redhat.com, kvm@vger.kernel.org, jmattson@google.com, wanpeng.li@hotmail.com, idan.brown@ORACLE.COM, Krish Sadhukhan To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: Received: from userp1040.oracle.com ([156.151.31.81]:46052 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752931AbdKWSUP (ORCPT ); Thu, 23 Nov 2017 13:20:15 -0500 In-Reply-To: <20171122202503.GB21279@flask> Sender: kvm-owner@vger.kernel.org List-ID: On 22/11/17 22:25, Radim Krčmář wrote: > 2017-11-21 17:30+0200, Liran Alon: >> Do not consider pending exception when return injected exception >> to user-mode. A "pending" exception means it's side-effect have not >> been applied yet. In contrast, an "injected" exception means it's >> side-effect have been already applied. >> Therefore, we only need to report of injected exceptions to user-mode. >> This is aligned with how interrupts are reported in same ioctl. > > Pending interrupts are stored in IRR, but we don't have anything for > exceptions -- we would lose a trap exception that was made pending after > handling inject_pending_event() if the VCPU got a userspace signal and > save+restored before starting the next vcpu_enter_guest() cycle. > (Non-trap exceptions should be generated again when re-executing, so > losing them isn't that bad.) I see your point. I was indeed not considering correctly what will happen with trap exceptions. > > I think we should add state for pending exceptions in kvm_vcpu_events, > like the FIXME says. Pending and injected are actually exclusive (for > now?), so maybe we can use only one bit for that, The FIXME is a bit incorrect as it states "This is only needed for nested virtualization". As I understand this, it is not true as the issue relates to handling trap exceptions in general. But it is true that we should return extra state here. I also observed that pending & injected should be exclusive but didn't want to change code too much in one shot. You are right that in order to handle trap exceptions correctly we should also return to user-space if exception is in pending or injected state. I can change struct kvm_vcpu_events.exception such that: 1. "injected" field will remain logical OR of "exception.pending | .injected". 2. "pad" will become a "flags" field which currently contain a single flag indicating if exception is pending or not. That way, I think I don't break any backwards compatibility. What do you think? Regards, -Liran P.S: I would be glad if you could review also the rest of the patches in this series as they are independent of this change and are also a bit complex. Especially "[PATCH 7/8]: KVM: nVMX: Require immediate-exit when event reinjected to L2 and L1 event pending". > > thanks. > > An alternative, probably unattainable, would be to process the > side-effects as we hit the exception. Using IRR to store pending > interrupts also seems possible, but I'd expect more problems down the > road. >