linux-kernel - Re: [PATCH 1/3] resource: shared I/O region support

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, 29 Mar 2010 11:06:33 -0700
From:	Jesse Barnes <jbarnes@...tuousgeek.org>
To:	"H. Peter Anvin" <hpa@...or.com>
Cc:	Giel van Schijndel <me@...tis.eu>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Hans de Goede <hdegoede@...hat.com>,
	Jean Delvare <khali@...ux-fr.org>,
	Jonathan Cameron <jic23@....ac.uk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Bjorn Helgaas <bjorn.helgaas@...com>,
	Dominik Brodowski <linux@...inikbrodowski.net>,
	Laurens Leemans <laurens@...nips.com>,
	lm-sensors@...sensors.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] resource: shared I/O region support

On Mon, 29 Mar 2010 10:45:35 -0700
"H. Peter Anvin" <hpa@...or.com> wrote:

> On 03/29/2010 10:38 AM, Giel van Schijndel wrote:
> > 
> > Patch after this line:
> > ========================================================================
> > resource: shared I/O region support
> > 
> > SuperIO devices share regions and use lock/unlock operations to chip
> > select.  We therefore need to be able to request a resource and wait for
> > it to be freed by whichever other SuperIO device currently hogs it.
> > Right now you have to poll which is horrible.
> > 
> > Add a MUXED field to IO port resources. If the MUXED field is set on the
> > resource and on the request (via request_muxed_region) then we block
> > until the previous owner of the muxed resource releases their region.
> > 
> > This allows us to implement proper resource sharing and locking for
> > superio chips using code of the form
> > 
> > enable_my_superio_dev() {
> > 	request_muxed_region(0x44, 0x02, "superio:watchdog");
> > 	outb() ..sequence to enable chip
> > }
> > 
> > disable_my_superio_dev() {
> > 	outb() .. sequence of disable chip
> > 	release_region(0x44, 0x02);
> > }
> > 
> > Signed-off-by: Giel van Schijndel <me@...tis.eu>
> > Signed-off-by: Alan Cox <alan@...ux.intel.com>
> 
> I have to question this approach a bit.
> 
> I would much rather see this as a two-step process, where multiple
> devices request the same region with a "sharable" flag, and then have a
> mutex associated with the struct resource (perhaps we need an outer
> container called "struct muxed_resource" or some such.)
> 
> What I *really* object to with this patch is that it inherently assumes
> that there is only one multiplexed resource in the entire system... but
> of course nowhere enforces that.

Well that does keep it simple, and with just one user that's probably
best.

But why not use the common bus driver method?  Muxing at the resource
level only seems to solve part of the problem...  It doesn't guarantee
for example that driver A does something to a shared region that breaks
driver B; it just makes sure they don't access the same region at the
same time.

-- 
Jesse Barnes, Intel Open Source Technology Center
--
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/