linux-kernel - Re: [Xen-devel] Re: [PATCH] xen: do not clear and mask evtchns in __xen_evtchn_do

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1301993495.23887.128.camel@zakaz.uk.xensource.com>
Date:	Tue, 5 Apr 2011 09:51:35 +0100
From:	Ian Campbell <Ian.Campbell@...rix.com>
To:	Stefano Stabellini <Stefano.Stabellini@...citrix.com>
CC:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Jeremy Fitzhardinge <Jeremy.Fitzhardinge@...rix.com>,
	"xen-devel@...ts.xensource.com" <xen-devel@...ts.xensource.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [Xen-devel] Re: [PATCH] xen: do not clear and mask evtchns in
 __xen_evtchn_do_upcall

Since this patch revolves around the interaction with the irq core I
think it would be worth running it by tglx as well as xen-devel for the
next iteration.

Does something about the switch of handler type fix the issue with
drivers which call disable_irq() in their interrupt handler and leave it
disabled until a later time? (e.g. the userspace evtchn driver). I think
I know why this is the case but it would be useful to mention in the
commit message.

On Mon, 2011-04-04 at 17:46 +0100, Stefano Stabellini wrote:
> 
>     - it uses these handlers in the same way Linux would use them: it let
>     Linux mask\unmask and ack the irq when Linux want to mask\unmask and ack
>     the irq;

This code is in Linux, perhaps you mean "the same way native code would
use them" or "with the semantics which the generic code defines" or
something along those lines.

> +       pirq_query_unmask(irq);
> +       /* we try to follow the same convention as Linux on native:

That's basically a coincidence, isn't it?

What we are actually trying to do is use the handler with the
appropriate semantics for the type of interrupt, e.g. event channels are
naturally edge triggered but some event channels can be bound to pirqs
with configurations that require an EOI and therefore have a level of
levelness about them.

> +        * handle_edge_irq for edge irqs and handle_fasteoi_irq for level
> +        * irqs

should probably mention the relationship between pirq_needs_eoi=>level
triggering.

Is there any relationship between pirq_needs_eoi and the trigger value
passed to PHYSDEVOP_setup_gsi at setup time?

> , see ioapic_register_intr (handle_level_irq is not used
> +        * anymore).
> +        */
> +       if (pirq_needs_eoi(irq))
> +               irq_set_chip_and_handler_name(irq, &xen_pirq_chip,
> +                               handle_fasteoi_irq, name);
> +       else
> +               irq_set_chip_and_handler_name(irq, &xen_pirq_chip,
> +                               handle_edge_irq, name);
> +
>  out:
>         spin_unlock(&irq_mapping_update_lock);

Ian.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/