Re: [PATCH] RFC: alias rework
On 01/25/2010 10:40 PM, Izik Eidus wrote: Or is this a feature you need? I dont need it (I asked Avi to do something), So he said he want to nuke the aliasing from kvm and keep supporting the old userspace`s Do you have any other way to achive this? Btw I do realize it might be better not to push this patch and just keep the old way of treating aliasing as we have now, I really don`t mind. How about implementing an alias pointing at a deleted slot as an invalid slot? If the slot comes back later, we can revalidate it. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] RFC: alias rework
On Tue, 26 Jan 2010 16:14:47 +0200 Avi Kivity a...@redhat.com wrote: On 01/25/2010 10:40 PM, Izik Eidus wrote: Or is this a feature you need? I dont need it (I asked Avi to do something), So he said he want to nuke the aliasing from kvm and keep supporting the old userspace`s Do you have any other way to achive this? Btw I do realize it might be better not to push this patch and just keep the old way of treating aliasing as we have now, I really don`t mind. How about implementing an alias pointing at a deleted slot as an invalid slot? If the slot comes back later, we can revalidate it. Ok, didn`t notice this invalid memslot flag, I will add this, I will still leave the update_aliased_memslot() in order to update the userspace virtual address... -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] RFC: alias rework
From f94dcd1ccabbcdb51ed7c37c5f58f00a5c1b7eec Mon Sep 17 00:00:00 2001
From: Izik Eidus iei...@redhat.com
Date: Mon, 25 Jan 2010 15:49:41 +0200
Subject: [PATCH] RFC: alias rework
This patch remove the old way of aliasing inside kvm
and move into using aliasing with the same virtual addresses
This patch is really just early RFC just to know if you guys
like this direction, and I need to clean some parts of it
and test it more before I feel it ready to be merged...
Comments are more than welcome.
Thanks.
Signed-off-by: Izik Eidus iei...@redhat.com
---
arch/ia64/include/asm/kvm_host.h |1 +
arch/ia64/kvm/kvm-ia64.c |5 --
arch/powerpc/kvm/powerpc.c |5 --
arch/s390/include/asm/kvm_host.h |1 +
arch/s390/kvm/kvm-s390.c |5 --
arch/x86/include/asm/kvm_host.h | 19 --
arch/x86/include/asm/vmx.h |6 +-
arch/x86/kvm/mmu.c | 19 ++-
arch/x86/kvm/x86.c | 114 +++--
include/linux/kvm_host.h | 11 +--
virt/kvm/kvm_main.c | 80 +++---
11 files changed, 107 insertions(+), 159 deletions(-)
diff --git a/arch/ia64/include/asm/kvm_host.h b/arch/ia64/include/asm/kvm_host.h
index a362e67..d5377c2 100644
--- a/arch/ia64/include/asm/kvm_host.h
+++ b/arch/ia64/include/asm/kvm_host.h
@@ -24,6 +24,7 @@
#define __ASM_KVM_HOST_H
#define KVM_MEMORY_SLOTS 32
+#define KVM_ALIAS_SLOTS 0
/* memory slots that does not exposed to userspace */
#define KVM_PRIVATE_MEM_SLOTS 4
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 0618898..3d2559e 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1947,11 +1947,6 @@ int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
return vcpu-arch.timer_fired;
}
-gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn)
-{
- return gfn;
-}
-
int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
{
return (vcpu-arch.mp_state == KVM_MP_STATE_RUNNABLE) ||
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 51aedd7..50b7d5f 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -35,11 +35,6 @@
#define CREATE_TRACE_POINTS
#include trace.h
-gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn)
-{
- return gfn;
-}
-
int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
{
return !(v-arch.msr MSR_WE) || !!(v-arch.pending_exceptions);
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 27605b6..6a2112e 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -21,6 +21,7 @@
#define KVM_MAX_VCPUS 64
#define KVM_MEMORY_SLOTS 32
+#define KVM_ALIAS_SLOTS 0
/* memory slots that does not exposed to userspace */
#define KVM_PRIVATE_MEM_SLOTS 4
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 8f09959..5d63f6b 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -741,11 +741,6 @@ void kvm_arch_flush_shadow(struct kvm *kvm)
{
}
-gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn)
-{
- return gfn;
-}
-
static int __init kvm_s390_init(void)
{
int ret;
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index a1f0b5d..2d2509f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -367,24 +367,7 @@ struct kvm_vcpu_arch {
u64 hv_vapic;
};
-struct kvm_mem_alias {
- gfn_t base_gfn;
- unsigned long npages;
- gfn_t target_gfn;
-#define KVM_ALIAS_INVALID 1UL
- unsigned long flags;
-};
-
-#define KVM_ARCH_HAS_UNALIAS_INSTANTIATION
-
-struct kvm_mem_aliases {
- struct kvm_mem_alias aliases[KVM_ALIAS_SLOTS];
- int naliases;
-};
-
struct kvm_arch {
- struct kvm_mem_aliases *aliases;
-
unsigned int n_free_mmu_pages;
unsigned int n_requested_mmu_pages;
unsigned int n_alloc_mmu_pages;
@@ -674,8 +657,6 @@ void kvm_disable_tdp(void);
int load_pdptrs(struct kvm_vcpu *vcpu, unsigned long cr3);
int complete_pio(struct kvm_vcpu *vcpu);
-struct kvm_memory_slot *gfn_to_memslot_unaliased(struct kvm *kvm, gfn_t gfn);
-
static inline struct kvm_mmu_page *page_header(hpa_t shadow_page)
{
struct page *page = pfn_to_page(shadow_page PAGE_SHIFT);
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 43f1e9b..bf52a32 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -347,9 +347,9 @@ enum vmcs_field {
#define AR_RESERVD_MASK 0xfffe0f00
-#define TSS_PRIVATE_MEMSLOT(KVM_MEMORY_SLOTS + 0)
-#define APIC_ACCESS_PAGE_PRIVATE_MEMSLOT (KVM_MEMORY_SLOTS + 1)
-#define IDENTITY_PAGETABLE_PRIVATE_MEMSLOT (KVM_MEMORY_SLOTS + 2)
+#define TSS_PRIVATE_MEMSLOT(KVM_MEMORY_SLOTS +
KVM_ALIAS_SLOTS + 0)
+#define APIC_ACCESS_PAGE_PRIVATE_MEMSLOT (KVM_MEMORY_SLOTS +
KVM_ALIAS_SLOTS + 1)
+#define
Re: [PATCH] RFC: alias rework
On Mon, 25 Jan 2010 17:45:53 -0200
Marcelo Tosatti mtosa...@redhat.com wrote:
Izik,
On Mon, Jan 25, 2010 at 03:53:44PM +0200, Izik Eidus wrote:
From f94dcd1ccabbcdb51ed7c37c5f58f00a5c1b7eec Mon Sep 17 00:00:00 2001
From: Izik Eidus iei...@redhat.com
Date: Mon, 25 Jan 2010 15:49:41 +0200
Subject: [PATCH] RFC: alias rework
This patch remove the old way of aliasing inside kvm
and move into using aliasing with the same virtual addresses
This patch is really just early RFC just to know if you guys
like this direction, and I need to clean some parts of it
and test it more before I feel it ready to be merged...
Comments are more than welcome.
Thanks.
Signed-off-by: Izik Eidus iei...@redhat.com
---
arch/ia64/include/asm/kvm_host.h |1 +
arch/ia64/kvm/kvm-ia64.c |5 --
arch/powerpc/kvm/powerpc.c |5 --
arch/s390/include/asm/kvm_host.h |1 +
arch/s390/kvm/kvm-s390.c |5 --
arch/x86/include/asm/kvm_host.h | 19 --
arch/x86/include/asm/vmx.h |6 +-
arch/x86/kvm/mmu.c | 19 ++-
arch/x86/kvm/x86.c | 114
+++--
include/linux/kvm_host.h | 11 +--
virt/kvm/kvm_main.c | 80 +++---
11 files changed, 107 insertions(+), 159 deletions(-)
@@ -2661,7 +2611,18 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_memslots *slots, *old_slots;
spin_lock(kvm-mmu_lock);
+ for (i = KVM_MEMORY_SLOTS; i KVM_MEMORY_SLOTS +
+ KVM_ALIAS_SLOTS; ++i) {
The plan is to kill KVM_ALIAS_SLOTS (aliases will share the 32 mem
slots), right?
Hrmm I think we got to have this addition 4 KVM_MEMORY_SLOTS to keep
the same beahivor with old userspaces
beacuse maybe some userspace apps use 32 slots already?
I dont mind remove it if you guys don`t think this is the case.
+#ifdef CONFIG_X86
+
+static void update_alias_slots(struct kvm *kvm, struct kvm_memory_slot
*slot)
+{
+ int i;
+
+ for (i = KVM_MEMORY_SLOTS; i KVM_MEMORY_SLOTS + KVM_ALIAS_SLOTS;
+++i) {
+ struct kvm_memory_slot *alias_memslot =
+ kvm-memslots-memslots[i];
+ unsigned long size = slot-npages PAGE_SHIFT;
+
+ if (alias_memslot-real_base_gfn = slot-base_gfn
+ alias_memslot-real_base_gfn slot-base_gfn + size) {
+ if (slot-dirty_bitmap) {
+ unsigned long bitmap_addr;
+ unsigned long dirty_offset;
+ unsigned long offset_addr =
+ (alias_memslot-real_base_gfn -
+ slot-base_gfn) PAGE_SHIFT;
+ alias_memslot-userspace_addr =
+ slot-userspace_addr + offset_addr;
+
+ dirty_offset =
+ ALIGN(offset_addr, BITS_PER_LONG) / 8;
+ bitmap_addr = (unsigned long)
slot-dirty_bitmap;
+ bitmap_addr += dirty_offset;
+ alias_memslot-dirty_bitmap = (unsigned long
*)bitmap_addr;
+ alias_memslot-base_gfn =
alias_memslot-real_base_gfn;
+ alias_memslot-npages =
alias_memslot-real_npages;
+ } else if (!slot-rmap) {
+ alias_memslot-base_gfn = 0;
+ alias_memslot-npages = 0;
+ }
+ }
+ }
+}
+
+#endif
Can't see why is this needed. What is the problem with nuking child
aliases when deleting a real memslot?
The problem is that this memslot still point in the virtual address of the host,
This mean that gfn_to_memslot/page will still work on gfns and will result in
pages that are mapped into the virtual address that the userspace requested to
remove from KVM.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] RFC: alias rework
On Mon, Jan 25, 2010 at 10:40:32PM +0200, Izik Eidus wrote: On Mon, 25 Jan 2010 18:20:39 -0200 Marcelo Tosatti mtosa...@redhat.com wrote: With current code, if a memslot is deleted, access through any aliases that use it will fail (BTW it looks this is not properly handled, but thats a separate problem). Yea I had some still open concerns about this code (this why I sent it on RFC) So AFAICS there is no requirement for an alias to continue operable if its parent memslot is deleted. With this patch alias will stop to opearte when the parent is deleted just like the behivor with the current code... base_gfn will be set to 0 and npages will be set to 0 as well (the true values wil be hide in real_base_gfn...), so gfn_to_memslot and gfn_to_page will fail But you adjust the alias (and keep it valid) if dirty logging is enabled? Or is this a feature you need? I dont need it (I asked Avi to do something), So he said he want to nuke the aliasing from kvm and keep supporting the old userspace`s With feature i meant keeping the alias around when parent slot is deleted. Do you have any other way to achive this? No. Btw I do realize it might be better not to push this patch and just keep the old way of treating aliasing as we have now, I really don`t mind. Motivation is that nukeing aliases is simpler than adjusting them. Agree. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] RFC: alias rework
On Mon, 25 Jan 2010 18:49:25 -0200
Marcelo Tosatti mtosa...@redhat.com wrote:
On Mon, Jan 25, 2010 at 10:40:32PM +0200, Izik Eidus wrote:
On Mon, 25 Jan 2010 18:20:39 -0200
Marcelo Tosatti mtosa...@redhat.com wrote:
With current code, if a memslot is deleted, access through any aliases
that use it will fail (BTW it looks this is not properly handled, but
thats a separate problem).
Yea I had some still open concerns about this code (this why I sent it on
RFC)
So AFAICS there is no requirement for an alias to continue operable
if its parent memslot is deleted.
With this patch alias will stop to opearte when the parent is deleted
just like the behivor with the current code...
base_gfn will be set to 0 and npages will be set to 0 as well
(the true values wil be hide in real_base_gfn...), so gfn_to_memslot
and gfn_to_page will fail
But you adjust the alias (and keep it valid) if dirty logging is
enabled?
I am sorry, but probaby you got confused beacuse the code is wrong
the adjust of aliasing should happen in every case of:
if(slot-rmap - valid (!NULL)):
this mean we got NEW parent slot that mapped into the gfn
that the alias is mapped to, and we want the userspace address
of the alias slot to intersect with the new parent slot.
and the latter adjustmant of the dirty_bitmap should happen only in case
of - if(slot-dirty_bitmap - valid (!NULL)):
the alias slot need to mark_page_dirty the bitmap of the new parent slot
I hope this will make things more clear
(I think there is another small issue there, but I will send it when it wont be
RFC)
Or is this a feature you need?
I dont need it (I asked Avi to do something), So he said he want to nuke
the aliasing
from kvm and keep supporting the old userspace`s
With feature i meant keeping the alias around when parent slot is
deleted.
The code doesnt try to do this, infact:
} else if (!slot-rmap) {
alias_memslot-base_gfn = 0;
alias_memslot-npages = 0;
}
came to invalidate the alias slot.
Sorry if I made to much mess :).
Do you have any other way to achive this?
No.
Btw I do realize it might be better not to push this patch and just keep
the old
way of treating aliasing as we have now, I really don`t mind.
Motivation is that nukeing aliases is simpler than adjusting them.
Agree.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
