| Message ID | 1385394599-15439-2-git-send-email-anup.patel@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
(Adding Christoffer, forgot to add him to CC) On Mon, Nov 25, 2013 at 9:19 PM, 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 > > /* For KVM_EXIT_INTERNAL_ERROR */ > /* Emulate instruction failed. */ > -- > 1.7.9.5 > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
On Mon, Nov 25, 2013 at 09:19:58PM +0530, Anup Patel 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. Acked-by: Christoffer Dall <christoffer.dall@linaro.org> > > 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 > > /* For KVM_EXIT_INTERNAL_ERROR */ > /* Emulate instruction failed. */ > -- > 1.7.9.5 >
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? 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. Alex
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. > > 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. May be we can rename KVM_EXIT_SHUTDOWN and KVM_EXIT_RESET to KVM_EXIT_SYSTEM_SHUTDOWN and KVM_EXIT_SYSTEM_RESET ?? -- Anup > > > Alex > > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
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. > >> >> 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. Alex
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. -- Anup > > > Alex >
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. Alex
On Tue, Dec 10, 2013 at 03:13:34AM +0100, Alexander Graf 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? > > 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 didn't realize what the KVM_EXIT_SHUTDOWN really was, thanks for explaining that. In that case, the SYSTEM_EVENT sounds good to me. How do you propose the parameter gets passed? As a new struct to the untion in kvm_run ? -Christoffer
On Tue, Dec 10, 2013 at 04:49:19PM +0100, Alexander Graf 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. > Did you grab this documentation from somewhere that I can't find with grep, or did you just come up with it? Shouldn't we have all exit reasons documented in Documentation/virtual/kvm/... ? > > > >> > >> 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. > > -Christoffer
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
On 10.12.2013, at 23:30, Christoffer Dall <christoffer.dall@linaro.org> wrote: > On Tue, Dec 10, 2013 at 04:49:19PM +0100, Alexander Graf 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. >> > > Did you grab this documentation from somewhere that I can't find with > grep, or did you just come up with it? That was from the AMD64 architecture reference manual :). > Shouldn't we have all exit reasons documented in > Documentation/virtual/kvm/... ? Yes, that would be great. The only reason I knew something was fishy was that there's no way for a guest to trigger a non-MMIO/PIO/HCALL event that would end up in a genuine shutdown request ;). Alex
On 10.12.2013, at 23:27, Christoffer Dall <christoffer.dall@linaro.org> wrote: > On Tue, Dec 10, 2013 at 03:13:34AM +0100, Alexander Graf 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? >> >> 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 didn't realize what the KVM_EXIT_SHUTDOWN really was, thanks for > explaining that. In that case, the SYSTEM_EVENT sounds good to me. > > How do you propose the parameter gets passed? As a new struct to the > untion in kvm_run ? Yup, that's how we usually pass information along with an exit reason. Make sure to add a flags field so it can be extended if we need to pass in more later for future unknown events. PPC for example has fun system wide hypercalls like "enable little endian mode on all virtual cpus". I don't even want to start imagining what people will come up with next. Alex
On Wed, Dec 11, 2013 at 4:06 AM, Alexander Graf <agraf@suse.de> wrote: > > On 10.12.2013, at 23:27, Christoffer Dall <christoffer.dall@linaro.org> wrote: > >> On Tue, Dec 10, 2013 at 03:13:34AM +0100, Alexander Graf 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? >>> >>> 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 didn't realize what the KVM_EXIT_SHUTDOWN really was, thanks for >> explaining that. In that case, the SYSTEM_EVENT sounds good to me. >> >> How do you propose the parameter gets passed? As a new struct to the >> untion in kvm_run ? > > Yup, that's how we usually pass information along with an exit reason. Make sure to add a flags field so it can be extended if we need to pass in more later for future unknown events. PPC for example has fun system wide hypercalls like "enable little endian mode on all virtual cpus". I don't even want to start imagining what people will come up with next. Sure, I will add KVM_EXIT_SYSTEM_EVENT exit reason and related documentation. -- Anup > > > Alex > > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
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 /* For KVM_EXIT_INTERNAL_ERROR */ /* Emulate instruction failed. */