From: Tom Lendacky <thomas.lendacky@amd.com>
To: Sean Christopherson <seanjc@google.com>, Peter Gonda <pgonda@google.com>
Cc: kvm@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Joerg Roedel <joro@8bytes.org>,
Brijesh Singh <brijesh.singh@amd.com>,
x86@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2] Fix unsynchronized access to sev members through svm_register_enc_region
Date: Wed, 27 Jan 2021 16:51:56 -0600 [thread overview]
Message-ID: <fecafe83-5f45-a0ab-c208-9ea4720fcb6a@amd.com> (raw)
In-Reply-To: <YBHhF8ktuMfivQEP@google.com>
On 1/27/21 3:54 PM, Sean Christopherson wrote:
> On Wed, Jan 27, 2021, Peter Gonda wrote:
>> Grab kvm->lock before pinning memory when registering an encrypted
>> region; sev_pin_memory() relies on kvm->lock being held to ensure
>> correctness when checking and updating the number of pinned pages.
>>
...
>> +
>> + list_add_tail(®ion->list, &sev->regions_list);
>> + mutex_unlock(&kvm->lock);
>> +
>> /*
>> * The guest may change the memory encryption attribute from C=0 -> C=1
>> * or vice versa for this memory range. Lets make sure caches are
>> @@ -1133,13 +1143,6 @@ int svm_register_enc_region(struct kvm *kvm,
>> */
>> sev_clflush_pages(region->pages, region->npages);
>
> I don't think it actually matters, but it feels like the flush should be done
> before adding the region to the list. That would also make this sequence
> consistent with the other flows.
>
> Tom, any thoughts?
I don't think it matters, either. This does keep the flushing outside of
the mutex, so if you are doing parallel operations, that should help speed
things up a bit.
Thanks,
Tom
>
next prev parent reply other threads:[~2021-01-27 22:54 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-27 16:15 [PATCH V2] Fix unsynchronized access to sev members through svm_register_enc_region Peter Gonda
2021-01-27 21:54 ` Sean Christopherson
2021-01-27 22:51 ` Tom Lendacky [this message]
2021-01-28 10:15 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fecafe83-5f45-a0ab-c208-9ea4720fcb6a@amd.com \
--to=thomas.lendacky@amd.com \
--cc=brijesh.singh@amd.com \
--cc=hpa@zytor.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=pgonda@google.com \
--cc=seanjc@google.com \
--cc=stable@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).