Skip to content

Commit

Permalink
ACPICA: Preserve memory opregion mappings
Browse files Browse the repository at this point in the history
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 <dan.j.williams@intel.com>
Tested-by: Xiang Li <xiang.z.li@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
  • Loading branch information
Rafael J. Wysocki committed Jul 27, 2020
1 parent 1757659 commit b8fcd0e
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 27 deletions.
14 changes: 8 additions & 6 deletions drivers/acpi/acpica/evrgnini.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ acpi_ev_system_memory_region_setup(acpi_handle handle,
union acpi_operand_object *region_desc =
(union acpi_operand_object *)handle;
struct acpi_mem_space_context *local_region_context;
struct acpi_mem_mapping *mm;

ACPI_FUNCTION_TRACE(ev_system_memory_region_setup);

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

/* Delete a cached mapping if present */
/* Delete memory mappings if present */

if (local_region_context->mapped_length) {
acpi_os_unmap_memory(local_region_context->
mapped_logical_address,
local_region_context->
mapped_length);
while (local_region_context->first_mm) {
mm = local_region_context->first_mm;
local_region_context->first_mm = mm->next_mm;
acpi_os_unmap_memory(mm->logical_address,
mm->length);
ACPI_FREE(mm);
}
ACPI_FREE(local_region_context);
*region_context = NULL;
Expand Down
64 changes: 46 additions & 18 deletions drivers/acpi/acpica/exregion.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ acpi_ex_system_memory_space_handler(u32 function,
acpi_status status = AE_OK;
void *logical_addr_ptr = NULL;
struct acpi_mem_space_context *mem_info = region_context;
struct acpi_mem_mapping *mm = mem_info->cur_mm;
u32 length;
acpi_size map_length;
acpi_size page_boundary_map_length;
Expand Down Expand Up @@ -96,20 +97,37 @@ acpi_ex_system_memory_space_handler(u32 function,
* Is 1) Address below the current mapping? OR
* 2) Address beyond the current mapping?
*/
if ((address < mem_info->mapped_physical_address) ||
(((u64) address + length) > ((u64)
mem_info->mapped_physical_address +
mem_info->mapped_length))) {
if (!mm || (address < mm->physical_address) ||
((u64) address + length > (u64) mm->physical_address + mm->length)) {
/*
* The request cannot be resolved by the current memory mapping;
* Delete the existing mapping and create a new one.
* The request cannot be resolved by the current memory mapping.
*
* Look for an existing saved mapping covering the address range
* at hand. If found, save it as the current one and carry out
* the access.
*/
if (mem_info->mapped_length) {
for (mm = mem_info->first_mm; mm; mm = mm->next_mm) {
if (mm == mem_info->cur_mm)
continue;

if (address < mm->physical_address)
continue;

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

acpi_os_unmap_memory(mem_info->mapped_logical_address,
mem_info->mapped_length);
mem_info->cur_mm = mm;
goto access;
}

/* Create a new mappings list entry */
mm = ACPI_ALLOCATE_ZEROED(sizeof(*mm));
if (!mm) {
ACPI_ERROR((AE_INFO,
"Unable to save memory mapping at 0x%8.8X%8.8X, size %u",
ACPI_FORMAT_UINT64(address), length));
return_ACPI_STATUS(AE_NO_MEMORY);
}

/*
Expand Down Expand Up @@ -143,29 +161,39 @@ acpi_ex_system_memory_space_handler(u32 function,

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

mem_info->mapped_logical_address =
acpi_os_map_memory(address, map_length);
if (!mem_info->mapped_logical_address) {
logical_addr_ptr = acpi_os_map_memory(address, map_length);
if (!logical_addr_ptr) {
ACPI_ERROR((AE_INFO,
"Could not map memory at 0x%8.8X%8.8X, size %u",
ACPI_FORMAT_UINT64(address),
(u32)map_length));
mem_info->mapped_length = 0;
ACPI_FREE(mm);
return_ACPI_STATUS(AE_NO_MEMORY);
}

/* Save the physical address and mapping size */

mem_info->mapped_physical_address = address;
mem_info->mapped_length = map_length;
mm->logical_address = logical_addr_ptr;
mm->physical_address = address;
mm->length = map_length;

/*
* Add the new entry to the mappigs list and save it as the
* current mapping.
*/
mm->next_mm = mem_info->first_mm;
mem_info->first_mm = mm;

mem_info->cur_mm = mm;
}

access:
/*
* Generate a logical pointer corresponding to the address we want to
* access
*/
logical_addr_ptr = mem_info->mapped_logical_address +
((u64) address - (u64) mem_info->mapped_physical_address);
logical_addr_ptr = mm->logical_address +
((u64) address - (u64) mm->physical_address);

ACPI_DEBUG_PRINT((ACPI_DB_INFO,
"System-Memory (width %u) R/W %u Address=%8.8X%8.8X\n",
Expand Down
12 changes: 9 additions & 3 deletions include/acpi/actypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1201,12 +1201,18 @@ struct acpi_pci_id {
u16 function;
};

struct acpi_mem_mapping {
acpi_physical_address physical_address;
u8 *logical_address;
acpi_size length;
struct acpi_mem_mapping *next_mm;
};

struct acpi_mem_space_context {
u32 length;
acpi_physical_address address;
acpi_physical_address mapped_physical_address;
u8 *mapped_logical_address;
acpi_size mapped_length;
struct acpi_mem_mapping *cur_mm;
struct acpi_mem_mapping *first_mm;
};

/*
Expand Down

0 comments on commit b8fcd0e

Please sign in to comment.