On Thu, Oct 24, 2019 at 05:08:59AM -0400, Jagannathan Raman wrote:

I don't know the interrupt code well enough to decide whether it's
necessary to do so much work and tie the protocol to the KVM API.  The
main QEMU process already has the KVM API code and the ability to deal
with these things.  I was expecting something much simpler, like
protocol messages that pass a single eventfd for raising an interrupt
and no state tracking interrupt levels.

> +static void intr_resample_handler(void *opaque)
> +{
> +    ResampleToken *token = opaque;
> +    RemoteIOHubState *iohub = token->iohub;
> +    uint64_t val;
> +    int pirq, s;
> +
> +    pirq = token->pirq;
> +
> +    s = read(event_notifier_get_fd(&iohub->resamplefds[pirq]), &val,
> +             sizeof(uint64_t));

Please use event_notifier_test_and_clear().

> +
> +    assert(s >= 0);
> +
> +    qemu_mutex_lock(&iohub->irq_level_lock[pirq]);
> +
> +    if (iohub->irq_level[pirq]) {
> +        event_notifier_set(&iohub->irqfds[pirq]);
> +    }
> +
> +    qemu_mutex_unlock(&iohub->irq_level_lock[pirq]);
> +}
> +
> +void process_set_irqfd_msg(PCIDevice *pci_dev, MPQemuMsg *msg)

This function doesn't handle the case where SET_IRQFD is sent multiple
times for the same interrupt gracefully.

> +{
> +    RemMachineState *machine = REMOTE_MACHINE(current_machine);
> +    RemoteIOHubState *iohub = machine->iohub;
> +    ResampleToken *token;
> +    int pirq = remote_iohub_map_irq(pci_dev, msg->data1.set_irqfd.intx);
> +
> +    assert(msg->num_fds == 2);
> +
> +    event_notifier_init_fd(&iohub->irqfds[pirq], msg->fds[0]);
> +    event_notifier_init_fd(&iohub->resamplefds[pirq], msg->fds[1]);

event_notifier_cleanup() is missing.

> +
> +    token = g_malloc0(sizeof(ResampleToken));

I couldn't find a g_free() and wonder if this needs to be malloced at
all.