Re: [Qemu-devel] [PATCH 03/26] Add a hook to allow hypercalls to be emulated on PowerPC

[David Gibson <david@gibson.dropbear.id.au>]

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 :).

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson