| Message ID | 1300251423-6715-4-git-send-email-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
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
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
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;
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.
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.
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
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 :).
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 >
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;