linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: christoffer.dall@linaro.org (Christoffer Dall)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] KVM: Add KVM_EXIT_RESET to user space API header
Date: Tue, 10 Dec 2013 14:32:53 -0800	[thread overview]
Message-ID: <20131210223253.GC2871@cbox> (raw)
In-Reply-To: <DF4E61FD-DBF8-43FA-A070-3ADA4E4874B4@suse.de>

On Tue, Dec 10, 2013 at 05:45:31PM +0100, Alexander Graf wrote:
> 
> On 10.12.2013, at 17:07, Anup Patel <anup@brainfault.org> wrote:
> 
> > On Tue, Dec 10, 2013 at 9:19 PM, Alexander Graf <agraf@suse.de> wrote:
> >> 
> >> On 10.12.2013, at 05:23, Anup Patel <anup@brainfault.org> wrote:
> >> 
> >>> On Tue, Dec 10, 2013 at 7:43 AM, Alexander Graf <agraf@suse.de> wrote:
> >>>> 
> >>>> On 25.11.2013, at 16:49, Anup Patel <anup.patel@linaro.org> wrote:
> >>>> 
> >>>>> Currently, we don't have an exit reason for VM reset emulation
> >>>>> in user space hence this patch adds exit reason KVM_EXIT_RESET
> >>>>> for this purpose.
> >>>>> 
> >>>>> This newly added KVM_EXIT_RESET will be used by KVM arm/arm64
> >>>>> in-kernel PSCI support to reset VMs.
> >>>>> 
> >>>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> >>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> >>>>> ---
> >>>>> include/uapi/linux/kvm.h |    1 +
> >>>>> 1 file changed, 1 insertion(+)
> >>>>> 
> >>>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> >>>>> index 902f124..64a04cc 100644
> >>>>> --- a/include/uapi/linux/kvm.h
> >>>>> +++ b/include/uapi/linux/kvm.h
> >>>>> @@ -171,6 +171,7 @@ struct kvm_pit_config {
> >>>>> #define KVM_EXIT_WATCHDOG         21
> >>>>> #define KVM_EXIT_S390_TSCH        22
> >>>>> #define KVM_EXIT_EPR              23
> >>>>> +#define KVM_EXIT_RESET            24
> >>>> 
> >>>> I have to admit that I'm not particularly happy with the exit name. It's not obvious from the name under which circumstances it gets triggered. Does it get triggered when a core level reset happens? Does it get triggered when a system level reset happened? When the guest requests one?
> >>> 
> >>> The KVM_EXIT_RESET gets triggered when system level reset is
> >>> initiated by VCPU. For arm/arm64, this is through SYSTEM_RESET
> >>> PSCI call. In KVM x86 SVM/VMX, we have KVM_EXIT_SHUTDOWN
> >>> being used for system shutdown which we have re-used for arm/arm64.
> >> 
> >> Yeah, that name already did mislead you once :).
> >> 
> >> KVM_EXIT_SHUTDOWN happens on
> >> 
> >>  * triple fault
> >>  * CPU internal severe problems
> >> 
> >> the latter is defined as:
> >> 
> >> In contrast, an error that cannot be contained and is of such severity that it has compromised the continued operation of a processor core requires immediate action to terminate system processing and may result in a hardware-enforced shutdown. In the shutdown state, the execution of instructions by that processor core is halted. See Section 8.2.9 ?#DF?Double-Fault Exception (Vector 8)? on page 220 for a description of the shutdown processor state.
> >> 
> >> Triple faults are used commonly in 286 code to switch from PG to real mode. So they _have_ to be emulated as core reset. Otherwise you break old guests.
> >> 
> >> However, the scope of this exit is definitely vcpu wide. What you are looking for is a system wide notification. Commonly this happens through MMIO, but I can see why you wouldn't want that with PSCI interpreted in the kernel. That's why I asked you to create a completely new one to not add up the the confusion.
> > 
> > Thanks for the info on the x86 part.
> > 
> > I think all this info should have been part of KVM api documentation
> > for KVM_EXIT_SHUTDOWN.
> > 
> >> 
> >>> 
> >>>> 
> >>>> I know what it does, but I find the name too generic for what it is. What you're really doing is introduce a new communication channel in parallel to MMIO / PIO / HCALL which is only used for system level reset / shutdown today.
> >>>> 
> >>>> Can we treat it as such? Could you please make this a common exit number that's called something like
> >>>> 
> >>>> KVM_EXIT_SYSTEM_EVENT
> >>>> 
> >>>> with a parameter that can either be TRIGGER_SHUTDOWN or TRIGGER_RESET.
> >>>> 
> >>>> That way it's obvious what's going on and people don't get confused.
> >>> 
> >>> I don't foresee any system level operations other than SHUTDOWN
> >>> and RESET to be handled from KVM in-kernel code but I might be
> >>> wrong.
> >> 
> >> The good but about the EXIT_SYSTEM_EVENT is that it's immediately obvious that we're not talking about a vcpu local event. But I'm open to better names.
> >> 
> >>> May be we can rename KVM_EXIT_SHUTDOWN and KVM_EXIT_RESET
> >>> to KVM_EXIT_SYSTEM_SHUTDOWN and KVM_EXIT_SYSTEM_RESET ??
> >> 
> >> You definitely can not rename KVM_EXIT_SHUTDOWN. It's part of the KVM API. In fact, I think it's a bad idea to even reuse the name as it clearly works on vcpu level.
> > 
> > Sure, it makes sense to avoid use of KVM_EXIT_SHUTDOWN
> > in arm/arm64.
> > 
> > How about adding exit reasons KVM_EXIT_SYSTEM_RESET
> > and KVM_EXIT_SYSTEM_SHUTDOWN ?
> > 
> > These exit reasons will be used for arm/arm64 but can also be
> > used by other architectures if they want.
> 
> I don't think we'll be able to reuse anything for other archs. For hcall PPC for example, we want to keep the hcall number scheme as the same between guest <-> kvm and kvm <-> qemu. That makes the overall logic easier, as the hcall number space is already properly standardized.
> 
> 
It should also make the first switch in userspace on the exit reason
easier, as the same code is likely to handle all sorts of system events.

-Christoffer

  reply	other threads:[~2013-12-10 22:32 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-25 15:49 [PATCH 0/2] PSCI system off and reset for KVM ARM/ARM64 Anup Patel
2013-11-25 15:49 ` [PATCH 1/2] KVM: Add KVM_EXIT_RESET to user space API header Anup Patel
2013-11-26  3:59   ` Anup Patel
2013-12-09 22:52   ` Christoffer Dall
2013-12-10  2:13   ` Alexander Graf
2013-12-10  4:23     ` Anup Patel
2013-12-10 15:49       ` Alexander Graf
2013-12-10 16:07         ` Anup Patel
2013-12-10 16:45           ` Alexander Graf
2013-12-10 22:32             ` Christoffer Dall [this message]
2013-12-10 22:30         ` Christoffer Dall
2013-12-10 22:34           ` Alexander Graf
2013-12-10 22:27     ` Christoffer Dall
2013-12-10 22:36       ` Alexander Graf
2013-12-11  4:30         ` Anup Patel
2013-11-25 15:49 ` [PATCH 2/2] ARM/ARM64: KVM: Forward PSCI SYSTEM_OFF and SYSTEM_RESET to user space Anup Patel
2013-11-26  3:59   ` Anup Patel
2013-12-09 22:51   ` Christoffer Dall
2013-12-10  5:05     ` Anup Patel
2013-12-10 10:57       ` Marc Zyngier
2013-12-10 15:31         ` Anup Patel
2013-11-25 15:57 ` [PATCH 0/2] PSCI system off and reset for KVM ARM/ARM64 Anup Patel
2013-11-26  4:00   ` Anup Patel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131210223253.GC2871@cbox \
    --to=christoffer.dall@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).