Re: [Xen-devel] [PATCH v2 06/20] piix4: Add a i8257 DMA Controller as specified in datasheet

[Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>]

On 10/21/19 5:25 PM, Li Qiang wrote:
> Philippe Mathieu-Daudé <philmd@xxxxxxxxxx <mailto:philmd@xxxxxxxxxx>> 于 
> 2019年10月18日周五 下午9:55写道：
>     From: Hervé Poussineau <hpoussin@xxxxxxxxxxx
>     <mailto:hpoussin@xxxxxxxxxxx>>
>
>     Remove i8257 instantiated in malta board, to not have it twice.
>
>     Acked-by: Michael S. Tsirkin <mst@xxxxxxxxxx <mailto:mst@xxxxxxxxxx>>
>     Acked-by: Paolo Bonzini <pbonzini@xxxxxxxxxx
>     <mailto:pbonzini@xxxxxxxxxx>>
>     Signed-off-by: Hervé Poussineau <hpoussin@xxxxxxxxxxx
>     <mailto:hpoussin@xxxxxxxxxxx>>
>     Message-Id: <20171216090228.28505-9-hpoussin@xxxxxxxxxxx
>     <mailto:20171216090228.28505-9-hpoussin@xxxxxxxxxxx>>
>     Reviewed-by: Aleksandar Markovic <amarkovic@xxxxxxxxxxxx
>     <mailto:amarkovic@xxxxxxxxxxxx>>
>     [PMD: rebased]
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx
>     <mailto:philmd@xxxxxxxxxx>>
>     ---
>       hw/isa/piix4.c       | 4 ++++
>       hw/mips/mips_malta.c | 2 --
>       2 files changed, 4 insertions(+), 2 deletions(-)
>
>     diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>     index ac9383a658..0b24d8323c 100644
>     --- a/hw/isa/piix4.c
>     +++ b/hw/isa/piix4.c
>     @@ -29,6 +29,7 @@
>       #include "hw/pci/pci.h"
>       #include "hw/isa/isa.h"
>       #include "hw/sysbus.h"
>     +#include "hw/dma/i8257.h"
>       #include "migration/vmstate.h"
>       #include "sysemu/reset.h"
>       #include "sysemu/runstate.h"
>     @@ -167,6 +168,9 @@ static void piix4_realize(PCIDevice *dev, Error
>     **errp)
>           /* initialize ISA irqs */
>           isa_bus_irqs(isa_bus, s->isa);
>
>     +    /* DMA */
>     +    i8257_dma_init(isa_bus, 0);
>     +
>           piix4_dev = dev;
>       }
>
>
> Could you please explain why this is better calling 'i8257_dma_init' in 
> piix4 realize function
> instead of calling it in mips_malta_init.
i8257_dma_init() is a bit misnamed as it instantiate 2x i8257.

The PIIX4 integrates 2x i8237 (very similar to the i8257).

The i8237 are parts of the PIIX4 chip, and are not chips on the Malta 
board PCB.
So when you instantiate a PIIX4 in QEMU, one expects them integrated, 
and should not have to manually manage them outside of the southbridge 
chipset.
> I'm still a little of which things should be done in realize and which 
> should be done in qom instance init function.
I remember a thread started by Peter Maydell when he was working on the 
MPS2 boards, but I can't find it.
Anyway this thread is more recent:
"Object instantiation vs. device realization: what to do when?"
https://www.mail-archive.com/qemu-devel@xxxxxxxxxx/msg596361.html

> Thanks,
> Li Qiang
>
>     diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
>     index e499b7a6bb..df247177ca 100644
>     --- a/hw/mips/mips_malta.c
>     +++ b/hw/mips/mips_malta.c
>     @@ -28,7 +28,6 @@
>       #include "cpu.h"
>       #include "hw/i386/pc.h"
>       #include "hw/isa/superio.h"
>     -#include "hw/dma/i8257.h"
>       #include "hw/char/serial.h"
>       #include "net/net.h"
>       #include "hw/boards.h"
>     @@ -1430,7 +1429,6 @@ void mips_malta_init(MachineState *machine)
>           smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100,
>                                 isa_get_irq(NULL, 9), NULL, 0, NULL);
>           pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
>     -    i8257_dma_init(isa_bus, 0);
>           mc146818_rtc_init(isa_bus, 2000, NULL);
>
>           /* generate SPD EEPROM data */
>     -- 
>     2.21.0
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel