Re: [PATCH v7 13/21] multi-process: Forward PCI config space acceses to the remote process

[Stefan Hajnoczi]

On Sat, Jun 27, 2020 at 10:09:35AM -0700, elena.ufimtseva@oracle.com wrote:
> @@ -42,6 +48,12 @@ gboolean mpqemu_process_msg(QIOChannel *ioc, GIOCondition 
> cond,
>      case CONNECT_DEV:
>          process_connect_dev_msg(&msg, ioc, &local_err);
>          break;
> +    case PCI_CONFIG_WRITE:
> +        process_config_write(ioc, pci_dev, &msg);
> +        break;
> +    case PCI_CONFIG_READ:
> +        process_config_read(ioc, pci_dev, &msg);
> +        break;

pci_dev is NULL when mpqemu_process_msg() is called on the main socket.
This is an example of how the N:1 model complicates things.  Now
process_config_read/write() need to check that pci_dev is non-NULL to
avoid crashing.

>      default:
>          error_setg(&local_err, "Unknown command (%d) received from proxy \
>                     in remote process pid=%d", msg.cmd, getpid());
> @@ -89,3 +101,45 @@ exit:
>  
>      mpqemu_msg_send(&ret, com);
>  }
> +
> +static void process_config_write(QIOChannel *ioc, PCIDevice *dev,
> +                                 MPQemuMsg *msg)
> +{
> +    struct conf_data_msg *conf = (struct conf_data_msg *)msg->data2;
> +    MPQemuMsg ret = { 0 };
> +
> +    if (conf->addr >= PCI_CFG_SPACE_EXP_SIZE) {

This check treats all devices as PCIe devices. Traditional PCI devices
have a smaller config space and pci_default_write_config() has an
assertion that fails on out-of-bounds writes:

  assert(addr + l <= pci_config_size(d));

Are you sure all devices are PCIe? If yes, please enforce that in the
code. If no, then please fix the size check.

> +struct conf_data_msg {
> +    uint32_t addr;
> +    uint32_t val;
> +    int l;
> +};

QEMU coding style uses typedefs:

  typedef struct {
      uint32_t addr;
      uint32_t val;
      int l;
  } ConfDataMsg;

signature.asc
Description: PGP signature