On 7/30/21 6:31 AM, Vlastimil Babka wrote:

On 7/15/21 9:38 PM, Brijesh Singh wrote:

quoted

On 7/15/21 1:39 PM, Sean Christopherson wrote:

quoted

On Thu, Jul 15, 2021, Brijesh Singh wrote:

quoted

The memfd_secrets uses the set_direct_map_{invalid,default}_noflush() and it
is designed to remove/add the present bit in the direct map. We can't use
them, because in our case the page may get accessed by the KVM (e.g
kvm_guest_write, kvm_guest_map etc).

But KVM should never access a guest private page, i.e. the direct map should
always be restored to PRESENT before KVM attempts to access the page.

Yes, KVM should *never* access the guest private pages. So, we could potentially
enhance the RMPUPDATE() to check for the assigned and act accordingly.

I think I'm not the first one suggesting it, but IMHO the best solution would be
to leave direct map alone (except maybe in some debugging/development mode where
you could do the unmapping to catch unexpected host accesses), and once there's
a situation with host accessing a shared page of the guest, it would temporarily
kmap() it outside of the direct map. Shouldn't these situations be rare enough,
and already recognizable due to the need to set up the page sharing in the first
place, that this approach would be feasible?

The host accessing a guest shared is not rare, some shared pages are
accessed on every VM-Entry and Exit. However, there are limited number
of such pages and we could track and temporary kmap() outside of the
direct map. The main challenge is when a host pages are added as
'private' in the RMP table.

e.g

page = alloc_page(GFP_KERNEL_ACCOUNT);

// firmware context page

rmp_make_firmware(page_to_pfn(page), PG_LEVEL_4K);

or

// VMSA page

rmp_make_private(page_to_pfn(page), 0, sev->asid, PG_LEVEL_4K);

The page is added as 4K in the RMP entry; If the pfn is part of the
large direct map then hardware will raise an RMP violation and to
resolve the fault we must split the direct map.

thanks

quoted

Are you thinking something along the line of this:

int rmpupdate(struct page *page, struct rmpupdate *val)
{
    ...
    
    /*
     * If page is getting assigned in the RMP entry then unmap
     * it from the direct map before its added in the RMP table.
     */
    if (val.assigned)
        set_direct_map_invalid_noflush(page_to_virt(page), 1);

    ...

    /*
     * If the page is getting unassigned then restore the mapping
     * in the direct map after its removed from the RMP table.
     */
    if (!val.assigned)
        set_direct_map_default_noflush(page_to_virt(page), 1);
    
    ...
}

thanks