On Sat, Jan 27, 2024 at 09:45:06AM -0600, Michael Roth wrote:
> directmap maps all physical memory accessible by kernel, including text
> pages, so those are valid PFNs as far as this function is concerned.
Why don't you have a look at
Documentation/arch/x86/x86_64/mm.rst
to sync up on the nomenclature first?
ffff888000000000 | -119.5 TB | ffffc87fffffffff | 64 TB | direct mapping of all physical memory (page_offset_base)
...
ffffffff80000000 | -2 GB | ffffffff9fffffff | 512 MB | kernel text mapping, mapped to physical address 0
and so on.
> The expectation is that the caller is aware of what PFNs it is passing in,
There are no expectations. Have you written them down somewhere?
> This function only splits mappings in the 0xffff888000000000 directmap
> range.
This function takes any PFN it gets passed in as it is. I don't care
who its users are now or in the future and whether they pay attention
what they pass into - it needs to be properly defined.
Mike, please get on with the program. Use the right naming for the
function and basta.
IOW, this:
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 0a8f9334ec6e..652ee63e87fd 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -394,7 +394,7 @@ EXPORT_SYMBOL_GPL(psmash);
* More specifics on how these checks are carried out can be found in APM
* Volume 2, "RMP and VMPL Access Checks".
*/
-static int adjust_direct_map(u64 pfn, int rmp_level)
+static int split_pfn(u64 pfn, int rmp_level)
{
unsigned long vaddr = (unsigned long)pfn_to_kaddr(pfn);
unsigned int level;
@@ -405,7 +405,12 @@ static int adjust_direct_map(u64 pfn, int rmp_level)
if (WARN_ON_ONCE(rmp_level > PG_LEVEL_2M))
return -EINVAL;
- if (WARN_ON_ONCE(rmp_level == PG_LEVEL_2M && !IS_ALIGNED(pfn, PTRS_PER_PMD)))
+ if (!pfn_valid(pfn))
+ return -EINVAL;
+
+ if (rmp_level == PG_LEVEL_2M &&
+ (!IS_ALIGNED(pfn, PTRS_PER_PMD) ||
+ !pfn_valid(pfn + PTRS_PER_PMD - 1)))
return -EINVAL;
/*
@@ -456,7 +461,7 @@ static int rmpupdate(u64 pfn, struct rmp_state *state)
level = RMP_TO_PG_LEVEL(state->pagesize);
- if (adjust_direct_map(pfn, level))
+ if (split_pfn(pfn, level))
return -EFAULT;
do {
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
