Skip to content

Commit b8fcd0e

Browse files
committed
ACPICA: Preserve memory opregion mappings
The ACPICA's strategy with respect to the handling of memory mappings associated with memory operation regions is to avoid mapping the entire region at once which may be problematic at least in principle (for example, it may lead to conflicts with overlapping mappings having different attributes created by drivers). It may also be wasteful, because memory opregions on some systems take up vast chunks of address space while the fields in those regions actually accessed by AML are sparsely distributed. For this reason, a one-page "window" is mapped for a given opregion on the first memory access through it and if that "window" does not cover an address range accessed through that opregion subsequently, it is unmapped and a new "window" is mapped to replace it. Next, if the new "window" is not sufficient to acess memory through the opregion in question in the future, it will be replaced with yet another "window" and so on. That may lead to a suboptimal sequence of memory mapping and unmapping operations, for example if two fields in one opregion separated from each other by a sufficiently wide chunk of unused address space are accessed in an alternating pattern. The situation may still be suboptimal if the deferred unmapping introduced previously is supported by the OS layer. For instance, the alternating memory access pattern mentioned above may produce a relatively long list of mappings to release with substantial duplication among the entries in it, which could be avoided if acpi_ex_system_memory_space_handler() did not release the mapping used by it previously as soon as the current access was not covered by it. In order to improve that, modify acpi_ex_system_memory_space_handler() to preserve all of the memory mappings created by it until the memory regions associated with them go away. Accordingly, update acpi_ev_system_memory_region_setup() to unmap all memory associated with memory opregions that go away. Reported-by: Dan Williams <[email protected]> Tested-by: Xiang Li <[email protected]> Signed-off-by: Rafael J. Wysocki <[email protected]>
1 parent 1757659 commit b8fcd0e

File tree

3 files changed

+63
-27
lines changed

3 files changed

+63
-27
lines changed

drivers/acpi/acpica/evrgnini.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ acpi_ev_system_memory_region_setup(acpi_handle handle,
3838
union acpi_operand_object *region_desc =
3939
(union acpi_operand_object *)handle;
4040
struct acpi_mem_space_context *local_region_context;
41+
struct acpi_mem_mapping *mm;
4142

4243
ACPI_FUNCTION_TRACE(ev_system_memory_region_setup);
4344

@@ -46,13 +47,14 @@ acpi_ev_system_memory_region_setup(acpi_handle handle,
4647
local_region_context =
4748
(struct acpi_mem_space_context *)*region_context;
4849

49-
/* Delete a cached mapping if present */
50+
/* Delete memory mappings if present */
5051

51-
if (local_region_context->mapped_length) {
52-
acpi_os_unmap_memory(local_region_context->
53-
mapped_logical_address,
54-
local_region_context->
55-
mapped_length);
52+
while (local_region_context->first_mm) {
53+
mm = local_region_context->first_mm;
54+
local_region_context->first_mm = mm->next_mm;
55+
acpi_os_unmap_memory(mm->logical_address,
56+
mm->length);
57+
ACPI_FREE(mm);
5658
}
5759
ACPI_FREE(local_region_context);
5860
*region_context = NULL;

drivers/acpi/acpica/exregion.c

Lines changed: 46 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ acpi_ex_system_memory_space_handler(u32 function,
4141
acpi_status status = AE_OK;
4242
void *logical_addr_ptr = NULL;
4343
struct acpi_mem_space_context *mem_info = region_context;
44+
struct acpi_mem_mapping *mm = mem_info->cur_mm;
4445
u32 length;
4546
acpi_size map_length;
4647
acpi_size page_boundary_map_length;
@@ -96,20 +97,37 @@ acpi_ex_system_memory_space_handler(u32 function,
9697
* Is 1) Address below the current mapping? OR
9798
* 2) Address beyond the current mapping?
9899
*/
99-
if ((address < mem_info->mapped_physical_address) ||
100-
(((u64) address + length) > ((u64)
101-
mem_info->mapped_physical_address +
102-
mem_info->mapped_length))) {
100+
if (!mm || (address < mm->physical_address) ||
101+
((u64) address + length > (u64) mm->physical_address + mm->length)) {
103102
/*
104-
* The request cannot be resolved by the current memory mapping;
105-
* Delete the existing mapping and create a new one.
103+
* The request cannot be resolved by the current memory mapping.
104+
*
105+
* Look for an existing saved mapping covering the address range
106+
* at hand. If found, save it as the current one and carry out
107+
* the access.
106108
*/
107-
if (mem_info->mapped_length) {
109+
for (mm = mem_info->first_mm; mm; mm = mm->next_mm) {
110+
if (mm == mem_info->cur_mm)
111+
continue;
112+
113+
if (address < mm->physical_address)
114+
continue;
108115

109-
/* Valid mapping, delete it */
116+
if ((u64) address + length >
117+
(u64) mm->physical_address + mm->length)
118+
continue;
110119

111-
acpi_os_unmap_memory(mem_info->mapped_logical_address,
112-
mem_info->mapped_length);
120+
mem_info->cur_mm = mm;
121+
goto access;
122+
}
123+
124+
/* Create a new mappings list entry */
125+
mm = ACPI_ALLOCATE_ZEROED(sizeof(*mm));
126+
if (!mm) {
127+
ACPI_ERROR((AE_INFO,
128+
"Unable to save memory mapping at 0x%8.8X%8.8X, size %u",
129+
ACPI_FORMAT_UINT64(address), length));
130+
return_ACPI_STATUS(AE_NO_MEMORY);
113131
}
114132

115133
/*
@@ -143,29 +161,39 @@ acpi_ex_system_memory_space_handler(u32 function,
143161

144162
/* Create a new mapping starting at the address given */
145163

146-
mem_info->mapped_logical_address =
147-
acpi_os_map_memory(address, map_length);
148-
if (!mem_info->mapped_logical_address) {
164+
logical_addr_ptr = acpi_os_map_memory(address, map_length);
165+
if (!logical_addr_ptr) {
149166
ACPI_ERROR((AE_INFO,
150167
"Could not map memory at 0x%8.8X%8.8X, size %u",
151168
ACPI_FORMAT_UINT64(address),
152169
(u32)map_length));
153-
mem_info->mapped_length = 0;
170+
ACPI_FREE(mm);
154171
return_ACPI_STATUS(AE_NO_MEMORY);
155172
}
156173

157174
/* Save the physical address and mapping size */
158175

159-
mem_info->mapped_physical_address = address;
160-
mem_info->mapped_length = map_length;
176+
mm->logical_address = logical_addr_ptr;
177+
mm->physical_address = address;
178+
mm->length = map_length;
179+
180+
/*
181+
* Add the new entry to the mappigs list and save it as the
182+
* current mapping.
183+
*/
184+
mm->next_mm = mem_info->first_mm;
185+
mem_info->first_mm = mm;
186+
187+
mem_info->cur_mm = mm;
161188
}
162189

190+
access:
163191
/*
164192
* Generate a logical pointer corresponding to the address we want to
165193
* access
166194
*/
167-
logical_addr_ptr = mem_info->mapped_logical_address +
168-
((u64) address - (u64) mem_info->mapped_physical_address);
195+
logical_addr_ptr = mm->logical_address +
196+
((u64) address - (u64) mm->physical_address);
169197

170198
ACPI_DEBUG_PRINT((ACPI_DB_INFO,
171199
"System-Memory (width %u) R/W %u Address=%8.8X%8.8X\n",

include/acpi/actypes.h

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,12 +1201,18 @@ struct acpi_pci_id {
12011201
u16 function;
12021202
};
12031203

1204+
struct acpi_mem_mapping {
1205+
acpi_physical_address physical_address;
1206+
u8 *logical_address;
1207+
acpi_size length;
1208+
struct acpi_mem_mapping *next_mm;
1209+
};
1210+
12041211
struct acpi_mem_space_context {
12051212
u32 length;
12061213
acpi_physical_address address;
1207-
acpi_physical_address mapped_physical_address;
1208-
u8 *mapped_logical_address;
1209-
acpi_size mapped_length;
1214+
struct acpi_mem_mapping *cur_mm;
1215+
struct acpi_mem_mapping *first_mm;
12101216
};
12111217

12121218
/*

0 commit comments

Comments
 (0)