linux-kernel - Re: [PATCH] uio_pdrv: Unique IRQ Mode

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]

Date:	Mon, 9 Jun 2008 09:57:01 +0200
From:	Uwe Kleine-König <Uwe.Kleine-Koenig@...i.com>
To:	"Hans J. Koch" <hjk@...utronix.de>
CC:	Magnus Damm <magnus.damm@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"gregkh@...e.de" <gregkh@...e.de>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"lethal@...ux-sh.org" <lethal@...ux-sh.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>
Subject: Re: [PATCH] uio_pdrv: Unique IRQ Mode

Hello Hans,

> Did you notice that in this thread nobody spoke up to support your
> patch?
Actually I like what the patch tries to achieve.  I'd like to have it a
bit more explicit tough:

- Provide the irq disabling handler in uio_pdrv.c (or even uio.c) with a
  prototype in an adequate header.  Then the platforms that want this
  kind of handling can request it explicitly.

- Don't use this handler automatically.

- Provide the function named uio_pdrv_unique_irqcontrol in Magnus' patch
  in uio_pdrv.c and in an adequate header.

- Either rely on userspace to enable the irq before reading/polling or
  assert that in kernel space.  See also
  http://thread.gmane.org/gmane.linux.kernel/684683/focus=689635
  (I asked tglx about the race condition via irc, but without a response
  so far.)
  Currently the former is done, but if we decide to let it as it is, I'd
  like to have it documented.  (I.e. something like:  "Before
  polling/reading /dev/uioX assert that irqs are enabled.")

The last point is a bit independent from that mode, but applies to
devices that have a irqcontrol function in general.

Apart from the general things above, I'd change a few things in the
implementation:

 - call dev_info->irqcontrol(OFF) in the handler (instead of
   disable_irq()) and demand that calling this is idempotent.
   With this change it isn't uio_pdrv specific any more and could go to
   uio.c.

 - rename "Unique IRQ Mode" to something like "No IRQ Handler Mode".
   I'm not completely lucky with that name, but it's better than the
   former.  Of course the function should be renamed accordingly.

 - s/unsigned long irq_disabled/unsigned int irq_disabled : 1/ in struct
   uio_platdata.

Best regards
Uwe

-- 
Uwe Kleine-König, Software Engineer
Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962
--
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/