| Message ID | 20200522125214.31348-14-kirill.shutemov@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM protected memory extension | expand |
"Kirill A. Shutemov" <kirill@shutemov.name> writes: > hvclock is shared between the guest and the hypervisor. It has to be > accessible by host. > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > --- > arch/x86/kernel/kvmclock.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c > index 34b18f6eeb2c..ac6c2abe0d0f 100644 > --- a/arch/x86/kernel/kvmclock.c > +++ b/arch/x86/kernel/kvmclock.c > @@ -253,7 +253,7 @@ static void __init kvmclock_init_mem(void) > * hvclock is shared between the guest and the hypervisor, must > * be mapped decrypted. > */ > - if (sev_active()) { > + if (sev_active() || kvm_mem_protected()) { > r = set_memory_decrypted((unsigned long) hvclock_mem, > 1UL << order); > if (r) { Sorry if I missed something but we have other structures which KVM guest share with the host, sev_map_percpu_data(): ... for_each_possible_cpu(cpu) { __set_percpu_decrypted(&per_cpu(apf_reason, cpu), sizeof(apf_reason)); __set_percpu_decrypted(&per_cpu(steal_time, cpu), sizeof(steal_time)); __set_percpu_decrypted(&per_cpu(kvm_apic_eoi, cpu), sizeof(kvm_apic_eoi)); } ... Do you handle them somehow in the patchset? (I'm probably just blind failing to see how 'early_set_memory_decrypted()' is wired up)
On Mon, May 25, 2020 at 05:22:10PM +0200, Vitaly Kuznetsov wrote: > "Kirill A. Shutemov" <kirill@shutemov.name> writes: > > > hvclock is shared between the guest and the hypervisor. It has to be > > accessible by host. > > > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > --- > > arch/x86/kernel/kvmclock.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c > > index 34b18f6eeb2c..ac6c2abe0d0f 100644 > > --- a/arch/x86/kernel/kvmclock.c > > +++ b/arch/x86/kernel/kvmclock.c > > @@ -253,7 +253,7 @@ static void __init kvmclock_init_mem(void) > > * hvclock is shared between the guest and the hypervisor, must > > * be mapped decrypted. > > */ > > - if (sev_active()) { > > + if (sev_active() || kvm_mem_protected()) { > > r = set_memory_decrypted((unsigned long) hvclock_mem, > > 1UL << order); > > if (r) { > > Sorry if I missed something but we have other structures which KVM guest > share with the host, > > sev_map_percpu_data(): > ... > for_each_possible_cpu(cpu) { > __set_percpu_decrypted(&per_cpu(apf_reason, cpu), sizeof(apf_reason)); > __set_percpu_decrypted(&per_cpu(steal_time, cpu), sizeof(steal_time)); > __set_percpu_decrypted(&per_cpu(kvm_apic_eoi, cpu), sizeof(kvm_apic_eoi)); > } > ... > > Do you handle them somehow in the patchset? (I'm probably just blind > failing to see how 'early_set_memory_decrypted()' is wired up) I don't handle them yet: I've seen the function, but have not modified it. I want to understand first why it doesn't blow up for me without the change. Any clues?
"Kirill A. Shutemov" <kirill@shutemov.name> writes: > On Mon, May 25, 2020 at 05:22:10PM +0200, Vitaly Kuznetsov wrote: >> "Kirill A. Shutemov" <kirill@shutemov.name> writes: >> >> > hvclock is shared between the guest and the hypervisor. It has to be >> > accessible by host. >> > >> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> >> > --- >> > arch/x86/kernel/kvmclock.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c >> > index 34b18f6eeb2c..ac6c2abe0d0f 100644 >> > --- a/arch/x86/kernel/kvmclock.c >> > +++ b/arch/x86/kernel/kvmclock.c >> > @@ -253,7 +253,7 @@ static void __init kvmclock_init_mem(void) >> > * hvclock is shared between the guest and the hypervisor, must >> > * be mapped decrypted. >> > */ >> > - if (sev_active()) { >> > + if (sev_active() || kvm_mem_protected()) { >> > r = set_memory_decrypted((unsigned long) hvclock_mem, >> > 1UL << order); >> > if (r) { >> >> Sorry if I missed something but we have other structures which KVM guest >> share with the host, >> >> sev_map_percpu_data(): >> ... >> for_each_possible_cpu(cpu) { >> __set_percpu_decrypted(&per_cpu(apf_reason, cpu), sizeof(apf_reason)); >> __set_percpu_decrypted(&per_cpu(steal_time, cpu), sizeof(steal_time)); >> __set_percpu_decrypted(&per_cpu(kvm_apic_eoi, cpu), sizeof(kvm_apic_eoi)); >> } >> ... >> >> Do you handle them somehow in the patchset? (I'm probably just blind >> failing to see how 'early_set_memory_decrypted()' is wired up) > > I don't handle them yet: I've seen the function, but have not modified it. > I want to understand first why it doesn't blow up for me without the > change. Any clues? (if I got the idea of the patchset right) these features are kernel-only (e.g. QEMU doesn't need to access these areas). E.g. for APF KVM will do kvm_write_guest_cached() and this will use FOLL_KVM. Guests should not rely on that and mark all shared areas as unprotected.
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c index 34b18f6eeb2c..ac6c2abe0d0f 100644 --- a/arch/x86/kernel/kvmclock.c +++ b/arch/x86/kernel/kvmclock.c @@ -253,7 +253,7 @@ static void __init kvmclock_init_mem(void) * hvclock is shared between the guest and the hypervisor, must * be mapped decrypted. */ - if (sev_active()) { + if (sev_active() || kvm_mem_protected()) { r = set_memory_decrypted((unsigned long) hvclock_mem, 1UL << order); if (r) {
hvclock is shared between the guest and the hypervisor. It has to be accessible by host. Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> --- arch/x86/kernel/kvmclock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)