Re: [PATCH - 0/2] Identify native drivers and ACPI subsystem accessing the same resources

On Wed, 2007-07-18 at 16:10 -0600, Bjorn Helgaas wrote:
On Wednesday 18 July 2007 11:50:38 am Thomas Renninger wrote:
On Fri, 2007-07-13 at 10:56 -0600, Bjorn Helgaas wrote:
The PNP core does not request resources for devices that are active
at boot. Those resources currently don't get requested until a driver
claims the device. I think this is a bug that we should fix.
Yep, this is also a problem for the ACPI variables (in fact not really,
as long as not overlapping like the rtc resource) and probably the
reason why pnpacpi ignores addresses below 0x100?

By "ACPI variables," do you mean PM1a_EVT_BLK and the like? Those
should be subsets of the _CRS resources for some device.
No, I mean SystemIO OperationRegion declarations, like:
OperationRegion (IOID, SystemIO, 0x2E, 0x02)
Field (IOID, ByteAcc, NoLock, Preserve)
{
INDX, 8,
DATA, 8
}
It may happen that you see e.g.:
0080-008f : dma page reg # statically requested early in setup.c
0080-0080 : ACPI DEB0 # ACPI SystemIO OperationRegion
# requested at ACPI table parse time

It's not nice..., but also does not hurt, as long as those do not
overlap. Even if these overlap the ACPI region/variable just won't be
able to reserve the region, not perfect, but better than before when
ACPI regions/variables were simply ignored.

IMO:
- Making sure ACPI claimed regions do not interfere with native
drivers is very important and will get much more important in near
future

I agree with this. I know some of these native drivers (lm-sensors, etc)
are pretty essential right now. But they just aren't safe because they
use resources that ACPI thinks it owns. I think the drivers should be
changed to explicitly override (with appropriate KERN_WARN messages)
any prior ACPI resource reservations.

The rtc driver seems to request the whole rtc space (0x70-0x77)
which fails because 0x72-0x73 has already been requested SHARED by
an ACPI SystemIO variable. At least parts of the rtc space got
requested (rtc0), I haven't checked whether the device was working
correctly...

I tripped over a couple of these when I changed the PNP core to request
resources for active devices. The RTC is one; often BIOS says the RTC
is at 0x70-0x71 and 0x72-0x73, but the rtc drivers like to claim 0x8
(RTC_IO_EXTENT) ports. However, I think the driver only *uses* two
ports, so we should change the driver to only request what it is
using.

But there could be more?

I think the PNP core should request all the ports the device responds
to (as reported by the BIOS). The driver should request only the ports
it uses. If the rtc driver only uses two ports, there's no reason for
it to request all eight.

Argggggh, normally it should be:
5000-50fe : ACPI PMIO
...
50c0-50df : pnp 00:07
50e0-50ff : pnp 00:07 # this one is missing because
# it's bigger than the parent
50e0-50ef : amd756_smbus

IMO, this is backwards. "pnp 00:07" should be the parent and should
be reserved by the PNP core. A side-effect of this is that
drivers/pnp/system.c would not be needed at all.

This patch is about to reserve the ACPI regions/variables (see the AML
descriptions at the beginning).
If I interpreted the code and your comment right, only PNP0c02 and
PNP0c01 is served (reserved) by pnpacpi layer currently (didn't see this
first). And you want to let pnpacpi reserve all ACPI device specific
resources (_CRS)? This should be done by removing system.c and the
interface to identify specific PNP devices (probe, device_id,..), but
just check for a _CRS function for all ACPI devices in
drivers/pnp/pnpacpi/core.c and reserve the resources.
IMO yes, this should be done and would be the first step.
Also reserve ACPI variables/OperationRegions, this is one is for, would
be the second step (and needs some more rework anyway).

I am off for a week, but will read and try out a bit more.
The next iteration will take some days (please stop me if you already
see some undoable hurdles that I may have overseen)...

Thanks a lot Bjorn, for the detailed review and all your input.

Thomas

Summary:
...
- Not fixable (maybe someone has an idea): If ACPI IO region declares
a smaller IO part than the later native driver (e.g. example above
with rtc driver), the partially overlapping resource cannot be
registered and the native driver fails to load with strict and lax
option.

Assuming the BIOS describes the region correctly, I'd say a driver
that claims a larger region is buggy and should be fixed.

Yep, but a temporary solution where everything just works fine and a
message: "This driver needs fixing" is needed IMO (if the code gets
accepted... It's possible, but ...).

How about something like this: We fix all the native drivers we know
about. We make the PNP and ACPI cores request resources for all active
devices by default, but add a flag like "pnp=no_reservations" that turns
this off. Then native drivers that request conflicting resources
will fail, but the user can limp along by using the boot-time option.

Yes sounds good.
What I wanted to do is to also reserve the OperationRegions. I didn't
realize that pnp layer only reserves resources for specific devices and
this should be done first (reserve all).
I am pretty sure arbitrary AML code using variables from
OperationRegions can still be outside device specific resources and we
need to reserve those too (or at least have a boot option to identify
possible interference) to make sure ACPI interpreted code does not clash
with native drivers or at least identify which drivers potentially
interfere.
However, this needs an ugly implementation (extension of
kernel/resources.c) because those could partly overlap with device
specific ACPI resources (the pnp ones):

5000-50fe : ACPI PMIO # ACPI variable/OperationRegion -> for ASL
# example definition, see beginning of the mail
...
50c0-50df : pnp 00:07 # These two are device specific ACPI resources
50e0-50ff : pnp 00:07 # This one's end address overlaps by one byte
# with its parent -> must get accepted
50e0-50ef : amd756_smbus

Then you'd have the pnp (pnpacpi only) and ACPI
variables/OperationRegions that should always be the parents (where the
variables are the parents of the pnpacpi resources) and those are
allowed to overlap and represent/reserve all ACPI resources.

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

Relevant Pages