Patchwork [12/15] piix_pci: introduce a write_config notifier

login
register
mail settings
Submitter Stefano Stabellini
Date Aug. 12, 2010, 2:09 p.m.
Message ID <1281622202-3453-12-git-send-email-stefano.stabellini@eu.citrix.com>
Download mbox | patch
Permalink /patch/61612/
State New
Headers show

Comments

Stefano Stabellini - Aug. 12, 2010, 2:09 p.m.
From: Anthony PERARD <anthony.perard@citrix.com>

Introduce a write config notifier in piix_pci, so that clients can be
notified every time a pci config write happens.
The patch also makes use of the notification mechanism in
xen_machine_fv.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/pc.h             |    1 +
 hw/piix_pci.c       |   28 ++++++++++++++++++++++++++++
 hw/xen_machine_fv.c |   16 ++++++++++++++++
 3 files changed, 45 insertions(+), 0 deletions(-)
Blue Swirl - Aug. 12, 2010, 6:35 p.m.
On Thu, Aug 12, 2010 at 2:09 PM,  <stefano.stabellini@eu.citrix.com> wrote:
> From: Anthony PERARD <anthony.perard@citrix.com>
>
> Introduce a write config notifier in piix_pci, so that clients can be
> notified every time a pci config write happens.
> The patch also makes use of the notification mechanism in
> xen_machine_fv.

Will the mechanism be used elsewhere? If not, I'd just add a call to
xen_piix_pci_write_config_client() to piix_pci.c. It can be surrounded
by Xen #ifdeffery, or you could introduce stubs like kvm-stub.c and
friends.

>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  hw/pc.h             |    1 +
>  hw/piix_pci.c       |   28 ++++++++++++++++++++++++++++
>  hw/xen_machine_fv.c |   16 ++++++++++++++++
>  3 files changed, 45 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pc.h b/hw/pc.h
> index ee562cd..3a745ae 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -141,6 +141,7 @@ typedef struct PCII440FXState PCII440FXState;
>
>  void piix3_register_set_irq(pci_set_irq_fn set_irq);
>  void piix3_register_map_irq(pci_map_irq_fn map_irq);
> +void piix_pci_register_write_config_notifier(PCII440FXState *d, PCIConfigWriteFunc *write_config);
>  PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn, qemu_irq *pic, ram_addr_t ram_size);
>  void i440fx_init_memory_mappings(PCII440FXState *d);
>
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 56e3f61..afa9e9d 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -49,6 +49,13 @@ struct PCII440FXState {
>     PIIX3State *piix3;
>  };
>
> +typedef struct PCII440FXWriteConfigNotifier {
> +    PCIConfigWriteFunc *write_config;
> +    QLIST_ENTRY(PCII440FXWriteConfigNotifier) next;
> +} PCII440FXWriteConfigNotifier;
> +
> +static QLIST_HEAD(write_config_list, PCII440FXWriteConfigNotifier) write_config_list
> +    = QLIST_HEAD_INITIALIZER(write_config_list);
>
>  #define I440FX_PAM      0x59
>  #define I440FX_PAM_SIZE 7
> @@ -71,6 +78,25 @@ void piix3_register_map_irq(pci_map_irq_fn map_irq)
>     piix3_map_irq_handler = map_irq;
>  }
>
> +void piix_pci_register_write_config_notifier(PCII440FXState *d, PCIConfigWriteFunc *write_config)
> +{
> +    PCII440FXWriteConfigNotifier *new_notifier;
> +
> +    assert(write_config);
> +    new_notifier = qemu_mallocz(sizeof(PCII440FXWriteConfigNotifier));
> +    new_notifier->write_config = write_config;
> +    QLIST_INSERT_HEAD(&write_config_list, new_notifier, next);
> +}
> +
> +static void piix_pci_notify_write_config(PCIDevice *dev, uint32_t address, uint32_t val, int len)
> +{
> +    PCII440FXWriteConfigNotifier *notifier;
> +
> +    QLIST_FOREACH(notifier, &write_config_list, next) {
> +        notifier->write_config(dev, address, val, len);
> +    }
> +}
> +
>  /* return the global irq number corresponding to a given device irq
>    pin. We could also use the bus number to have a more precise
>    mapping. */
> @@ -157,6 +183,8 @@ static void i440fx_write_config(PCIDevice *dev,
>  {
>     PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev);
>
> +    piix_pci_notify_write_config(dev, address, val, len);
> +
>     /* XXX: implement SMRAM.D_LOCK */
>     pci_default_write_config(dev, address, val, len);
>     if (ranges_overlap(address, len, I440FX_PAM, I440FX_PAM_SIZE) ||
> diff --git a/hw/xen_machine_fv.c b/hw/xen_machine_fv.c
> index 5d553b6..77563db 100644
> --- a/hw/xen_machine_fv.c
> +++ b/hw/xen_machine_fv.c
> @@ -61,6 +61,21 @@ static void xen_piix3_set_irq(void *opaque, int irq_num, int level)
>             irq_num & 3, level);
>  }
>
> +static void xen_piix_pci_write_config_client(PCIDevice *dev,
> +        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);
> +    }
> +}
>
>  static void xen_init_fv(ram_addr_t ram_size,
>                         const char *boot_device,
> @@ -141,6 +156,7 @@ static void xen_init_fv(ram_addr_t ram_size,
>     piix3_register_set_irq(xen_piix3_set_irq);
>     piix3_register_map_irq(xen_piix3_map_irq);
>     pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, isa_irq, ram_size);
> +    piix_pci_register_write_config_notifier(i440fx_state, xen_piix_pci_write_config_client);
>     isa_bus_irqs(isa_irq);
>
>     pc_register_ferr_irq(isa_reserve_irq(13));
> --
> 1.7.0.4
>
>
>
Stefano Stabellini - Aug. 13, 2010, 1:10 p.m.
On Thu, 12 Aug 2010, Blue Swirl wrote:
> On Thu, Aug 12, 2010 at 2:09 PM,  <stefano.stabellini@eu.citrix.com> wrote:
> > From: Anthony PERARD <anthony.perard@citrix.com>
> >
> > Introduce a write config notifier in piix_pci, so that clients can be
> > notified every time a pci config write happens.
> > The patch also makes use of the notification mechanism in
> > xen_machine_fv.
> 
> Will the mechanism be used elsewhere? If not, I'd just add a call to
> xen_piix_pci_write_config_client() to piix_pci.c. It can be surrounded
> by Xen #ifdeffery, or you could introduce stubs like kvm-stub.c and
> friends.
> 

we were trying to avoid ifdef's in piix_pci, but if you are OK with just a
couple of them we'll gladly remove the hook.
Michael S. Tsirkin - Sept. 5, 2010, 7:34 a.m.
On Fri, Aug 13, 2010 at 02:10:01PM +0100, Stefano Stabellini wrote:
> On Thu, 12 Aug 2010, Blue Swirl wrote:
> > On Thu, Aug 12, 2010 at 2:09 PM,  <stefano.stabellini@eu.citrix.com> wrote:
> > > From: Anthony PERARD <anthony.perard@citrix.com>
> > >
> > > Introduce a write config notifier in piix_pci, so that clients can be
> > > notified every time a pci config write happens.
> > > The patch also makes use of the notification mechanism in
> > > xen_machine_fv.
> > 
> > Will the mechanism be used elsewhere? If not, I'd just add a call to
> > xen_piix_pci_write_config_client() to piix_pci.c. It can be surrounded
> > by Xen #ifdeffery, or you could introduce stubs like kvm-stub.c and
> > friends.
> > 
> 
> we were trying to avoid ifdef's in piix_pci, but if you are OK with just a
> couple of them we'll gladly remove the hook.
> 

I second this. Callbacks complicate code significantly.
If there's a single user we are better off without.

Patch

diff --git a/hw/pc.h b/hw/pc.h
index ee562cd..3a745ae 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -141,6 +141,7 @@  typedef struct PCII440FXState PCII440FXState;
 
 void piix3_register_set_irq(pci_set_irq_fn set_irq);
 void piix3_register_map_irq(pci_map_irq_fn map_irq);
+void piix_pci_register_write_config_notifier(PCII440FXState *d, PCIConfigWriteFunc *write_config);
 PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn, qemu_irq *pic, ram_addr_t ram_size);
 void i440fx_init_memory_mappings(PCII440FXState *d);
 
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 56e3f61..afa9e9d 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -49,6 +49,13 @@  struct PCII440FXState {
     PIIX3State *piix3;
 };
 
+typedef struct PCII440FXWriteConfigNotifier {
+    PCIConfigWriteFunc *write_config;
+    QLIST_ENTRY(PCII440FXWriteConfigNotifier) next;
+} PCII440FXWriteConfigNotifier;
+
+static QLIST_HEAD(write_config_list, PCII440FXWriteConfigNotifier) write_config_list
+    = QLIST_HEAD_INITIALIZER(write_config_list);
 
 #define I440FX_PAM      0x59
 #define I440FX_PAM_SIZE 7
@@ -71,6 +78,25 @@  void piix3_register_map_irq(pci_map_irq_fn map_irq)
     piix3_map_irq_handler = map_irq;
 }
 
+void piix_pci_register_write_config_notifier(PCII440FXState *d, PCIConfigWriteFunc *write_config)
+{
+    PCII440FXWriteConfigNotifier *new_notifier;
+
+    assert(write_config);
+    new_notifier = qemu_mallocz(sizeof(PCII440FXWriteConfigNotifier));
+    new_notifier->write_config = write_config;
+    QLIST_INSERT_HEAD(&write_config_list, new_notifier, next);
+}
+
+static void piix_pci_notify_write_config(PCIDevice *dev, uint32_t address, uint32_t val, int len)
+{
+    PCII440FXWriteConfigNotifier *notifier;
+
+    QLIST_FOREACH(notifier, &write_config_list, next) {
+        notifier->write_config(dev, address, val, len);
+    }
+}
+
 /* return the global irq number corresponding to a given device irq
    pin. We could also use the bus number to have a more precise
    mapping. */
@@ -157,6 +183,8 @@  static void i440fx_write_config(PCIDevice *dev,
 {
     PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev);
 
+    piix_pci_notify_write_config(dev, address, val, len);
+
     /* XXX: implement SMRAM.D_LOCK */
     pci_default_write_config(dev, address, val, len);
     if (ranges_overlap(address, len, I440FX_PAM, I440FX_PAM_SIZE) ||
diff --git a/hw/xen_machine_fv.c b/hw/xen_machine_fv.c
index 5d553b6..77563db 100644
--- a/hw/xen_machine_fv.c
+++ b/hw/xen_machine_fv.c
@@ -61,6 +61,21 @@  static void xen_piix3_set_irq(void *opaque, int irq_num, int level)
             irq_num & 3, level);
 }
 
+static void xen_piix_pci_write_config_client(PCIDevice *dev,
+        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);
+    }
+}
 
 static void xen_init_fv(ram_addr_t ram_size,
                         const char *boot_device,
@@ -141,6 +156,7 @@  static void xen_init_fv(ram_addr_t ram_size,
     piix3_register_set_irq(xen_piix3_set_irq);
     piix3_register_map_irq(xen_piix3_map_irq);
     pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, isa_irq, ram_size);
+    piix_pci_register_write_config_notifier(i440fx_state, xen_piix_pci_write_config_client);
     isa_bus_irqs(isa_irq);
 
     pc_register_ferr_irq(isa_reserve_irq(13));