On Wed, Feb 20, 2013 at 06:28:24PM -0300, Marcelo Tosatti wrote: > On Wed, Feb 20, 2013 at 03:09:49PM +0200, Gleb Natapov wrote: > > On Tue, Feb 19, 2013 at 03:16:37PM -0600, Scott Wood wrote: > > > On 02/19/2013 06:24:18 AM, Gleb Natapov wrote: > > > >On Mon, Feb 18, 2013 at 05:01:40PM -0600, Scott Wood wrote: > > > >> The ability to set/get attributes is needed. Sorry, but "get or set > > > >> one blob of data, up to 512 bytes, for the entire irqchip" is just > > > >> not good enough -- assuming you don't want us to start sticking > > > >> pointers and commands in *that* data. :-) > > > >> > > > >Proposed interface sticks pointers into ioctl data, so why doing > > > >the same > > > >for KVM_SET_IRQCHIP/KVM_GET_IRQCHIP makes you smile. > > > > > > There's a difference between putting a pointer in an ioctl control > > > structure that is specifically documented as being that way (as in > > > ONE_REG), versus taking an ioctl that claims to be setting/getting a > > > blob of state and embedding pointers in it. It would be like > > > sticking a pointer in the attribute payload of this API, which I > > > think is something to be discouraged. > > If documentation is what differentiate for you between silly and smart > > then write documentation instead of new interfaces. > > > > KVM_SET_IRQCHIP/KVM_GET_IRQCHIP is defined to operate on blob of data on > > x86, nothing prevent you from adding MPIC specifics to the interface, > > Add mpic state into kvm_irqchip structure and if 512 bytes is not enough > > for you to transfer the state put pointers there and _document_ them. > > But with 512 bytes you can transfer properties inline, so you probably > > do not need pointer there anyway. I see you have three properties 2 of > > them 32bit and one 64bit. > > > > > It'd also be using > > > KVM_SET_IRQCHIP to read data, which is the sort of thing you object > > > to later on regarding KVM_IRQ_LINE_STATUS. > > > > > Do not see why. > > > > > Then there's the silliness of transporting 512 bytes just to read a > > > descriptor for transporting something else. > > > > > Yes, agree. But is this enough of a reason to introduce entirely new > > interface? Is it on performance critical path? Doubt it, unless you > > abuse the interface to send interrupts, but then isn't it silty to > > do copy_from_user() twice to inject an interrupt like proposed interface > > does? > > > > > >For signaling irqs (I think this is what you mean by "commands") > > > >we have KVM_IRQ_LINE. > > > > > > It's one type of command. Another is setting the address. Another > > > is writing to registers that have side effects (this is how MSI > > > injection is done on MPIC, just as in real hardware). > > > > > Setting the address is setting an attribute. Sending MSI is a command. > > Things you set/get during init/migration are attributes. Things you do > > to cause side-effects are commands. > > Yes, it would be good to restrict what can be done via the interface > (to avoid abstracting away problems). At a first glance, i would say > it should allow for "initial configuration of device state", such as > registers etc. > > Why exactly you need to set attributes Scott? > > > > What is the benefit of KVM_IRQ_LINE over what MPIC does? What real > > > (non-glue/wrapper) code can become common? > > > > > No new ioctl with exactly same result (well actually even faster since > > less copying is done). You need to show us the benefits of the new interface > > vs existing one, not vice versa. > > Also related to this question is the point of avoiding the > implementation of devices to be spread across userspace and the kernel > (this is one point Avi brought up often). If the device emulation > is implemented entirely in the kernel, all is necessary are initial > values of device registers (and retrieval of those values later for > savevm/migration). > > It is then not necessary to set device attributes on a live guest and > deal with the complications associated with that. > > <snip> I suppose the atomic-test-and-set type operations introduced to ONE_REG ioctls are one example of such complications. Avi, can you remind us what are the drawbacks of having separate userspace/kernel device emulation? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
