[Qemu-devel] [PATCH v2 0/3] Improve 40p, make AIX 5.1 boot

[Qemu-devel] [PATCH v2 0/3] Improve 40p, make AIX 5.1 boot

[Artyom Tarasenko]

This series makes various fixes for the -M 40p target.
Particularly now it's possible to boot AIX 5.1 (both from a cdrom
and a hard disk).

The OFW image: 
https://github.com/artyom-tarasenko/openfirmware/releases/download/20190406-AIX-boots/q40pofw-serial.rom

The command line:
qemu-system-ppc -M 40p -bios q40pofw-serial.rom -serial telnet::4441,server -hda aix-hdd.qcow2 -cdrom \
/path/to/aix-5.1-cd1.iso  -vga none -nographic

More info can be found here:
https://tyom.blogspot.com/2019/04/aixprep-under-qemu-how-to.html

Artyom Tarasenko (3):
  lsi53c895a: hide 53c895a registers in 53c810
  40p and prep: implement PCI bus mastering
  hw/isa/i82378.c: use 1900 as a base year

 hw/isa/i82378.c      |  4 +++-
 hw/pci-host/prep.c   | 15 +++++++++++----
 hw/scsi/lsi53c895a.c | 17 ++++++++++++++---
 3 files changed, 28 insertions(+), 8 deletions(-)

-- 
2.7.2

[Qemu-devel] [PATCH v2 1/3] lsi53c895a: hide 53c895a registers in 53c810

[Artyom Tarasenko]

AIX/PReP does access to the aliased IO registers of 53810.
Implement aliasing to make the AIX driver work.

Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 hw/scsi/lsi53c895a.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index da7239d..6b95699 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2271,6 +2271,9 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
     LSIState *s = LSI53C895A(dev);
     DeviceState *d = DEVICE(dev);
     uint8_t *pci_conf;
+    uint64_t mmio_size;
+    MemoryRegion *mr;
+    uint16_t type = PCI_DEVICE_GET_CLASS(dev)->device_id;
 
     pci_conf = dev->config;
 
@@ -2279,13 +2282,21 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
     /* Interrupt pin A */
     pci_conf[PCI_INTERRUPT_PIN] = 0x01;
 
-    memory_region_init_io(&s->mmio_io, OBJECT(s), &lsi_mmio_ops, s,
-                          "lsi-mmio", 0x400);
     memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
                           "lsi-ram", 0x2000);
     memory_region_init_io(&s->io_io, OBJECT(s), &lsi_io_ops, s,
                           "lsi-io", 256);
-
+    if (type == PCI_DEVICE_ID_LSI_53C895A) {
+        mmio_size = 0x400;
+    } else {
+        mr = g_new(MemoryRegion, 1);
+        memory_region_init_alias(mr, OBJECT(d), "lsi-io-alias", &s->io_io,
+                                 0, 0x80);
+        memory_region_add_subregion_overlap(&s->io_io, 0x80, mr, -1);
+        mmio_size = 0x80;
+    }
+    memory_region_init_io(&s->mmio_io, OBJECT(s), &lsi_mmio_ops, s,
+                          "lsi-mmio", mmio_size);
     address_space_init(&s->pci_io_as, pci_address_space_io(dev), "lsi-pci-io");
     qdev_init_gpio_out(d, &s->ext_irq, 1);
 
-- 
2.7.2

[Qemu-devel] [PATCH v2 1/3] lsi53c895a: hide 53c895a registers in 53c810

[Artyom Tarasenko]

AIX/PReP does access to the aliased IO registers of 53810.
Implement aliasing to make the AIX driver work.

Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 hw/scsi/lsi53c895a.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index da7239d..6b95699 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2271,6 +2271,9 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
     LSIState *s = LSI53C895A(dev);
     DeviceState *d = DEVICE(dev);
     uint8_t *pci_conf;
+    uint64_t mmio_size;
+    MemoryRegion *mr;
+    uint16_t type = PCI_DEVICE_GET_CLASS(dev)->device_id;
 
     pci_conf = dev->config;
 
@@ -2279,13 +2282,21 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
     /* Interrupt pin A */
     pci_conf[PCI_INTERRUPT_PIN] = 0x01;
 
-    memory_region_init_io(&s->mmio_io, OBJECT(s), &lsi_mmio_ops, s,
-                          "lsi-mmio", 0x400);
     memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
                           "lsi-ram", 0x2000);
     memory_region_init_io(&s->io_io, OBJECT(s), &lsi_io_ops, s,
                           "lsi-io", 256);
-
+    if (type == PCI_DEVICE_ID_LSI_53C895A) {
+        mmio_size = 0x400;
+    } else {
+        mr = g_new(MemoryRegion, 1);
+        memory_region_init_alias(mr, OBJECT(d), "lsi-io-alias", &s->io_io,
+                                 0, 0x80);
+        memory_region_add_subregion_overlap(&s->io_io, 0x80, mr, -1);
+        mmio_size = 0x80;
+    }
+    memory_region_init_io(&s->mmio_io, OBJECT(s), &lsi_mmio_ops, s,
+                          "lsi-mmio", mmio_size);
     address_space_init(&s->pci_io_as, pci_address_space_io(dev), "lsi-pci-io");
     qdev_init_gpio_out(d, &s->ext_irq, 1);
 
-- 
2.7.2

[Qemu-devel] [PATCH 2/3] 40p and prep: implement PCI bus mastering

[Artyom Tarasenko]

Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/pci-host/prep.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 8b9e1fd..94b7465 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -62,6 +62,7 @@ typedef struct PRePPCIState {
     AddressSpace pci_io_as;
     MemoryRegion pci_io;
     MemoryRegion pci_io_non_contiguous;
+    MemoryRegion pci_io_system;
     MemoryRegion pci_memory;
     MemoryRegion pci_intack;
     MemoryRegion bm;
@@ -146,7 +147,7 @@ static uint64_t raven_io_read(void *opaque, hwaddr addr,
     uint8_t buf[4];
 
     addr = raven_io_address(s, addr);
-    address_space_read(&s->pci_io_as, addr + 0x80000000,
+    address_space_read(&s->pci_io_as, addr,
                        MEMTXATTRS_UNSPECIFIED, buf, size);
 
     if (size == 1) {
@@ -178,7 +179,7 @@ static void raven_io_write(void *opaque, hwaddr addr,
         g_assert_not_reached();
     }
 
-    address_space_write(&s->pci_io_as, addr + 0x80000000,
+    address_space_write(&s->pci_io_as, addr,
                         MEMTXATTRS_UNSPECIFIED, buf, size);
 }
 
@@ -276,14 +277,21 @@ static void raven_pcihost_initfn(Object *obj)
     MemoryRegion *address_space_mem = get_system_memory();
     DeviceState *pci_dev;
 
-    memory_region_init(&s->pci_io, obj, "pci-io", 0x3f800000);
+    memory_region_init(&s->pci_io, obj, "pci-io", UINT32_MAX);
     memory_region_init_io(&s->pci_io_non_contiguous, obj, &raven_io_ops, s,
                           "pci-io-non-contiguous", 0x00800000);
     memory_region_init(&s->pci_memory, obj, "pci-memory", 0x3f000000);
     address_space_init(&s->pci_io_as, &s->pci_io, "raven-io");
 
     /* CPU address space */
-    memory_region_add_subregion(address_space_mem, 0x80000000, &s->pci_io);
+    memory_region_add_subregion_overlap(address_space_mem, 0x0,
+                                        &s->pci_io, -1);
+
+    memory_region_init_alias(&s->pci_io_system,  obj, "pci-io-system",
+                             &s->pci_io, 0x0000000, 0x3f800000);
+
+    memory_region_add_subregion(address_space_mem, 0x80000000,
+                                &s->pci_io_system);
     memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
                                         &s->pci_io_non_contiguous, 1);
     memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory);
-- 
2.7.2

[Qemu-devel] [PATCH 2/3] 40p and prep: implement PCI bus mastering

[Artyom Tarasenko]

Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/pci-host/prep.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 8b9e1fd..94b7465 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -62,6 +62,7 @@ typedef struct PRePPCIState {
     AddressSpace pci_io_as;
     MemoryRegion pci_io;
     MemoryRegion pci_io_non_contiguous;
+    MemoryRegion pci_io_system;
     MemoryRegion pci_memory;
     MemoryRegion pci_intack;
     MemoryRegion bm;
@@ -146,7 +147,7 @@ static uint64_t raven_io_read(void *opaque, hwaddr addr,
     uint8_t buf[4];
 
     addr = raven_io_address(s, addr);
-    address_space_read(&s->pci_io_as, addr + 0x80000000,
+    address_space_read(&s->pci_io_as, addr,
                        MEMTXATTRS_UNSPECIFIED, buf, size);
 
     if (size == 1) {
@@ -178,7 +179,7 @@ static void raven_io_write(void *opaque, hwaddr addr,
         g_assert_not_reached();
     }
 
-    address_space_write(&s->pci_io_as, addr + 0x80000000,
+    address_space_write(&s->pci_io_as, addr,
                         MEMTXATTRS_UNSPECIFIED, buf, size);
 }
 
@@ -276,14 +277,21 @@ static void raven_pcihost_initfn(Object *obj)
     MemoryRegion *address_space_mem = get_system_memory();
     DeviceState *pci_dev;
 
-    memory_region_init(&s->pci_io, obj, "pci-io", 0x3f800000);
+    memory_region_init(&s->pci_io, obj, "pci-io", UINT32_MAX);
     memory_region_init_io(&s->pci_io_non_contiguous, obj, &raven_io_ops, s,
                           "pci-io-non-contiguous", 0x00800000);
     memory_region_init(&s->pci_memory, obj, "pci-memory", 0x3f000000);
     address_space_init(&s->pci_io_as, &s->pci_io, "raven-io");
 
     /* CPU address space */
-    memory_region_add_subregion(address_space_mem, 0x80000000, &s->pci_io);
+    memory_region_add_subregion_overlap(address_space_mem, 0x0,
+                                        &s->pci_io, -1);
+
+    memory_region_init_alias(&s->pci_io_system,  obj, "pci-io-system",
+                             &s->pci_io, 0x0000000, 0x3f800000);
+
+    memory_region_add_subregion(address_space_mem, 0x80000000,
+                                &s->pci_io_system);
     memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
                                         &s->pci_io_non_contiguous, 1);
     memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory);
-- 
2.7.2

[Qemu-devel] [PATCH v2 3/3] hw/isa/i82378.c: use 1900 as a base year

[Artyom Tarasenko]

AIX 5.1 expects the base year to be 1900. Adjust accordingly.

Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/isa/i82378.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index a5d67bc..546c928 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -107,7 +107,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
     isa = isa_create_simple(isabus, "i82374");
 
     /* timer */
-    isa_create_simple(isabus, TYPE_MC146818_RTC);
+    isa = isa_create(isabus, TYPE_MC146818_RTC);
+    qdev_prop_set_int32(DEVICE(isa), "base_year", 1900);
+    qdev_init_nofail(DEVICE(isa));
 }
 
 static void i82378_init(Object *obj)
-- 
2.7.2

[Qemu-devel] [PATCH v2 3/3] hw/isa/i82378.c: use 1900 as a base year

[Artyom Tarasenko]

AIX 5.1 expects the base year to be 1900. Adjust accordingly.

Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/isa/i82378.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index a5d67bc..546c928 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -107,7 +107,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
     isa = isa_create_simple(isabus, "i82374");
 
     /* timer */
-    isa_create_simple(isabus, TYPE_MC146818_RTC);
+    isa = isa_create(isabus, TYPE_MC146818_RTC);
+    qdev_prop_set_int32(DEVICE(isa), "base_year", 1900);
+    qdev_init_nofail(DEVICE(isa));
 }
 
 static void i82378_init(Object *obj)
-- 
2.7.2

Re: [Qemu-devel] [PATCH v2 1/3] lsi53c895a: hide 53c895a registers in 53c810

[Mark Cave-Ayland]

On 04/05/2019 22:02, Artyom Tarasenko wrote:

> AIX/PReP does access to the aliased IO registers of 53810.
> Implement aliasing to make the AIX driver work.
> 
> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
>  hw/scsi/lsi53c895a.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index da7239d..6b95699 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -2271,6 +2271,9 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
>      LSIState *s = LSI53C895A(dev);
>      DeviceState *d = DEVICE(dev);
>      uint8_t *pci_conf;
> +    uint64_t mmio_size;
> +    MemoryRegion *mr;
> +    uint16_t type = PCI_DEVICE_GET_CLASS(dev)->device_id;
>  
>      pci_conf = dev->config;
>  
> @@ -2279,13 +2282,21 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
>      /* Interrupt pin A */
>      pci_conf[PCI_INTERRUPT_PIN] = 0x01;
>  
> -    memory_region_init_io(&s->mmio_io, OBJECT(s), &lsi_mmio_ops, s,
> -                          "lsi-mmio", 0x400);
>      memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
>                            "lsi-ram", 0x2000);
>      memory_region_init_io(&s->io_io, OBJECT(s), &lsi_io_ops, s,
>                            "lsi-io", 256);
> -
> +    if (type == PCI_DEVICE_ID_LSI_53C895A) {
> +        mmio_size = 0x400;
> +    } else {
> +        mr = g_new(MemoryRegion, 1);

In general these days it's worth keeping the reference to the MemoryRegion within
LSIState since then its lifecycle is more clearly defined.

> +        memory_region_init_alias(mr, OBJECT(d), "lsi-io-alias", &s->io_io,
> +                                 0, 0x80);
> +        memory_region_add_subregion_overlap(&s->io_io, 0x80, mr, -1);
> +        mmio_size = 0x80;

This feels a little strange - is it possible to see from the datasheets that the
53C895A has 0x400 bytes MMIO whilst the 53C810 has 0x80 bytes MMIO? It's not clear to
me where the aliasing is happening.

> +    }
> +    memory_region_init_io(&s->mmio_io, OBJECT(s), &lsi_mmio_ops, s,
> +                          "lsi-mmio", mmio_size);
>      address_space_init(&s->pci_io_as, pci_address_space_io(dev), "lsi-pci-io");
>      qdev_init_gpio_out(d, &s->ext_irq, 1);
>  
> 


ATB,

Mark.

Re: [Qemu-devel] [PATCH v2 3/3] hw/isa/i82378.c: use 1900 as a base year

[Mark Cave-Ayland]

On 04/05/2019 22:02, Artyom Tarasenko wrote:

> AIX 5.1 expects the base year to be 1900. Adjust accordingly.
> 
> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
> Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/isa/i82378.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
> index a5d67bc..546c928 100644
> --- a/hw/isa/i82378.c
> +++ b/hw/isa/i82378.c
> @@ -107,7 +107,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
>      isa = isa_create_simple(isabus, "i82374");
>  
>      /* timer */
> -    isa_create_simple(isabus, TYPE_MC146818_RTC);
> +    isa = isa_create(isabus, TYPE_MC146818_RTC);
> +    qdev_prop_set_int32(DEVICE(isa), "base_year", 1900);
> +    qdev_init_nofail(DEVICE(isa));
>  }
>  
>  static void i82378_init(Object *obj)

Is this true for the 82378 in general, or is it a particular quirk of the 40p
machine/PReP specification?


ATB,

Mark.

Re: [Qemu-devel] [PATCH v2 3/3] hw/isa/i82378.c: use 1900 as a base year

[Mark Cave-Ayland]

On 04/05/2019 22:02, Artyom Tarasenko wrote:

> AIX 5.1 expects the base year to be 1900. Adjust accordingly.
> 
> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
> Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/isa/i82378.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
> index a5d67bc..546c928 100644
> --- a/hw/isa/i82378.c
> +++ b/hw/isa/i82378.c
> @@ -107,7 +107,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
>      isa = isa_create_simple(isabus, "i82374");
>  
>      /* timer */
> -    isa_create_simple(isabus, TYPE_MC146818_RTC);
> +    isa = isa_create(isabus, TYPE_MC146818_RTC);
> +    qdev_prop_set_int32(DEVICE(isa), "base_year", 1900);
> +    qdev_init_nofail(DEVICE(isa));
>  }
>  
>  static void i82378_init(Object *obj)

Is this true for the 82378 in general, or is it a particular quirk of the 40p
machine/PReP specification?


ATB,

Mark.

Re: [Qemu-devel] [PATCH v2 3/3] hw/isa/i82378.c: use 1900 as a base year

[Philippe Mathieu-Daudé]

Hi Mark, Artyom.

On 5/5/19 12:46 PM, Mark Cave-Ayland wrote:
> On 04/05/2019 22:02, Artyom Tarasenko wrote:
> 
>> AIX 5.1 expects the base year to be 1900. Adjust accordingly.
>>
>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
>> Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>>  hw/isa/i82378.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
>> index a5d67bc..546c928 100644
>> --- a/hw/isa/i82378.c
>> +++ b/hw/isa/i82378.c
>> @@ -107,7 +107,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
>>      isa = isa_create_simple(isabus, "i82374");
>>  
>>      /* timer */
>> -    isa_create_simple(isabus, TYPE_MC146818_RTC);
>> +    isa = isa_create(isabus, TYPE_MC146818_RTC);
>> +    qdev_prop_set_int32(DEVICE(isa), "base_year", 1900);
>> +    qdev_init_nofail(DEVICE(isa));
>>  }
>>  
>>  static void i82378_init(Object *obj)
> 
> Is this true for the 82378 in general, or is it a particular quirk of the 40p
> machine/PReP specification?

This is indeed incorrect (see
https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02452.html) so:
Nack

Artyom: I did wrote the patch and included it in another series based on
top of Joel Stanley's
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00657.html
which is also depending of another from Cédric, so I'll just extract it
and send directly, sorry for the delay.

Regards,

Phil.

Re: [Qemu-devel] [PATCH v2 1/3] lsi53c895a: hide 53c895a registers in 53c810

[Artyom Tarasenko]

On Sun, May 5, 2019 at 12:43 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> On 04/05/2019 22:02, Artyom Tarasenko wrote:
>
> > AIX/PReP does access to the aliased IO registers of 53810.
> > Implement aliasing to make the AIX driver work.
> >
> > Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
> > ---
> >  hw/scsi/lsi53c895a.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> > index da7239d..6b95699 100644
> > --- a/hw/scsi/lsi53c895a.c
> > +++ b/hw/scsi/lsi53c895a.c
> > @@ -2271,6 +2271,9 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
> >      LSIState *s = LSI53C895A(dev);
> >      DeviceState *d = DEVICE(dev);
> >      uint8_t *pci_conf;
> > +    uint64_t mmio_size;
> > +    MemoryRegion *mr;
> > +    uint16_t type = PCI_DEVICE_GET_CLASS(dev)->device_id;
> >
> >      pci_conf = dev->config;
> >
> > @@ -2279,13 +2282,21 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
> >      /* Interrupt pin A */
> >      pci_conf[PCI_INTERRUPT_PIN] = 0x01;
> >
> > -    memory_region_init_io(&s->mmio_io, OBJECT(s), &lsi_mmio_ops, s,
> > -                          "lsi-mmio", 0x400);
> >      memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
> >                            "lsi-ram", 0x2000);
> >      memory_region_init_io(&s->io_io, OBJECT(s), &lsi_io_ops, s,
> >                            "lsi-io", 256);
> > -
> > +    if (type == PCI_DEVICE_ID_LSI_53C895A) {
> > +        mmio_size = 0x400;
> > +    } else {
> > +        mr = g_new(MemoryRegion, 1);
>
> In general these days it's worth keeping the reference to the MemoryRegion within
> LSIState since then its lifecycle is more clearly defined.

On the other hand, it's a PCI card, and can not be
hot-plugged/removed, so the lifecycle is pretty simple here.
Or am I missing something?

> > +        memory_region_init_alias(mr, OBJECT(d), "lsi-io-alias", &s->io_io,
> > +                                 0, 0x80);
> > +        memory_region_add_subregion_overlap(&s->io_io, 0x80, mr, -1);
> > +        mmio_size = 0x80;
>
> This feels a little strange - is it possible to see from the datasheets that the
> 53C895A has 0x400 bytes MMIO whilst the 53C810 has 0x80 bytes MMIO? It's not clear to
> me where the aliasing is happening.

These values are empiric. For 810 it can not be more than 0x80,
because the AIX does access the registers with the shift of 0x80.
For  895A we did already have 0x400.

> > +    }
> > +    memory_region_init_io(&s->mmio_io, OBJECT(s), &lsi_mmio_ops, s,
> > +                          "lsi-mmio", mmio_size);
> >      address_space_init(&s->pci_io_as, pci_address_space_io(dev), "lsi-pci-io");
> >      qdev_init_gpio_out(d, &s->ext_irq, 1);
> >
> >
>
>
> ATB,
>
> Mark.
>


-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH v2 3/3] hw/isa/i82378.c: use 1900 as a base year

[Artyom Tarasenko]

On Sun, May 5, 2019 at 5:19 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Hi Mark, Artyom.
>
> On 5/5/19 12:46 PM, Mark Cave-Ayland wrote:
> > On 04/05/2019 22:02, Artyom Tarasenko wrote:
> >
> >> AIX 5.1 expects the base year to be 1900. Adjust accordingly.
> >>
> >> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
> >> Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
> >> ---
> >>  hw/isa/i82378.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
> >> index a5d67bc..546c928 100644
> >> --- a/hw/isa/i82378.c
> >> +++ b/hw/isa/i82378.c
> >> @@ -107,7 +107,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
> >>      isa = isa_create_simple(isabus, "i82374");
> >>
> >>      /* timer */
> >> -    isa_create_simple(isabus, TYPE_MC146818_RTC);
> >> +    isa = isa_create(isabus, TYPE_MC146818_RTC);
> >> +    qdev_prop_set_int32(DEVICE(isa), "base_year", 1900);
> >> +    qdev_init_nofail(DEVICE(isa));
> >>  }
> >>
> >>  static void i82378_init(Object *obj)
> >
> > Is this true for the 82378 in general, or is it a particular quirk of the 40p
> > machine/PReP specification?
>
> This is indeed incorrect (see
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02452.html) so:
> Nack
>
> Artyom: I did wrote the patch and included it in another series based on
> top of Joel Stanley's
> https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00657.html
> which is also depending of another from Cédric, so I'll just extract it
> and send directly, sorry for the delay.

Thanks, it works for me.

-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH v2 1/3] lsi53c895a: hide 53c895a registers in 53c810

[Mark Cave-Ayland]

On 06/05/2019 09:42, Artyom Tarasenko wrote:

> On Sun, May 5, 2019 at 12:43 PM Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>>
>> On 04/05/2019 22:02, Artyom Tarasenko wrote:
>>
>>> AIX/PReP does access to the aliased IO registers of 53810.
>>> Implement aliasing to make the AIX driver work.
>>>
>>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
>>> ---
>>>  hw/scsi/lsi53c895a.c | 17 ++++++++++++++---
>>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
>>> index da7239d..6b95699 100644
>>> --- a/hw/scsi/lsi53c895a.c
>>> +++ b/hw/scsi/lsi53c895a.c
>>> @@ -2271,6 +2271,9 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
>>>      LSIState *s = LSI53C895A(dev);
>>>      DeviceState *d = DEVICE(dev);
>>>      uint8_t *pci_conf;
>>> +    uint64_t mmio_size;
>>> +    MemoryRegion *mr;
>>> +    uint16_t type = PCI_DEVICE_GET_CLASS(dev)->device_id;
>>>
>>>      pci_conf = dev->config;
>>>
>>> @@ -2279,13 +2282,21 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
>>>      /* Interrupt pin A */
>>>      pci_conf[PCI_INTERRUPT_PIN] = 0x01;
>>>
>>> -    memory_region_init_io(&s->mmio_io, OBJECT(s), &lsi_mmio_ops, s,
>>> -                          "lsi-mmio", 0x400);
>>>      memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
>>>                            "lsi-ram", 0x2000);
>>>      memory_region_init_io(&s->io_io, OBJECT(s), &lsi_io_ops, s,
>>>                            "lsi-io", 256);
>>> -
>>> +    if (type == PCI_DEVICE_ID_LSI_53C895A) {
>>> +        mmio_size = 0x400;
>>> +    } else {
>>> +        mr = g_new(MemoryRegion, 1);
>>
>> In general these days it's worth keeping the reference to the MemoryRegion within
>> LSIState since then its lifecycle is more clearly defined.
> 
> On the other hand, it's a PCI card, and can not be
> hot-plugged/removed, so the lifecycle is pretty simple here.
> Or am I missing something?

Well Thomas has been working on a set of tests that for each machine will plug and
unplug each device via the monitor to make sure that init/realize/unrealize work
correctly so it would be good to ensure that these tests don't leak.

However...

>>> +        memory_region_init_alias(mr, OBJECT(d), "lsi-io-alias", &s->io_io,
>>> +                                 0, 0x80);
>>> +        memory_region_add_subregion_overlap(&s->io_io, 0x80, mr, -1);
>>> +        mmio_size = 0x80;
>>
>> This feels a little strange - is it possible to see from the datasheets that the
>> 53C895A has 0x400 bytes MMIO whilst the 53C810 has 0x80 bytes MMIO? It's not clear to
>> me where the aliasing is happening.
> 
> These values are empiric. For 810 it can not be more than 0x80,
> because the AIX does access the registers with the shift of 0x80.
> For  895A we did already have 0x400.

After a bit of searching I managed to locate an 810 datasheet and in Chapter 5 it
clearly describes the IO space (s->io_io) as being 256 bytes in size which is the
same as the 895A, but with 0x80-0xff aliased onto 0x00 - 0x7f.

It feels to me that rather than complicate things with an additional alias
MemoryRegion, the simplest solution would be to simply change the mask in
lsi_io_read() and lsi_io_write() to be 0x7f rather than 0xff if we've instantiated a
810 rather than an 895A.


ATB,

Mark.

Re: [Qemu-devel] [PATCH v2 1/3] lsi53c895a: hide 53c895a registers in 53c810

[Artyom Tarasenko]

On Mon, May 6, 2019 at 4:27 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> On 06/05/2019 09:42, Artyom Tarasenko wrote:
>
> > On Sun, May 5, 2019 at 12:43 PM Mark Cave-Ayland
> > <mark.cave-ayland@ilande.co.uk> wrote:
> >>
> >> On 04/05/2019 22:02, Artyom Tarasenko wrote:
> >>
> >>> AIX/PReP does access to the aliased IO registers of 53810.
> >>> Implement aliasing to make the AIX driver work.
> >>>
> >>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
> >>> ---
> >>>  hw/scsi/lsi53c895a.c | 17 ++++++++++++++---
> >>>  1 file changed, 14 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> >>> index da7239d..6b95699 100644
> >>> --- a/hw/scsi/lsi53c895a.c
> >>> +++ b/hw/scsi/lsi53c895a.c
> >>> @@ -2271,6 +2271,9 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
> >>>      LSIState *s = LSI53C895A(dev);
> >>>      DeviceState *d = DEVICE(dev);
> >>>      uint8_t *pci_conf;
> >>> +    uint64_t mmio_size;
> >>> +    MemoryRegion *mr;
> >>> +    uint16_t type = PCI_DEVICE_GET_CLASS(dev)->device_id;
> >>>
> >>>      pci_conf = dev->config;
> >>>
> >>> @@ -2279,13 +2282,21 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
> >>>      /* Interrupt pin A */
> >>>      pci_conf[PCI_INTERRUPT_PIN] = 0x01;
> >>>
> >>> -    memory_region_init_io(&s->mmio_io, OBJECT(s), &lsi_mmio_ops, s,
> >>> -                          "lsi-mmio", 0x400);
> >>>      memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
> >>>                            "lsi-ram", 0x2000);
> >>>      memory_region_init_io(&s->io_io, OBJECT(s), &lsi_io_ops, s,
> >>>                            "lsi-io", 256);
> >>> -
> >>> +    if (type == PCI_DEVICE_ID_LSI_53C895A) {
> >>> +        mmio_size = 0x400;
> >>> +    } else {
> >>> +        mr = g_new(MemoryRegion, 1);
> >>
> >> In general these days it's worth keeping the reference to the MemoryRegion within
> >> LSIState since then its lifecycle is more clearly defined.
> >
> > On the other hand, it's a PCI card, and can not be
> > hot-plugged/removed, so the lifecycle is pretty simple here.
> > Or am I missing something?
>
> Well Thomas has been working on a set of tests that for each machine will plug and
> unplug each device via the monitor to make sure that init/realize/unrealize work
> correctly so it would be good to ensure that these tests don't leak.

Makes sense, indeed.

> However...
>
> >>> +        memory_region_init_alias(mr, OBJECT(d), "lsi-io-alias", &s->io_io,
> >>> +                                 0, 0x80);
> >>> +        memory_region_add_subregion_overlap(&s->io_io, 0x80, mr, -1);
> >>> +        mmio_size = 0x80;
> >>
> >> This feels a little strange - is it possible to see from the datasheets that the
> >> 53C895A has 0x400 bytes MMIO whilst the 53C810 has 0x80 bytes MMIO? It's not clear to
> >> me where the aliasing is happening.
> >
> > These values are empiric. For 810 it can not be more than 0x80,
> > because the AIX does access the registers with the shift of 0x80.
> > For  895A we did already have 0x400.
>
> After a bit of searching I managed to locate an 810 datasheet and in Chapter 5 it
> clearly describes the IO space (s->io_io) as being 256 bytes in size which is the
> same as the 895A, but with 0x80-0xff aliased onto 0x00 - 0x7f.
>
> It feels to me that rather than complicate things with an additional alias
> MemoryRegion, the simplest solution would be to simply change the mask in
> lsi_io_read() and lsi_io_write() to be 0x7f rather than 0xff if we've instantiated a
> 810 rather than an 895A.

Initially I implemented it exactly as you suggest, via mask. But then
I thought that memory_region_init_alias makes aliasing more obvious.
I don't have a strong opinion on this one though.

@Paolo, what do you think?

-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH v2 1/3] lsi53c895a: hide 53c895a registers in 53c810

[Mark Cave-Ayland]

On 07/05/2019 16:03, Artyom Tarasenko wrote:

> On Mon, May 6, 2019 at 4:27 PM Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>>
>> On 06/05/2019 09:42, Artyom Tarasenko wrote:
>>
>>> On Sun, May 5, 2019 at 12:43 PM Mark Cave-Ayland
>>> <mark.cave-ayland@ilande.co.uk> wrote:
>>>>
>>>> On 04/05/2019 22:02, Artyom Tarasenko wrote:
>>>>
>>>>> AIX/PReP does access to the aliased IO registers of 53810.
>>>>> Implement aliasing to make the AIX driver work.
>>>>>
>>>>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
>>>>> ---
>>>>>  hw/scsi/lsi53c895a.c | 17 ++++++++++++++---
>>>>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
>>>>> index da7239d..6b95699 100644
>>>>> --- a/hw/scsi/lsi53c895a.c
>>>>> +++ b/hw/scsi/lsi53c895a.c
>>>>> @@ -2271,6 +2271,9 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
>>>>>      LSIState *s = LSI53C895A(dev);
>>>>>      DeviceState *d = DEVICE(dev);
>>>>>      uint8_t *pci_conf;
>>>>> +    uint64_t mmio_size;
>>>>> +    MemoryRegion *mr;
>>>>> +    uint16_t type = PCI_DEVICE_GET_CLASS(dev)->device_id;
>>>>>
>>>>>      pci_conf = dev->config;
>>>>>
>>>>> @@ -2279,13 +2282,21 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
>>>>>      /* Interrupt pin A */
>>>>>      pci_conf[PCI_INTERRUPT_PIN] = 0x01;
>>>>>
>>>>> -    memory_region_init_io(&s->mmio_io, OBJECT(s), &lsi_mmio_ops, s,
>>>>> -                          "lsi-mmio", 0x400);
>>>>>      memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
>>>>>                            "lsi-ram", 0x2000);
>>>>>      memory_region_init_io(&s->io_io, OBJECT(s), &lsi_io_ops, s,
>>>>>                            "lsi-io", 256);
>>>>> -
>>>>> +    if (type == PCI_DEVICE_ID_LSI_53C895A) {
>>>>> +        mmio_size = 0x400;
>>>>> +    } else {
>>>>> +        mr = g_new(MemoryRegion, 1);
>>>>
>>>> In general these days it's worth keeping the reference to the MemoryRegion within
>>>> LSIState since then its lifecycle is more clearly defined.
>>>
>>> On the other hand, it's a PCI card, and can not be
>>> hot-plugged/removed, so the lifecycle is pretty simple here.
>>> Or am I missing something?
>>
>> Well Thomas has been working on a set of tests that for each machine will plug and
>> unplug each device via the monitor to make sure that init/realize/unrealize work
>> correctly so it would be good to ensure that these tests don't leak.
> 
> Makes sense, indeed.
> 
>> However...
>>
>>>>> +        memory_region_init_alias(mr, OBJECT(d), "lsi-io-alias", &s->io_io,
>>>>> +                                 0, 0x80);
>>>>> +        memory_region_add_subregion_overlap(&s->io_io, 0x80, mr, -1);
>>>>> +        mmio_size = 0x80;
>>>>
>>>> This feels a little strange - is it possible to see from the datasheets that the
>>>> 53C895A has 0x400 bytes MMIO whilst the 53C810 has 0x80 bytes MMIO? It's not clear to
>>>> me where the aliasing is happening.
>>>
>>> These values are empiric. For 810 it can not be more than 0x80,
>>> because the AIX does access the registers with the shift of 0x80.
>>> For  895A we did already have 0x400.
>>
>> After a bit of searching I managed to locate an 810 datasheet and in Chapter 5 it
>> clearly describes the IO space (s->io_io) as being 256 bytes in size which is the
>> same as the 895A, but with 0x80-0xff aliased onto 0x00 - 0x7f.
>>
>> It feels to me that rather than complicate things with an additional alias
>> MemoryRegion, the simplest solution would be to simply change the mask in
>> lsi_io_read() and lsi_io_write() to be 0x7f rather than 0xff if we've instantiated a
>> 810 rather than an 895A.
> 
> Initially I implemented it exactly as you suggest, via mask. But then
> I thought that memory_region_init_alias makes aliasing more obvious.
> I don't have a strong opinion on this one though.
> 
> @Paolo, what do you think?

My general feeling is that memory region aliases are more aimed at mapping areas into
different address spaces. In this particular case it seems to me that the memory
region for both devices is still 256 bytes, but it's just that internally the address
decoder ignores bit 7 on the 810.


ATB,

Mark.

Re: [Qemu-devel] [PATCH v2 1/3] lsi53c895a: hide 53c895a registers in 53c810

[Paolo Bonzini]

On 17/05/19 16:12, Mark Cave-Ayland wrote:
>> Initially I implemented it exactly as you suggest, via mask. But then
>> I thought that memory_region_init_alias makes aliasing more obvious.
>> I don't have a strong opinion on this one though.
>>
>> @Paolo, what do you think?
> My general feeling is that memory region aliases are more aimed at mapping areas into
> different address spaces. In this particular case it seems to me that the memory
> region for both devices is still 256 bytes, but it's just that internally the address
> decoder ignores bit 7 on the 810.

It's the same for me; whatever makes the code simpler is good.  RAM
regions need to use aliases in order to simulate ignored bits in the
decoder, so it's okay to use that for MMIO as well.  On the other hand,
it's true that a simple "&" does the job just as well.

Paolo