[Qemu-devel] [PATCH 1/2] piix: use type-safe cast instead of directly access of parent dev

[Qemu-devel] [PATCH 1/2] piix: use type-safe cast instead of directly access of parent dev

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/pci-host/piix.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index f9e68c3..ba9a8f0 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -121,22 +121,24 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
 static void i440fx_update_memory_mappings(PCII440FXState *d)
 {
     int i;
+    PCIDevice *pd = PCI_DEVICE(d);
 
     memory_region_transaction_begin();
     for (i = 0; i < 13; i++) {
         pam_update(&d->pam_regions[i], i,
-                   d->dev.config[I440FX_PAM + ((i + 1) / 2)]);
+                   pd->config[I440FX_PAM + ((i + 1) / 2)]);
     }
-    smram_update(&d->smram_region, d->dev.config[I440FX_SMRAM], d->smm_enabled);
+    smram_update(&d->smram_region, pd->config[I440FX_SMRAM], d->smm_enabled);
     memory_region_transaction_commit();
 }
 
 static void i440fx_set_smm(int val, void *arg)
 {
     PCII440FXState *d = arg;
+    PCIDevice *pd = PCI_DEVICE(d);
 
     memory_region_transaction_begin();
-    smram_set_smm(&d->smm_enabled, val, d->dev.config[I440FX_SMRAM],
+    smram_set_smm(&d->smm_enabled, val, pd->config[I440FX_SMRAM],
                   &d->smram_region);
     memory_region_transaction_commit();
 }
@@ -158,9 +160,10 @@ static void i440fx_write_config(PCIDevice *dev,
 static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
 {
     PCII440FXState *d = opaque;
+    PCIDevice *pd = PCI_DEVICE(d);
     int ret, i;
 
-    ret = pci_device_load(&d->dev, f);
+    ret = pci_device_load(pd, f);
     if (ret < 0)
         return ret;
     i440fx_update_memory_mappings(d);
-- 
1.8.2.3

[Qemu-devel] [PATCH 2/2] q35: use type-safe cast instead of directly access of parent dev

[Hu Tao]

And remove variables if possible.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/pci-host/q35.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 24df6b5..efe96d0 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -36,28 +36,26 @@
 
 static int q35_host_init(SysBusDevice *dev)
 {
-    PCIBus *b;
-    PCIHostState *pci = FROM_SYSBUS(PCIHostState, dev);
-    Q35PCIHost *s = Q35_HOST_DEVICE(&dev->qdev);
+    PCIHostState *pci = PCI_HOST_BRIDGE(dev);
+    Q35PCIHost *s = Q35_HOST_DEVICE(dev);
 
     memory_region_init_io(&pci->conf_mem, &pci_host_conf_le_ops, pci,
                           "pci-conf-idx", 4);
     sysbus_add_io(dev, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
-    sysbus_init_ioports(&pci->busdev, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
+    sysbus_init_ioports(dev, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
 
     memory_region_init_io(&pci->data_mem, &pci_host_data_le_ops, pci,
                           "pci-conf-data", 4);
     sysbus_add_io(dev, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
-    sysbus_init_ioports(&pci->busdev, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
+    sysbus_init_ioports(dev, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
 
-    if (pcie_host_init(&s->host) < 0) {
+    if (pcie_host_init(PCIE_HOST_BRIDGE(dev)) < 0) {
         return -1;
     }
-    b = pci_bus_new(&s->host.pci.busdev.qdev, "pcie.0",
-                    s->mch.pci_address_space, s->mch.address_space_io,
-                    0, TYPE_PCIE_BUS);
-    s->host.pci.bus = b;
-    qdev_set_parent_bus(DEVICE(&s->mch), BUS(b));
+    pci->bus = pci_bus_new(DEVICE(dev), "pcie.0",
+                           s->mch.pci_address_space, s->mch.address_space_io,
+                           0, TYPE_PCIE_BUS);
+    qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));
     qdev_init_nofail(DEVICE(&s->mch));
 
     return 0;
@@ -105,9 +103,8 @@ static const TypeInfo q35_host_info = {
 static void mch_update_pciexbar(MCHPCIState *mch)
 {
     PCIDevice *pci_dev = &mch->d;
-    BusState *bus = qdev_get_parent_bus(&pci_dev->qdev);
+    BusState *bus = qdev_get_parent_bus(DEVICE(pci_dev));
     DeviceState *qdev = bus->parent;
-    Q35PCIHost *s = Q35_HOST_DEVICE(qdev);
 
     uint64_t pciexbar;
     int enable;
@@ -139,18 +136,19 @@ static void mch_update_pciexbar(MCHPCIState *mch)
         break;
     }
     addr = pciexbar & addr_mask;
-    pcie_host_mmcfg_update(&s->host, enable, addr, length);
+    pcie_host_mmcfg_update(PCIE_HOST_BRIDGE(qdev), enable, addr, length);
 }
 
 /* PAM */
 static void mch_update_pam(MCHPCIState *mch)
 {
+    PCIDevice *pd = PCI_DEVICE(mch);
     int i;
 
     memory_region_transaction_begin();
     for (i = 0; i < 13; i++) {
         pam_update(&mch->pam_regions[i], i,
-                   mch->d.config[MCH_HOST_BRIDGE_PAM0 + ((i + 1) / 2)]);
+                   pd->config[MCH_HOST_BRIDGE_PAM0 + ((i + 1) / 2)]);
     }
     memory_region_transaction_commit();
 }
@@ -158,8 +156,10 @@ static void mch_update_pam(MCHPCIState *mch)
 /* SMRAM */
 static void mch_update_smram(MCHPCIState *mch)
 {
+    PCIDevice *pd = PCI_DEVICE(mch);
+
     memory_region_transaction_begin();
-    smram_update(&mch->smram_region, mch->d.config[MCH_HOST_BRDIGE_SMRAM],
+    smram_update(&mch->smram_region, pd->config[MCH_HOST_BRDIGE_SMRAM],
                     mch->smm_enabled);
     memory_region_transaction_commit();
 }
@@ -167,9 +167,10 @@ static void mch_update_smram(MCHPCIState *mch)
 static void mch_set_smm(int smm, void *arg)
 {
     MCHPCIState *mch = arg;
+    PCIDevice *pd = PCI_DEVICE(mch);
 
     memory_region_transaction_begin();
-    smram_set_smm(&mch->smm_enabled, smm, mch->d.config[MCH_HOST_BRDIGE_SMRAM],
+    smram_set_smm(&mch->smm_enabled, smm, pd->config[MCH_HOST_BRDIGE_SMRAM],
                     &mch->smram_region);
     memory_region_transaction_commit();
 }
-- 
1.8.2.3

Re: [Qemu-devel] [PATCH 1/2] piix: use type-safe cast instead of directly access of parent dev

[Andreas Färber]

Hi,

Am 14.06.2013 08:49, schrieb Hu Tao:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/pci-host/piix.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index f9e68c3..ba9a8f0 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -121,22 +121,24 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
>  static void i440fx_update_memory_mappings(PCII440FXState *d)
>  {
>      int i;
> +    PCIDevice *pd = PCI_DEVICE(d);
>  
>      memory_region_transaction_begin();
>      for (i = 0; i < 13; i++) {
>          pam_update(&d->pam_regions[i], i,
> -                   d->dev.config[I440FX_PAM + ((i + 1) / 2)]);
> +                   pd->config[I440FX_PAM + ((i + 1) / 2)]);
[snip]

These two patches will likely conflict with Peter C.'s recent PCI
patchset doing the same thing.

They look okay, although in one case changing to "dev" variable will
force yet another change when later switching to QOM realize with
DeviceState *dev.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 1/2] piix: use type-safe cast instead of directly access of parent dev

[Hu Tao]

On Fri, Jun 14, 2013 at 11:20:55AM +0200, Andreas Färber wrote:
> Hi,
> 
> Am 14.06.2013 08:49, schrieb Hu Tao:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  hw/pci-host/piix.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> > index f9e68c3..ba9a8f0 100644
> > --- a/hw/pci-host/piix.c
> > +++ b/hw/pci-host/piix.c
> > @@ -121,22 +121,24 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
> >  static void i440fx_update_memory_mappings(PCII440FXState *d)
> >  {
> >      int i;
> > +    PCIDevice *pd = PCI_DEVICE(d);
> >  
> >      memory_region_transaction_begin();
> >      for (i = 0; i < 13; i++) {
> >          pam_update(&d->pam_regions[i], i,
> > -                   d->dev.config[I440FX_PAM + ((i + 1) / 2)]);
> > +                   pd->config[I440FX_PAM + ((i + 1) / 2)]);
> [snip]
> 
> These two patches will likely conflict with Peter C.'s recent PCI
> patchset doing the same thing.

Thanks for reminding! I applied these two on top of Peter's patchset,
only patch 2 conflicts on one hunk.

> 
> They look okay, although in one case changing to "dev" variable will
> force yet another change when later switching to QOM realize with
> DeviceState *dev.

Do you mean q35_host_init(SysBusDevice *dev)?  Would you like to hold it?

Re: [Qemu-devel] [PATCH 1/2] piix: use type-safe cast instead of directly access of parent dev

[Hu Tao]

Andreas,

On Mon, Jun 17, 2013 at 09:56:47AM +0800, Hu Tao wrote:
> On Fri, Jun 14, 2013 at 11:20:55AM +0200, Andreas Färber wrote:
> > Hi,
> > 
> > Am 14.06.2013 08:49, schrieb Hu Tao:
> > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > > ---
> > >  hw/pci-host/piix.c | 11 +++++++----
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> > > index f9e68c3..ba9a8f0 100644
> > > --- a/hw/pci-host/piix.c
> > > +++ b/hw/pci-host/piix.c
> > > @@ -121,22 +121,24 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
> > >  static void i440fx_update_memory_mappings(PCII440FXState *d)
> > >  {
> > >      int i;
> > > +    PCIDevice *pd = PCI_DEVICE(d);
> > >  
> > >      memory_region_transaction_begin();
> > >      for (i = 0; i < 13; i++) {
> > >          pam_update(&d->pam_regions[i], i,
> > > -                   d->dev.config[I440FX_PAM + ((i + 1) / 2)]);
> > > +                   pd->config[I440FX_PAM + ((i + 1) / 2)]);
> > [snip]
> > 
> > These two patches will likely conflict with Peter C.'s recent PCI
> > patchset doing the same thing.
> 
> Thanks for reminding! I applied these two on top of Peter's patchset,
> only patch 2 conflicts on one hunk.
> 
> > 
> > They look okay, although in one case changing to "dev" variable will
> > force yet another change when later switching to QOM realize with
> > DeviceState *dev.
> 
> Do you mean q35_host_init(SysBusDevice *dev)?  Would you like to hold it?

I'm trying to switch SysbusDevice to QOM realize if you're not working
on it.

Re: [Qemu-devel] [PATCH 1/2] piix: use type-safe cast instead of directly access of parent dev

[Peter Crosthwaite]

Hi Hu,

On Mon, Jun 17, 2013 at 1:27 PM, Hu Tao <hutao@cn.fujitsu.com> wrote:
> Andreas,
>
> On Mon, Jun 17, 2013 at 09:56:47AM +0800, Hu Tao wrote:
>> On Fri, Jun 14, 2013 at 11:20:55AM +0200, Andreas Färber wrote:
>> > Hi,
>> >
>> > Am 14.06.2013 08:49, schrieb Hu Tao:
>> > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
>> > > ---
>> > >  hw/pci-host/piix.c | 11 +++++++----
>> > >  1 file changed, 7 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> > > index f9e68c3..ba9a8f0 100644
>> > > --- a/hw/pci-host/piix.c
>> > > +++ b/hw/pci-host/piix.c
>> > > @@ -121,22 +121,24 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
>> > >  static void i440fx_update_memory_mappings(PCII440FXState *d)
>> > >  {
>> > >      int i;
>> > > +    PCIDevice *pd = PCI_DEVICE(d);
>> > >
>> > >      memory_region_transaction_begin();
>> > >      for (i = 0; i < 13; i++) {
>> > >          pam_update(&d->pam_regions[i], i,
>> > > -                   d->dev.config[I440FX_PAM + ((i + 1) / 2)]);
>> > > +                   pd->config[I440FX_PAM + ((i + 1) / 2)]);
>> > [snip]
>> >
>> > These two patches will likely conflict with Peter C.'s recent PCI
>> > patchset doing the same thing.
>>
>> Thanks for reminding! I applied these two on top of Peter's patchset,
>> only patch 2 conflicts on one hunk.
>>
>> >
>> > They look okay, although in one case changing to "dev" variable will
>> > force yet another change when later switching to QOM realize with
>> > DeviceState *dev.
>>
>> Do you mean q35_host_init(SysBusDevice *dev)?  Would you like to hold it?
>
> I'm trying to switch SysbusDevice to QOM realize if you're not working
> on it.

Thats a big job. FYI, I have already done hw/block/nand.c so you can
skip that one. Will send patches shortly.

Regards,
Peter

>

Re: [Qemu-devel] [PATCH 1/2] piix: use type-safe cast instead of directly access of parent dev

[Hu Tao]

On Mon, Jun 17, 2013 at 06:14:47PM +1000, Peter Crosthwaite wrote:
> Hi Hu,
> 
> On Mon, Jun 17, 2013 at 1:27 PM, Hu Tao <hutao@cn.fujitsu.com> wrote:
> > Andreas,
> >
> > On Mon, Jun 17, 2013 at 09:56:47AM +0800, Hu Tao wrote:
> >> On Fri, Jun 14, 2013 at 11:20:55AM +0200, Andreas Färber wrote:
> >> > Hi,
> >> >
> >> > Am 14.06.2013 08:49, schrieb Hu Tao:
> >> > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> >> > > ---
> >> > >  hw/pci-host/piix.c | 11 +++++++----
> >> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> >> > >
> >> > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >> > > index f9e68c3..ba9a8f0 100644
> >> > > --- a/hw/pci-host/piix.c
> >> > > +++ b/hw/pci-host/piix.c
> >> > > @@ -121,22 +121,24 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
> >> > >  static void i440fx_update_memory_mappings(PCII440FXState *d)
> >> > >  {
> >> > >      int i;
> >> > > +    PCIDevice *pd = PCI_DEVICE(d);
> >> > >
> >> > >      memory_region_transaction_begin();
> >> > >      for (i = 0; i < 13; i++) {
> >> > >          pam_update(&d->pam_regions[i], i,
> >> > > -                   d->dev.config[I440FX_PAM + ((i + 1) / 2)]);
> >> > > +                   pd->config[I440FX_PAM + ((i + 1) / 2)]);
> >> > [snip]
> >> >
> >> > These two patches will likely conflict with Peter C.'s recent PCI
> >> > patchset doing the same thing.
> >>
> >> Thanks for reminding! I applied these two on top of Peter's patchset,
> >> only patch 2 conflicts on one hunk.
> >>
> >> >
> >> > They look okay, although in one case changing to "dev" variable will
> >> > force yet another change when later switching to QOM realize with
> >> > DeviceState *dev.
> >>
> >> Do you mean q35_host_init(SysBusDevice *dev)?  Would you like to hold it?
> >
> > I'm trying to switch SysbusDevice to QOM realize if you're not working
> > on it.
> 
> Thats a big job. FYI, I have already done hw/block/nand.c so you can
> skip that one. Will send patches shortly.

OK. Thanks.