Re: [PATCH RESEND v6 24/36] multi-process: Retrieve PCI info from remote process

[Stefan Hajnoczi]

On Wed, Apr 22, 2020 at 09:13:59PM -0700, address@hidden wrote:
> @@ -291,3 +299,79 @@ const MemoryRegionOps proxy_default_ops = {
>          .max_access_size = 1,
>      },
>  };
> +
> +static void probe_pci_info(PCIDevice *dev)
> +{
> +    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> +    DeviceClass *dc = DEVICE_CLASS(pc);
> +    PCIProxyDev *pdev = PCI_PROXY_DEV(dev);
> +    MPQemuLinkState *mpqemu_link = pdev->mpqemu_link;
> +    MPQemuMsg msg, ret;
> +    uint32_t orig_val, new_val, class;
> +    uint8_t type;
> +    int i, size;
> +    char *name;
> +
> +    memset(&msg, 0, sizeof(MPQemuMsg));
> +    msg.bytestream = 0;
> +    msg.size = 0;
> +    msg.cmd = GET_PCI_INFO;
> +    mpqemu_msg_send(&msg, mpqemu_link->dev);
> +
> +    mpqemu_msg_recv(&ret, mpqemu_link->dev);
> +
> +    pc->vendor_id = ret.data1.ret_pci_info.vendor_id;
> +    pc->device_id = ret.data1.ret_pci_info.device_id;
> +    pc->class_id = ret.data1.ret_pci_info.class_id;
> +    pc->subsystem_id = ret.data1.ret_pci_info.subsystem_id;
> +

Why introduce the GET_PCI_INFO/RET_PCI_INFO messages when the same
information can already be fetched using PCI_CONFIG_READ?

> +    config_op_send(pdev, 11, &class, 1, PCI_CONFIG_READ);

Please use constants like PCI_CLASS_DEVICE instead of magic numbers.

> diff --git a/include/io/mpqemu-link.h b/include/io/mpqemu-link.h
> index 14e4be2bd0..102c736705 100644
> --- a/include/io/mpqemu-link.h
> +++ b/include/io/mpqemu-link.h
> @@ -48,6 +48,8 @@ typedef enum {
>      BAR_WRITE,
>      BAR_READ,
>      SET_IRQFD,
> +    GET_PCI_INFO,
> +    RET_PCI_INFO,

Ah, I see reply messages are being used after all. I suggest dropping
the eventfd wait mechanism and using reply messages instead. Otherwise
you need to duplicate the wait timeout for reply messages. Using
temporary eventfds is more complex and maybe also slower.

signature.asc
Description: PGP signature