On 3/17/2017 9:32 AM, Tom Lendacky wrote:
On 3/16/2017 3:04 PM, Tom Lendacky wrote:
On 3/7/2017 8:59 AM, Borislav Petkov wrote:
On Thu, Mar 02, 2017 at 10:13:32AM -0500, Brijesh Singh wrote:
From: Tom Lendacky <thomas.lendacky@xxxxxxx>
In order for memory pages to be properly mapped when SEV is active, we
need to use the PAGE_KERNEL protection attribute as the base
protection.
This will insure that memory mapping of, e.g. ACPI tables, receives the
proper mapping attributes.
Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
---
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index c400ab5..481c999 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -151,7 +151,15 @@ static void __iomem
*__ioremap_caller(resource_size_t phys_addr,
pcm = new_pcm;
}
+ /*
+ * If the page being mapped is in memory and SEV is active then
+ * make sure the memory encryption attribute is enabled in the
+ * resulting mapping.
+ */
prot = PAGE_KERNEL_IO;
+ if (sev_active() && page_is_mem(pfn))
Hmm, a resource tree walk per ioremap call. This could get expensive for
ioremap-heavy workloads.
__ioremap_caller() gets called here during boot 55 times so not a whole
lot but I wouldn't be surprised if there were some nasty use cases which
ioremap a lot.
...
diff --git a/kernel/resource.c b/kernel/resource.c
index 9b5f044..db56ba3 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -518,6 +518,46 @@ int __weak page_is_ram(unsigned long pfn)
}
EXPORT_SYMBOL_GPL(page_is_ram);
+/*
+ * This function returns true if the target memory is marked as
+ * IORESOURCE_MEM and IORESOUCE_BUSY and described as other than
+ * IORES_DESC_NONE (e.g. IORES_DESC_ACPI_TABLES).
+ */
+static int walk_mem_range(unsigned long start_pfn, unsigned long
nr_pages)
+{
+ struct resource res;
+ unsigned long pfn, end_pfn;
+ u64 orig_end;
+ int ret = -1;
+
+ res.start = (u64) start_pfn << PAGE_SHIFT;
+ res.end = ((u64)(start_pfn + nr_pages) << PAGE_SHIFT) - 1;
+ res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+ orig_end = res.end;
+ while ((res.start < res.end) &&
+ (find_next_iomem_res(&res, IORES_DESC_NONE, true) >= 0)) {
+ pfn = (res.start + PAGE_SIZE - 1) >> PAGE_SHIFT;
+ end_pfn = (res.end + 1) >> PAGE_SHIFT;
+ if (end_pfn > pfn)
+ ret = (res.desc != IORES_DESC_NONE) ? 1 : 0;
+ if (ret)
+ break;
+ res.start = res.end + 1;
+ res.end = orig_end;
+ }
+ return ret;
+}
So the relevant difference between this one and walk_system_ram_range()
is this:
- ret = (*func)(pfn, end_pfn - pfn, arg);
+ ret = (res.desc != IORES_DESC_NONE) ? 1 : 0;
so it seems to me you can have your own *func() pointer which does that
IORES_DESC_NONE comparison. And then you can define your own workhorse
__walk_memory_range() which gets called by both walk_mem_range() and
walk_system_ram_range() instead of almost duplicating them.
And looking at walk_system_ram_res(), that one looks similar too except
the pfn computation. But AFAICT the pfn/end_pfn things are computed from
res.start and res.end so it looks to me like all those three functions
are crying for unification...
I'll take a look at what it takes to consolidate these with a pre-patch.
Then I'll add the new support.
It looks pretty straight forward to combine walk_iomem_res_desc() and
walk_system_ram_res(). The walk_system_ram_range() function would fit
easily into this, also, except for the fact that the callback function
takes unsigned longs vs the u64s of the other functions. Is it worth
modifying all of the callers of walk_system_ram_range() (which are only
about 8 locations) to change the callback functions to accept u64s in
order to consolidate the walk_system_ram_range() function, too?
The more I dig, the more I find that the changes keep expanding. I'll
leave walk_system_ram_range() out of the consolidation for now.
Thanks,
Tom
Thanks,
Tom
Thanks,
Tom
