On 5/11/10, Isaku Yamahata <yamah...@valinux.co.jp> 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 <blauwir...@gmail.com> > > --- > > 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 >
