Re: [PATCH-for-9.1 v3 1/2] hw/pci-host/gt64120: Reset config registers during RESET phase

[Philippe Mathieu-Daudé]

+Peter who is tackling our Reset interface limitations,
+Daniel for deprecation advices.

On 1/8/24 17:37, Michael S. Tsirkin wrote:
> On Thu, Aug 01, 2024 at 05:30:38PM +0200, BALATON Zoltan wrote:
> > On Thu, 1 Aug 2024, Philippe Mathieu-Daudé wrote:
> > > Reset config values in the device RESET phase, not only once
> > > when the device is realized, because otherwise the device can
> > > use unknown values at reset.
> > >
> > > Mention the datasheet referenced. Remove the "Malta assumptions
> > > ahead" comment since the reset values from the datasheet are used.
> > >
> > > Reported-by: Michael S. Tsirkin <mst@redhat.com>
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > ---
> > > hw/pci-host/gt64120.c | 14 +++++++++++---
> > > 1 file changed, 11 insertions(+), 3 deletions(-)


> > > @@ -1231,8 +1238,9 @@ static void gt64120_pci_class_init(ObjectClass *klass, 
> > > void *data)
> > > {
> > >      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> > >      DeviceClass *dc = DEVICE_CLASS(klass);
> > > +    ResettableClass *rc = RESETTABLE_CLASS(klass);
> > >
> > > -    k->realize = gt64120_pci_realize;
> > > +    rc->phases.hold = gt64120_pci_reset_hold;
> >
> > Why reset_hold and not a simple reset method which is more usual?

DeviceReset is deprecated since 4 years now, see commit c11256aa6f
("hw/core: add Resettable support to BusClass and DeviceClass") and
the effort to convert the legacy interface to this new API:

$ git log --oneline |egrep -i 'convert.*(Resettable|3.*phase)'
cee78fa513 hw/arm/stellaris: Convert I2C controller to Resettable interface
bebd89e166 hw/arm/stellaris: Convert ADC controller to Resettable interface
ead17ebf53 hw/intc/pxa2xx: Convert to Resettable interface
d43e967f69 q800-glue.c: convert to Resettable interface
4dd5cb5d84 hw/pci-host/bonito: Convert to 3-phase reset
a0c2e80afc hw/pci-host/pnv_phb3_msi: Convert TYPE_PHB3_MSI to 3-phase reset
a359da4c62 hw/intc/xics: Convert TYPE_ICS to 3-phase reset
f4c636b0c2 pci: Convert child classes of TYPE_PCIE_ROOT_PORT to 3-phase 
reset
bb27210c8c pci: Convert TYPE_PCIE_ROOT_PORT to 3-phase reset
0d89890466 hw/display/virtio-vga: Convert TYPE_VIRTIO_VGA_BASE to 
3-phase reset
54da41834f hw/virtio: Convert TYPE_VIRTIO_PCI to 3-phase reset
d66e64dd00 target/xtensa: Convert to 3-phase reset
efcc10682e target/tricore: Convert to 3-phase reset
3b4fff1bd5 target/sparc: Convert to 3-phase reset
9049383002 target/sh4: Convert to 3-phase reset
88c41e4082 target/rx: Convert to 3-phase reset
4fa485a78e target/riscv: Convert to 3-phase reset
a1c5d644b7 target/ppc: Convert to 3-phase reset
0409750479 target/openrisc: Convert to 3-phase reset
4245a71662 target/nios2: Convert to 3-phase reset
c08dfb7ae2 target/mips: Convert to 3-phase reset
d4bc6c1a79 target/microblaze: Convert to 3-phase reset
bf90b345d7 target/m68k: Convert to 3-phase reset
f78b49ae8d target/loongarch: Convert to 3-phase reset
e86787d33b target/i386: Convert to 3-phase reset
ab85156d8a target/hexagon: Convert to 3-phase reset
1d2eb1c0c5 target/cris: Convert to 3-phase reset
605787606e target/avr: Convert to 3-phase reset
9130cade5f target/arm: Convert to 3-phase reset
3b750f1b1a hw/core/cpu-common: Convert TYPE_CPU class to 3-phase reset
ed053e8997 hw/misc: Convert TYPE_MOS6522 subclasses to 3-phase reset
8bdaed0f30 hw/misc/mos6522: Convert TYPE_MOS6522 to 3-phase reset
fc2fc3c1ed hw/input/ps2.c: Convert TYPE_PS2_{KBD, MOUSE}_DEVICE to 
3-phase reset
2bb3f93037 hw/input/ps2: Convert TYPE_PS2_DEVICE to 3-phase reset
227b5866c0 hw/intc: Convert TYPE_KVM_ARM_ITS to 3-phase reset
1bcb90762b hw/intc: Convert TYPE_ARM_GICV3_ITS to 3-phase reset
1f6887616f hw/intc: Convert TYPE_ARM_GICV3_ITS_COMMON to 3-phase reset
823300f0fc hw/intc: Convert TYPE_KVM_ARM_GICV3 to 3-phase reset
183cac319e hw/intc: Convert TYPE_ARM_GICV3_COMMON to 3-phase reset
d39270b559 hw/intc: Convert TYPE_ARM_GIC_KVM to 3-phase reset
fe3c6174f2 hw/intc: Convert TYPE_ARM_GIC_COMMON to 3-phase reset
503819a347 hw/arm: Convert TYPE_ARM_SMMUV3 to 3-phase reset
3c1a7c4197 hw/arm: Convert TYPE_ARM_SMMU to 3-phase reset
ef4989b0a8 hw/gpio/pl061: Convert to 3-phase reset and assert GPIO lines 
correctly on reset
fae5a04207 hw/rtc/mc146818rtc: Convert to 3-phase reset (Resettable 
interface)
72fe4742c6 hw/timer/etraxfs_timer: Convert to 3-phase reset (Resettable 
interface)

Peter, Daniel, do we have a way to hint developers about
deprecated API uses (like for versioned machines macros,
commit a35f8577a0 "hw: add macros for deprecation & removal
of versioned machines"), to settle on a release when API
must be converted by?

> Good point.And I'd keep it limited, e.g. wmask can be set once,
> no need to tweak it on reset.

I'll set wmask in the DeviceRealize handler.

Thanks,

Phil.