Patchwork [03/26] Add a hook to allow hypercalls to be emulated on PowerPC

login
register
mail settings
Submitter David Gibson
Date March 16, 2011, 4:56 a.m.
Message ID <1300251423-6715-4-git-send-email-david@gibson.dropbear.id.au>
Download mbox | patch
Permalink /patch/87177/
State New
Headers show

Comments

David Gibson - March 16, 2011, 4:56 a.m.
From: David Gibson <dwg@au1.ibm.com>

PowerPC and POWER chips since the POWER4 and 970 have a special
hypervisor mode, and a corresponding form of the system call
instruction which traps to the hypervisor.

qemu currently has stub implementations of hypervisor mode.  That
is, the outline is there to allow qemu to run a PowerPC hypervisor
under emulation.  There are a number of details missing so this
won't actually work at present, but the idea is there.

What there is no provision at all, is for qemu to instead emulate
the hypervisor itself.  That is to have hypercalls trap into qemu
and their result be emulated from qemu, rather than running
hypervisor code within the emulated system.

Hypervisor hardware aware KVM implementations are in the works and
it would  be useful for debugging and development to also allow
full emulation of the same para-virtualized guests as such a KVM.

Therefore, this patch adds a hook which will allow a machine to
set up emulation of hypervisor calls.

Signed-off-by: David Gibson <dwg@au1.ibm.com>
---
 target-ppc/cpu.h    |    2 ++
 target-ppc/helper.c |    4 ++++
 2 files changed, 6 insertions(+), 0 deletions(-)
Alexander Graf - March 16, 2011, 1:46 p.m.
On 03/16/2011 05:56 AM, David Gibson wrote:
> From: David Gibson<dwg@au1.ibm.com>
>
> PowerPC and POWER chips since the POWER4 and 970 have a special
> hypervisor mode, and a corresponding form of the system call
> instruction which traps to the hypervisor.
>
> qemu currently has stub implementations of hypervisor mode.  That
> is, the outline is there to allow qemu to run a PowerPC hypervisor
> under emulation.  There are a number of details missing so this
> won't actually work at present, but the idea is there.
>
> What there is no provision at all, is for qemu to instead emulate
> the hypervisor itself.  That is to have hypercalls trap into qemu
> and their result be emulated from qemu, rather than running
> hypervisor code within the emulated system.
>
> Hypervisor hardware aware KVM implementations are in the works and
> it would  be useful for debugging and development to also allow
> full emulation of the same para-virtualized guests as such a KVM.
>
> Therefore, this patch adds a hook which will allow a machine to
> set up emulation of hypervisor calls.
>
> Signed-off-by: David Gibson<dwg@au1.ibm.com>
> ---
>   target-ppc/cpu.h    |    2 ++
>   target-ppc/helper.c |    4 ++++
>   2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index a20c132..eaddc27 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -692,6 +692,8 @@ struct CPUPPCState {
>       int bfd_mach;
>       uint32_t flags;
>       uint64_t insns_flags;
> +    void (*emulate_hypercall)(CPUState *, void *);
> +    void *hcall_opaque;
>
>       int error_code;
>       uint32_t pending_interrupts;
> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
> index 2094ca3..19aa067 100644
> --- a/target-ppc/helper.c
> +++ b/target-ppc/helper.c
> @@ -2152,6 +2152,10 @@ static inline void powerpc_excp(CPUState *env, int excp_model, int excp)
>       case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
>           dump_syscall(env);
>           lev = env->error_code;
> +	if ((lev == 1)&&  env->emulate_hypercall) {
> +	    env->emulate_hypercall(env, env->hcall_opaque);
> +	    return;
> +	}	

Tabs! Please go through all your patches and make sure there are no tabs 
in there :(.


Alex
Stefan Hajnoczi - March 16, 2011, 4:58 p.m.
On Wed, Mar 16, 2011 at 1:46 PM, Alexander Graf <agraf@suse.de> wrote:
> On 03/16/2011 05:56 AM, David Gibson wrote:
>>
>> From: David Gibson<dwg@au1.ibm.com>
>>
>> PowerPC and POWER chips since the POWER4 and 970 have a special
>> hypervisor mode, and a corresponding form of the system call
>> instruction which traps to the hypervisor.
>>
>> qemu currently has stub implementations of hypervisor mode.  That
>> is, the outline is there to allow qemu to run a PowerPC hypervisor
>> under emulation.  There are a number of details missing so this
>> won't actually work at present, but the idea is there.
>>
>> What there is no provision at all, is for qemu to instead emulate
>> the hypervisor itself.  That is to have hypercalls trap into qemu
>> and their result be emulated from qemu, rather than running
>> hypervisor code within the emulated system.
>>
>> Hypervisor hardware aware KVM implementations are in the works and
>> it would  be useful for debugging and development to also allow
>> full emulation of the same para-virtualized guests as such a KVM.
>>
>> Therefore, this patch adds a hook which will allow a machine to
>> set up emulation of hypervisor calls.
>>
>> Signed-off-by: David Gibson<dwg@au1.ibm.com>
>> ---
>>  target-ppc/cpu.h    |    2 ++
>>  target-ppc/helper.c |    4 ++++
>>  2 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index a20c132..eaddc27 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -692,6 +692,8 @@ struct CPUPPCState {
>>      int bfd_mach;
>>      uint32_t flags;
>>      uint64_t insns_flags;
>> +    void (*emulate_hypercall)(CPUState *, void *);
>> +    void *hcall_opaque;
>>
>>      int error_code;
>>      uint32_t pending_interrupts;
>> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
>> index 2094ca3..19aa067 100644
>> --- a/target-ppc/helper.c
>> +++ b/target-ppc/helper.c
>> @@ -2152,6 +2152,10 @@ static inline void powerpc_excp(CPUState *env, int
>> excp_model, int excp)
>>      case POWERPC_EXCP_SYSCALL:   /* System call exception
>>    */
>>          dump_syscall(env);
>>          lev = env->error_code;
>> +       if ((lev == 1)&&  env->emulate_hypercall) {
>> +           env->emulate_hypercall(env, env->hcall_opaque);
>> +           return;
>> +       }
>
> Tabs! Please go through all your patches and make sure there are no tabs in
> there :(.

scripts/checkpatch.pl is there to automate style checking.  That's the
easiest way to check patches before submitting them.

Stefan
Anthony Liguori - March 16, 2011, 8:44 p.m.
On 03/15/2011 11:56 PM, David Gibson wrote:
> From: David Gibson<dwg@au1.ibm.com>
>
> PowerPC and POWER chips since the POWER4 and 970 have a special
> hypervisor mode, and a corresponding form of the system call
> instruction which traps to the hypervisor.
>
> qemu currently has stub implementations of hypervisor mode.  That
> is, the outline is there to allow qemu to run a PowerPC hypervisor
> under emulation.  There are a number of details missing so this
> won't actually work at present, but the idea is there.
>
> What there is no provision at all, is for qemu to instead emulate
> the hypervisor itself.  That is to have hypercalls trap into qemu
> and their result be emulated from qemu, rather than running
> hypervisor code within the emulated system.
>
> Hypervisor hardware aware KVM implementations are in the works and
> it would  be useful for debugging and development to also allow
> full emulation of the same para-virtualized guests as such a KVM.
>
> Therefore, this patch adds a hook which will allow a machine to
> set up emulation of hypervisor calls.
>
> Signed-off-by: David Gibson<dwg@au1.ibm.com>
> ---
>   target-ppc/cpu.h    |    2 ++
>   target-ppc/helper.c |    4 ++++
>   2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index a20c132..eaddc27 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -692,6 +692,8 @@ struct CPUPPCState {
>       int bfd_mach;
>       uint32_t flags;
>       uint64_t insns_flags;
> +    void (*emulate_hypercall)(CPUState *, void *);
> +    void *hcall_opaque;

Is the hypercall handler ever specific to a CPU?

I'd prefer to see this as a generic interface that wasn't specific to 
target-ppc.

Basically, add a:

void cpu_hypercall(CPUState *env);

And then implement it within your target.  I'm not sure I get the opaque 
argument.

Regards,

Anthony Liguori

>
>       int error_code;
>       uint32_t pending_interrupts;
> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
> index 2094ca3..19aa067 100644
> --- a/target-ppc/helper.c
> +++ b/target-ppc/helper.c
> @@ -2152,6 +2152,10 @@ static inline void powerpc_excp(CPUState *env, int excp_model, int excp)
>       case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
>           dump_syscall(env);
>           lev = env->error_code;
> +	if ((lev == 1)&&  env->emulate_hypercall) {
> +	    env->emulate_hypercall(env, env->hcall_opaque);
> +	    return;
> +	}	
>           if (lev == 1 || (lpes0 == 0&&  lpes1 == 0))
>               new_msr |= (target_ulong)MSR_HVB;
>           goto store_next;
David Gibson - March 17, 2011, 2:26 a.m.
On Wed, Mar 16, 2011 at 04:58:41PM +0000, Stefan Hajnoczi wrote:
> On Wed, Mar 16, 2011 at 1:46 PM, Alexander Graf <agraf@suse.de> wrote:
> > On 03/16/2011 05:56 AM, David Gibson wrote:
[snip]
> scripts/checkpatch.pl is there to automate style checking.  That's the
> easiest way to check patches before submitting them.

Ah, thanks.  I was hoping for a tool like that but somehow missed it.
David Gibson - March 17, 2011, 4:55 a.m.
On Wed, Mar 16, 2011 at 03:44:49PM -0500, Anthony Liguori wrote:
> On 03/15/2011 11:56 PM, David Gibson wrote:
> >From: David Gibson<dwg@au1.ibm.com>
> >
> >PowerPC and POWER chips since the POWER4 and 970 have a special
> >hypervisor mode, and a corresponding form of the system call
> >instruction which traps to the hypervisor.
> >
> >qemu currently has stub implementations of hypervisor mode.  That
> >is, the outline is there to allow qemu to run a PowerPC hypervisor
> >under emulation.  There are a number of details missing so this
> >won't actually work at present, but the idea is there.
> >
> >What there is no provision at all, is for qemu to instead emulate
> >the hypervisor itself.  That is to have hypercalls trap into qemu
> >and their result be emulated from qemu, rather than running
> >hypervisor code within the emulated system.
> >
> >Hypervisor hardware aware KVM implementations are in the works and
> >it would  be useful for debugging and development to also allow
> >full emulation of the same para-virtualized guests as such a KVM.
> >
> >Therefore, this patch adds a hook which will allow a machine to
> >set up emulation of hypervisor calls.
> >
> >Signed-off-by: David Gibson<dwg@au1.ibm.com>
> >---
> >  target-ppc/cpu.h    |    2 ++
> >  target-ppc/helper.c |    4 ++++
> >  2 files changed, 6 insertions(+), 0 deletions(-)
> >
> >diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> >index a20c132..eaddc27 100644
> >--- a/target-ppc/cpu.h
> >+++ b/target-ppc/cpu.h
> >@@ -692,6 +692,8 @@ struct CPUPPCState {
> >      int bfd_mach;
> >      uint32_t flags;
> >      uint64_t insns_flags;
> >+    void (*emulate_hypercall)(CPUState *, void *);
> >+    void *hcall_opaque;
> 
> Is the hypercall handler ever specific to a CPU?

If you mean, "is the hypercall environment ever different from one cpu
to another within the same guest at the same time", then no.  Or at
least, no for any platform that exists now, and anything plausible I
can think of.

If you mean can the hypercall ABI and handling be different for
different CPU models within an architecture, then yes.  It's not there
yet, but BookE CPUs *will* have a quite different hypercall
environment to the PAPR hypercall environment used on IBM servers.

> I'd prefer to see this as a generic interface that wasn't specific
> to target-ppc.

> 
> Basically, add a:
> 
> void cpu_hypercall(CPUState *env);
> 
> And then implement it within your target.

I'm not exactly sure what you mean by "target" here.  It is *not*
sufficient to make the hypercall function per guest architecture, it
must be per machine.  However, it could be a global hook rather than
in the CPUState.

>  I'm not sure I get the
> opaque argument.

Well, my hypercall code needs to get at various device structures
established during machine init.  I use the opaque argument to pass a
context with this information, rather than having globals for the
things I need.  I could use a global instead, if you'd prefer.
Anthony Liguori - March 17, 2011, 1:20 p.m.
On 03/16/2011 11:55 PM, David Gibson wrote:
> On Wed, Mar 16, 2011 at 03:44:49PM -0500, Anthony Liguori wrote:
>> On 03/15/2011 11:56 PM, David Gibson wrote:
>>> From: David Gibson<dwg@au1.ibm.com>
>>>
>>> PowerPC and POWER chips since the POWER4 and 970 have a special
>>> hypervisor mode, and a corresponding form of the system call
>>> instruction which traps to the hypervisor.
>>>
>>> qemu currently has stub implementations of hypervisor mode.  That
>>> is, the outline is there to allow qemu to run a PowerPC hypervisor
>>> under emulation.  There are a number of details missing so this
>>> won't actually work at present, but the idea is there.
>>>
>>> What there is no provision at all, is for qemu to instead emulate
>>> the hypervisor itself.  That is to have hypercalls trap into qemu
>>> and their result be emulated from qemu, rather than running
>>> hypervisor code within the emulated system.
>>>
>>> Hypervisor hardware aware KVM implementations are in the works and
>>> it would  be useful for debugging and development to also allow
>>> full emulation of the same para-virtualized guests as such a KVM.
>>>
>>> Therefore, this patch adds a hook which will allow a machine to
>>> set up emulation of hypervisor calls.
>>>
>>> Signed-off-by: David Gibson<dwg@au1.ibm.com>
>>> ---
>>>   target-ppc/cpu.h    |    2 ++
>>>   target-ppc/helper.c |    4 ++++
>>>   2 files changed, 6 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>> index a20c132..eaddc27 100644
>>> --- a/target-ppc/cpu.h
>>> +++ b/target-ppc/cpu.h
>>> @@ -692,6 +692,8 @@ struct CPUPPCState {
>>>       int bfd_mach;
>>>       uint32_t flags;
>>>       uint64_t insns_flags;
>>> +    void (*emulate_hypercall)(CPUState *, void *);
>>> +    void *hcall_opaque;
>> Is the hypercall handler ever specific to a CPU?
> If you mean, "is the hypercall environment ever different from one cpu
> to another within the same guest at the same time", then no.  Or at
> least, no for any platform that exists now, and anything plausible I
> can think of.

Yes, that's what I was asking.  So having a function pointer in each 
CPUState isn't necessary.

> If you mean can the hypercall ABI and handling be different for
> different CPU models within an architecture, then yes.  It's not there
> yet, but BookE CPUs *will* have a quite different hypercall
> environment to the PAPR hypercall environment used on IBM servers.
>
>> I'd prefer to see this as a generic interface that wasn't specific
>> to target-ppc.
>> Basically, add a:
>>
>> void cpu_hypercall(CPUState *env);
>>
>> And then implement it within your target.
> I'm not exactly sure what you mean by "target" here.  It is *not*
> sufficient to make the hypercall function per guest architecture, it
> must be per machine.  However, it could be a global hook rather than
> in the CPUState.

I'd suggest a totally generic hypercall infrastructure but I know that's 
not plausible for Power.  So I'm suggesting defining cpu_hypercall() in 
cpu.h, and then somewhere in target-ppc/, you can implement whatever 
logic you need to support that function.

This fits well with how we dispatch other forms of I/O (cpu_outb, 
cpu_physical_memory_rw, etc).

Regards,

Anthony Liguori
David Gibson - March 18, 2011, 4:03 a.m.
On Thu, Mar 17, 2011 at 08:20:52AM -0500, Anthony Liguori wrote:
> On 03/16/2011 11:55 PM, David Gibson wrote:
> >On Wed, Mar 16, 2011 at 03:44:49PM -0500, Anthony Liguori wrote:
> >>On 03/15/2011 11:56 PM, David Gibson wrote:
[snip]
> >>Is the hypercall handler ever specific to a CPU?
> >If you mean, "is the hypercall environment ever different from one cpu
> >to another within the same guest at the same time", then no.  Or at
> >least, no for any platform that exists now, and anything plausible I
> >can think of.
> 
> Yes, that's what I was asking.  So having a function pointer in each
> CPUState isn't necessary.

That's right.

> >If you mean can the hypercall ABI and handling be different for
> >different CPU models within an architecture, then yes.  It's not there
> >yet, but BookE CPUs *will* have a quite different hypercall
> >environment to the PAPR hypercall environment used on IBM servers.
> >
> >>I'd prefer to see this as a generic interface that wasn't specific
> >>to target-ppc.
> >>Basically, add a:
> >>
> >>void cpu_hypercall(CPUState *env);
> >>
> >>And then implement it within your target.
> >I'm not exactly sure what you mean by "target" here.  It is *not*
> >sufficient to make the hypercall function per guest architecture, it
> >must be per machine.  However, it could be a global hook rather than
> >in the CPUState.
> 
> I'd suggest a totally generic hypercall infrastructure but I know
> that's not plausible for Power.

I'm still not sure what you're getting at here.  I can't see how a
generic (as in across architectures) hypercall infrastructure makes
sense when clearly both the implemenentation of a hypercall, and the
trigger to fire it off will be ISA specific.

>  So I'm suggesting defining
> cpu_hypercall() in cpu.h, and then somewhere in target-ppc/, you can
> implement whatever logic you need to support that function.

So I don't see the point of having this arch-specific wrapper function
which will do nothing but call a hypervisor platform specific hook,
presumably set by the machine.  There's only one callsite for the
hypercall function, why not just call the hook straight from there.

So, for the moment, I'm just going to take the hypercall hook out of
the CPUState and make it a global.  We can have the next round of
objections from there :).
Alexander Graf - March 18, 2011, 6:57 a.m.
On 18.03.2011, at 05:03, David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Mar 17, 2011 at 08:20:52AM -0500, Anthony Liguori wrote:
>> On 03/16/2011 11:55 PM, David Gibson wrote:
>>> On Wed, Mar 16, 2011 at 03:44:49PM -0500, Anthony Liguori wrote:
>>>> On 03/15/2011 11:56 PM, David Gibson wrote:
> [snip]
>>>> Is the hypercall handler ever specific to a CPU?
>>> If you mean, "is the hypercall environment ever different from one cpu
>>> to another within the same guest at the same time", then no.  Or at
>>> least, no for any platform that exists now, and anything plausible I
>>> can think of.
>> 
>> Yes, that's what I was asking.  So having a function pointer in each
>> CPUState isn't necessary.
> 
> That's right.
> 
>>> If you mean can the hypercall ABI and handling be different for
>>> different CPU models within an architecture, then yes.  It's not there
>>> yet, but BookE CPUs *will* have a quite different hypercall
>>> environment to the PAPR hypercall environment used on IBM servers.
>>> 
>>>> I'd prefer to see this as a generic interface that wasn't specific
>>>> to target-ppc.
>>>> Basically, add a:
>>>> 
>>>> void cpu_hypercall(CPUState *env);
>>>> 
>>>> And then implement it within your target.
>>> I'm not exactly sure what you mean by "target" here.  It is *not*
>>> sufficient to make the hypercall function per guest architecture, it
>>> must be per machine.  However, it could be a global hook rather than
>>> in the CPUState.
>> 
>> I'd suggest a totally generic hypercall infrastructure but I know
>> that's not plausible for Power.
> 
> I'm still not sure what you're getting at here.  I can't see how a
> generic (as in across architectures) hypercall infrastructure makes
> sense when clearly both the implemenentation of a hypercall, and the
> trigger to fire it off will be ISA specific.

I second that feeling. The reason for abstracting mmio and pio calls is so that multiple targets can share device emulation code. Hypercall code is 100% platform specific anyways.

Alex

>

Patch

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index a20c132..eaddc27 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -692,6 +692,8 @@  struct CPUPPCState {
     int bfd_mach;
     uint32_t flags;
     uint64_t insns_flags;
+    void (*emulate_hypercall)(CPUState *, void *);
+    void *hcall_opaque;
 
     int error_code;
     uint32_t pending_interrupts;
diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index 2094ca3..19aa067 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -2152,6 +2152,10 @@  static inline void powerpc_excp(CPUState *env, int excp_model, int excp)
     case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
         dump_syscall(env);
         lev = env->error_code;
+	if ((lev == 1) && env->emulate_hypercall) {
+	    env->emulate_hypercall(env, env->hcall_opaque);
+	    return;
+	}	    
         if (lev == 1 || (lpes0 == 0 && lpes1 == 0))
             new_msr |= (target_ulong)MSR_HVB;
         goto store_next;