Re: [Xen-devel] irq_guest_eoi_timer interaction with MSI

["Jan Beulich" <jbeulich@xxxxxxxxxx>]

>>> Keir Fraser <keir.fraser@xxxxxxxxxxxxx> 24.11.08 18:02 >>>
>On 24/11/08 16:34, "Jan Beulich" <jbeulich@xxxxxxxxxx> wrote:
>
>> Perhaps the other way around: Make PHYSDEVOP_eoi imply an unmask,
>> as that's what is always happening (whereas not every unmask also wants
>> an EOI to be signaled). Below is a draft (compile-tested only) patch that,
>> before coding the guest side, I'd appreciate to get comments on -
>> especially if it appears reasonable to be done that way, if it meets your
>> naming and coding preferences (I'm pretty sure it won't), and of course
>> whether it's obviously broken in some respect.
>
>I don't care which way round you do it (PHYSDEVOP_eoi implies unmask, or
>vice versa) although you had just about convinced me that you should do it
>the other way round to how you've chosen. I don't really mind either way
>though.

I may have expressed myself a little imprecisely. I really just meant the two
operations to be folded into one. While looking at it in more detail I realized
that tying the EOI to the unmask wouldn't be nice as 'normal' unmasks must
still be possible.

My main concern was what the bitmap is to express, and indeed following
your approach of simply indicating the need for an EOI notification (rather
than indicating a need for unmask through hypercall) finally seemed more
consistent (and apparently making for simpler guest side code).

>The fixmap stuff is a bit ugly and I would just have done a
>map_domain_page_global() for 32-bit Xen (good enough as far as I'm
>concerned). I'm not dead set against your approach if you like it very much,
>though.

But just as map_domain_page(), map_domain_page_global() can't be used
out of IRQ context...

>Setting the need-a-hypercall bit looks racey. Don't you need to set the bit,
>then check the guest didn't unmask meanwhile?

We could, but I don't think it's strictly needed: The bit geting temporarily
set (as opposed to the case where it's being set for the lifetime of the IRQ)
is a performance optimization only anyway, i.e. to prevent the IRQ from
remaining masked for longer than it really needs to be. But yes, I'll see
whether the unmasking case can be taken care of without making the code
much more complicated.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel