Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
Hollis Blanchard wrote: These cannot use the same method, since we need to support both vmx and svm in the same binary. The arch specific members aren't the same size, nor do the symbols they use have the same visibility. I have never understood this. Why on earth do you need to support VMX and SVM in the same binary? For example, when would you overwrite kvm_x86_ops after initialization? If you wouldn't, then why are you using function pointers instead of the linker? Consider a non-modular build; common code needs to call either svm_vcpu_load() or vmx_vcpu_load() depending on the current hardware. I imagine it could be done using linker tricks (duplicating the common code to be linked twice, once per subarch, and leaving a stub to detect which duplicate needs to be used), but I never found either the time or necessity to do them. PowerPC will also need to support multiple processor types, and so I expect to have one kvm_arch structure for each. That also means struct kvm_arch must be the *last* member in struct kvm, which is not how it is shown above. Do you plan to support multiple processor types in a single binary as well? Or require modules and compile them multiple times? (that was how AMD support was initially added; it was changed before merging). -- Any sufficiently difficult bug is indistinguishable from a feature. - SF.Net email is sponsored by: The Future of Linux Business White Paper from Novell. From the desktop to the data center, Linux is going mainstream. Let it simplify your IT future. http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4 ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
Avi Kivity wrote:
Hollis Blanchard wrote:
On Wed, 2007-11-21 at 11:18 +0200, Avi Kivity wrote:
Well, I hate to say it, but the resulting code doesn't look too well
(all the kvm_x86 variables), and it's entirely my fault as I
recommended this approach. Not like it was difficult to predict.
I guess we still have reached no conclusion on this question?
Right. Thanks for re-raising it.
Thanks too. I have almost done the rebase work for IA64 support, maybe
we should work out a solution for that :)
I'm thinking again of
struct kvm {
struct kvm_arch a;
...
}
Where each arch defines its own kvm_arch. Now the changes look
like a bunch of kvm-blah to kvm-a.blah conversions.
The nicer one:
struct kvm {
struct kvm_arch arch;
// common fields
}
I prefer this one, seems it is more direct and readable. Same thinking
about kvm_vcpu structure:)
-
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
Hollis Blanchard wrote:
On Fri, 2007-11-30 at 11:04 +0200, Avi Kivity wrote:
Zhang, Xiantao wrote:
The nicer one:
struct kvm {
struct kvm_arch arch;
// common fields
}
I prefer this one, seems it is more direct and readable. Same thinking
about kvm_vcpu structure:)
I agree, kvm_vcpu should use the same method.
And we will convert vcpu_vmx/vcpu_svm as well?
These cannot use the same method, since we need to support both vmx and
svm in the same binary. The arch specific members aren't the same size,
nor do the symbols they use have the same visibility.
--
Do not meddle in the internals of kernels, for they are subtle and quick to
panic.
-
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
On Fri, 2007-11-30 at 15:43 -0600, Anthony Liguori wrote: Hollis Blanchard wrote: On Fri, 2007-11-30 at 22:31 +0200, Avi Kivity wrote: These cannot use the same method, since we need to support both vmx and svm in the same binary. The arch specific members aren't the same size, nor do the symbols they use have the same visibility. I have never understood this. Why on earth do you need to support VMX and SVM in the same binary? For example, when would you overwrite kvm_x86_ops after initialization? If you wouldn't, then why are you using function pointers instead of the linker? It's necessary for the distros to be able to ship both AMD and Intel support in a single binary. We aren't talking, in general, about a single static binary but instead loadable modules. There maybe some cases where it's useful to support both in a static kernel binary. I think the monolithic case is the one I overlooked. As long as everything is a module, there should be no problem loading the appropriate module for the host processor type. However, once you want to support both processor types in a monolithic kernel, that's where you need the function pointer flexibility. If you used the linker instead of function pointers, it would be impossible to build a static kernel binary that supported both. Plus, depmod would get very confused because two modules would be providing the same symbols. It can be made to work, but it's kind of funky. PowerPC will also need to support multiple processor types, and so I expect to have one kvm_arch structure for each. That also means struct kvm_arch must be the *last* member in struct kvm, which is not how it is shown above. Instead of having a kvm.ko and a kvm-ppc-440.ko, you probably should have a kvm.ko and a kvm-ppc.ko and then build the kvm-ppc.ko based on the board. You would never build multiple kvm-ppc-XXX.ko modules in the same binary right? I hope to have multiple kvm-ppc-XXX.ko modules loaded simultaneously to support different guest types on the same host. I haven't yet figured out what that interface should look like, but obviously linking is preferable to function pointers where feasible. -- Hollis Blanchard IBM Linux Technology Center - SF.Net email is sponsored by: The Future of Linux Business White Paper from Novell. From the desktop to the data center, Linux is going mainstream. Let it simplify your IT future. http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4 ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
On Fri, 2007-11-30 at 22:31 +0200, Avi Kivity wrote:
Hollis Blanchard wrote:
On Fri, 2007-11-30 at 11:04 +0200, Avi Kivity wrote:
Zhang, Xiantao wrote:
The nicer one:
struct kvm {
struct kvm_arch arch;
// common fields
}
I prefer this one, seems it is more direct and readable. Same thinking
about kvm_vcpu structure:)
I agree, kvm_vcpu should use the same method.
And we will convert vcpu_vmx/vcpu_svm as well?
These cannot use the same method, since we need to support both vmx and
svm in the same binary. The arch specific members aren't the same size,
nor do the symbols they use have the same visibility.
I have never understood this. Why on earth do you need to support VMX
and SVM in the same binary? For example, when would you overwrite
kvm_x86_ops after initialization? If you wouldn't, then why are you
using function pointers instead of the linker?
PowerPC will also need to support multiple processor types, and so I
expect to have one kvm_arch structure for each. That also means struct
kvm_arch must be the *last* member in struct kvm, which is not how it is
shown above.
--
Hollis Blanchard
IBM Linux Technology Center
-
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
Hollis Blanchard wrote:
On Fri, 2007-11-30 at 22:31 +0200, Avi Kivity wrote:
Hollis Blanchard wrote:
On Fri, 2007-11-30 at 11:04 +0200, Avi Kivity wrote:
Zhang, Xiantao wrote:
The nicer one:
struct kvm {
struct kvm_arch arch;
// common fields
}
I prefer this one, seems it is more direct and readable. Same thinking
about kvm_vcpu structure:)
I agree, kvm_vcpu should use the same method.
And we will convert vcpu_vmx/vcpu_svm as well?
These cannot use the same method, since we need to support both vmx and
svm in the same binary. The arch specific members aren't the same size,
nor do the symbols they use have the same visibility.
I have never understood this. Why on earth do you need to support VMX
and SVM in the same binary? For example, when would you overwrite
kvm_x86_ops after initialization? If you wouldn't, then why are you
using function pointers instead of the linker?
It's necessary for the distros to be able to ship both AMD and Intel
support in a single binary. We aren't talking, in general, about a
single static binary but instead loadable modules. There maybe some
cases where it's useful to support both in a static kernel binary.
If you used the linker instead of function pointers, it would be
impossible to build a static kernel binary that supported both. Plus,
depmod would get very confused because two modules would be providing
the same symbols. It can be made to work, but it's kind of funky.
PowerPC will also need to support multiple processor types, and so I
expect to have one kvm_arch structure for each. That also means struct
kvm_arch must be the *last* member in struct kvm, which is not how it is
shown above.
Instead of having a kvm.ko and a kvm-ppc-440.ko, you probably should
have a kvm.ko and a kvm-ppc.ko and then build the kvm-ppc.ko based on
the board. You would never build multiple kvm-ppc-XXX.ko modules in the
same binary right?
Regards,
Anthony Liguori
-
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
On Fri, 2007-11-30 at 11:04 +0200, Avi Kivity wrote:
Zhang, Xiantao wrote:
The nicer one:
struct kvm {
struct kvm_arch arch;
// common fields
}
I prefer this one, seems it is more direct and readable. Same thinking
about kvm_vcpu structure:)
I agree, kvm_vcpu should use the same method.
And we will convert vcpu_vmx/vcpu_svm as well?
--
Hollis Blanchard
IBM Linux Technology Center
-
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
Avi Kivity wrote:
Zhang, Xiantao wrote:
The nicer one:
struct kvm {
struct kvm_arch arch;
// common fields
}
I prefer this one, seems it is more direct and readable. Same
thinking about kvm_vcpu structure:)
I agree, kvm_vcpu should use the same method.
OK, I will rebase the kvm structure split patches according to this
method. :-)
-
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
Zhang, Xiantao wrote:
The nicer one:
struct kvm {
struct kvm_arch arch;
// common fields
}
I prefer this one, seems it is more direct and readable. Same thinking
about kvm_vcpu structure:)
I agree, kvm_vcpu should use the same method.
--
Do not meddle in the internals of kernels, for they are subtle and quick to
panic.
-
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
Hollis Blanchard wrote:
On Wed, 2007-11-21 at 11:18 +0200, Avi Kivity wrote:
Carsten Otte wrote:
Hollis Blanchard wrote:
These patches are based on Xiantao's work to create struct kvm_x86. Patch
1 replaces his KVM Portability split: Splitting kvm structure (V2), and
patches 2 and 3 build on it.
Looks like a clean approach with to to_kvm_x86 macro. Whole series:
Acked-by: Carsten Otte [EMAIL PROTECTED]
Well, I hate to say it, but the resulting code doesn't look too well
(all the kvm_x86 variables), and it's entirely my fault as I recommended
this approach. Not like it was difficult to predict.
I guess we still have reached no conclusion on this question?
Right. Thanks for re-raising it.
I'm thinking again of
struct kvm {
struct kvm_arch a;
...
}
Where each arch defines its own kvm_arch. Now the changes look like a
bunch of kvm-blah to kvm-a.blah conversions.
The simplest container changes would be a bunch of kvm-blah to
to_x86(kvm)-blah conversions. How is that worse? If it's the
kvm_x86 variables you're objecting to, it would be easy enough to
remove them in favor of this approach.
It's horribly obfuscated. I'm accessing a member of a structure but
that is hidden in a no-op function call.
IIRC a downside was mentioned that it is easier to cause a build failure
for another arch now.
Opinions? In theory correctness should win over style every time, no?
Which approach is not correct?
The nicer one:
struct kvm {
struct kvm_arch arch;
// common fields
}
or the similar
struct kvm {
struct kvm_common s;
// arch specific fields
}
not correct is an exaggeration; more prone to breaking the build is
more accurate. Maybe we can set up an hourly cross-compile to
compensate. Code clarity is important to me.
--
Do not meddle in the internals of kernels, for they are subtle and quick to
panic.
-
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
On Wed, 2007-11-21 at 11:18 +0200, Avi Kivity wrote:
Carsten Otte wrote:
Hollis Blanchard wrote:
These patches are based on Xiantao's work to create struct kvm_x86. Patch
1 replaces his KVM Portability split: Splitting kvm structure (V2), and
patches 2 and 3 build on it.
Looks like a clean approach with to to_kvm_x86 macro. Whole series:
Acked-by: Carsten Otte [EMAIL PROTECTED]
Well, I hate to say it, but the resulting code doesn't look too well
(all the kvm_x86 variables), and it's entirely my fault as I recommended
this approach. Not like it was difficult to predict.
I guess we still have reached no conclusion on this question?
I'm thinking again of
struct kvm {
struct kvm_arch a;
...
}
Where each arch defines its own kvm_arch. Now the changes look like a
bunch of kvm-blah to kvm-a.blah conversions.
The simplest container changes would be a bunch of kvm-blah to
to_x86(kvm)-blah conversions. How is that worse? If it's the
kvm_x86 variables you're objecting to, it would be easy enough to
remove them in favor of this approach.
IIRC a downside was mentioned that it is easier to cause a build failure
for another arch now.
Opinions? In theory correctness should win over style every time, no?
Which approach is not correct?
--
Hollis Blanchard
IBM Linux Technology Center
-
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
Hollis Blanchard wrote: These patches are based on Xiantao's work to create struct kvm_x86. Patch 1 replaces his KVM Portability split: Splitting kvm structure (V2), and patches 2 and 3 build on it. Looks like a clean approach with to to_kvm_x86 macro. Whole series: Acked-by: Carsten Otte [EMAIL PROTECTED] - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
* Avi Kivity wrote:
Carsten Otte wrote:
Hollis Blanchard wrote:
These patches are based on Xiantao's work to create struct kvm_x86.
Patch 1 replaces his KVM Portability split: Splitting kvm structure
(V2), and patches 2 and 3 build on it.
Looks like a clean approach with to to_kvm_x86 macro. Whole series:
Acked-by: Carsten Otte [EMAIL PROTECTED]
Well, I hate to say it, but the resulting code doesn't look too well
(all the kvm_x86 variables), and it's entirely my fault as I recommended
this approach. Not like it was difficult to predict.
I'm thinking again of
struct kvm {
struct kvm_arch a;
...
}
Where each arch defines its own kvm_arch. Now the changes look like a
bunch of kvm-blah to kvm-a.blah conversions.
I was thinking the exact same thing. Having an arch-specific kvm as the
super-structure decouples 'struct kvm' from everything and it's extremely
clumsy to look at and program with. container_of() for getting struct kvm
might be better than getting the arch-specific thing.
IIRC a downside was mentioned that it is easier to cause a build failure
for another arch now.
How? I think it would work the same way in either case. Any arch-specific
stuff happen in kvm_arch and generic stuff in kvm.
Opinions? In theory correctness should win over style every time, no?
But are those two conflicting here? And if you don't like what you're looking
at, it's going to be tougher to make it correct without making it look better
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] create kvm_x86
Avi Kivity wrote:
Carsten Otte wrote:
Hollis Blanchard wrote:
These patches are based on Xiantao's work to create struct kvm_x86.
Patch 1 replaces his KVM Portability split: Splitting kvm
structure (V2), and patches 2 and 3 build on it.
Looks like a clean approach with to to_kvm_x86 macro. Whole series:
Acked-by: Carsten Otte [EMAIL PROTECTED]
Well, I hate to say it, but the resulting code doesn't look too well
(all the kvm_x86 variables), and it's entirely my fault as I
recommended this approach. Not like it was difficult to predict.
I'm thinking again of
struct kvm {
struct kvm_arch a;
...
}
Agree. The kvm_arch approach is natural, and easy to understand. I
prefer it here.
Where each arch defines its own kvm_arch. Now the changes look like a
bunch of kvm-blah to kvm-a.blah conversions.
container_of approach has similar trobles, and has to use to_kvm_x86 to
translate kvm to kvm_x86 for every arch-specific field access.
IIRC a downside was mentioned that it is easier to cause a build
failure for another arch now.
I can't figure out why it can cause more build failure.
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] [PATCH 0 of 3] create kvm_x86
These patches are based on Xiantao's work to create struct kvm_x86. Patch 1 replaces his KVM Portability split: Splitting kvm structure (V2), and patches 2 and 3 build on it. 9 files changed, 180 insertions(+), 116 deletions(-) drivers/kvm/i8259.c|1 drivers/kvm/ioapic.c | 16 --- drivers/kvm/irq.h |3 - drivers/kvm/kvm.h | 30 -- drivers/kvm/kvm_main.c | 13 ++ drivers/kvm/mmu.c | 99 drivers/kvm/vmx.c | 20 ++--- drivers/kvm/x86.c | 67 ++-- drivers/kvm/x86.h | 47 ++ - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
