linux-kernel - Re: [patch 22/53] PNP: factor pnp_init_resource_table() and pnp_clean_resource

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] [day] [month] [year] [list]

Date:	Thu, 24 Apr 2008 16:45:50 -0600
From:	Bjorn Helgaas <bjorn.helgaas@...com>
To:	Rene Herman <rene.herman@...access.nl>
Cc:	Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org,
	linux-kernel@...r.kernel.org, Adam Belay <ambx1@....rr.com>,
	Li Shaohua <shaohua.li@...el.com>,
	Matthieu Castet <castet.matthieu@...e.fr>,
	Thomas Renninger <trenn@...e.de>,
	Jaroslav Kysela <perex@...ex.cz>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [patch 22/53] PNP: factor pnp_init_resource_table() and pnp_clean_resource_table()

On Tuesday 22 April 2008 03:01:58 pm Rene Herman wrote:
> On 22-04-08 01:10, Bjorn Helgaas wrote:
> >> Getting things working also needs setting pnp_res->index (to nport, nmem, 
> >> nirq, ndma in pnp_assign_resources) so that the isapnp_set_resources which 
> >> follows sets to the correct hardware index, but at that point position in 
> >> the list and the index are mixing together in unhealthy ways -- in the 
> >> pnp_assign_foo helpers, pnp_get_resource(.., idx) just get the "idx-th" 
> >> resource of the correct type in the list but it seems it really should be 
> >> getting the resource of the correct type with its ->index set to "idx".
> > 
> > I don't mind setting pnp_res->index in the generic pnp_assign_* code.
> > We have to do that already in pnp_set_current_resources() (the /sys
> > interface), and I don't see a good way around it.
> > 
> > In pnp_assign_resources(), we currently assume that all independent
> > options appear before any dependent ones because we compute nport,
> > nmem, etc by iterating through the independent options first.  Then
> > we use those nport, nmem, etc values as the "index" (CSR index for
> > ISAPNP, nth resource type in the template for PNPBIOS and PNPACPI).
> > I don't know whether this assumption is in the spec, but at least
> > we've assumed it for a long time.
> 
> Did this just address my position/index worry above?

Here's something interesting.  I was mistaken about the ordering
of independent vs. dependent options coming from BIOS -- I have
several examples where the dependent ones come first, including
this one:

    pnp 00:0a: parse resource options
    pnp 00:0a: new independent option
    pnp 00:0a: new dependent option (priority 0x1)
    pnp 00:0a:   io  min 0x3f8 max 0x3f8 align 1 size 8 flags 0x1
    pnp 00:0a: new dependent option (priority 0x1)
    pnp 00:0a:   io  min 0x2e8 max 0x2e8 align 1 size 8 flags 0x1
    pnp 00:0a: new dependent option (priority 0x1)
    pnp 00:0a:   io  min 0x2f8 max 0x2f8 align 1 size 8 flags 0x1
    pnp 00:0a: new dependent option (priority 0x1)
    pnp 00:0a:   io  min 0x3e8 max 0x3e8 align 1 size 8 flags 0x1
    pnp 00:0a: end dependent options
    pnp 00:0a:   irq bitmask 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000cf8 flags 0x1
    pnp 00:0a:   io  min 0x100 max 0x130 align 16 size 8 flags 0x1
    pnp 00:0a:   dma bitmask 0xe flags 0x2
    pnp 00:0a: Plug and Play ACPI device, IDs SMCf010 (disabled)

The interesting thing here is that this IRDA device comes up disabled,
and we use pnp_assign_resources() before enabling it.  But we
assign independent options first, so we end up reversing the order
of these IO regions:

    state = active
    io 0x100-0x107
    io 0x2e8-0x2ef
    irq 5
    dma 1

The driver for this device (smsc-ircc2.c) currently can't claim it
via PNP, but by long painful trial and error, I figured out last
summer that I could get the PNP claim code to work if I manually
swapped the IO regions!

I haven't done anything about this yet, but it sure looks to me like
we need to preserve the order of the independent/dependent options
as we get them from the BIOS.

Bjorn
--
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/