Re: [PATCH 09/14] KVM: PPC: Add generic single register ioctls

Alexander Graf
2011-11-10T14:22:39Z

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