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

[Esteban Bosse]

El vie, 18-10-2019 a las 15:47 +0200, Philippe Mathieu-Daudé escribió:
> From: Hervé Poussineau 
> 
> Remove i8257 instantiated in malta board, to not have it twice.
> 
> Acked-by: Michael S. Tsirkin 
> Acked-by: Paolo Bonzini 
> Signed-off-by: Hervé Poussineau 
> Message-Id: <20171216090228.28505-9-hpous...@reactos.org>
> Reviewed-by: Aleksandar Markovic 
> [PMD: rebased]
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  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;
>  }
>  
> 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 */
Reviewed-by: Esteban Bosse 


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Philippe Mathieu-Daudé]

On 10/21/19 5:25 PM, Li Qiang wrote:



Philippe Mathieu-Daudé mailto:phi...@redhat.com>> 于 
2019年10月18日周五 下午9:55写道：


From: Hervé Poussineau mailto:hpous...@reactos.org>>

Remove i8257 instantiated in malta board, to not have it twice.

Acked-by: Michael S. Tsirkin mailto:m...@redhat.com>>
Acked-by: Paolo Bonzini mailto:pbonz...@redhat.com>>
Signed-off-by: Hervé Poussineau mailto:hpous...@reactos.org>>
Message-Id: <20171216090228.28505-9-hpous...@reactos.org
>
Reviewed-by: Aleksandar Markovic mailto:amarko...@wavecomp.com>>
[PMD: rebased]
Signed-off-by: Philippe Mathieu-Daudé mailto:phi...@redhat.com>>
---
  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@nongnu.org/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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Li Qiang]

Philippe Mathieu-Daudé  于2019年10月18日周五 下午9:55写道：

> From: Hervé Poussineau 
>
> Remove i8257 instantiated in malta board, to not have it twice.
>
> Acked-by: Michael S. Tsirkin 
> Acked-by: Paolo Bonzini 
> Signed-off-by: Hervé Poussineau 
> Message-Id: <20171216090228.28505-9-hpous...@reactos.org>
> Reviewed-by: Aleksandar Markovic 
> [PMD: rebased]
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  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.

I'm still a little of which things should be done in realize and which
should be done in qom instance init function.

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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel