Xen devel - Jun 2006 - [PATCH, resend] replacement for noirqdebug hack

Instead of re-establishing the noirqdebug hack earlier present in the i386 Linux
code, communicate the information about
whether a particular IRQ is shared across domains from hypervisor to kernel.

Signed-off-by: Jan Beulich <jbeulich@novell.com>



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

On 2 Jun 2006, at 11:02, Jan Beulich wrote:
> Instead of re-establishing the noirqdebug hack earlier present in the 
> i386 Linux code, communicate the information about
> whether a particular IRQ is shared across domains from hypervisor to 
> kernel.
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
This is gross for a few reasons. Firstly it adds Xen cruft to a file we 
don''t currently modify, and the changes would never be mergable 
upstream. Plus I''d prefer a lighter weight solution for handling this 
x86 crufty corner case -- adding extra bitmaps to shared_info and 
updating bits on every IRQ delivery is overkill imo.

What I would suggest is change note_interrupt() to only increment 
irqs_unhandled if some new function spurious_irqs_ok(irq) returns 
false. We can then define that function as Xen-specific and extend 
physdev_irq_status_query to be able to determine if an irq is shared 
across guests. We can avoid frequent hypercalls by caching the shared 
status the first time it evaluates true (so sharedness is sticky; I''d 
assume we would rarely take the path that executes the hypercall if the 
irq is really not shared).

  -- Keir


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

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 02.06.06 12:32
>>>
>
>On 2 Jun 2006, at 11:02, Jan Beulich wrote:
>
>> Instead of re-establishing the noirqdebug hack earlier present in the 
>> i386 Linux code, communicate the information about
>> whether a particular IRQ is shared across domains from hypervisor to 
>> kernel.
>>
>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
>This is gross for a few reasons. Firstly it adds Xen cruft to a file we 
>don''t currently modify, and the changes would never be mergable 
>upstream. Plus I''d prefer a lighter weight solution for handling
this
>x86 crufty corner case -- adding extra bitmaps to shared_info and 
>updating bits on every IRQ delivery is overkill imo.
I don''t think this has less chances of merging than other changes to
pre-existing files. Besides that, I don''t think
sharing of IRQs across domains is an x86-only issue. And where do you see
overkill in updating a single bit?
>What I would suggest is change note_interrupt() to only increment 
>irqs_unhandled if some new function spurious_irqs_ok(irq) returns 
>false. We can then define that function as Xen-specific and extend 
>physdev_irq_status_query to be able to determine if an irq is shared 
>across guests. We can avoid frequent hypercalls by caching the shared 
>status the first time it evaluates true (so sharedness is sticky;
I''d
>assume we would rarely take the path that executes the hypercall if the 
>irq is really not shared).
That stickiness is clearly undesirable to me - if you bring up a domU for
testing or other temporary purposes that
makes use of an interrupt already in use elsewhere, all other users of the
interrupt will from there on not do proper
accounting anymore. Further, how would you want to prevent doing the hypercall
if by default interrupts are non-shared
(I hope you didn''t have other intentions) and hence the (sticky) flag
wasn''t set, yet.

Jan

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

On 2 Jun 2006, at 12:01, Jan Beulich wrote:
> That stickiness is clearly undesirable to me - if you bring up a domU 
> for testing or other temporary purposes that
> makes use of an interrupt already in use elsewhere, all other users of 
> the interrupt will from there on not do proper
> accounting anymore.
It''s hardly a core part of interrupt management. It''d just be
disabling
a code path that only matters for broken hardware. Now that the IOAPIC 
ACking is fixed we could arguably just disable that path completely 
again (noirqdebug). So I think all the interface changes and additions 
to shared_info are overkill because this is just enabling a 
broken-hardware workaround.
>  Further, how would you want to prevent doing the hypercall if by 
> default interrupts are non-shared
> (I hope you didn''t have other intentions) and hence the (sticky)
flag
> wasn''t set, yet.
We''d only execute the hypercall if the return code was IRQ_NONE.

  -- Keir


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

On 2 Jun 2006, at 11:02, Jan Beulich wrote:
> Instead of re-establishing the noirqdebug hack earlier present in the 
> i386 Linux code, communicate the information about
> whether a particular IRQ is shared across domains from hypervisor to 
> kernel.
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
How about the following patch, which is a cleaner modification to Linux 
and does not affect shared_info layout? Note that it depends on a Xen 
patch that is currently in the staging tree (extends the irq info 
hypercall to inform about sharing status), and that the patch is 
against xen-unstable. The status check is much slower than reading from 
shared memory, but ought to be on a rare path (unless you have a 
high-rate interrupt line shared across guests, which is going to be 
rather suckful anyway).

  -- Keir




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

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 07.06.06 17:57
>>>
>
>How about the following patch, which is a cleaner modification to Linux 
>and does not affect shared_info layout? Note that it depends on a Xen 
>patch that is currently in the staging tree (extends the irq info 
>hypercall to inform about sharing status), and that the patch is 
>against xen-unstable. The status check is much slower than reading from 
>shared memory, but ought to be on a rare path (unless you have a 
>high-rate interrupt line shared across guests, which is going to be 
>rather suckful anyway).
Yes, that looks fine to me (assuming that the other patch you mention is the one
introducing
PHYSDEVOP_irq_status_query, and that this is a non-intrusive addition).

Jan

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