Re: [Qemu-devel] [QEMU][RFC PATCH 4/6] xen-pci: Register PCI in Xen

[Anthony Liguori <anthony@codemonkey.ws>]

On 03/22/2012 11:01 AM, Julien Grall wrote:
> QEMU will now register PCI in Xen. It will usefull to forward
> IO config space to the right QEMU.
>
> Before to register a PCI device, QEMU will check with XenStore if it is
> autorized to register the PCI associate to a given BDF.
>
> Signed-off-by: Julien Grall<julien.grall@citrix.com>

As a general rule, any time you call to XenStore from QEMU, you're doing 
something wrong.

You should explicitly launch QEMU with the PCI devices that you want it to have. 
  If there are platform devices you don't want it to have, then you need to 
construct a machine that lacks those devices.

Random hooks in non-Xen code that call into XenStore are always wrong.

Regards,

Anthony Liguori

> ---
>   hw/pci.c  |    6 +++++
>   xen-all.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 76 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index bf046bf..4df4449 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -31,6 +31,7 @@
>   #include "loader.h"
>   #include "range.h"
>   #include "qmp-commands.h"
> +#include "xen.h"
>
>   //#define DEBUG_PCI
>   #ifdef DEBUG_PCI
> @@ -764,6 +765,11 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>       pci_dev->devfn = devfn;
>       pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
>       pci_dev->irq_state = 0;
> +
> +    if (xen_enabled()&&  xen_register_pcidev(pci_dev)) {
> +        return NULL;
> +    }
> +
>       pci_config_alloc(pci_dev);
>
>       pci_config_set_vendor_id(pci_dev->config, pc->vendor_id);
> diff --git a/xen-all.c b/xen-all.c
> index 366bafe..2f5405c 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -107,6 +107,76 @@ void xen_piix3_set_irq(void *opaque, int irq_num, int level)
>                                 irq_num&  3, level);
>   }
>
> +static uint32_t str_to_bdf(const char *str)
> +{
> +    /* We assume that bdf is valid */
> +    char *buf;
> +    uint32_t bdf;
> +    uint32_t tmp;
> +
> +    bdf = strtol(str,&buf, 16);
> +    buf++;
> +
> +    str = buf;
> +    tmp = strtol(str,&buf, 16);
> +    bdf = (bdf<<  16) | (tmp<<  11);
> +    buf++;
> +
> +    str = buf;
> +    tmp = strtol(str,&buf, 16);
> +    bdf = bdf | (tmp<<  8);
> +
> +    return bdf;
> +}
> +
> +int xen_register_pcidev(PCIDevice *pci_dev)
> +{
> +    struct xs_handle *xs = NULL;
> +    uint32_t bdf = 0;
> +    uint32_t allowed_bdf = 0;
> +    char **dir;
> +    char path[50];
> +    unsigned int nb;
> +    unsigned int i;
> +    unsigned int len;
> +    char *str;
> +    int rc = 0;
> +
> +
> +    /* Fix : missing bus id to be more generic */
> +    bdf |= pci_dev->devfn<<  8;
> +
> +    xs = xs_open(0);
> +    if (!xs) {
> +        fprintf(stderr, "pci_register: Unable to open xenstore\n");
> +        return 1;
> +    }
> +
> +    snprintf(path, sizeof (path), "/local/domain/%u/image/dms/%u/pci",
> +             xen_domid, xen_dmid);
> +
> +    dir = xs_directory(xs, XBT_NULL, path,&nb);
> +    if (dir) {
> +        for (i = 0; i<  nb; i++) {
> +            snprintf(path, sizeof (path), "/local/domain/%u/image/dms/%u/pci/%s",
> +                     xen_domid, xen_dmid, dir[i]);
> +            str = xs_read(xs, XBT_NULL, path,&len);
> +            allowed_bdf = str_to_bdf(str);
> +            free(str);
> +            if (bdf == allowed_bdf)
> +            {
> +                rc = xc_hvm_register_pcidev(xen_xc, xen_domid, serverid, bdf);
> +                break;
> +            }
> +        }
> +        free(dir);
> +    }
> +
> +    xs_close(xs);
> +
> +    return rc;
> +}
> +
>   void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
>   {
>       int i;