[AMD Official Use Only - General] Hello Boris, >> +static struct rmpentry *__snp_lookup_rmpentry(u64 pfn, int *level) { >> + unsigned long vaddr, paddr = pfn << PAGE_SHIFT; >> + struct rmpentry *entry, *large_entry; >.> + >> + if (!pfn_valid(pfn)) >> + return ERR_PTR(-EINVAL); >> + >> + if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP)) >> + return ERR_PTR(-ENXIO); >That test should happen first. Ok. >> + vaddr = rmptable_start + rmptable_page_offset(paddr); >Wait, what does that macro do? >It takes the physical address and gives the offset from the beginning of the RMP table in VA space? >So why don't you do > entry = rmptable_entry(paddr) >instead which simply gives you directly the entry in the RMP table with which you can work further? >Instead of this macro doing half the work and then callers having to add the RMP start address and cast. >And make it small function so that you can have typechecking too, while at it. Ok, I will add a new corresponding rmptable_entry() function here, should be usable for getting the large RMP entry below too. >> + if (unlikely(vaddr > rmptable_end)) >> + return ERR_PTR(-ENXIO); >> + >> + entry = (struct rmpentry *)vaddr; >> + >> + /* Read a large RMP entry to get the correct page level used in RMP entry. */ >> + vaddr = rmptable_start + rmptable_page_offset(paddr & PMD_MASK); >> + large_entry = (struct rmpentry *)vaddr; >> + *level = RMP_TO_X86_PG_LEVEL(rmpentry_pagesize(large_entry)); >> + >> + return entry; >> +} >> + >> +/* >> diff --git a/include/linux/sev.h b/include/linux/sev.h new file mode >> 100644 index 000000000000..1a68842789e1 >> --- /dev/null >> +++ b/include/linux/sev.h >Why is this header in the linux/ namespace and not in arch/x86/ ? >All that stuff in here doesn't have any meaning outside of x86... Yes, makes sense to move it to arch/x86. Thanks, Ashish