linux-kernel - Re: [PATCH 01/11] Add shared memory hypercall to PV Linux guest.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 2 Nov 2009 18:18:09 +0200
From:	Gleb Natapov <gleb@...hat.com>
To:	Avi Kivity <avi@...hat.com>
Cc:	kvm@...r.kernel.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/11] Add shared memory hypercall to PV Linux guest.

On Mon, Nov 02, 2009 at 02:18:54PM +0200, Avi Kivity wrote:
> On 11/01/2009 01:56 PM, Gleb Natapov wrote:
> >Add hypercall that allows guest and host to setup per cpu shared
> >memory.
> >
> 
> Better to set this up as an MSR (with bit zero enabling, bits 1-5
> features, and 64-byte alignment).  This allows auto-reset on INIT
> and live migration using the existing MSR save/restore
> infrastructure.
> 
Hmm. Will do.

> >  arch/x86/include/asm/kvm_host.h |    3 +
> >  arch/x86/include/asm/kvm_para.h |   11 +++++
> >  arch/x86/kernel/kvm.c           |   82 +++++++++++++++++++++++++++++++++++++++
> >  arch/x86/kernel/setup.c         |    1 +
> >  arch/x86/kernel/smpboot.c       |    3 +
> >  arch/x86/kvm/x86.c              |   70 +++++++++++++++++++++++++++++++++
> >  include/linux/kvm.h             |    1 +
> >  include/linux/kvm_para.h        |    4 ++
> >  8 files changed, 175 insertions(+), 0 deletions(-)
> 
> Please separate into guest and host patches.
> 
OK.

> >+#define KVM_PV_SHM_VERSION 1
> 
> versions = bad, feature bits = good
> 
I have both! Do you want me to drop version?

> >+
> >+#define KVM_PV_SHM_FEATURES_ASYNC_PF		(1<<  0)
> >+
> >+struct kvm_vcpu_pv_shm {
> >+	__u64 features;
> >+	__u64 reason;
> >+	__u64 param;
> >+};
> >+
> 
> Some documentation for this?
> 
> Also, the name should reflect the pv pagefault use.  For other uses
> we can register other areas.
> 
I wanted it to be generic, but I am fine with making it apf specific.
It will allow to make it smaller too.

> >  #define MMU_QUEUE_SIZE 1024
> >
> >@@ -37,6 +41,7 @@ struct kvm_para_state {
> >  };
> >
> >  static DEFINE_PER_CPU(struct kvm_para_state, para_state);
> >+static DEFINE_PER_CPU(struct kvm_vcpu_pv_shm *, kvm_vcpu_pv_shm);
> 
> Easier to put the entire structure here, not a pointer.
OK.

> 
> >+
> >+static int kvm_pv_reboot_notify(struct notifier_block *nb,
> >+				unsigned long code, void *unused)
> >+{
> >+	if (code == SYS_RESTART)
> >+		on_each_cpu(kvm_pv_unregister_shm, NULL, 1);
> >+	return NOTIFY_DONE;
> >+}
> >+
> >+static struct notifier_block kvm_pv_reboot_nb = {
> >+        .notifier_call = kvm_pv_reboot_notify,
> >+};
> 
> Is this called on kexec, or do we need another hook?
> 
This was added specifically for kexec to work. It was called in my test,

> >+static int kvm_pv_setup_shm(struct kvm_vcpu *vcpu, unsigned long gpa,
> >+			    unsigned long size, unsigned long version,
> >+			    unsigned long *ret)
> >+{
> >+	addr = gfn_to_hva(vcpu->kvm, gfn);
> >+	if (kvm_is_error_hva(addr))
> >+		return -EFAULT;
> >+
> >+	/* pin page with pv shared memory */
> >+	down_read(&mm->mmap_sem);
> >+	r = get_user_pages(current, mm, addr, 1, 1, 0,&vcpu->arch.pv_shm_page,
> >+			   NULL);
> >+	up_read(&mm->mmap_sem);
> 
> This fails if the memory area straddles a page boundary.  Aligning
Good point.

> would solve this.  I prefer using put_user() though than a permanent
> get_user_pages().
> 
I want to prevent it from been swapped out.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/