Re: [PATCH 10/12] mm: x86: Invoke hypercall when page encryption status is changed

From: Brijesh Singh
Date: Thu Feb 20 2020 - 17:43:30 EST

On 2/20/20 2:43 PM, Steve Rutherford wrote:
On Thu, Feb 20, 2020 at 7:55 AM Brijesh Singh <brijesh.singh@xxxxxxx> wrote:

On 2/19/20 8:12 PM, Andy Lutomirski wrote:

On Feb 19, 2020, at 5:58 PM, Steve Rutherford <srutherford@xxxxxxxxxx> wrote:

ïOn Wed, Feb 12, 2020 at 5:18 PM Ashish Kalra <Ashish.Kalra@xxxxxxx> wrote:

From: Brijesh Singh <brijesh.singh@xxxxxxx>

Invoke a hypercall when a memory region is changed from encrypted ->
decrypted and vice versa. Hypervisor need to know the page encryption
status during the guest migration.

One messy aspect, which I think is fine in practice, is that this
presumes that pages are either treated as encrypted or decrypted. If
also done on SEV, the in-place re-encryption supported by SME would
break SEV migration. Linux doesn't do this now on SEV, and I don't
have an intuition for why Linux might want this, but we will need to
ensure it is never done in order to ensure that migration works down
the line. I don't believe the AMD manual promises this will work

Something feels a bit wasteful about having all future kernels
universally announce c-bit status when SEV is enabled, even if KVM
isn't listening, since it may be too old (or just not want to know).
Might be worth eliding the hypercalls if you get ENOSYS back? There
might be a better way of passing paravirt config metadata across than
just trying and seeing if the hypercall succeeds, but I'm not super
familiar with it.

I actually think this should be a hard requirement to merge this. The host needs to tell the guest that it supports this particular migration strategy and the guest needs to tell the host that it is using it. And the guest needs a way to tell the host that itâs *not* using it right now due to kexec, for example.

Iâm still uneasy about a guest being migrated in the window where the hypercall tracking and the page encryption bit donât match. I guess maybe corruption in this window doesnât matter?

I don't think there is a corruption issue here. Let's consider the below

1) A page is transmitted as C=1 (encrypted)

2) During the migration window, the page encryption bit is changed
to C=0 (decrypted)

3) #2 will cause a change in page table memory, thus dirty memory
the tracker will create retransmission of the page table memory.

4) The page itself will not be re-transmitted because there was
no change to the content of the page.

On destination, the read from the page will get the ciphertext.

The encryption bit change in the page table is used on the next access.
The user of the page needs to ensure that data is written with the
correct encryption bit before reading.


I think the issue results from a slightly different perspective than
the one you are using. I think the situation Andy is interested in is
when a c-bit change and a write happen close in time. There are five
events, and the ordering matters:
1) Guest dirties the c-bit in the guest
2) Guest dirties the page
3) Host userspace observes the c-bit logs
4) Host userspace observes the page dirty logs
5) Host transmits the page

If these are reordered to:
3) Host userspace observes the c-bit logs
1) Guest dirties the c-bit in the guest
2) Guest dirties the page
4) Host userspace observes the page dirty logs
5) Host transmits the page (from the wrong c-bit perspective!)

Then the host will transmit a page with the wrong c-bit status and
clear the dirty bit for that page. If the guest page is not
retransmitted incidentally later, then this page will be corrupted.

If you treat pages with dirty c-bits as dirty pages, then you will
check the c-bit logs later and observe the dirty c-bit and retransmit.
There might be some cleverness around enforcing that you always fetch
the c-bit logs after fetching the dirty logs, but I haven't convinced
myself that this works yet. I think it might, since then the c-bits
are at least as fresh as the dirty bits.

Unlike the dirty log, the c-bit log maintains the complete state.
So, I think it is the Host userspace responsibility to ensure that it
either keeps track of any c-bit log changes since it last sync'ed.
During the migration, after pausing the guest it can get the recent
c-bit log and compare if something has changed since it last sync'ed.
If so, then retransmit the page with new c-bit state.

The main uncertainty that comes to mind for that strategy is if, on
multi-vCPU VMs, the page dirtying event (from the new c-bit
perspective) and the c-bit status change hypercall can themselves
race. If a write from the new c-bit perspective can arrive before the
c-bit status change arrives in the c-bit logs, we will need to treat
pages with dirty c-bits as dirty pages.

I believe if host userspace tracks the changes in the c-bit log since
it last synced then this problem can be avoided. Do you think we should
consider tracking the last sync changes in KVM or let the host userspace
handle it.

Note that I do agree that if the c-bit status flips, and no one writes
to the page, it doesn't really matter if you retransmit that page. If
a guest wants to read nonsense, it can.