Re: [PATCH 16/30] KVM: PPC: e500mc: Add doorbell emulation support

On 20.02.2012, at 16:39, Scott Wood wrote:

On Mon, Feb 20, 2012 at 12:49:46PM +0100, Alexander Graf wrote:

quoted

=20
On 17.02.2012, at 22:55, Scott Wood wrote:
=20

quoted

On 02/17/2012 11:13 AM, Alexander Graf wrote:

quoted

	case BOOKE_IRQPRIO_EXTERNAL:
+#ifdef CONFIG_KVM_E500MC
+	case BOOKE_IRQPRIO_DBELL:
+#endif

=20
This isn't e500mc specific -- it's in the ISA as "Embedded.Processor
Control".
=20
Any harm in just removing the ifdef (similar to tlbilx)?

=20
Well, to me this is more of an indication "this should become a =

runtime

quoted

check one day" in case we want to combine the two targets.  On =

e500v2,

quoted

we don't know what a doorbell interrupt is, so we really shouldn't be
delivering one either.

=20
Should at least have a comment saying that eventually the decision =

should

be based on ISA category support rather than e500mc.

Hrm. How about a new #define that is a bit more verbose?

#ifdef CONFIG_KVM_E500MC
#define KVM_CAT_DOORBELL
#endif

=20

quoted

quoted

Should this be a kvm_make_request instead (with a separate
pending_doorbell bool in vcpu that msgclr can act on), considering
earlier discussion of phasing out atomics on pending_exceptions, in
favor of requests?

=20
Yeah, I was thinking about that too, but right now we already have =

some

quoted

direct use of pending_exceptions in different places around the code.=20=

quoted

So today, that is our public interface.
=20
I'd rather go in and clean up the whole thing to make
pending_exceptions private in a separate cleanup round, rather than
having it part of e500mc support.

=20
We already use requests for timers, and it seems simple enough to add =

one

for doorbells now rather than come back and clean it up later (it's =

not

tied to what we'd have to do for external IRQs), but if you'd rather =

do

it later that's fine with me.

Yeah, because I want the external interface to include the kick, which =
we don't model today. So instead of touching basically every line of =
code again, I'd rather do it in one roll.


Alex