lkml.org 
[lkml]   [2021]   [Dec]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 08/13] KVM: selftests: add SEV boot tests
On Tue, Dec 21, 2021 at 09:40:36AM -0600, Michael Roth wrote:
> On Mon, Dec 20, 2021 at 01:49:15AM +0000, Mingwei Zhang wrote:
> > On Thu, Dec 16, 2021, Michael Roth wrote:
> > > +}
> > > +
> > > +static void
> > > +test_common(struct kvm_vm *vm, struct ucall *uc,
> > > + uint8_t *shared_buf, uint8_t *private_buf)
> > > +{
> > > + bool success;
> > > +
> > > + /* Initial guest check-in. */
> > > + vcpu_run(vm, VCPU_ID);
> > > + CHECK_SHARED_SYNC(vm, VCPU_ID, uc, 100);
> > > +
> > > + /* Ensure initial private pages are intact/encrypted. */
> > > + success = check_buf(private_buf, PRIVATE_PAGES, PAGE_STRIDE, 0x42);
> > > + TEST_ASSERT(!success, "Initial guest memory not encrypted!");
> > > +
> > > + vcpu_run(vm, VCPU_ID);
> > > + CHECK_SHARED_SYNC(vm, VCPU_ID, uc, 101);
> > > +
> > > + /* Ensure host userspace can't read newly-written encrypted data. */
> > > + success = check_buf(private_buf, PRIVATE_PAGES, PAGE_STRIDE, 0x43);
> >
> > I am not sure if it is safe here. Since the cache coherency is not there
> > for neither SEV or SEV-ES. Reading confidential memory from host side
> > will generate cache lines that is not coherent with the guest. So might
> > be better to add clfush here?
>
> On the guest side, the cachelines are tagged based on ASID, so in this case
> the guest would populate it's own cachelines when it writes new data to
> private buf.
>
> On a host without SME coherency bit, there is a possibility that whatever
> data the host had previously written to private_buf with C=0/ASID=0, prior
> to the guest writing to it, might still be present in the cache, but for
> this test that's okay since the guest has purposely written new data to
> confirm that the host does not see the new data. What data the host
> *actually* sees, stale cache data vs. new reads of guest private memory
> with C=0 (e.g. ciphertext) are both okay as far as the test is concerned.
> clflush() would probably make sense here, but if failure to do so
> somehow results in the above assumptions not holding, and the test ends
> up seeing the newly-written data, we definitely want this test to fail
> loudly, so leaving out the clflush() to cover that corner case seems
> like a good idea.

Actually it might be good to check both of those cases, e.g.:

//check private buf (possibly with stale cache for sme_coherency=0)
clflush()
//check private buf again (with fresh read of guest memory)

I'll take a look at that.

-Mike

\
 
 \ /
  Last update: 2021-12-21 18:27    [W:2.641 / U:0.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site