* [Qemu-devel] [PATCH 2/2] acpi: remove static gpe and pci0_status variables
@ 2010-05-10 20:51 Blue Swirl
2010-05-11 11:27 ` Isaku Yamahata
0 siblings, 1 reply; 4+ messages in thread
From: Blue Swirl @ 2010-05-10 20:51 UTC (permalink / raw)
To: qemu-devel
Make gpe and pci0_status fields in PIIX4PMState.
Signed-off-by: Blue Swirl <blauwirbel@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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] acpi: remove static gpe and pci0_status variables
2010-05-10 20:51 [Qemu-devel] [PATCH 2/2] acpi: remove static gpe and pci0_status variables Blue Swirl
@ 2010-05-11 11:27 ` Isaku Yamahata
2010-05-11 20:48 ` Blue Swirl
0 siblings, 1 reply; 4+ messages in thread
From: Isaku Yamahata @ 2010-05-11 11:27 UTC (permalink / raw)
To: Blue Swirl, Gerd Hoffmann, Anthony Liguori; +Cc: qemu-devel
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.
As for code iteself, the hotplug callback argument was DeviceState
instead of PCIDevice as Gerd suggested.
[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 <blauwirbel@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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] acpi: remove static gpe and pci0_status variables
2010-05-11 11:27 ` Isaku Yamahata
@ 2010-05-11 20:48 ` Blue Swirl
2010-05-12 2:37 ` Isaku Yamahata
0 siblings, 1 reply; 4+ messages in thread
From: Blue Swirl @ 2010-05-11 20:48 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: Anthony Liguori, Gerd Hoffmann, qemu-devel
On 5/11/10, Isaku Yamahata <yamahata@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 <blauwirbel@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
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] acpi: remove static gpe and pci0_status variables
2010-05-11 20:48 ` Blue Swirl
@ 2010-05-12 2:37 ` Isaku Yamahata
0 siblings, 0 replies; 4+ messages in thread
From: Isaku Yamahata @ 2010-05-12 2:37 UTC (permalink / raw)
To: Blue Swirl; +Cc: Anthony Liguori, Gerd Hoffmann, qemu-devel
On Tue, May 11, 2010 at 11:48:24PM +0300, Blue Swirl wrote:
> On 5/11/10, Isaku Yamahata <yamahata@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.
Okay, will do.
--
yamahata
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-05-12 2:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-10 20:51 [Qemu-devel] [PATCH 2/2] acpi: remove static gpe and pci0_status variables Blue Swirl
2010-05-11 11:27 ` Isaku Yamahata
2010-05-11 20:48 ` Blue Swirl
2010-05-12 2:37 ` Isaku Yamahata
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).