acpi_ex_system_memory_space_handler() is implemented
to round up mapping requests to ACPI_SYSMEM_REGION_WINDOW_SIZE.
However, sometimes this overlaps with other mappings.
At least in one case, this is because doing mappings of this
size crosses a page boundary.
In Linux 2.6.28 a check was added to see if overlapping
mapping requests with different properties are made,
and various AML requests have been triggering Linux
warnings ever since:
The fix for this problem is likely to round up the mapping
request to a page boundary, but to not cross a page boundary.
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
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.)
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 <email@example.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?
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.
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