This can probably be easily added.

ACPI_SYSMEM_REGION_WINDOW_SIZE is 4K by default. Most operation regions are smaller than this anyway, so it certainly makes sense to add some intelligence to the mapping operations. And 4K is the miminum page size AFAIK.

How about this: if the start and end addresses of an operation region fit in a single page, just map that page. if the region crosses a page boundary, map up to the first page boundary immediately, then successive pages later as necessary.

Will this solve the problem?

Yes Bob. That should fix the issue.

Looking at the code, it looks to me that when creating a mapping, the length it uses is the smaller of:

1) The end of the operation region minus the requested address, and
2) ACPI_SYSMEM_REGION_WINDOW_SIZE 

Therefore, the code is not blindly just mapping ACPI_SYSMEM_REGION_WINDOW_SIZE; it is only mapping (at most) to the end of the operation region.

So, page boundaries should have no bearing on the problem; the code will only map to the end of the operation region and presumably sooner or later a mapping will be created that conflicts (assuming that the definition of the region actually intrudes into a page that is used for something else.)



Below,
MemInfo->Address is the base physical address of the region
MemInfo->Length is the length of the region
Address is the requested physical address


/*
 * Don't attempt to map memory beyond the end of the region, and
 * constrain the maximum mapping size to something reasonable.
 */
WindowSize = (ACPI_SIZE)
    ((MemInfo->Address + MemInfo->Length) - Address);

if (WindowSize > ACPI_SYSMEM_REGION_WINDOW_SIZE)
{
    WindowSize = ACPI_SYSMEM_REGION_WINDOW_SIZE;
}

Instead of from start to start+ACPI_SYSTEM_REGION_WINDOW_SIZE, we should map only from start to 4k_roundedown(start+ACPI_SYSTEM_REGION_WINDOW_SIZE).

Though we are in the same operation region, this region is being chopped into sub portions used by multiple ACPI entities. And mapping up to start+ACPI_SYSTEM_REGION_WINDOW_SIZE is going beyond the 4k boundary and checks in the ioremap() complains that we are trying to map beyond one of those sub portions (these subportions are registered in the resource tree as separate entities). Sub-portion in one of the reports we saw was ACPI non-volatile storage.

Mapping upto just the 4k boundary avoids the ioremap() request potentially crossing a resource entity.

For example, in the reported kernel log, this is the error:

resource map sanity check conflict: 0x7fffff00 0x800000ff 0x7ffff000 0x7fffffff ACPI Non-volatile Storage

Here, acpi is trying to map from 0x7fffff00 0x800000ff, and in the resource tree we have a ACPI Non-volatile storage which is from 0x7ffff000 0x7fffffff.

So acpi map request is spanning more than one resource entity.

We should chnage the acpi map request to map only from 0x7fffff00 to 0x7fffffff.

So, in your example, it looks like we have an operation region that is defined from 0x7fffff00 to 0x800000ff.

Are you saying that it would be OK if we created two separate mappings (not at the same time) of:

0x7fffff00 to 0x7fffffff, and
0x80000000 to 0x800000ff

Yes. First ioremap() from 0x7fffff00 to 0x7fffffff followed by another ioremap() from 0x80000000 to 0x800000ff (while the first mapping is active/inactive) is ok.

As the intelligence lies with the caller, we have to do this.

ioremap() checks happen at the resource level and it just happens that we span two different resource entities when we try to map the whole of 0x7fffff00 to 0x800000ff in one ioremap() request.

Hope this helps.

I think I understand the issue.

However, in ACPICA, we don't know what the page size is.

hmm, for now, can't we round down to ACPI_SYSTEM_REGION_WINDOW_SIZE? In general that also makes sense.

Yes, we can do that. Probably rename the #define since we are now dealing with actual pages, not some arbitrary (and configurable) window size.

Another question: Why would there exist an operation region which would cross the boundary into ACPI Non-volatile storage?

Created attachment 23491 [details]
proposed patch from Bob

Patch to ensure that memory mappings created for operation regions do not cross page boundaries.
Crossing a page boundary while mapping regions can cause warnings if the pages have different attributes.
Such regions are probably BIOS bugs, and this is the workaround.

regarding Comment #12, Lin Ming, Please add my Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>

regarding Comment #11,

in the lkml report mentioned by the bug description,

BIOS-e820: 000000007fff9000 - 000000007ffff000 (ACPI data)
BIOS-e820: 000000007ffff000 - 0000000080000000 (ACPI NVS)

And we are trying to map from 0x7fffff00 to 0x800000ff and we are crossing the resource boundary of "0x7ffff000 0x7fffffff ACPI Non-volatile Storage"

some bug some where (not sure where! perhaps bios) that was mentioning that resource map is spanning upto 0x800000ff.

>>some bug some where (not sure where! perhaps bios)

Please post the acpidump for the machine, we would like to look at the operation region definitions.

Bob, Have you read the  bug description?

> http://lkml.org/lkml/2008/12/30/128

this url in bug description seems to have the acpi dump. Nevertheless  Idon't have a system where I see this.

Lin Ming, Forgot to add one more request. Can you please add a line in the changelog of the patch that says

[Kernel summit hacking hour]

We root caused it during kernel summit hacking hour. Arjan and others wanted to track the patches using a tag like this, for evaluating the usefulness of the hacking hour during kernel summit. Thanks.

OK.

The patch will first go into ACPICA tree and then be merged into kernel.

marking RESOLVED, per patch in comment #12

patch in comment #12 applied to acpi tree

shipped in v2.6.32-rc7
closed