KVM: VMX: Move private memory slot position

PATCH Applied Yang, Sheng 4 participants 8 messages Sep 4, 2008 – Oct 19, 2008

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Yang, Sheng

author

2008-09-04 03:27 UTC #0 lore raw

PATCH 1/1 KVM: VMX: Move private memory slot position

From ebe4ea311305d2910dcdcff2510662da0dc2c742 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Thu, 4 Sep 2008 03:11:48 +0800
Subject: [PATCH] KVM: VMX: Move private memory slot position

PCI device assignment would map guest MMIO spaces as separate slot, so it is possible that the device has more than 2 MMIO spaces and overwrite current private memslot.

The patch move private memory slot to the top of userspace visible memory slots.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/kvm/vmx.c |    2 +-
 arch/x86/kvm/vmx.h |    5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

arch/x86/kvm/vmx.c +0 -0

index 004d24a..27c3bb7 100644
@@ -2448,7 +2448,7 @@ static int vmx_set_tss_addr(struct kvm *kvm, unsigned

int addr)
{
int ret;
struct kvm_userspace_memory_region tss_mem = {
- .slot = 8,
+ .slot = TSS_PRIVATE_MEMSLOT,
.guest_phys_addr = addr, .memory_size = PAGE_SIZE * 3, .flags = 0,

arch/x86/kvm/vmx.h +3 -2

index 16b3cfb..dd0eea9 100644
@@ -356,8 +356,9 @@ enum vmcs_field {
 #define IA32_FEATURE_CONTROL_LOCKED_BIT		0x1
 #define IA32_FEATURE_CONTROL_VMXON_ENABLED_BIT	0x4
 
-#define APIC_ACCESS_PAGE_PRIVATE_MEMSLOT	9
-#define IDENTITY_PAGETABLE_PRIVATE_MEMSLOT	10
+#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 VMX_NR_VPIDS				(1 << 16)
 #define VMX_VPID_EXTENT_SINGLE_CONTEXT		1

--
1.5.4.5

PCI device assignment would map guest MMIO spaces as separate slot, so it is possible that the device has more than 2 MMIO spaces and overwrite current private memslot.

The patch move private memory slot to the top of userspace visible memory slots.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/kvm/vmx.c |    2 +-
 arch/x86/kvm/vmx.h |    5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

arch/x86/kvm/vmx.c +1 -1

index 004d24a..27c3bb7 100644
@@ -2448,7 +2448,7 @@ static int vmx_set_tss_addr(struct kvm *kvm, unsigned int addr)
 {
 	int ret;
 	struct kvm_userspace_memory_region tss_mem = {
-		.slot = 8,
+		.slot = TSS_PRIVATE_MEMSLOT,
 		.guest_phys_addr = addr,
 		.memory_size = PAGE_SIZE * 3,
 		.flags = 0,

arch/x86/kvm/vmx.h +3 -2

index 16b3cfb..dd0eea9 100644
@@ -356,8 +356,9 @@ enum vmcs_field {
 #define IA32_FEATURE_CONTROL_LOCKED_BIT		0x1
 #define IA32_FEATURE_CONTROL_VMXON_ENABLED_BIT	0x4
 
-#define APIC_ACCESS_PAGE_PRIVATE_MEMSLOT	9
-#define IDENTITY_PAGETABLE_PRIVATE_MEMSLOT	10
+#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 VMX_NR_VPIDS				(1 << 16)
 #define VMX_VPID_EXTENT_SINGLE_CONTEXT		1

Signature · 1 line

1.5.4.5

Yang, Sheng

author

2008-09-11 09:27 UTC #1 lore raw

Quoted 5 lines · Sheng

From ebe4ea311305d2910dcdcff2510662da0dc2c742 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Thu, 4 Sep 2008 03:11:48 +0800
Subject: [PATCH] KVM: VMX: Move private memory slot position

Quoted 7 lines

PCI device assignment would map guest MMIO spaces as separate slot, so it is possible that the device has more than 2 MMIO spaces and overwrite current private memslot.
The patch move private memory slot to the top of userspace visible memory slots.

Avi, these two?

(Oh, it's a little old, next time I will use git-send-email :) )

Signature · 2 lines

regards
Yang, Sheng

Quoted 38 lines

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/kvm/vmx.c |    2 +-
 arch/x86/kvm/vmx.h |    5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

arch/x86/kvm/vmx.c +1 -1

index 004d24a..27c3bb7 100644
@@ -2448,7 +2448,7 @@ static int vmx_set_tss_addr(struct kvm *kvm, unsigned
 int addr)
 {
 	int ret;
 	struct kvm_userspace_memory_region tss_mem = {
-		.slot = 8,
+		.slot = TSS_PRIVATE_MEMSLOT,
 		.guest_phys_addr = addr,
 		.memory_size = PAGE_SIZE * 3,

arch/x86/kvm/vmx.c +0 -0

 		.flags = 0,

arch/x86/kvm/vmx.h +3 -3

index 16b3cfb..dd0eea9 100644
@@ -356,8 +356,9 @@ enum vmcs_field {
 #define IA32_FEATURE_CONTROL_LOCKED_BIT		0x1
 #define IA32_FEATURE_CONTROL_VMXON_ENABLED_BIT	0x4
 
-#define APIC_ACCESS_PAGE_PRIVATE_MEMSLOT	9
-#define IDENTITY_PAGETABLE_PRIVATE_MEMSLOT	10
+#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 VMX_NR_VPIDS				(1 << 16)
 #define VMX_VPID_EXTENT_SINGLE_CONTEXT		1
--

1.5.4.5

Avi Kivity

Applied

2008-09-13 05:04 UTC #2 lore raw

Quoted 11 lines · Sheng

Quoted 9 lines · Sheng
From ebe4ea311305d2910dcdcff2510662da0dc2c742 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Thu, 4 Sep 2008 03:11:48 +0800
Subject: [PATCH] KVM: VMX: Move private memory slot position
PCI device assignment would map guest MMIO spaces as separate slot, so it is possible that the device has more than 2 MMIO spaces and overwrite current private memslot.

Quoted 7 lines

Quoted 4 lines
The patch move private memory slot to the top of userspace visible memory slots.
Avi, these two?

Thanks, applied both.

Note that kvm now exports the number of slots using KVM_CAP_NR_MEMSLOTS, so userspace could be made dynamic.

Signature · 1 line

Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Avi Kivity

2008-09-13 08:58 UTC #3 lore raw

Quoted 19 lines · Avi Kivity

Quoted 17 lines · Sheng
Quoted 14 lines · Sheng
From ebe4ea311305d2910dcdcff2510662da0dc2c742 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Thu, 4 Sep 2008 03:11:48 +0800
Subject: [PATCH] KVM: VMX: Move private memory slot position
PCI device assignment would map guest MMIO spaces as separate slot, so it is possible that the device has more than 2 MMIO spaces and overwrite current private memslot.
The patch move private memory slot to the top of userspace visible memory slots.

Quoted 9 lines

Quoted 3 lines
Avi, these two?
Thanks, applied both.
Note that kvm now exports the number of slots using KVM_CAP_NR_MEMSLOTS, so userspace could be made dynamic.

Well, the kernel change causes the host to oops while booting Windows on an i386 pae host. No idea why.

Signature · 1 line

Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Yang, Sheng

author

2008-10-13 09:20 UTC #4 lore raw

Quoted 17 lines · On Saturday 13 September 2008 16:55:27 Avi Kivity wrote:

Quoted 16 lines · Avi Kivity
Quoted 14 lines · Sheng
Quoted 11 lines · Sheng
From ebe4ea311305d2910dcdcff2510662da0dc2c742 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Thu, 4 Sep 2008 03:11:48 +0800
Subject: [PATCH] KVM: VMX: Move private memory slot position
PCI device assignment would map guest MMIO spaces as separate slot, so it is possible that the device has more than 2 MMIO spaces and overwrite current private memslot.
The patch move private memory slot to the top of userspace visible memory slots.
Avi, these two?

Quoted 7 lines

Quoted 4 lines
Thanks, applied both.
Note that kvm now exports the number of slots using KVM_CAP_NR_MEMSLOTS, so userspace could be made dynamic.
Well, the kernel change causes the host to oops while booting Windows on an i386 pae host. No idea why.

I've found the reason... It's because that kvm_mmu_page->slot_bitmap is unsigned long, and if use KVM_MEMORY_SLOTS + xxx, it would beyond 32 in pae, then memory corrupted.

But reduce supported memory slot number to 28 or extend slot_bitmap, or other methods? Slot_bitmap have bitops, so keep unsigned long would be better... Now reduce supported memory slot number seems reasonable to me.

(I also want to have this fix into 2.6.28, for some device would easily overlapped with current private memory slot)

--
regards
Yang, Sheng

Avi Kivity

2008-10-16 08:26 UTC #5 lore raw

Quoted 5 lines · Sheng

I've found the reason... It's because that kvm_mmu_page->slot_bitmap is unsigned long, and if use KVM_MEMORY_SLOTS + xxx, it would beyond 32 in pae, then memory corrupted.
But reduce supported memory slot number to 28 or extend slot_bitmap, or other methods? Slot_bitmap have bitops, so keep unsigned long would be better... Now reduce supported memory slot number seems reasonable to me.

We could change it to DECLARE_BITMAP, and thus support >= 32 slots even on i386. But I agree that 28 slots would be sufficient.

Quoted 2 lines

(I also want to have this fix into 2.6.28, for some device would easily overlapped with current private memory slot)

I've been thinking that we can get rid of internal slots, by placing the TSS, real mode identity map, and APIC access page in the bios. Of course we would need a new ioctl to let the kernel know where the scratch memory is located and how much of it is available.

Signature · 1 line

error compiling committee.c: too many arguments to function

Sheng Yang

2008-10-16 08:44 UTC #6 lore raw

Quoted 11 lines · On Thursday 16 October 2008 16:26:07 Avi Kivity wrote:

Quoted 7 lines · Sheng
I've found the reason... It's because that kvm_mmu_page->slot_bitmap is unsigned long, and if use KVM_MEMORY_SLOTS + xxx, it would beyond 32 in pae, then memory corrupted.
But reduce supported memory slot number to 28 or extend slot_bitmap, or other methods? Slot_bitmap have bitops, so keep unsigned long would be better... Now reduce supported memory slot number seems reasonable to me.
We could change it to DECLARE_BITMAP, and thus support >= 32 slots even on i386. But I agree that 28 slots would be sufficient.

DECLARE_BITMAP looks like a flexible solution, I forgot it...(and I also don't like to change a lot of macro in kernel and userspace). I will post the patch soon.

Quoted 9 lines

Quoted 3 lines
(I also want to have this fix into 2.6.28, for some device would easily overlapped with current private memory slot)
I've been thinking that we can get rid of internal slots, by placing the TSS, real mode identity map, and APIC access page in the bios. Of course we would need a new ioctl to let the kernel know where the scratch memory is located and how much of it is available.

How about put into userspace e.g. pc_init()? I think more easy than keeping bios sync with KVM. Anyway, vmx specific thing would become generic. I'd like to keep it as what it is at least for now...

--
regards
Yang, Sheng

Avi Kivity

2008-10-19 11:02 UTC #7 lore raw

Quoted 8 lines · Sheng Yang

Quoted 6 lines
I've been thinking that we can get rid of internal slots, by placing the TSS, real mode identity map, and APIC access page in the bios. Of course we would need a new ioctl to let the kernel know where the scratch memory is located and how much of it is available.
How about put into userspace e.g. pc_init()? I think more easy than keeping bios sync with KVM.

That was my plan -- bios reserves space, qemu tells the kernel where it is, kernel uses the space.

Quoted 2 lines

Anyway, vmx specific thing would become generic. I'd like to keep it as what it is at least for now...

It's already exposed to userspace, as userspace tells the kernel where to put the slots. The only advantage is that we reduce slot count and don't have to mess with the e820 map.

Signature · 1 line

error compiling committee.c: too many arguments to function