Skip to content

piix_pci cleanup

Juan Quintela qemu-devel 3 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v4 Latest16 patches 1 replyIn review
  2. v213 patches 0 repliesSuperseded
  3. v15 patches 2 repliesSuperseded

Changes in v4 from the cover letter

- rebase to today git. conflicted with now init functions return int.
- moved to new vmstate infrastructure
- fix vmstate to work well with substructures (we need subsections to do it right).

Patches in v4 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/16piix4 don't use pci_irq_levels at allhw/piix_pci.c+0 -20 replies
2/16Split piix4 support from piix_pci.cMakefile.target hw/pc.h hw/piix4.c +1 more+130 -730 replies
3/16low_set_irq is not used anywherehw/pci.c+0 -20 replies
4/16Use PCII440FXState instead of generic PCIDevicehw/pc.c hw/pc.h hw/piix_pci.c+38 -270 replies
5/16Move smm_enabled and isa_memory_mappings to PCII440FXStatehw/piix_pci.c+10 -100 replies
6/16We want the argument pass to set_irq to be opaquehw/apb_pci.c hw/grackle_pci.c hw/gt64xxx.c +10 more+37 -190 replies
7/16Create PIIX3State instead of using PCIDevice for PIIX3hw/piix_pci.c+14 -90 replies
8/16Introduce PIIX3IrqState for piix3 irq's statehw/piix_pci.c+9 -30 replies
9/16Fold piix3_init() intto i440fx_inithw/pc.c hw/pc.h hw/piix_pci.c+8 -160 replies
10/16We can add piix3_dev now to PIIX3IrqStatehw/piix_pci.c+6 -60 replies
11/16Save irq_state into PCII440FXStatehw/piix_pci.c+2 -00 replies
12/16pci_irq_levels[] belong to PIIX3Statehw/piix_pci.c+6 -70 replies
13/16Update SaveVM versionshw/piix_pci.c+25 -150 replies
14/16VMState: Fix sub-structs versioningsavevm.c+1 -10 replies
15/16i440fx: use new vmstate infrastructurehw/piix_pci.c+24 -120 replies
16/16piix3: use new vmstate infrastructurehw/piix_pci.c+11 -270 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v4 yet.

Read the v4 thread

Cover letter [Qemu-devel] [PATCH v4 00/16] piix_pci cleanup

Hi

v4:
- rebase to today git. conflicted with now init functions return int.
- moved to new vmstate infrastructure
- fix vmstate to work well with substructures (we need subsections to do it right).

Later, Juan.

v3:
- rebase to today git. Fix conflicts with avi pic/isa cleanup.

v2:
- bit the bullet, change irq_opaque to void *, instead of qemu_irq * that allows us to pass anything there.
- Rearrange piix_pci to be able to add piix3_dev to the irq_state
- Once rearrangmente is fixed, make sane SaveVM *_load/_save() functions. We can still load old state, but we only write the sane one.

mips_malta can do a similar cleanup, it is not difficult to do. But I can't test it.

About irq_state in i440fx_init(). It would have been easier, if there were a way to create i440FX-pcihost giving it the already allocated variable. Basically:

pci_bus = pci_register_bus() -> needs irq_state
i440fx = pci_create_simple(pci_bus, ..) -> needs pci_bus piix3 = pci_create_simple (pci_bus, ...) -> needs pci_bus

irq_state -> needs piix3

We have a nice circular dependency here. to fix it, I just create an empty irq_state, passed it to pci_register_bus() and fill it later.

It would have been a bit cleaner if there were a finction like i440fx = pci_create_simple_here(..., &i440fx).

Later, Juan.

v1:
- split piix4 from piix_pci. The only shared code where piix_save/load, i.e. almost nothing. Once there, compile piix4.o only for mips (it was not used anywhere else).
- Move global variables to PCII440FXState. Nice and clean until....
- pci_irq_levels: This is great:
* it is saved/loaded from i440fx
* it is cleaned during reset in piix3
* it is used in piix3_set_irq, that don't receive neither i440fx nor piix state, it needs to be a global variable. (created a global link until a better solution appear).

I looked where to "hide" pci_irq_levels and piix3_dev (both needed in piix3_set_irq), and didn't found where to pass them, out of:
- hack i8259 to hide it into PicState2 (that one got passed through the pic) ugly for words, but will work
- try to add <something> at setup_irq time, but at that point we have:
* opaque -> pci_dev of device that we are working with
* from there we can get to the bus that the device is attached to, but no way to go from there to the Host bridge (that is what we wanted in the 1st place)

Later, Juan.

Juan Quintela (16):
piix4 don't use pci_irq_levels at all
Split piix4 support from piix_pci.c
low_set_irq is not used anywhere
Use PCII440FXState instead of generic PCIDevice Move smm_enabled and isa_memory_mappings to PCII440FXState We want the argument pass to set_irq to be opaque Create PIIX3State instead of using PCIDevice for PIIX3 Introduce PIIX3IrqState for piix3 irq's state Fold piix3_init() intto i440fx_init
We can add piix3_dev now to PIIX3IrqState
Save irq_state into PCII440FXState pci_irq_levels[] belong to PIIX3State
Update SaveVM versions
VMState: Fix sub-structs versioning
i440fx: use new vmstate infrastructure
piix3: use new vmstate infrastructure

 Makefile.target    |    2 +-
 hw/apb_pci.c       |    4 +-
 hw/grackle_pci.c   |    4 +-
 hw/gt64xxx.c       |    3 +-
 hw/pc.c            |    5 +-
 hw/pc.h            |   11 ++-
 hw/pci.c           |    8 +-
 hw/pci.h           |    6 +-
 hw/piix4.c         |  128 ++++++++++++++++++++++++
 hw/piix_pci.c      |  275 +++++++++++++++++++++-------------------------------
 hw/ppc4xx_pci.c    |    4 +-
 hw/ppce500_pci.c   |    4 +-
 hw/prep_pci.c      |    4 +-
 hw/r2d.c           |    4 +-
 hw/sh_pci.c        |    4 +-
 hw/unin_pci.c      |    4 +-
 hw/versatile_pci.c |    4 +-
 savevm.c           |    2 +-
 18 files changed, 284 insertions(+), 192 deletions(-)
 create mode 100644 hw/piix4.c