[AMD Official Use Only - General]

Hello Boris,

quoted

quoted

You need to elaborate more here: a RMP fault can happen and then the 
page can get unmapped? What is the exact scenario here?

Yes, if the page gets unmapped while the RMP fault was being handled, 
will add more explanation here.

So what's the logic here to return 1, i.e., retry?

Why should a fault for a page that gets unmapped be retried? The fault in that case should be ignored, IMO. It'll have the same effect to return from do_user_addr_fault() there, without splitting but you need to have a separate return value >definition so that it is clear what needs to happen. And that return value should be != 0 so that the current check still works.

if (!pte || !pte_present(*pte))
                return 1;

This is more like a sanity check and returning 1 will cause the fault handler to return and ignore the fault for current #PF case.
If the page got unmapped, the fault will not happen again and there will be no retry, so the fault in this case is
being ignored.
The other case where 1 is returned is RMP table lookup failure, in that case the faulting process is being terminated,
that resolves the fault. 

quoted

Actually, the above computes an index into the RMP table.

What index in the RMP table?

quoted

It is basically an index into the 4K page within the hugepage mapped 
in the RMP table or in other words an index into the RMP table entry 
for 4K page(s) corresponding to a hugepage.

So pte_index(address) and for 1G pages, pmd_index(address).

So no reinventing the wheel if we already have helpers for that.

Yes that makes sense and pte_index(address) is exactly what is
required for 2M hugepages.

Will use pte_index() for 2M pages and pmd_index() for 1G pages. 

quoted

It is mainly a wrapper around__split_huge_pmd() for SNP use case where 
the host hugepage is split to be in sync with the RMP table.

I see what it is. And I'm saying this looks wrong. You're enforcing page splitting to be a valid thing to do only for SEV machines. Why?

Why is

       if (!IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT))
               return VM_FAULT_SIGBUS;

there at all?

This is generic code you're touching - not arch/x86/.

Ok, so you are suggesting that we remove this check and simply keep this function wrapping around __split_huge_pmd(). 
This becomes a generic utility function. 

Thanks,
Ashish