On 01/03/2012 07:10 PM, 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.
If we end up replacing get/set sregs with this (either totally or just for new registers), we should consider a runtime facility for userspace to extract from the kernel a list of supported registers (with sizes) that are supported, so that userspace doesn't have to know about every register in order to migrate (this could also be useful if there is non-architecturally-readable state that needs to be preserved, such as a pending book3e doorbell exception, or the MCAR to be set when machine checks are unmasked). It could also avoid the need for explicit capabilities in some situations when new registers are added (like KVM_CAP_PPC_HIOR). > 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. Does it really make sense to consider these large things as single registers (even if that's how hw documents it)? Do they need to be set atomically? How do you get/set them in hardware if GPRs aren't that large? > +4.65 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; > + union { > + __u8 reg8; > + __u16 reg16; > + __u32 reg32; > + __u64 reg64; > + __u8 reg128[16]; > + __u8 reg256[32]; > + __u8 reg512[64]; > + __u8 reg1024[128]; > + } u; > +}; Do we need reg8/16/32, or can we simplify (and reduce potential size mismatch errors) by using __u64 for everything that doesn't need to be an array? > +/* Available with KVM_CAP_ONE_REG */ > + > +#define KVM_ONE_REG_GENERIC 0x0000000000000000ULL Generic registers? Is the idea to use this in place of dedicated ioctls for certain KVM knobs? > +/* > + * Architecture specific registers are to be defined in arch headers and > + * ORed with the arch identifier. > + */ > +#define KVM_ONE_REG_PPC 0x1000000000000000ULL > +#define KVM_ONE_REG_X86 0x2000000000000000ULL > +#define KVM_ONE_REG_IA64 0x3000000000000000ULL > +#define KVM_ONE_REG_ARM 0x4000000000000000ULL > +#define KVM_ONE_REG_S390 0x5000000000000000ULL Might want to allow space for more than 16 architectures -- there's room to spare. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html