Re: [PATCH 09/14] KVM: PPC: Add generic single register ioctls
On 23.11.2011, at 13:47, Marcelo Tosatti wrote: On Wed, Nov 16, 2011 at 12:45:45AM +0100, Alexander Graf wrote: On 10.11.2011, at 18:35, Marcelo Tosatti wrote: On Thu, Nov 10, 2011 at 05:49:42PM +0100, Alexander Graf wrote: Documentation/virtual/kvm/api.txt | 47 ++ arch/powerpc/kvm/powerpc.c| 51 + include/linux/kvm.h | 32 +++ 3 files changed, 130 insertions(+), 0 deletions(-) I don't see the benefit of this generalization, the current structure where context information is hardcoded in the data transmitted works well. Well, unfortunately it doesn't work quite as well for us because we are a much more evolving platform. Also, there are a lot of edges and corners of the architecture that simply aren't implemented in KVM as of now. I want to have something extensible enough so we don't break the ABI along the way. You still have to agree on format between userspace and kernel, right? If either party fails to conform to that, you're doomed. Yes, but we can shove registers back and forth without allocating 8kb of ram each time. If all we need to do is poke one register, we poke one register. If we poke 10, we poke the 10 we need to touch. The problem with two interfaces is potential ambiguity: is register X implemented through KVM_GET_ONE_REG and also through KVM_GET_XYZ_REGISTER_SET ? If its accessible by two interfaces, what is the register writeback order? Is there a plan to convert, etc. Why writeback order? Register modification operations should always happen from the same thread the vCPU would run on at the end of the day, no? Yes, but there is a specified order which the registers must be written back, in case there are dependencies between them (the QEMU x86's code does its best to document these dependencies). All i'm saying is that two distinct interfaces make it potentially confusing for the programmer. That said, its up to Avi to decide. I still don't fully understand. You pass in a list of register modifications. The same would happen from guest code. You have a code stream of register modifications. They should both end up calling the same functions in the kernel at the end of the day with the same order. If you call XYZ_REGISTER_SET and then GET_ONE_REG, you get the same the guest would get. If it's difficult to implement for specific registers then just don't implement those with the ONE_REG interface. You're not forced to implement all registers with either interface - it's mostly a nicely extensible interface for architectures that evolve quite a bit with people implementing things only partially and then later realizing what's missing :). In other words, it should work great for us ppc folks and I'm fairly sure the ARM guys will appreciate it too. X86 is rather stable and well-exploited, so I can see how it doesn't make sense to use it there. Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/14] KVM: PPC: Add generic single register ioctls
On 10.11.2011, at 18:35, Marcelo Tosatti wrote: On Thu, Nov 10, 2011 at 05:49:42PM +0100, Alexander Graf wrote: Documentation/virtual/kvm/api.txt | 47 ++ arch/powerpc/kvm/powerpc.c| 51 + include/linux/kvm.h | 32 +++ 3 files changed, 130 insertions(+), 0 deletions(-) I don't see the benefit of this generalization, the current structure where context information is hardcoded in the data transmitted works well. Well, unfortunately it doesn't work quite as well for us because we are a much more evolving platform. Also, there are a lot of edges and corners of the architecture that simply aren't implemented in KVM as of now. I want to have something extensible enough so we don't break the ABI along the way. You still have to agree on format between userspace and kernel, right? If either party fails to conform to that, you're doomed. Yes, but we can shove registers back and forth without allocating 8kb of ram each time. If all we need to do is poke one register, we poke one register. If we poke 10, we poke the 10 we need to touch. The problem with two interfaces is potential ambiguity: is register X implemented through KVM_GET_ONE_REG and also through KVM_GET_XYZ_REGISTER_SET ? If its accessible by two interfaces, what is the register writeback order? Is there a plan to convert, etc. Why writeback order? Register modification operations should always happen from the same thread the vCPU would run on at the end of the day, no? Avi wanted to go as far as making this a syscall interface even. If you agree these concerns are valid, perhaps this interface can be PPC specific. I can always make it PPC specific, but I believe it would make sense as a generic interface for everyone, similar to how ENABLE_CAP can make sense for any arch. Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/14] KVM: PPC: Add generic single register ioctls
On 10/31/2011 02:36 PM, Avi Kivity wrote:
On 10/31/2011 09:53 AM, Alexander Graf wrote:
Right now we transfer a static struct every time we want to get or set
registers. Unfortunately, over time we realize that there are more of
these than we thought of before and the extensibility and flexibility of
transferring a full struct every time is limited.
So this is a new approach to the problem. With these new ioctls, we can
get and set a single register that is identified by an ID. This allows for
very precise and limited transmittal of data. When we later realize that
it's a better idea to shove over multiple registers at once, we can reuse
most of the infrastructure and simply implement a GET_MANY_REGS / SET_MANY_REGS
interface.
The only downpoint I see to this one is that it needs to pad to 1024 bits
(hardware is already on 512 bit registers, so I wanted to leave some room)
which is slightly too much for transmitting only 64 bits. But if that's all
the tradeoff we have to do for getting an extensible interface, I'd say go
for it nevertheless.
Do we want this for x86 too? How often do we want just one register?
I'm not sure. Depends on your user space I suppose :). If you want a
simple debugging tool that exposes register poking directly to user
space, then it can be handy.
+4.64 KVM_SET_ONE_REG
+
+Capability: KVM_CAP_ONE_REG
+Architectures: all
+Type: vcpu ioctl
+Parameters: struct kvm_one_reg (in)
+Returns: 0 on success, negative value on failure
+
+struct kvm_one_reg {
+ __u64 id;
would be better to have a register set (in x86 terms,
gpr/x86/sse/cr/xcr/msr/special) and an ID within the set. __u64 is
excessive, I hope.
Yeah, we have that in the ID. But since the sets are arch specific I'd
rather keep the definition of which parts of the ID are used for the set
and which are used for the actual register id inside that set to the arch.
+ union {
+ __u8 reg8;
+ __u16 reg16;
+ __u32 reg32;
+ __u64 reg64;
+ __u8 reg128[16];
+ __u8 reg256[32];
+ __u8 reg512[64];
+ __u8 reg1024[128];
+ } u;
+};
+
+Using this ioctl, a single vcpu register can be set to a specific value
+defined by user space with the passed in struct kvm_one_reg. There can
+be architecture agnostic and architecture specific registers. Each have
+their own range of operation and their own constants and width. To keep
+track of the implemented registers, find a list below:
+
+ Arch | Register| Width (bits)
+| |
+
One possible issue is that certain register have mutually exclusive
values, so you may need to issue multiple calls to get the right
sequence. You probably don't have that on ppc.
I'm fairly sure we don't. But even if so, it's the same as running code
inside the guest, so it should come natural, no?
Alex
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/14] KVM: PPC: Add generic single register ioctls
On Thu, Nov 10, 2011 at 05:49:42PM +0100, Alexander Graf wrote: Documentation/virtual/kvm/api.txt | 47 ++ arch/powerpc/kvm/powerpc.c| 51 + include/linux/kvm.h | 32 +++ 3 files changed, 130 insertions(+), 0 deletions(-) I don't see the benefit of this generalization, the current structure where context information is hardcoded in the data transmitted works well. Well, unfortunately it doesn't work quite as well for us because we are a much more evolving platform. Also, there are a lot of edges and corners of the architecture that simply aren't implemented in KVM as of now. I want to have something extensible enough so we don't break the ABI along the way. You still have to agree on format between userspace and kernel, right? If either party fails to conform to that, you're doomed. The problem with two interfaces is potential ambiguity: is register X implemented through KVM_GET_ONE_REG and also through KVM_GET_XYZ_REGISTER_SET ? If its accessible by two interfaces, what is the register writeback order? Is there a plan to convert, etc. If you agree these concerns are valid, perhaps this interface can be PPC specific. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/14] KVM: PPC: Add generic single register ioctls
On 10/31/2011 09:53 AM, Alexander Graf wrote:
Right now we transfer a static struct every time we want to get or set
registers. Unfortunately, over time we realize that there are more of
these than we thought of before and the extensibility and flexibility of
transferring a full struct every time is limited.
So this is a new approach to the problem. With these new ioctls, we can
get and set a single register that is identified by an ID. This allows for
very precise and limited transmittal of data. When we later realize that
it's a better idea to shove over multiple registers at once, we can reuse
most of the infrastructure and simply implement a GET_MANY_REGS /
SET_MANY_REGS
interface.
The only downpoint I see to this one is that it needs to pad to 1024 bits
(hardware is already on 512 bit registers, so I wanted to leave some room)
which is slightly too much for transmitting only 64 bits. But if that's all
the tradeoff we have to do for getting an extensible interface, I'd say go
for it nevertheless.
Do we want this for x86 too? How often do we want just one register?
+4.64 KVM_SET_ONE_REG
+
+Capability: KVM_CAP_ONE_REG
+Architectures: all
+Type: vcpu ioctl
+Parameters: struct kvm_one_reg (in)
+Returns: 0 on success, negative value on failure
+
+struct kvm_one_reg {
+ __u64 id;
would be better to have a register set (in x86 terms,
gpr/x86/sse/cr/xcr/msr/special) and an ID within the set. __u64 is
excessive, I hope.
+ union {
+ __u8 reg8;
+ __u16 reg16;
+ __u32 reg32;
+ __u64 reg64;
+ __u8 reg128[16];
+ __u8 reg256[32];
+ __u8 reg512[64];
+ __u8 reg1024[128];
+ } u;
+};
+
+Using this ioctl, a single vcpu register can be set to a specific value
+defined by user space with the passed in struct kvm_one_reg. There can
+be architecture agnostic and architecture specific registers. Each have
+their own range of operation and their own constants and width. To keep
+track of the implemented registers, find a list below:
+
+ Arch | Register| Width (bits)
+| |
+
One possible issue is that certain register have mutually exclusive
values, so you may need to issue multiple calls to get the right
sequence. You probably don't have that on ppc.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/14] KVM: PPC: Add generic single register ioctls
On 2011-10-31 14:36, Avi Kivity wrote: On 10/31/2011 09:53 AM, Alexander Graf wrote: Right now we transfer a static struct every time we want to get or set registers. Unfortunately, over time we realize that there are more of these than we thought of before and the extensibility and flexibility of transferring a full struct every time is limited. So this is a new approach to the problem. With these new ioctls, we can get and set a single register that is identified by an ID. This allows for very precise and limited transmittal of data. When we later realize that it's a better idea to shove over multiple registers at once, we can reuse most of the infrastructure and simply implement a GET_MANY_REGS / SET_MANY_REGS interface. The only downpoint I see to this one is that it needs to pad to 1024 bits (hardware is already on 512 bit registers, so I wanted to leave some room) which is slightly too much for transmitting only 64 bits. But if that's all the tradeoff we have to do for getting an extensible interface, I'd say go for it nevertheless. Do we want this for x86 too? How often do we want just one register? On x86, a single register is probably only interesting for debugging purposes. Things that matter (performance wise) are in kvm.run, anything else does not worry that much about speed. At least for now. I'm still waiting for Kemari to propose some get/set optimizations, but there is obviously not that much to gain or still bigger fish to fry. Also, x86 is less regular than PPC. And where it is fairly regular, we already have a GET/SET_MANY interface: MSRs. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
