A couple ifdefs

PATCH In review Hollis Blanchard 4 participants 10 messages Jan 15, 2008 – Jan 24, 2008 4 messages not in the archive

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Hollis Blanchard

author

2008-01-15 21:59 UTC #0 lore raw

PATCH 1/1 A couple ifdefs

These small ifdefs are necessary for integration of the PowerPC port.

3 files changed, 9 insertions(+)
arch/x86/kvm/Kconfig     |    5 +++++
include/linux/kvm_host.h |    2 ++
virt/kvm/kvm_main.c      |    2 ++

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Hollis Blanchard

author

2008-01-15 21:59 UTC #1 lore raw

PATCH 1/1 Define and use CONFIG_KVM_HAS_PIO so that we don't need pio_data in kvm_arch_vcpu

# HG changeset patch
# User Hollis Blanchard <hollisb-r/[email protected]> # Date 1200434310 21600
# Node ID 7fa5947a2da8c0c7424ebdcfaebcae624d6cf015
# Parent ee0c227fe3f6632f4b1b5fde3f7e05c8ea0a4378

Signed-off-by Hollis Blanchard <hollisb-r/[email protected]>

Signed-off-by Christian Ehrhardt <[email protected]>

---

2 files changed, 7 insertions(+)
arch/x86/kvm/Kconfig |    5 +++++
virt/kvm/kvm_main.c  |    2 ++

arch/x86/kvm/Kconfig +5 -0

@@ -33,9 +33,13 @@ config KVM
 
 	  If unsure, say N.
 
+config KVM_HAS_PIO
+	bool
+
 config KVM_INTEL
 	tristate "KVM for Intel processors support"
 	depends on KVM
+	select KVM_HAS_PIO
 	---help---
 	  Provides support for KVM on Intel processors equipped with the VT
 	  extensions.

@@ -43,6 +47,7 @@ config KVM_AMD
 config KVM_AMD
 	tristate "KVM for AMD processors support"
 	depends on KVM
+	select KVM_HAS_PIO
 	---help---
 	  Provides support for KVM on AMD processors equipped with the AMD-V
 	  (SVM) extensions.

virt/kvm/kvm_main.c +2 -0

@@ -677,8 +677,10 @@ static int kvm_vcpu_fault(struct vm_area
 
 	if (vmf->pgoff == 0)
 		page = virt_to_page(vcpu->run);
+#ifdef CONFIG_KVM_HAS_PIO
 	else if (vmf->pgoff == KVM_PIO_PAGE_OFFSET)
 		page = virt_to_page(vcpu->arch.pio_data);
+#endif /* CONFIG_KVM_HAS_PIO */
 	else
 		return VM_FAULT_SIGBUS;
 	get_page(page);

Hollis Blanchard

author

2008-01-15 21:59 UTC #7 lore raw

PATCH 1/1 Use CONFIG_PREEMPT_NOTIFIERS around struct preempt_notifier

In reply to Hollis Blanchard

# HG changeset patch
# User Hollis Blanchard <hollisb-r/[email protected]> # Date 1200434370 21600
# Node ID 9878c9cec5f831ff5e9b97539aabc5fa3d934501
# Parent 931a81e1002110be0e8bf5b335bf199d43534c2c
This allows kvm_host.h to be #included even when struct preempt_notifier is undefined.

Signed-off-by Hollis Blanchard <hollisb-r/[email protected]>

---

1 file changed, 2 insertions(+)

include/linux/kvm_host.h | 2 ++

include/linux/kvm_host.h +2 -0

@@ -66,7 +66,9 @@ void kvm_io_bus_register_dev(struct kvm_
 
 struct kvm_vcpu {
 	struct kvm *kvm;
+#ifdef CONFIG_PREEMPT_NOTIFIERS
 	struct preempt_notifier preempt_notifier;
+#endif
 	int vcpu_id;
 	struct mutex mutex;
 	int   cpu;

Avi Kivity

2008-01-16 08:03 UTC #11 lore raw

In reply to Hollis Blanchard

Quoted 3 lines · Hollis Blanchard

These small ifdefs are necessary for integration of the PowerPC port.

Only patch 2 of 2 made it.

Signature · 7 lines

error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Avi Kivity

2008-01-16 08:08 UTC #8 lore raw

Re: [PATCH 2 of 2] Use CONFIG_PREEMPT_NOTIFIERS around struct preempt_notifier

In reply to Hollis Blanchard

Quoted 9 lines · Hollis Blanchard

# HG changeset patch
# User Hollis Blanchard <hollisb-r/[email protected]> # Date 1200434370 21600
# Node ID 9878c9cec5f831ff5e9b97539aabc5fa3d934501
# Parent 931a81e1002110be0e8bf5b335bf199d43534c2c
This allows kvm_host.h to be #included even when struct preempt_notifier is undefined.

Don't you actually need preempt notifiers? They are useful if you have state that is only needed from userspace, but is expensive to switch. For x86, this is the syscall msrs (which define the syscall entry point), the fpu (which is not used in the kernel), and a few other bits (which I'm too lazy too look up and are esoteric anyway).

Signature · 7 lines

error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Christian Ehrhardt

2008-01-16 09:24 UTC #13 lore raw

In reply to a message not in the archive

Quoted 7 lines · Avi Kivity

Quoted 3 lines · Hollis Blanchard
These small ifdefs are necessary for integration of the PowerPC port.
Only patch 2 of 2 made it.

As Hollis should be sleeping right now I resend 1/2 as it arrived on kvm-powerpc-devel (I hope my mail-app keeps the format this time)

Signature · 20 lines

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization

original mail
---

# HG changeset patch
# User Hollis Blanchard <hollisb-r/[email protected]>
# Date 1200434310 21600
# Node ID 7fa5947a2da8c0c7424ebdcfaebcae624d6cf015
# Parent  ee0c227fe3f6632f4b1b5fde3f7e05c8ea0a4378

Signed-off-by: Hollis Blanchard <hollisb-r/[email protected]>
Signed-off-by: Christian Ehrhardt <[email protected]>

---
2 files changed, 7 insertions(+)
arch/x86/kvm/Kconfig |    5 +++++
virt/kvm/kvm_main.c  |    2 ++

arch/x86/kvm/Kconfig +5 -0

@@ -33,9 +33,13 @@ config KVM
 
 	  If unsure, say N.
 
+config KVM_HAS_PIO
+	bool
+
 config KVM_INTEL
 	tristate "KVM for Intel processors support"
 	depends on KVM
+	select KVM_HAS_PIO
 	---help---
 	  Provides support for KVM on Intel processors equipped with the VT
 	  extensions.

@@ -43,6 +47,7 @@ config KVM_AMD
 config KVM_AMD
 	tristate "KVM for AMD processors support"
 	depends on KVM
+	select KVM_HAS_PIO
 	---help---
 	  Provides support for KVM on AMD processors equipped with the AMD-V
 	  (SVM) extensions.

virt/kvm/kvm_main.c +2 -0

@@ -677,8 +677,10 @@ static int kvm_vcpu_fault(struct vm_area
 
 	if (vmf->pgoff == 0)
 		page = virt_to_page(vcpu->run);
+#ifdef CONFIG_KVM_HAS_PIO
 	else if (vmf->pgoff == KVM_PIO_PAGE_OFFSET)
 		page = virt_to_page(vcpu->arch.pio_data);
+#endif /* CONFIG_KVM_HAS_PIO */
 	else
 		return VM_FAULT_SIGBUS;
 	get_page(page);

Hollis Blanchard

author

2008-01-16 16:23 UTC #10 lore raw

Re: [PATCH 2 of 2] Use CONFIG_PREEMPT_NOTIFIERS around struct preempt_notifier

In reply to a message not in the archive

Quoted 9 lines · Avi Kivity

Quoted 7 lines · Hollis Blanchard
# HG changeset patch
# User Hollis Blanchard <hollisb-r/[email protected]> # Date 1200434370 21600
# Node ID 9878c9cec5f831ff5e9b97539aabc5fa3d934501
# Parent 931a81e1002110be0e8bf5b335bf199d43534c2c
This allows kvm_host.h to be #included even when struct preempt_notifier is undefined.

Quoted 5 lines

Don't you actually need preempt notifiers? They are useful if you have state that is only needed from userspace, but is expensive to switch. For x86, this is the syscall msrs (which define the syscall entry point), the fpu (which is not used in the kernel), and a few other bits (which I'm too lazy too look up and are esoteric anyway).

Yes, I do. However, if you #include <kvm_host.h> *without* CONFIG_VIRTUALIZATION=y, CONFIG_PREEMPT_NOTIFIERS is not set and the structure is undefined.

It is Linux policy to be able to unconditionally include headers, and indeed I already hit this problem when I added that #include to arch/powerpc/kernel/asm-offsets.c.

Signature · 8 lines

Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Avi Kivity

2008-01-20 13:15 UTC #2 lore raw

Re: [PATCH 1 of 2] Define and use CONFIG_KVM_HAS_PIO so that we don't need pio_data in kvm_arch_vcpu

In reply to Hollis Blanchard

Quoted 34 lines · Hollis Blanchard

# HG changeset patch
# User Hollis Blanchard <hollisb-r/[email protected]> # Date 1200434310 21600
# Node ID 7fa5947a2da8c0c7424ebdcfaebcae624d6cf015
# Parent ee0c227fe3f6632f4b1b5fde3f7e05c8ea0a4378
Signed-off-by Hollis Blanchard <hollisb-r/[email protected]>
Signed-off-by Christian Ehrhardt <[email protected]>
---
2 files changed, 7 insertions(+)
arch/x86/kvm/Kconfig |    5 +++++
virt/kvm/kvm_main.c  |    2 ++
arch/x86/kvm/Kconfig +4 -0
@@ -33,9 +33,13 @@ config KVM
 
 	  If unsure, say N.
 
+config KVM_HAS_PIO
+	bool
+
 config KVM_INTEL
 	tristate "KVM for Intel processors support"
 	depends on KVM
+	select KVM_HAS_PIO
 	---help---
 	  Provides support for KVM on Intel processors equipped with the VT
 	  extensions.

arch/x86/kvm/Kconfig config KVM_AMD · line 53

Quoted 7 lines

arch/x86/kvm/Kconfig +1 -0

@@ -43,6 +47,7 @@ config KVM_AMD
 config KVM_AMD
 	tristate "KVM for AMD processors support"
 	depends on KVM
+	select KVM_HAS_PIO
 	---help---
 	  Provides support for KVM on AMD processors equipped with the AMD-V
 	  (SVM) extensions.

Too much Kconfig for this. Since I want to do some Kconfig changes anyway, I'll do this another way.

Signature · 7 lines

error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Carsten Otte

2008-01-24 12:59 UTC #4 lore raw

Re: [PATCH 1 of 2] Define and use CONFIG_KVM_HAS_PIO so that we don't need pio_data in kvm_arch_vcpu

In reply to a message not in the archive

Quoted 2 lines · Avi Kivity

Too much Kconfig for this. Since I want to do some Kconfig changes anyway, I'll do this another way.

I think you overlooked something here. Actually this should change too if we don't have pio:
-.......case KVM_GET_VCPU_MMAP_SIZE:
-.......-.......r = -EINVAL;
-.......-.......if (arg)
-.......-.......-.......goto out;
-.......-.......r = 2 * PAGE_SIZE;
-.......-.......break;

so long,
Carsten

Avi Kivity

2008-01-24 13:14 UTC #6 lore raw

Re: [PATCH 1 of 2] Define and use CONFIG_KVM_HAS_PIO so that we don't need pio_data in kvm_arch_vcpu

In reply to a message not in the archive

Quoted 12 lines · Carsten Otte

Quoted 2 lines · Avi Kivity
Too much Kconfig for this. Since I want to do some Kconfig changes anyway, I'll do this another way.
I think you overlooked something here. Actually this should change too if we don't have pio:
-.......case KVM_GET_VCPU_MMAP_SIZE:
-.......-.......r = -EINVAL;
-.......-.......if (arg)
-.......-.......-.......goto out;
-.......-.......r = 2 * PAGE_SIZE;
-.......-.......break;

Right you are. Fixup patch pushed. Thanks.

Signature · 7 lines

Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/