Re: [PATCH RESEND v6 23/36] multi-process: create IOHUB object to handle irq

[Stefan Hajnoczi]

On Wed, Apr 22, 2020 at 09:13:58PM -0700, address@hidden wrote:
> diff --git a/include/remote/iohub.h b/include/remote/iohub.h
> new file mode 100644
> index 0000000000..7a488a8c38
> --- /dev/null
> +++ b/include/remote/iohub.h
> @@ -0,0 +1,50 @@
> +/*
> + * IO Hub for remote device
> + *
> + * Copyright © 2018, 2020 Oracle and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef REMOTE_IOHUB_H
> +#define REMOTE_IOHUB_H
> +
> +#include <sys/types.h>
> +
> +#include "qemu/osdep.h"
> +#include "hw/pci/pci.h"
> +#include "qemu/event_notifier.h"
> +#include "qemu/thread-posix.h"
> +#include "io/mpqemu-link.h"
> +
> +#define REMOTE_IOHUB_NB_PIRQS    8

We only use pirqs 4-7 based on the (slot + intx) % 4 + 4 formula. Why is
it necessary to reserve 8 irqs?

> +
> +#define REMOTE_IOHUB_DEV         31
> +#define REMOTE_IOHUB_FUNC        0
> +
> +#define TYPE_REMOTE_IOHUB_DEVICE "remote-iohub"
> +#define REMOTE_IOHUB_DEVICE(obj) \
> +    OBJECT_CHECK(RemoteIOHubState, (obj), TYPE_REMOTE_IOHUB_DEVICE)
> +
> +typedef struct ResampleToken {
> +    void *iohub;
> +    int pirq;
> +} ResampleToken;
> +
> +typedef struct RemoteIOHubState {
> +    PCIDevice d;
> +    uint8_t irq_num[PCI_SLOT_MAX][PCI_NUM_PINS];

This is a fixed mapping based on a formula. Is there any need to store
it in an array?

> diff --git a/io/mpqemu-link.c b/io/mpqemu-link.c
> index 5cb93fc47b..ea519a980e 100644
> --- a/io/mpqemu-link.c
> +++ b/io/mpqemu-link.c
> @@ -389,6 +389,7 @@ bool mpqemu_msg_valid(MPQemuMsg *msg)
>          break;
>      case BAR_WRITE:
>      case BAR_READ:
> +    case SET_IRQFD:
>          if (msg->size != sizeof(msg->data1)) {

Wait, this is checking for sizeof(msg->data1) but the message defines
its own struct!

> +void process_set_irqfd_msg(PCIDevice *pci_dev, MPQemuMsg *msg)
> +{
> +    RemMachineState *machine = REMOTE_MACHINE(current_machine);
> +    RemoteIOHubState *iohub = machine->iohub;
> +    int pirq = remote_iohub_map_irq(pci_dev, msg->data1.set_irqfd.intx);

intx has not been validated.

signature.asc
Description: PGP signature