On 02/20/2013 07:09:55 PM, Marcelo Tosatti wrote:
On Wed, Feb 20, 2013 at 06:20:51PM -0600, Scott Wood wrote:
> On 02/20/2013 01:58:54 PM, Marcelo Tosatti wrote:
> >This is probably a stupid question, but why the
> >KVM_SET_IRQCHIP/KVM_SET_GSI_ROUTING interface is not appropriate
for
> >your purposes?
> >
> >x86 sets up a default GSI->IRQCHIP PIN mapping on creation (during
> >KVM_SET_IRQCHIP), but it can be modified with KVM_SET_GSI_ROUTING.
>
> To start, the whole IRQ routing stuff is poorly documented.
>
> Am I supposed to make up GSI numbers and use the routing thing to
> associate them with real interrupts?
I have no idea. Is mapping from one integer linear space (GSIs)
to real interrupts suitable for you?
I can live with it.
> Where does the code live to manage this table, and how APICy is it
(looks like the
> answer is "irq_comm.c, and very")?
Thinking faster than typing? Not sure what you mean.
Sorry... The code to manage the table lives in virt/kvm/irq_comm.c.
That code is very APIC-specific and not currently in a state that
invites sharing.
> It looks like I'm going to have to do this anyway for irqfd, though
> that doesn't make the other uses of the device control api go away.
> Even KVM_DEV_MPIC_GRP_IRQ_ACTIVE would still be useful for reading
> the status for debugging (reading device registers doesn't quite do
> it, since the "active" bit won't show up if the interrupt is
> masked).
> At that point, is it more offensive to make it read-only
> even though it would be trivial to make it read/write (which would
> allow users who don't need it to bypass the routing API), or to make
> it read/write and live with there being more than one way to do
> something?
Can't follow this sentence.
Suppose, for the sake of irqfd (and maillist tranquility) MPIC uses
existing KVM_IRQ_LINE and such. Will there be objection to being able
to use KVM_GET_DEVICE_ATTR to *get* the irq line status for debugging
purposes (maybe also useful for migration)? If there's no objection to
that, would there be any actual reason, beyond saving a few lines of
glue code, to make it a read-only attribute?
> KVM_SET_IRQCHIP is not suitable because we have more than 512 bytes
> of state, and because it doesn't allow debugging access to device
> registers (e.g. inspecting from the QEMU command line), and because
> it's hard to add new pieces of state if we realize we left something
> out. It reminds be of GET/SET_SREGS. With that, I did what you
> seem to want here, which is to adapt the existing interfaces, using
> feature flags to control optional state. It ended up being a mess,
> and ONE_REG was introduced as a replacement. The device control API
> is the equivalent of ONE_REG for things other than vcpus.
>
> -Scott
- ACK on 512 bytes not sufficient. Add another ioctl, SET_IRQCHIP2?
Well, that's what KVM_SET_DEVICE_ATTR is.
- Agree on poor extensibility of interface. Adding a reserved amount
of space as padding and versioning such as has been done so far
is not acceptable?
That's exactly what we did with SREGS, and it got declared a mess and
replaced with ONE_REG. I'm trying to learn from my mistakes. :-)
- Debugging: why is reading entire register state not acceptable? Yes,
its slow.
For one, it's more work. The current way works by simulating a guest
MMIO access. No blob format to design, implement, and keep compatible.
-Scott
--
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
