qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/2] acpi: remove static gpe and pci0_status va


From: Blue Swirl
Subject: Re: [Qemu-devel] [PATCH 2/2] acpi: remove static gpe and pci0_status variables
Date: Tue, 11 May 2010 23:48:24 +0300

On 5/11/10, Isaku Yamahata <address@hidden> wrote:
> Hi Blue.
>  I send out very similar patches before and got acked-by from Gerd.
>  But they haven't been merged yet. Please look at them.
>  Instead of reinventing similar patches, those patches should be merged.
>  If necessary, I'm willing to rebase them and resend them.

Please resend those, I'll apply them if there are no objections. Could
you include my pm_state patch? I think the pm_state changes don't
depend on the other changes so it would be nice to keep them separate.

>  As for code iteself, the hotplug callback argument was DeviceState
>  instead of PCIDevice as Gerd suggested.

That would be slightly better.

>  [Qemu-devel] [PATCH V12 00/27] split out piix specific part from pc emulator 
> and some clean ups
>  [Qemu-devel] [PATCH V12 21/27] acpi_piix4: qdevfy.
>  [Qemu-devel] [PATCH V12 22/27] pci hotplug: add argument to pci hot plug
>  [Qemu-devel] [PATCH V12 23/27] pci hotadd, acpi_piix4: remove global var
>
>  http://lists.nongnu.org/archive/html/qemu-devel/2010-01/msg00282.html
>  http://lists.nongnu.org/archive/html/qemu-devel/2010-01/msg00297.html
>  http://lists.nongnu.org/archive/html/qemu-devel/2010-01/msg00293.html
>  http://lists.nongnu.org/archive/html/qemu-devel/2010-01/msg00285.html
>
>
>
>
>  On Mon, May 10, 2010 at 11:51:22PM +0300, Blue Swirl wrote:
>  > Make gpe and pci0_status fields in PIIX4PMState.
>  >
>  > Signed-off-by: Blue Swirl <address@hidden>
>  > ---
>  >  hw/acpi.c |   93 
> +++++++++++++++++++++++++++++++++---------------------------
>  >  hw/pc.c   |    1 -
>  >  hw/pc.h   |    1 -
>  >  hw/pci.c  |   12 +++++--
>  >  hw/pci.h  |    6 ++-
>  >  5 files changed, 63 insertions(+), 50 deletions(-)
>  >
>  > diff --git a/hw/acpi.c b/hw/acpi.c
>  > index bb2974e..6db1a12 100644
>  > --- a/hw/acpi.c
>  > +++ b/hw/acpi.c
>  > @@ -30,6 +30,20 @@
>  >
>  >  #define ACPI_DBG_IO_ADDR  0xb044
>  >
>  > +#define GPE_BASE 0xafe0
>  > +#define PCI_BASE 0xae00
>  > +#define PCI_EJ_BASE 0xae08
>  > +
>  > +struct gpe_regs {
>  > +    uint16_t sts; /* status */
>  > +    uint16_t en;  /* enabled */
>  > +};
>  > +
>  > +struct pci_status {
>  > +    uint32_t up;
>  > +    uint32_t down;
>  > +};
>  > +
>  >  typedef struct PIIX4PMState {
>  >      PCIDevice dev;
>  >      uint16_t pmsts;
>  > @@ -52,6 +66,8 @@ typedef struct PIIX4PMState {
>  >      qemu_irq cmos_s3;
>  >      qemu_irq smi_irq;
>  >      int kvm_enabled;
>  > +    struct gpe_regs gpe;
>  > +    struct pci_status pci0_status;
>  >  } PIIX4PMState;
>  >
>  >  #define RSM_STS (1 << 15)
>  > @@ -497,16 +513,19 @@ static void piix4_powerdown(void *opaque, int
>  > irq, int power_failing)
>  >      }
>  >  }
>  >
>  > +static void piix4_acpi_system_hot_add_init(PCIBus *bus, PCIDevice
>  > *hotplug_dev);
>  > +
>  >  i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
>  >                         qemu_irq sci_irq, qemu_irq cmos_s3, qemu_irq 
> smi_irq,
>  >                         int kvm_enabled)
>  >  {
>  >      PIIX4PMState *s;
>  > +    PCIDevice *d;
>  >      uint8_t *pci_conf;
>  >
>  > -    s = (PIIX4PMState *)pci_register_device(bus,
>  > -                                         "PM", sizeof(PIIX4PMState),
>  > -                                         devfn, NULL, pm_write_config);
>  > +    d = pci_register_device(bus, "PM", sizeof(PIIX4PMState), devfn, NULL,
>  > +                            pm_write_config);
>  > +    s = container_of(d, PIIX4PMState, dev);
>  >      pci_conf = s->dev.config;
>  >      pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
>  >      pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82371AB_3);
>  > @@ -557,26 +576,11 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn,
>  > uint32_t smb_io_base,
>  >      s->smi_irq = smi_irq;
>  >      qemu_register_reset(piix4_reset, s);
>  >
>  > +    piix4_acpi_system_hot_add_init(bus, d);
>  > +
>  >      return s->smbus;
>  >  }
>  >
>  > -#define GPE_BASE 0xafe0
>  > -#define PCI_BASE 0xae00
>  > -#define PCI_EJ_BASE 0xae08
>  > -
>  > -struct gpe_regs {
>  > -    uint16_t sts; /* status */
>  > -    uint16_t en;  /* enabled */
>  > -};
>  > -
>  > -struct pci_status {
>  > -    uint32_t up;
>  > -    uint32_t down;
>  > -};
>  > -
>  > -static struct gpe_regs gpe;
>  > -static struct pci_status pci0_status;
>  > -
>  >  static uint32_t gpe_read_val(uint16_t val, uint32_t addr)
>  >  {
>  >      if (addr & 1)
>  > @@ -714,46 +718,51 @@ static void pciej_write(void *opaque, uint32_t
>  > addr, uint32_t val)
>  >  #endif
>  >  }
>  >
>  > -static int piix4_device_hotplug(PCIDevice *dev, int state);
>  > +static int piix4_device_hotplug(PCIDevice *hotplug_dev, PCIDevice *dev,
>  > +                                int state);
>  >
>  > -void piix4_acpi_system_hot_add_init(PCIBus *bus)
>  > +static void piix4_acpi_system_hot_add_init(PCIBus *bus, PCIDevice 
> *hotplug_dev)
>  >  {
>  > -    register_ioport_write(GPE_BASE, 4, 1, gpe_writeb, &gpe);
>  > -    register_ioport_read(GPE_BASE, 4, 1,  gpe_readb, &gpe);
>  > +    PIIX4PMState *s = container_of(hotplug_dev, PIIX4PMState, dev);
>  >
>  > -    register_ioport_write(PCI_BASE, 8, 4, pcihotplug_write, &pci0_status);
>  > -    register_ioport_read(PCI_BASE, 8, 4,  pcihotplug_read, &pci0_status);
>  > +    register_ioport_write(GPE_BASE, 4, 1, gpe_writeb, s);
>  > +    register_ioport_read(GPE_BASE, 4, 1,  gpe_readb, s);
>  > +
>  > +    register_ioport_write(PCI_BASE, 8, 4, pcihotplug_write, s);
>  > +    register_ioport_read(PCI_BASE, 8, 4,  pcihotplug_read, s);
>  >
>  >      register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, bus);
>  >      register_ioport_read(PCI_EJ_BASE, 4, 4,  pciej_read, bus);
>  >
>  > -    pci_bus_hotplug(bus, piix4_device_hotplug);
>  > +    pci_bus_hotplug(bus, piix4_device_hotplug, hotplug_dev);
>  >  }
>  >
>  > -static void enable_device(struct pci_status *p, struct gpe_regs *g, int 
> slot)
>  > +static void enable_device(PIIX4PMState *s, int slot)
>  >  {
>  > -    g->sts |= 2;
>  > -    p->up |= (1 << slot);
>  > +    s->gpe.sts |= 2;
>  > +    s->pci0_status.up |= (1 << slot);
>  >  }
>  >
>  > -static void disable_device(struct pci_status *p, struct gpe_regs *g, int 
> slot)
>  > +static void disable_device(PIIX4PMState *s, int slot)
>  >  {
>  > -    g->sts |= 2;
>  > -    p->down |= (1 << slot);
>  > +    s->gpe.sts |= 2;
>  > +    s->pci0_status.down |= (1 << slot);
>  >  }
>  >
>  > -static int piix4_device_hotplug(PCIDevice *dev, int state)
>  > +static int piix4_device_hotplug(PCIDevice *hotplug_dev, PCIDevice *dev,
>  > +                                int state)
>  >  {
>  > -    PIIX4PMState *s = container_of(dev, PIIX4PMState, dev);
>  > +    PIIX4PMState *s = container_of(hotplug_dev, PIIX4PMState, dev);
>  >      int slot = PCI_SLOT(dev->devfn);
>  >
>  > -    pci0_status.up = 0;
>  > -    pci0_status.down = 0;
>  > -    if (state)
>  > -        enable_device(&pci0_status, &gpe, slot);
>  > -    else
>  > -        disable_device(&pci0_status, &gpe, slot);
>  > -    if (gpe.en & 2) {
>  > +    s->pci0_status.up = 0;
>  > +    s->pci0_status.down = 0;
>  > +    if (state) {
>  > +        enable_device(s, slot);
>  > +    } else {
>  > +        disable_device(s, slot);
>  > +    }
>  > +    if (s->gpe.en & 2) {
>  >          qemu_set_irq(s->irq, 1);
>  >          qemu_set_irq(s->irq, 0);
>  >      }
>  > diff --git a/hw/pc.c b/hw/pc.c
>  > index db2b9a2..e41db68 100644
>  > --- a/hw/pc.c
>  > +++ b/hw/pc.c
>  > @@ -1046,7 +1046,6 @@ static void pc_init1(ram_addr_t ram_size,
>  >              qdev_prop_set_ptr(eeprom, "data", eeprom_buf + (i * 256));
>  >              qdev_init_nofail(eeprom);
>  >          }
>  > -        piix4_acpi_system_hot_add_init(pci_bus);
>  >      }
>  >
>  >      if (i440fx_state) {
>  > diff --git a/hw/pc.h b/hw/pc.h
>  > index d11a576..088937a 100644
>  > --- a/hw/pc.h
>  > +++ b/hw/pc.h
>  > @@ -94,7 +94,6 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn,
>  > uint32_t smb_io_base,
>  >                         qemu_irq sci_irq, qemu_irq cmos_s3, qemu_irq 
> smi_irq,
>  >                         int kvm_enabled);
>  >  void piix4_smbus_register_device(SMBusDevice *dev, uint8_t addr);
>  > -void piix4_acpi_system_hot_add_init(PCIBus *bus);
>  >
>  >  /* hpet.c */
>  >  extern int no_hpet;
>  > diff --git a/hw/pci.c b/hw/pci.c
>  > index f167436..9d19cb8 100644
>  > --- a/hw/pci.c
>  > +++ b/hw/pci.c
>  > @@ -42,6 +42,7 @@ struct PCIBus {
>  >      pci_set_irq_fn set_irq;
>  >      pci_map_irq_fn map_irq;
>  >      pci_hotplug_fn hotplug;
>  > +    PCIDevice *hotplug_dev;
>  >      void *irq_opaque;
>  >      PCIDevice *devices[256];
>  >      PCIDevice *parent_dev;
>  > @@ -233,10 +234,12 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn
>  > set_irq, pci_map_irq_fn map_irq,
>  >      bus->irq_count = qemu_mallocz(nirq * sizeof(bus->irq_count[0]));
>  >  }
>  >
>  > -void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug)
>  > +void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug,
>  > +                     PCIDevice *hotplug_dev)
>  >  {
>  >      bus->qbus.allow_hotplug = 1;
>  >      bus->hotplug = hotplug;
>  > +    bus->hotplug_dev = hotplug_dev;
>  >  }
>  >
>  >  void pci_bus_set_mem_base(PCIBus *bus, target_phys_addr_t base)
>  > @@ -1660,8 +1663,9 @@ static int pci_qdev_init(DeviceState *qdev,
>  > DeviceInfo *base)
>  >          pci_dev->romfile = qemu_strdup(info->romfile);
>  >      pci_add_option_rom(pci_dev);
>  >
>  > -    if (qdev->hotplugged)
>  > -        bus->hotplug(pci_dev, 1);
>  > +    if (qdev->hotplugged) {
>  > +        bus->hotplug(bus->hotplug_dev, pci_dev, 1);
>  > +    }
>  >      return 0;
>  >  }
>  >
>  > @@ -1669,7 +1673,7 @@ static int pci_unplug_device(DeviceState *qdev)
>  >  {
>  >      PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev);
>  >
>  > -    dev->bus->hotplug(dev, 0);
>  > +    dev->bus->hotplug(dev->bus->hotplug_dev, dev, 0);
>  >      return 0;
>  >  }
>  >
>  > diff --git a/hw/pci.h b/hw/pci.h
>  > index 625188c..31e0438 100644
>  > --- a/hw/pci.h
>  > +++ b/hw/pci.h
>  > @@ -209,13 +209,15 @@ int pci_device_load(PCIDevice *s, QEMUFile *f);
>  >
>  >  typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
>  >  typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
>  > -typedef int (*pci_hotplug_fn)(PCIDevice *pci_dev, int state);
>  > +typedef int (*pci_hotplug_fn)(PCIDevice *hotplug_dev, PCIDevice *pci_dev,
>  > +                              int state);
>  >  void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
>  >                           const char *name, int devfn_min);
>  >  PCIBus *pci_bus_new(DeviceState *parent, const char *name, int devfn_min);
>  >  void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn 
> map_irq,
>  >                    void *irq_opaque, int nirq);
>  > -void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug);
>  > +void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug,
>  > +                     PCIDevice *hotplug_dev);
>  >  PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>  >                           pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>  >                           void *irq_opaque, int devfn_min, int nirq);
>  > --
>  > 1.6.2.4
>  >
>
>
> --
>
> yamahata
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]