Messages in this thread | | | Subject | Re: [PATCH Part2 RFC v4 07/40] x86/sev: Split the physmap when adding the page in RMP table | From | Brijesh Singh <> | Date | Fri, 30 Jul 2021 11:10:01 -0500 |
| |
On 7/30/21 6:31 AM, Vlastimil Babka wrote: > On 7/15/21 9:38 PM, Brijesh Singh wrote: >> >> On 7/15/21 1:39 PM, Sean Christopherson wrote: >>> On Thu, Jul 15, 2021, Brijesh Singh wrote: >>>> 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
> >> 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
| |