Re: [Qemu-devel] [PATCH V10 07/15] piix_pci: Introduces Xen specific call for irq.

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/02/2011 08:49 AM, anthony.perard@citrix.com wrote:
> From: Anthony PERARD<anthony.perard@citrix.com>
>
> This patch introduces Xen specific call in piix_pci.
>
> The specific part for Xen is in write_config, set_irq and get_pirq.
>
> Signed-off-by: Anthony PERARD<anthony.perard@citrix.com>
> Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
> Acked-by: Alexander Graf<agraf@suse.de>
> ---
>   hw/piix_pci.c |   28 ++++++++++++++++++++++++++--
>   hw/xen.h      |    6 ++++++
>   xen-all.c     |   31 +++++++++++++++++++++++++++++++
>   xen-stub.c    |   13 +++++++++++++
>   4 files changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 358da58..152fcc0 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -29,6 +29,7 @@
>   #include "isa.h"
>   #include "sysbus.h"
>   #include "range.h"
> +#include "xen.h"
>
>   /*
>    * I440FX chipset data sheet.
> @@ -151,6 +152,13 @@ static void i440fx_write_config(PCIDevice *dev,
>       }
>   }
>
> +static void i440fx_write_config_xen(PCIDevice *dev,
> +                                    uint32_t address, uint32_t val, int len)
> +{
> +    xen_piix_pci_write_config_client(address, val, len);
> +    i440fx_write_config(dev, address, val, len);
> +}
> +
>   static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
>   {
>       PCII440FXState *d = opaque;
> @@ -230,13 +238,21 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq *
>       s->bus = b;
>       qdev_init_nofail(dev);
>
> -    d = pci_create_simple(b, 0, "i440FX");
> +    if (xen_enabled()) {
> +        d = pci_create_simple(b, 0, "i440FX-xen");
> +    } else {
> +        d = pci_create_simple(b, 0, "i440FX");
>    

We don't really want to have a device that magically becomes another 
device if Xen is enabled.

You should introduce an i440fx_xen_init() and make enough of the code 
common here to simply it.  We don't want to have if (xen_enabled())'s 
sprinkled through the device model.

Regards,

Anthony Liguori

> +    }
>       *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
>
>       piix3 = DO_UPCAST(PIIX3State, dev,
>                         pci_create_simple_multifunction(b, -1, true, "PIIX3"));
>       piix3->pic = pic;
> -    pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, 4);
> +    if (xen_enabled()) {
> +        pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq, piix3, 4);
> +    } else {
> +        pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, 4);
> +    }
>       (*pi440fx_state)->piix3 = piix3;
>
>       *piix3_devfn = piix3->dev.devfn;
> @@ -352,6 +368,14 @@ static PCIDeviceInfo i440fx_info[] = {
>           .init         = i440fx_initfn,
>           .config_write = i440fx_write_config,
>       },{
> +        .qdev.name    = "i440FX-xen",
> +        .qdev.desc    = "Host bridge",
> +        .qdev.size    = sizeof(PCII440FXState),
> +        .qdev.vmsd    =&vmstate_i440fx,
> +        .qdev.no_user = 1,
> +        .init         = i440fx_initfn,
> +        .config_write = i440fx_write_config_xen,
> +    },{
>           .qdev.name    = "PIIX3",
>           .qdev.desc    = "ISA bridge",
>           .qdev.size    = sizeof(PIIX3State),
> diff --git a/hw/xen.h b/hw/xen.h
> index 53a2ca4..2a53f8b 100644
> --- a/hw/xen.h
> +++ b/hw/xen.h
> @@ -8,6 +8,8 @@
>    */
>   #include<inttypes.h>
>
> +#include "qemu-common.h"
> +
>   /* xen-machine.c */
>   enum xen_mode {
>       XEN_EMULATE = 0,  // xen emulation, using xenner (default)
> @@ -29,6 +31,10 @@ static inline int xen_enabled(void)
>   #endif
>   }
>
> +int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
> +void xen_piix3_set_irq(void *opaque, int irq_num, int level);
> +void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
> +
>   void pci_xen_platform_init(PCIBus *bus);
>
>   int xen_init(int smp_cpus);
> diff --git a/xen-all.c b/xen-all.c
> index 8d77d42..123decb 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -8,9 +8,40 @@
>
>   #include "config.h"
>
> +#include "hw/pci.h"
>   #include "hw/xen_common.h"
>   #include "hw/xen_backend.h"
>
> +/* Xen specific function for piix pci */
> +
> +int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
> +{
> +    return irq_num + ((pci_dev->devfn>>  3)<<  2);
> +}
> +
> +void xen_piix3_set_irq(void *opaque, int irq_num, int level)
> +{
> +    xc_hvm_set_pci_intx_level(xen_xc, xen_domid, 0, 0, irq_num>>  2,
> +                              irq_num&  3, level);
> +}
> +
> +void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
> +{
> +    int i;
> +
> +    /* Scan for updates to PCI link routes (0x60-0x63). */
> +    for (i = 0; i<  len; i++) {
> +        uint8_t v = (val>>  (8 * i))&  0xff;
> +        if (v&  0x80) {
> +            v = 0;
> +        }
> +        v&= 0xf;
> +        if (((address + i)>= 0x60)&&  ((address + i)<= 0x63)) {
> +            xc_hvm_set_pci_link_route(xen_xc, xen_domid, address + i - 0x60, v);
> +        }
> +    }
> +}
> +
>   /* Initialise Xen */
>
>   int xen_init(int smp_cpus)
> diff --git a/xen-stub.c b/xen-stub.c
> index a6d5850..ba95537 100644
> --- a/xen-stub.c
> +++ b/xen-stub.c
> @@ -11,6 +11,19 @@
>   #include "qemu-common.h"
>   #include "hw/xen.h"
>
> +int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
> +{
> +    return -1;
> +}
> +
> +void xen_piix3_set_irq(void *opaque, int irq_num, int level)
> +{
> +}
> +
> +void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
> +{
> +}
> +
>   void pci_xen_platform_init(PCIBus *bus)
>   {
>   }
>