Re: [PATCH 3/5] booke: define reset and shutdown hcalls

Gleb Natapov
2013-07-16T06:35:55Z

On Mon, Jul 15, 2013 at 01:17:33PM -0500, Scott Wood wrote:
> On 07/15/2013 06:30:20 AM, Gleb Natapov wrote:
> >On Mon, Jul 15, 2013 at 04:41:17PM +0530, Bharat Bhushan wrote:
> >> KVM_HC_VM_RESET: Requests that the virtual machine be reset.
> >> KVM_HC_VM_SHUTDOWN: Requests that the virtual machine be
> >powered-off/halted.
> >>
> >> These hcalls are handled by guest userspace.
> >>
> >> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> >> ---
> >> Documentation/virtual/kvm/hypercalls.txt | 16 ++++++++++++++++
> >> include/uapi/linux/kvm_para.h | 3 ++-
> >> 2 files changed, 18 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/Documentation/virtual/kvm/hypercalls.txt
> >b/Documentation/virtual/kvm/hypercalls.txt
> >> index ea113b5..58acdc1 100644
> >> --- a/Documentation/virtual/kvm/hypercalls.txt
> >> +++ b/Documentation/virtual/kvm/hypercalls.txt
> >> @@ -64,3 +64,19 @@ Purpose: To enable communication between the
> >hypervisor and guest there is a
> >> shared page that contains parts of supervisor visible register
> >state.
> >> The guest can map this shared page to access its supervisor
> >register through
> >> memory using this hypercall.
> >> +
> >> +5. KVM_HC_VM_RESET
> >> +------------------------
> >> +Architecture: PPC
> >> +Status: active
> >> +Purpose: Requests that the virtual machine be reset. The
> >hcall takes no
> >> +arguments. If successful the hcall does not return. If an error
> >occurs it
> >> +returns EV_INTERNAL.
> >> +
> >> +6. KVM_HC_VM_SHUTDOWN
> >> +------------------------
> >> +Architecture: PPC
> >> +Status: active
> >> +Purpose: Requests that the virtual machine be powered-off/halted.
> >> +The hcall takes no arguments. If successful the hcall does not
> >return.
> >> +If an error occurs it returns EV_INTERNAL.
> >> diff --git a/include/uapi/linux/kvm_para.h
> >b/include/uapi/linux/kvm_para.h
> >> index cea2c5c..218882d 100644
> >> --- a/include/uapi/linux/kvm_para.h
> >> +++ b/include/uapi/linux/kvm_para.h
> >> @@ -19,7 +19,8 @@
> >> #define KVM_HC_MMU_OP 2
> >> #define KVM_HC_FEATURES 3
> >> #define KVM_HC_PPC_MAP_MAGIC_PAGE 4
> >> -
> >> +#define KVM_HC_VM_RESET 5
> >> +#define KVM_HC_VM_SHUTDOWN 6
> >There is no much sense to share hypercalls between architectures.
> >There
> >is zero probability x86 will implement those for instance
>
> This is similar to the question of whether to keep device API
> enumerations per-architecture... It costs very little to keep it in
> a common place, and it's hard to go back in the other direction if
> we later realize there are things that should be shared.
>
This is different from device API since with device API all arches have
to create/destroy devices, so it make sense to put device lifecycle
management into the common code, and device API has single entry point
to the code - device fd ioctl - where it makes sense to handle common
tasks, if any, and despatch others to specific device implementation.

This is totally unlike hypercalls which are, by definition, very
architecture specific (the way they are triggered, the way parameter
are passed from guest to host, what hypercalls arch needs...). The
entry point of hypercalls is in arch specific code (again unlike device
API), so they way to reuse code if need arise is different too and does
not require common name space - just call common function after
retrieving hypercalls parameters in arch specific way.

> Keeping it in a common place also makes it more visible to people
> looking to add new hcalls, which could cut down on reinventing the
> wheel.
I do not want other arches to start using hypercalls in the way powerpc
started to use them: separate device io space, so it is better to hide
this as far away from common code as possible :) But on a more serious
note hypercalls should be a last resort and added only when no other
possibility exists, so people should not look what hcalls others
implemented, so they can add them to their favorite arch, but they
should have a problem at hand that they cannot solve without hcall, but
at this point they will have pretty good idea what this hcall should do.

>
> >(not sure why PPC will want them either instead of emulating
> >devices that do
> >shutdown/reset).
>
> Besides what Alex said, for shutdown we don't have any existing
> device to emulate (our real hardware just doesn't have that
> functionality). For reset we currently do emulate, but it's awkward
> to describe in the device tree what we actually emulate since the
> reset functionality is part of a kitchen-sink "device" of which we
> emulate virtually nothing other than the reset. Currently we
> advertise the entire thing and just ignore the rest, but that causes
> problems with the guest seeing the node and trying to use that
> functionality.
>
What about writing virtio device for shutdown and add missing emulation
to kitchen-sink device (yeah I know easily said that done), or make
the virtio device handle reset too? This of course raises the question
what address to use for a device hence the idea to use hcalls as
separate address space.

--
Gleb.