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

[Gleb Natapov <gleb@redhat.com>]

On Mon, Jul 15, 2013 at 02:21:51PM +0200, Alexander Graf wrote:
> 
> On 15.07.2013, at 14:15, Gleb Natapov wrote:
> 
> > On Mon, Jul 15, 2013 at 01:44:46PM +0200, Alexander Graf wrote:
> >> 
> >> On 15.07.2013, at 13:30, 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 (not sure
> >>> why PPC will want them either instead of emulating devices that do
> >>> shutdown/reset
> >> 
> >> Implementing devices gets pretty tricky. Usually all of your devices sit on the SoC with a strictly defined layout. We can randomly shove some device in there, but there's a good chance we're overlapping with another device.
> >> 
> > I thought we have device trees to sort these things out.
> 
> For Linux guests, yes :). For proprietary random other guests, no.
> 
But those can't use hcalls too, no?

> > 
> >> So having a separate namespace with hcalls makes things a lot easier. And the guest needs to learn how to access it either way.
> >> 
> >>> ).  So lets move them to arch headers.
> >> 
> >> Do we want to keep the numbering scheme interchangable? Maybe there will be hcalls that can get shared between archs? If so, leaving it in the same header file might make sense.
> >> 
> > hcalls will not be handled in shared code, so I do not see why would we
> > want to have interchangable numbering scheme. hcalls handlers of
> > different arches can call common code after intercepting hcall and
> > retrieving arguments from an arch vcpu state.
> 
> Works for me, but then we should make hcall numbers 100% arch specific and have no global hc namespace anymore.
> 
Yes, of course. Move all of them to arch headers.

--
			Gleb.