Re: [PATCH] KVM: PPC: Add generic single register ioctls
Am 09.01.2012 um 21:12 schrieb Scott Wood : > On 01/09/2012 02:11 PM, Alexander Graf wrote: >> >> >> Am 09.01.2012 um 20:07 schrieb Scott Wood : >> >>> On 01/06/2012 06:52 PM, Alexander Graf wrote: On 06.01.2012, at 20:32, Scott Wood wrote: > On 01/05/2012 10:15 PM, Alexander Graf wrote: > >> +/* >> + * Architecture specific registers are to be defined in arch headers and >> + * ORed with the arch identifier. >> + */ >> +#define KVM_REG_PPC0x1000ULL >> +#define KVM_REG_X860x2000ULL >> +#define KVM_REG_IA640x3000ULL >> +#define KVM_REG_ARM0x4000ULL >> +#define KVM_REG_S3900x5000ULL >> + >> +#define KVM_REG_SIZE_SHIFT52 >> +#define KVM_REG_SIZE_MASK0x00f0ULL >> +#define KVM_REG_SIZE_U80xULL >> +#define KVM_REG_SIZE_U160x0010ULL >> +#define KVM_REG_SIZE_U320x0020ULL >> +#define KVM_REG_SIZE_U640x0030ULL >> +#define KVM_REG_SIZE_U1280x0040ULL >> +#define KVM_REG_SIZE_U2560x0050ULL >> +#define KVM_REG_SIZE_U5120x0060ULL >> +#define KVM_REG_SIZE_U10240x0070ULL > > Why not just encode directly as number of bytes? Because this is 1 << n bytes :) >>> >>> Some registers may not be a power-of-2 number of bytes (e.g. x86 segment >>> descriptors), and we've got plenty of space to spare in the id. >> >> If they're not a power of 2, we can still bmp it to the next power of 2 >> size, no? > > Yes, that's what I meant by it not being a huge deal and documenting > whether the padding is on the right or the left. Just seems a little > more awkward than necessary, given that we have room to encode it as a > plain linear size. Well I thought it makes things easier, since variable types are usually in a power of 2 :). But I'm certainly not attached to it. Alex > > -Scott > -- 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] KVM: PPC: Add generic single register ioctls
On 01/09/2012 02:11 PM, Alexander Graf wrote: > > > Am 09.01.2012 um 20:07 schrieb Scott Wood : > >> On 01/06/2012 06:52 PM, Alexander Graf wrote: >>> >>> >>> On 06.01.2012, at 20:32, Scott Wood wrote: >>> On 01/05/2012 10:15 PM, Alexander Graf wrote: > +/* > + * Architecture specific registers are to be defined in arch headers and > + * ORed with the arch identifier. > + */ > +#define KVM_REG_PPC0x1000ULL > +#define KVM_REG_X860x2000ULL > +#define KVM_REG_IA640x3000ULL > +#define KVM_REG_ARM0x4000ULL > +#define KVM_REG_S3900x5000ULL > + > +#define KVM_REG_SIZE_SHIFT52 > +#define KVM_REG_SIZE_MASK0x00f0ULL > +#define KVM_REG_SIZE_U80xULL > +#define KVM_REG_SIZE_U160x0010ULL > +#define KVM_REG_SIZE_U320x0020ULL > +#define KVM_REG_SIZE_U640x0030ULL > +#define KVM_REG_SIZE_U1280x0040ULL > +#define KVM_REG_SIZE_U2560x0050ULL > +#define KVM_REG_SIZE_U5120x0060ULL > +#define KVM_REG_SIZE_U10240x0070ULL Why not just encode directly as number of bytes? >>> >>> Because this is 1 << n bytes :) >> >> Some registers may not be a power-of-2 number of bytes (e.g. x86 segment >> descriptors), and we've got plenty of space to spare in the id. > > If they're not a power of 2, we can still bmp it to the next power of 2 size, > no? Yes, that's what I meant by it not being a huge deal and documenting whether the padding is on the right or the left. Just seems a little more awkward than necessary, given that we have room to encode it as a plain linear size. -Scott -- 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] KVM: PPC: Add generic single register ioctls
Am 09.01.2012 um 20:07 schrieb Scott Wood : > On 01/06/2012 06:52 PM, Alexander Graf wrote: >> >> >> On 06.01.2012, at 20:32, Scott Wood wrote: >> >>> On 01/05/2012 10:15 PM, Alexander Graf wrote: >>> +/* + * Architecture specific registers are to be defined in arch headers and + * ORed with the arch identifier. + */ +#define KVM_REG_PPC0x1000ULL +#define KVM_REG_X860x2000ULL +#define KVM_REG_IA640x3000ULL +#define KVM_REG_ARM0x4000ULL +#define KVM_REG_S3900x5000ULL + +#define KVM_REG_SIZE_SHIFT52 +#define KVM_REG_SIZE_MASK0x00f0ULL +#define KVM_REG_SIZE_U80xULL +#define KVM_REG_SIZE_U160x0010ULL +#define KVM_REG_SIZE_U320x0020ULL +#define KVM_REG_SIZE_U640x0030ULL +#define KVM_REG_SIZE_U1280x0040ULL +#define KVM_REG_SIZE_U2560x0050ULL +#define KVM_REG_SIZE_U5120x0060ULL +#define KVM_REG_SIZE_U10240x0070ULL >>> >>> Why not just encode directly as number of bytes? >> >> Because this is 1 << n bytes :) > > Some registers may not be a power-of-2 number of bytes (e.g. x86 segment > descriptors), and we've got plenty of space to spare in the id. If they're not a power of 2, we can still bmp it to the next power of 2 size, no? Alex > > It's probably not worth another respin, though -- we can just document > right/left justification in the event that we have such a register. > > -Scott > -- 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] KVM: PPC: Add generic single register ioctls
On 01/06/2012 06:52 PM, Alexander Graf wrote: > > > On 06.01.2012, at 20:32, Scott Wood wrote: > >> On 01/05/2012 10:15 PM, Alexander Graf wrote: >> >>> +/* >>> + * Architecture specific registers are to be defined in arch headers and >>> + * ORed with the arch identifier. >>> + */ >>> +#define KVM_REG_PPC0x1000ULL >>> +#define KVM_REG_X860x2000ULL >>> +#define KVM_REG_IA640x3000ULL >>> +#define KVM_REG_ARM0x4000ULL >>> +#define KVM_REG_S3900x5000ULL >>> + >>> +#define KVM_REG_SIZE_SHIFT52 >>> +#define KVM_REG_SIZE_MASK0x00f0ULL >>> +#define KVM_REG_SIZE_U80xULL >>> +#define KVM_REG_SIZE_U160x0010ULL >>> +#define KVM_REG_SIZE_U320x0020ULL >>> +#define KVM_REG_SIZE_U640x0030ULL >>> +#define KVM_REG_SIZE_U1280x0040ULL >>> +#define KVM_REG_SIZE_U2560x0050ULL >>> +#define KVM_REG_SIZE_U5120x0060ULL >>> +#define KVM_REG_SIZE_U10240x0070ULL >> >> Why not just encode directly as number of bytes? > > Because this is 1 << n bytes :) Some registers may not be a power-of-2 number of bytes (e.g. x86 segment descriptors), and we've got plenty of space to spare in the id. It's probably not worth another respin, though -- we can just document right/left justification in the event that we have such a register. -Scott -- 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] KVM: PPC: Add generic single register ioctls
On 06.01.2012, at 20:32, Scott Wood wrote: > On 01/05/2012 10:15 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. >> >> 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. > > The comment about padding is no longer relevant. > >> +/* >> + * Architecture specific registers are to be defined in arch headers and >> + * ORed with the arch identifier. >> + */ >> +#define KVM_REG_PPC0x1000ULL >> +#define KVM_REG_X860x2000ULL >> +#define KVM_REG_IA640x3000ULL >> +#define KVM_REG_ARM0x4000ULL >> +#define KVM_REG_S3900x5000ULL >> + >> +#define KVM_REG_SIZE_SHIFT52 >> +#define KVM_REG_SIZE_MASK0x00f0ULL >> +#define KVM_REG_SIZE_U80xULL >> +#define KVM_REG_SIZE_U160x0010ULL >> +#define KVM_REG_SIZE_U320x0020ULL >> +#define KVM_REG_SIZE_U640x0030ULL >> +#define KVM_REG_SIZE_U1280x0040ULL >> +#define KVM_REG_SIZE_U2560x0050ULL >> +#define KVM_REG_SIZE_U5120x0060ULL >> +#define KVM_REG_SIZE_U10240x0070ULL > > Why not just encode directly as number of bytes? Because this is 1 << n bytes :) Alex > > -Scott > -- 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] KVM: PPC: Add generic single register ioctls
On 01/05/2012 10:15 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. > > 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. The comment about padding is no longer relevant. > +/* > + * Architecture specific registers are to be defined in arch headers and > + * ORed with the arch identifier. > + */ > +#define KVM_REG_PPC 0x1000ULL > +#define KVM_REG_X86 0x2000ULL > +#define KVM_REG_IA64 0x3000ULL > +#define KVM_REG_ARM 0x4000ULL > +#define KVM_REG_S390 0x5000ULL > + > +#define KVM_REG_SIZE_SHIFT 52 > +#define KVM_REG_SIZE_MASK0x00f0ULL > +#define KVM_REG_SIZE_U8 0xULL > +#define KVM_REG_SIZE_U16 0x0010ULL > +#define KVM_REG_SIZE_U32 0x0020ULL > +#define KVM_REG_SIZE_U64 0x0030ULL > +#define KVM_REG_SIZE_U1280x0040ULL > +#define KVM_REG_SIZE_U2560x0050ULL > +#define KVM_REG_SIZE_U5120x0060ULL > +#define KVM_REG_SIZE_U1024 0x0070ULL Why not just encode directly as number of bytes? -Scott -- 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
