[Qemu-devel] [PATCH 0/2] m48t59: add year offset and MMIO ISA mapping

[Qemu-devel] [PATCH 0/2] m48t59: add year offset and MMIO ISA mapping

[Mark Cave-Ayland]

This patch lays the groundwork for switching sun4u over from ioport NVRAM
access to MMIO NVRAM access.

Patch 1 introduces a new year_offset property which is the offset between the
year value stored in hardware and the actual year. In particular, Sun hardware
has a 68 year offset used to extend the date range of the IC. While the
kernel sources I have viewed contain this offset within a #ifdef CONFIG_SPARC
block, I've updated all the callers so that no change in behaviour will be
seen across all platforms. PPC users may wish to alter the callers to change
this behaviour as required.

Patch 2 mimics the mem_base parameter from m48t59_init() to m48t59_init_isa()
so that MMIO can be used for sun4u where the NVRAM is attached to the ebus
which is essentially the same as an ISA bus.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Mark Cave-Ayland (2):
  m48t59: introduce new year_offset qdev property
  m48t59: add mem_base value to m48t59_init_isa()

 hw/ppc/ppc405_boards.c    |    2 +-
 hw/ppc/prep.c             |    2 +-
 hw/sparc/sun4m.c          |    2 +-
 hw/sparc64/sun4u.c        |    2 +-
 hw/timer/m48t59.c         |   42 ++++++++++++++++++++++--------------------
 include/hw/timer/m48t59.h |    7 ++++---
 6 files changed, 30 insertions(+), 27 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH 1/2] m48t59: introduce new year_offset qdev property

[Mark Cave-Ayland]

Currently the m48t59 device uses the hardware model in order to determine
whether the year value is offset from the hardware value. As this will
soon be required by the x59 model, change the year offset to a qdev
property and update the callers appropriately.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/ppc405_boards.c    |    2 +-
 hw/ppc/prep.c             |    2 +-
 hw/sparc/sun4m.c          |    2 +-
 hw/sparc64/sun4u.c        |    2 +-
 hw/timer/m48t59.c         |   35 ++++++++++++++++-------------------
 include/hw/timer/m48t59.h |    5 +++--
 6 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 1dcea77..2d8d2aa 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -283,7 +283,7 @@ static void ref405ep_init(MachineState *machine)
 #ifdef DEBUG_BOARD_INIT
     printf("%s: register NVRAM\n", __func__);
 #endif
-    m48t59_init(NULL, 0xF0000000, 0, 8192, 8);
+    m48t59_init(NULL, 0xF0000000, 0, 8192, 68, 8);
     /* Load kernel */
     linux_boot = (kernel_filename != NULL);
     if (linux_boot) {
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 15df7f3..378a368 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -543,7 +543,7 @@ static void ppc_prep_init(MachineState *machine)
         pci_create_simple(pci_bus, -1, "pci-ohci");
     }
 
-    m48t59 = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 59);
+    m48t59 = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 0, 59);
     if (m48t59 == NULL)
         return;
     sysctrl->nvram = m48t59;
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index ecd9dc1..cde4a87 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -1012,7 +1012,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
 
     lance_init(&nd_table[0], hwdef->le_base, ledma, ledma_irq);
 
-    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 8);
+    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 68, 8);
 
     slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 3ff5bd8..6b46511 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -873,7 +873,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
         fd[i] = drive_get(IF_FLOPPY, 0, i);
     }
     fdctrl_init_isa(isa_bus, fd);
-    nvram = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 59);
+    nvram = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 0, 59);
 
     initrd_size = 0;
     initrd_addr = 0;
diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
index 31509d5..0a05100 100644
--- a/hw/timer/m48t59.c
+++ b/hw/timer/m48t59.c
@@ -56,6 +56,7 @@ struct M48t59State {
     MemoryRegion iomem;
     uint32_t io_base;
     uint32_t size;
+    uint32_t year_offset;
     /* RTC management */
     time_t   time_offset;
     time_t   stop_time;
@@ -346,11 +347,7 @@ void m48t59_write (void *opaque, uint32_t addr, uint32_t val)
 	tmp = from_bcd(val);
 	if (tmp >= 0 && tmp <= 99) {
 	    get_time(NVRAM, &tm);
-            if (NVRAM->model == 8) {
-                tm.tm_year = from_bcd(val) + 68; // Base year is 1968
-            } else {
-                tm.tm_year = from_bcd(val);
-            }
+            tm.tm_year = from_bcd(val) + NVRAM->year_offset;
 	    set_time(NVRAM, &tm);
 	}
         break;
@@ -453,11 +450,7 @@ uint32_t m48t59_read (void *opaque, uint32_t addr)
     case 0x07FF:
         /* year */
         get_time(NVRAM, &tm);
-        if (NVRAM->model == 8) {
-            retval = to_bcd(tm.tm_year - 68); // Base year is 1968
-        } else {
-            retval = to_bcd(tm.tm_year);
-        }
+        retval = to_bcd(tm.tm_year - NVRAM->year_offset);
         break;
     default:
         /* Check lock registers state */
@@ -641,8 +634,8 @@ static const MemoryRegionOps m48t59_io_ops = {
 };
 
 /* Initialisation routine */
-M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
-                         uint32_t io_base, uint16_t size, int model)
+M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base, uint32_t io_base,
+                         uint16_t size, uint32_t year_offset, int model)
 {
     DeviceState *dev;
     SysBusDevice *s;
@@ -653,6 +646,7 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
     qdev_prop_set_uint32(dev, "model", model);
     qdev_prop_set_uint32(dev, "size", size);
     qdev_prop_set_uint32(dev, "io_base", io_base);
+    qdev_prop_set_uint32(dev, "year_offset", year_offset);
     qdev_init_nofail(dev);
     s = SYS_BUS_DEVICE(dev);
     d = SYSBUS_M48T59(dev);
@@ -671,7 +665,7 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
 }
 
 M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
-                             int model)
+                             uint32_t year_offset, int model)
 {
     M48t59ISAState *d;
     ISADevice *isadev;
@@ -683,6 +677,7 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
     qdev_prop_set_uint32(dev, "model", model);
     qdev_prop_set_uint32(dev, "size", size);
     qdev_prop_set_uint32(dev, "io_base", io_base);
+    qdev_prop_set_uint32(dev, "year_offset", year_offset);
     qdev_init_nofail(dev);
     d = ISA_M48T59(isadev);
     s = &d->state;
@@ -738,9 +733,10 @@ static int m48t59_init1(SysBusDevice *dev)
 }
 
 static Property m48t59_isa_properties[] = {
-    DEFINE_PROP_UINT32("size",    M48t59ISAState, state.size,    -1),
-    DEFINE_PROP_UINT32("model",   M48t59ISAState, state.model,   -1),
-    DEFINE_PROP_UINT32("io_base", M48t59ISAState, state.io_base,  0),
+    DEFINE_PROP_UINT32("size",        M48t59ISAState, state.size,       -1),
+    DEFINE_PROP_UINT32("model",       M48t59ISAState, state.model,      -1),
+    DEFINE_PROP_UINT32("io_base",     M48t59ISAState, state.io_base,     0),
+    DEFINE_PROP_UINT32("year_offset", M48t59ISAState, state.year_offset, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -763,9 +759,10 @@ static const TypeInfo m48t59_isa_info = {
 };
 
 static Property m48t59_properties[] = {
-    DEFINE_PROP_UINT32("size",    M48t59SysBusState, state.size,    -1),
-    DEFINE_PROP_UINT32("model",   M48t59SysBusState, state.model,   -1),
-    DEFINE_PROP_UINT32("io_base", M48t59SysBusState, state.io_base,  0),
+    DEFINE_PROP_UINT32("size",        M48t59SysBusState, state.size,       -1),
+    DEFINE_PROP_UINT32("model",       M48t59SysBusState, state.model,      -1),
+    DEFINE_PROP_UINT32("io_base",     M48t59SysBusState, state.io_base,     0),
+    DEFINE_PROP_UINT32("year_offset", M48t59SysBusState, state.year_offset, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/timer/m48t59.h b/include/hw/timer/m48t59.h
index 8217522..08252b6 100644
--- a/include/hw/timer/m48t59.h
+++ b/include/hw/timer/m48t59.h
@@ -30,8 +30,9 @@ void m48t59_write (void *private, uint32_t addr, uint32_t val);
 uint32_t m48t59_read (void *private, uint32_t addr);
 void m48t59_toggle_lock (void *private, int lock);
 M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
-                             int type);
+                             uint32_t year_offset, int type);
 M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
-                         uint32_t io_base, uint16_t size, int type);
+                         uint32_t io_base, uint16_t size,
+                         uint32_t year_offset, int type);
 
 #endif /* !NVRAM_H */
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 1/2] m48t59: introduce new year_offset qdev property

[Artyom Tarasenko]

On Mon, Jan 19, 2015 at 12:35 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> Currently the m48t59 device uses the hardware model in order to determine
> whether the year value is offset from the hardware value. As this will
> soon be required by the x59 model, change the year offset to a qdev
> property and update the callers appropriately.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/ppc/ppc405_boards.c    |    2 +-
>  hw/ppc/prep.c             |    2 +-
>  hw/sparc/sun4m.c          |    2 +-
>  hw/sparc64/sun4u.c        |    2 +-
>  hw/timer/m48t59.c         |   35 ++++++++++++++++-------------------
>  include/hw/timer/m48t59.h |    5 +++--
>  6 files changed, 23 insertions(+), 25 deletions(-)
>
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index 1dcea77..2d8d2aa 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -283,7 +283,7 @@ static void ref405ep_init(MachineState *machine)
>  #ifdef DEBUG_BOARD_INIT
>      printf("%s: register NVRAM\n", __func__);
>  #endif
> -    m48t59_init(NULL, 0xF0000000, 0, 8192, 8);
> +    m48t59_init(NULL, 0xF0000000, 0, 8192, 68, 8);
>      /* Load kernel */
>      linux_boot = (kernel_filename != NULL);
>      if (linux_boot) {
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 15df7f3..378a368 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -543,7 +543,7 @@ static void ppc_prep_init(MachineState *machine)
>          pci_create_simple(pci_bus, -1, "pci-ohci");
>      }
>
> -    m48t59 = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 59);
> +    m48t59 = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 0, 59);
>      if (m48t59 == NULL)
>          return;
>      sysctrl->nvram = m48t59;
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index ecd9dc1..cde4a87 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -1012,7 +1012,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>
>      lance_init(&nd_table[0], hwdef->le_base, ledma, ledma_irq);
>
> -    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 8);
> +    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 68, 8);
>
>      slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);
>
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 3ff5bd8..6b46511 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -873,7 +873,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>          fd[i] = drive_get(IF_FLOPPY, 0, i);
>      }
>      fdctrl_init_isa(isa_bus, fd);
> -    nvram = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 59);
> +    nvram = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 0, 59);
>
>      initrd_size = 0;
>      initrd_addr = 0;
> diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
> index 31509d5..0a05100 100644
> --- a/hw/timer/m48t59.c
> +++ b/hw/timer/m48t59.c
> @@ -56,6 +56,7 @@ struct M48t59State {
>      MemoryRegion iomem;
>      uint32_t io_base;
>      uint32_t size;
> +    uint32_t year_offset;
>      /* RTC management */
>      time_t   time_offset;
>      time_t   stop_time;
> @@ -346,11 +347,7 @@ void m48t59_write (void *opaque, uint32_t addr, uint32_t val)
>         tmp = from_bcd(val);
>         if (tmp >= 0 && tmp <= 99) {
>             get_time(NVRAM, &tm);
> -            if (NVRAM->model == 8) {
> -                tm.tm_year = from_bcd(val) + 68; // Base year is 1968
> -            } else {
> -                tm.tm_year = from_bcd(val);
> -            }
> +            tm.tm_year = from_bcd(val) + NVRAM->year_offset;
>             set_time(NVRAM, &tm);
>         }
>          break;
> @@ -453,11 +450,7 @@ uint32_t m48t59_read (void *opaque, uint32_t addr)
>      case 0x07FF:
>          /* year */
>          get_time(NVRAM, &tm);
> -        if (NVRAM->model == 8) {
> -            retval = to_bcd(tm.tm_year - 68); // Base year is 1968
> -        } else {
> -            retval = to_bcd(tm.tm_year);
> -        }
> +        retval = to_bcd(tm.tm_year - NVRAM->year_offset);
>          break;
>      default:
>          /* Check lock registers state */
> @@ -641,8 +634,8 @@ static const MemoryRegionOps m48t59_io_ops = {
>  };
>
>  /* Initialisation routine */
> -M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
> -                         uint32_t io_base, uint16_t size, int model)
> +M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base, uint32_t io_base,
> +                         uint16_t size, uint32_t year_offset, int model)
>  {
>      DeviceState *dev;
>      SysBusDevice *s;
> @@ -653,6 +646,7 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>      qdev_prop_set_uint32(dev, "model", model);
>      qdev_prop_set_uint32(dev, "size", size);
>      qdev_prop_set_uint32(dev, "io_base", io_base);
> +    qdev_prop_set_uint32(dev, "year_offset", year_offset);
>      qdev_init_nofail(dev);
>      s = SYS_BUS_DEVICE(dev);
>      d = SYSBUS_M48T59(dev);
> @@ -671,7 +665,7 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>  }
>
>  M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
> -                             int model)
> +                             uint32_t year_offset, int model)
>  {
>      M48t59ISAState *d;
>      ISADevice *isadev;
> @@ -683,6 +677,7 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
>      qdev_prop_set_uint32(dev, "model", model);
>      qdev_prop_set_uint32(dev, "size", size);
>      qdev_prop_set_uint32(dev, "io_base", io_base);
> +    qdev_prop_set_uint32(dev, "year_offset", year_offset);
>      qdev_init_nofail(dev);
>      d = ISA_M48T59(isadev);
>      s = &d->state;
> @@ -738,9 +733,10 @@ static int m48t59_init1(SysBusDevice *dev)
>  }
>
>  static Property m48t59_isa_properties[] = {
> -    DEFINE_PROP_UINT32("size",    M48t59ISAState, state.size,    -1),
> -    DEFINE_PROP_UINT32("model",   M48t59ISAState, state.model,   -1),
> -    DEFINE_PROP_UINT32("io_base", M48t59ISAState, state.io_base,  0),
> +    DEFINE_PROP_UINT32("size",        M48t59ISAState, state.size,       -1),
> +    DEFINE_PROP_UINT32("model",       M48t59ISAState, state.model,      -1),
> +    DEFINE_PROP_UINT32("io_base",     M48t59ISAState, state.io_base,     0),

Formatting-only change?

> +    DEFINE_PROP_UINT32("year_offset", M48t59ISAState, state.year_offset, 0),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> @@ -763,9 +759,10 @@ static const TypeInfo m48t59_isa_info = {
>  };
>
>  static Property m48t59_properties[] = {
> -    DEFINE_PROP_UINT32("size",    M48t59SysBusState, state.size,    -1),
> -    DEFINE_PROP_UINT32("model",   M48t59SysBusState, state.model,   -1),
> -    DEFINE_PROP_UINT32("io_base", M48t59SysBusState, state.io_base,  0),
> +    DEFINE_PROP_UINT32("size",        M48t59SysBusState, state.size,       -1),
> +    DEFINE_PROP_UINT32("model",       M48t59SysBusState, state.model,      -1),
> +    DEFINE_PROP_UINT32("io_base",     M48t59SysBusState, state.io_base,     0),
> +    DEFINE_PROP_UINT32("year_offset", M48t59SysBusState, state.year_offset, 0),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> diff --git a/include/hw/timer/m48t59.h b/include/hw/timer/m48t59.h
> index 8217522..08252b6 100644
> --- a/include/hw/timer/m48t59.h
> +++ b/include/hw/timer/m48t59.h
> @@ -30,8 +30,9 @@ void m48t59_write (void *private, uint32_t addr, uint32_t val);
>  uint32_t m48t59_read (void *private, uint32_t addr);
>  void m48t59_toggle_lock (void *private, int lock);
>  M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
> -                             int type);
> +                             uint32_t year_offset, int type);
>  M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
> -                         uint32_t io_base, uint16_t size, int type);
> +                         uint32_t io_base, uint16_t size,
> +                         uint32_t year_offset, int type);
>
>  #endif /* !NVRAM_H */
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

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

Re: [Qemu-devel] [PATCH 1/2] m48t59: introduce new year_offset qdev property

[Mark Cave-Ayland]

On 19/01/15 12:06, Artyom Tarasenko wrote:

> On Mon, Jan 19, 2015 at 12:35 PM, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>> Currently the m48t59 device uses the hardware model in order to determine
>> whether the year value is offset from the hardware value. As this will
>> soon be required by the x59 model, change the year offset to a qdev
>> property and update the callers appropriately.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/ppc/ppc405_boards.c    |    2 +-
>>  hw/ppc/prep.c             |    2 +-
>>  hw/sparc/sun4m.c          |    2 +-
>>  hw/sparc64/sun4u.c        |    2 +-
>>  hw/timer/m48t59.c         |   35 ++++++++++++++++-------------------
>>  include/hw/timer/m48t59.h |    5 +++--
>>  6 files changed, 23 insertions(+), 25 deletions(-)
>>
>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
>> index 1dcea77..2d8d2aa 100644
>> --- a/hw/ppc/ppc405_boards.c
>> +++ b/hw/ppc/ppc405_boards.c
>> @@ -283,7 +283,7 @@ static void ref405ep_init(MachineState *machine)
>>  #ifdef DEBUG_BOARD_INIT
>>      printf("%s: register NVRAM\n", __func__);
>>  #endif
>> -    m48t59_init(NULL, 0xF0000000, 0, 8192, 8);
>> +    m48t59_init(NULL, 0xF0000000, 0, 8192, 68, 8);
>>      /* Load kernel */
>>      linux_boot = (kernel_filename != NULL);
>>      if (linux_boot) {
>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>> index 15df7f3..378a368 100644
>> --- a/hw/ppc/prep.c
>> +++ b/hw/ppc/prep.c
>> @@ -543,7 +543,7 @@ static void ppc_prep_init(MachineState *machine)
>>          pci_create_simple(pci_bus, -1, "pci-ohci");
>>      }
>>
>> -    m48t59 = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 59);
>> +    m48t59 = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 0, 59);
>>      if (m48t59 == NULL)
>>          return;
>>      sysctrl->nvram = m48t59;
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index ecd9dc1..cde4a87 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -1012,7 +1012,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>
>>      lance_init(&nd_table[0], hwdef->le_base, ledma, ledma_irq);
>>
>> -    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 8);
>> +    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 68, 8);
>>
>>      slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);
>>
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index 3ff5bd8..6b46511 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -873,7 +873,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>          fd[i] = drive_get(IF_FLOPPY, 0, i);
>>      }
>>      fdctrl_init_isa(isa_bus, fd);
>> -    nvram = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 59);
>> +    nvram = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 0, 59);
>>
>>      initrd_size = 0;
>>      initrd_addr = 0;
>> diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
>> index 31509d5..0a05100 100644
>> --- a/hw/timer/m48t59.c
>> +++ b/hw/timer/m48t59.c
>> @@ -56,6 +56,7 @@ struct M48t59State {
>>      MemoryRegion iomem;
>>      uint32_t io_base;
>>      uint32_t size;
>> +    uint32_t year_offset;
>>      /* RTC management */
>>      time_t   time_offset;
>>      time_t   stop_time;
>> @@ -346,11 +347,7 @@ void m48t59_write (void *opaque, uint32_t addr, uint32_t val)
>>         tmp = from_bcd(val);
>>         if (tmp >= 0 && tmp <= 99) {
>>             get_time(NVRAM, &tm);
>> -            if (NVRAM->model == 8) {
>> -                tm.tm_year = from_bcd(val) + 68; // Base year is 1968
>> -            } else {
>> -                tm.tm_year = from_bcd(val);
>> -            }
>> +            tm.tm_year = from_bcd(val) + NVRAM->year_offset;
>>             set_time(NVRAM, &tm);
>>         }
>>          break;
>> @@ -453,11 +450,7 @@ uint32_t m48t59_read (void *opaque, uint32_t addr)
>>      case 0x07FF:
>>          /* year */
>>          get_time(NVRAM, &tm);
>> -        if (NVRAM->model == 8) {
>> -            retval = to_bcd(tm.tm_year - 68); // Base year is 1968
>> -        } else {
>> -            retval = to_bcd(tm.tm_year);
>> -        }
>> +        retval = to_bcd(tm.tm_year - NVRAM->year_offset);
>>          break;
>>      default:
>>          /* Check lock registers state */
>> @@ -641,8 +634,8 @@ static const MemoryRegionOps m48t59_io_ops = {
>>  };
>>
>>  /* Initialisation routine */
>> -M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>> -                         uint32_t io_base, uint16_t size, int model)
>> +M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base, uint32_t io_base,
>> +                         uint16_t size, uint32_t year_offset, int model)
>>  {
>>      DeviceState *dev;
>>      SysBusDevice *s;
>> @@ -653,6 +646,7 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>>      qdev_prop_set_uint32(dev, "model", model);
>>      qdev_prop_set_uint32(dev, "size", size);
>>      qdev_prop_set_uint32(dev, "io_base", io_base);
>> +    qdev_prop_set_uint32(dev, "year_offset", year_offset);
>>      qdev_init_nofail(dev);
>>      s = SYS_BUS_DEVICE(dev);
>>      d = SYSBUS_M48T59(dev);
>> @@ -671,7 +665,7 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>>  }
>>
>>  M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
>> -                             int model)
>> +                             uint32_t year_offset, int model)
>>  {
>>      M48t59ISAState *d;
>>      ISADevice *isadev;
>> @@ -683,6 +677,7 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
>>      qdev_prop_set_uint32(dev, "model", model);
>>      qdev_prop_set_uint32(dev, "size", size);
>>      qdev_prop_set_uint32(dev, "io_base", io_base);
>> +    qdev_prop_set_uint32(dev, "year_offset", year_offset);
>>      qdev_init_nofail(dev);
>>      d = ISA_M48T59(isadev);
>>      s = &d->state;
>> @@ -738,9 +733,10 @@ static int m48t59_init1(SysBusDevice *dev)
>>  }
>>
>>  static Property m48t59_isa_properties[] = {
>> -    DEFINE_PROP_UINT32("size",    M48t59ISAState, state.size,    -1),
>> -    DEFINE_PROP_UINT32("model",   M48t59ISAState, state.model,   -1),
>> -    DEFINE_PROP_UINT32("io_base", M48t59ISAState, state.io_base,  0),
>> +    DEFINE_PROP_UINT32("size",        M48t59ISAState, state.size,       -1),
>> +    DEFINE_PROP_UINT32("model",       M48t59ISAState, state.model,      -1),
>> +    DEFINE_PROP_UINT32("io_base",     M48t59ISAState, state.io_base,     0),
> 
> Formatting-only change?

Yeah. It's one of those files whereby the properties are white-spaced
aligned according to the longest property name, and since "year_offset"
now becomes the longest property name everything else gets shifted...


ATB,

Mark.

[Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Mark Cave-Ayland]

Similar to m48t59_init(), add a mem_base value so that NVRAM can be mapped via
MMIO rather than ioport if required.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/prep.c             |    2 +-
 hw/sparc64/sun4u.c        |    2 +-
 hw/timer/m48t59.c         |    9 +++++++--
 include/hw/timer/m48t59.h |    4 ++--
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 378a368..4ed22ba 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -543,7 +543,7 @@ static void ppc_prep_init(MachineState *machine)
         pci_create_simple(pci_bus, -1, "pci-ohci");
     }
 
-    m48t59 = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 0, 59);
+    m48t59 = m48t59_init_isa(isa_bus, 0, 0x0074, NVRAM_SIZE, 0, 59);
     if (m48t59 == NULL)
         return;
     sysctrl->nvram = m48t59;
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 6b46511..86f5861 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -873,7 +873,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
         fd[i] = drive_get(IF_FLOPPY, 0, i);
     }
     fdctrl_init_isa(isa_bus, fd);
-    nvram = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 0, 59);
+    nvram = m48t59_init_isa(isa_bus, 0, 0x0074, NVRAM_SIZE, 0, 59);
 
     initrd_size = 0;
     initrd_addr = 0;
diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
index 0a05100..dc52ce3 100644
--- a/hw/timer/m48t59.c
+++ b/hw/timer/m48t59.c
@@ -664,8 +664,8 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base, uint32_t io_base,
     return state;
 }
 
-M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
-                             uint32_t year_offset, int model)
+M48t59State *m48t59_init_isa(ISABus *bus, hwaddr mem_base, uint32_t io_base,
+                             uint16_t size, uint32_t year_offset, int model)
 {
     M48t59ISAState *d;
     ISADevice *isadev;
@@ -686,6 +686,11 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
     if (io_base != 0) {
         isa_register_ioport(isadev, &d->io, io_base);
     }
+    if (mem_base != 0) {
+        memory_region_init_io(&s->iomem, OBJECT(d), &nvram_ops, s,
+                              "m48t59.nvram", s->size);
+        isa_register_ioport(isadev, &s->iomem, mem_base);
+    }
 
     return s;
 }
diff --git a/include/hw/timer/m48t59.h b/include/hw/timer/m48t59.h
index 08252b6..4bcd8fc 100644
--- a/include/hw/timer/m48t59.h
+++ b/include/hw/timer/m48t59.h
@@ -29,8 +29,8 @@ typedef struct M48t59State M48t59State;
 void m48t59_write (void *private, uint32_t addr, uint32_t val);
 uint32_t m48t59_read (void *private, uint32_t addr);
 void m48t59_toggle_lock (void *private, int lock);
-M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
-                             uint32_t year_offset, int type);
+M48t59State *m48t59_init_isa(ISABus *bus, hwaddr mem_base, uint32_t io_base,
+                             uint16_t size, uint32_t year_offset, int type);
 M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
                          uint32_t io_base, uint16_t size,
                          uint32_t year_offset, int type);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Paolo Bonzini]

On 19/01/2015 12:35, Mark Cave-Ayland wrote:
> Similar to m48t59_init(), add a mem_base value so that NVRAM can be mapped via
> MMIO rather than ioport if required.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---

Is it really ISA if it's MMIO?  In other words, why can't this be a
sysbus device?

Paolo

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Artyom Tarasenko]

On Mon, Jan 19, 2015 at 1:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 19/01/2015 12:35, Mark Cave-Ayland wrote:
>> Similar to m48t59_init(), add a mem_base value so that NVRAM can be mapped via
>> MMIO rather than ioport if required.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>
> Is it really ISA if it's MMIO?  In other words, why can't this be a
> sysbus device?

On physical machines it's EBus, which is pretty much like 8-bit ISA.
So, I think modelling it as ISA is closer to to the reality.
But out of curiosity, would it be possible to have a sysbus device
somewhere in a middle of PCI space? Do sysbus devices have higher
priority if the address spaces overlap? Or do you mean that the PCI
controller needs to be modified to have a hole for a sysbus device?

Artyom

-- 
Regards,
Artyom Tarasenko

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

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Paolo Bonzini]

On 19/01/2015 13:57, Artyom Tarasenko wrote:
>> > Is it really ISA if it's MMIO?  In other words, why can't this be a
>> > sysbus device?
> On physical machines it's EBus, which is pretty much like 8-bit ISA.
> So, I think modelling it as ISA is closer to to the reality.
> But out of curiosity, would it be possible to have a sysbus device
> somewhere in a middle of PCI space? Do sysbus devices have higher
> priority if the address spaces overlap? Or do you mean that the PCI
> controller needs to be modified to have a hole for a sysbus device?

What does the memory map look like (simplifying to "where can BARs be"
and "where is the RTC")?

Paolo

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Artyom Tarasenko]

On Mon, Jan 19, 2015 at 1:59 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 19/01/2015 13:57, Artyom Tarasenko wrote:
>>> > Is it really ISA if it's MMIO?  In other words, why can't this be a
>>> > sysbus device?
>> On physical machines it's EBus, which is pretty much like 8-bit ISA.
>> So, I think modelling it as ISA is closer to to the reality.
>> But out of curiosity, would it be possible to have a sysbus device
>> somewhere in a middle of PCI space? Do sysbus devices have higher
>> priority if the address spaces overlap? Or do you mean that the PCI
>> controller needs to be modified to have a hole for a sysbus device?
>
> What does the memory map look like (simplifying to "where can BARs be"
> and "where is the RTC")?

 dev: pbm, id ""
    mmio 000001fe00000000/0000000000010000
    mmio 000001fe01000000/0000000001000000
    mmio 000001fe02000000/0000000000010000
    bus: pci
      dev: ebus, id ""
        addr = 03.0
        class Bridge, addr 00:03.0, pci id 108e:1000 (sub 1af4:1100)
        bar 0: mem at 0x3000000 [0x3ffffff]
        bar 1: i/o at 0x4000 [0x7fff]
        bus: isa.0
          type ISA
          dev: m48t59_isa, id ""
            size = 8192 (0x2000)
            io_base = 8192 (0x2000)

(actually it should be better to have it at the beginning of the
ISA-space, 0x0, but it is not critical, since QEMU's sun4u machine
doesn't exactly match any known physical sun4u machine)


Artyom

-- 
Regards,
Artyom Tarasenko

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

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Andreas Färber]

Am 19.01.2015 um 13:57 schrieb Artyom Tarasenko:
> On Mon, Jan 19, 2015 at 1:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 19/01/2015 12:35, Mark Cave-Ayland wrote:
>>> Similar to m48t59_init(), add a mem_base value so that NVRAM can be mapped via
>>> MMIO rather than ioport if required.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>
>> Is it really ISA if it's MMIO?  In other words, why can't this be a
>> sysbus device?
> 
> On physical machines it's EBus, which is pretty much like 8-bit ISA.
> So, I think modelling it as ISA is closer to to the reality.
> But out of curiosity, would it be possible to have a sysbus device
> somewhere in a middle of PCI space? [...]

Why would you want to use a SysBusDevice in the first place? I
previously discussed with Mark that it should be an EBusDevice, not an
ISADevice or SysBusDevice. IndustryPack is an example of a custom bus
that sits behind a PCI bridge and doesn't need a global variable.

Also, wasn't Hervé's(?) plan to get rid of mem_base completely by always
passing a pointer to ISADevice/ISABus around? It should only be needed
when somewhere NULL is being passed, no?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Artyom Tarasenko]

On Mon, Jan 19, 2015 at 4:01 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 19.01.2015 um 13:57 schrieb Artyom Tarasenko:
>> On Mon, Jan 19, 2015 at 1:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> On 19/01/2015 12:35, Mark Cave-Ayland wrote:
>>>> Similar to m48t59_init(), add a mem_base value so that NVRAM can be mapped via
>>>> MMIO rather than ioport if required.
>>>>
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> ---
>>>
>>> Is it really ISA if it's MMIO?  In other words, why can't this be a
>>> sysbus device?
>>
>> On physical machines it's EBus, which is pretty much like 8-bit ISA.
>> So, I think modelling it as ISA is closer to to the reality.
>> But out of curiosity, would it be possible to have a sysbus device
>> somewhere in a middle of PCI space? [...]
>
> Why would you want to use a SysBusDevice in the first place?

Ask Paolo. :-) For me it's only important to have a MMIO device in the
proper address range.

> I previously discussed with Mark that it should be an EBusDevice, not an
> ISADevice or SysBusDevice.

Interesting. I can't find this discussion in the list archive. Do you suggest to
create EBusDevices for all ISA devices (serial, parallel, keyboard,
floppy) used in sun4u, or only for m48t59?
What would be the advantage of using EBusDevice over ISADevice?

Artyom

> IndustryPack is an example of a custom bus
> that sits behind a PCI bridge and doesn't need a global variable.
>
> Also, wasn't Hervé's(?) plan to get rid of mem_base completely by always
> passing a pointer to ISADevice/ISABus around? It should only be needed
> when somewhere NULL is being passed, no?
>
> Regards,
> Andreas
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
> Graham Norton; HRB 21284 (AG Nürnberg)



-- 
Regards,
Artyom Tarasenko

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

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Paolo Bonzini]

On 19/01/2015 16:22, Artyom Tarasenko wrote:
>>> >>
>>> >> On physical machines it's EBus, which is pretty much like 8-bit ISA.
>>> >> So, I think modelling it as ISA is closer to to the reality.
>>> >> But out of curiosity, would it be possible to have a sysbus device
>>> >> somewhere in a middle of PCI space? [...]
>> >
>> > Why would you want to use a SysBusDevice in the first place?
> Ask Paolo. :-) For me it's only important to have a MMIO device in the
> proper address range.

The reason I asked is simply because ISA devices never do MMIO (apart
for the VGA window).

>> > I previously discussed with Mark that it should be an EBusDevice, not an
>> > ISADevice or SysBusDevice.
> Interesting. I can't find this discussion in the list archive. Do you suggest to
> create EBusDevices for all ISA devices (serial, parallel, keyboard,
> floppy) used in sun4u, or only for m48t59?
> What would be the advantage of using EBusDevice over ISADevice?

Is there a description of EBus and the sun4u memory map somewhere?

Is there an "EBus bridge" PCI device similar to the PCI-to-ISA bridge?

Paolo

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Andreas Färber]

Am 19.01.2015 um 16:31 schrieb Paolo Bonzini:
> Is there an "EBus bridge" PCI device similar to the PCI-to-ISA bridge?

In the previous dump, the "ebus" entry looked like one to me?

    bus: pci
      dev: ebus, id ""
        addr = 03.0
        class Bridge, addr 00:03.0, pci id 108e:1000 (sub 1af4:1100)
        bar 0: mem at 0x3000000 [0x3ffffff]
        bar 1: i/o at 0x4000 [0x7fff]
        bus: isa.0
          type ISA

Confusingly named, as "ebus" would be a better type name for the actual
bus, but "EBus" might work as bus name if we actually need one (does the
user add devices to this bus or are these all chipset features
recursively set up by the machine?). Otherwise they could just be
child<> properties on the ebus device.

Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Paolo Bonzini]

On 19/01/2015 16:38, Andreas Färber wrote:
>> > Is there an "EBus bridge" PCI device similar to the PCI-to-ISA bridge?
> In the previous dump, the "ebus" entry looked like one to me?
> 
>     bus: pci
>       dev: ebus, id ""
>         addr = 03.0
>         class Bridge, addr 00:03.0, pci id 108e:1000 (sub 1af4:1100)
>         bar 0: mem at 0x3000000 [0x3ffffff]
>         bar 1: i/o at 0x4000 [0x7fff]
>         bus: isa.0
>           type ISA
> 
> Confusingly named, as "ebus" would be a better type name for the actual
> bus, but "EBus" might work as bus name if we actually need one (does the
> user add devices to this bus or are these all chipset features
> recursively set up by the machine?). Otherwise they could just be
> child<> properties on the ebus device.

So is this m48t59 device mapped inside a BAR of its parent ebus device?

What value will the sun4u patch pass for mem_base?

Paolo

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Artyom Tarasenko]

On Mon, Jan 19, 2015 at 4:31 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 19/01/2015 16:22, Artyom Tarasenko wrote:
>>>> >>
>>>> >> On physical machines it's EBus, which is pretty much like 8-bit ISA.
>>>> >> So, I think modelling it as ISA is closer to to the reality.
>>>> >> But out of curiosity, would it be possible to have a sysbus device
>>>> >> somewhere in a middle of PCI space? [...]
>>> >
>>> > Why would you want to use a SysBusDevice in the first place?
>> Ask Paolo. :-) For me it's only important to have a MMIO device in the
>> proper address range.
>
> The reason I asked is simply because ISA devices never do MMIO (apart
> for the VGA window).

You mean in the QEMU world? At least physical SCSI and Ethernet
adapters had a MMIO space for the onboard ROM.

>>> > I previously discussed with Mark that it should be an EBusDevice, not an
>>> > ISADevice or SysBusDevice.
>> Interesting. I can't find this discussion in the list archive. Do you suggest to
>> create EBusDevices for all ISA devices (serial, parallel, keyboard,
>> floppy) used in sun4u, or only for m48t59?
>> What would be the advantage of using EBusDevice over ISADevice?
>
> Is there a description of EBus and the sun4u memory map somewhere?

I could find only sparse pieces. "Uniprocessor System Controller
User's Manual" (805-0170.pdf) has some brief description, it's also
mentioned in the STP2223BGA  and STP2200ABGA data sheets.

> Is there an "EBus bridge" PCI device similar to the PCI-to-ISA bridge?

As physical devices there are integrated SBus-to-EBus and PCI-to-EBus bridges.

But actually I may have been wrong about NVRAM always sitting on the
EBus: looking at the page 28 of "UltraSPARC™-IIi User's Manual"
(805-0087.pdf), I see that NVRAM, Serial and other controllers reside
in a "PC compatible SuperIO" chip which sits on a PCI bus.

Artyom

-- 
Regards,
Artyom Tarasenko

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

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Paolo Bonzini]

On 19/01/2015 17:17, Artyom Tarasenko wrote:
> On Mon, Jan 19, 2015 at 4:31 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 19/01/2015 16:22, Artyom Tarasenko wrote:
>>>>>>>
>>>>>>> On physical machines it's EBus, which is pretty much like 8-bit ISA.
>>>>>>> So, I think modelling it as ISA is closer to to the reality.
>>>>>>> But out of curiosity, would it be possible to have a sysbus device
>>>>>>> somewhere in a middle of PCI space? [...]
>>>>>
>>>>> Why would you want to use a SysBusDevice in the first place?
>>> Ask Paolo. :-) For me it's only important to have a MMIO device in the
>>> proper address range.
>>
>> The reason I asked is simply because ISA devices never do MMIO (apart
>> for the VGA window).
> 
> You mean in the QEMU world? At least physical SCSI and Ethernet
> adapters had a MMIO space for the onboard ROM.

Uh right, ROMs count as MMIO too.

>>>>> I previously discussed with Mark that it should be an EBusDevice, not an
>>>>> ISADevice or SysBusDevice.
>>> Interesting. I can't find this discussion in the list archive. Do you suggest to
>>> create EBusDevices for all ISA devices (serial, parallel, keyboard,
>>> floppy) used in sun4u, or only for m48t59?
>>> What would be the advantage of using EBusDevice over ISADevice?
>>
>> Is there a description of EBus and the sun4u memory map somewhere?
> 
> I could find only sparse pieces. "Uniprocessor System Controller
> User's Manual" (805-0170.pdf) has some brief description, it's also
> mentioned in the STP2223BGA  and STP2200ABGA data sheets.
> 
>> Is there an "EBus bridge" PCI device similar to the PCI-to-ISA bridge?
> 
> As physical devices there are integrated SBus-to-EBus and PCI-to-EBus bridges.
> 
> But actually I may have been wrong about NVRAM always sitting on the
> EBus: looking at the page 28 of "UltraSPARC™-IIi User's Manual"
> (805-0087.pdf), I see that NVRAM, Serial and other controllers reside
> in a "PC compatible SuperIO" chip which sits on a PCI bus.

That's an ISA bridge basically.  I understand a little more of how this
is supposed to work now, but I think it makes little sense to add this
patch without the corresponding user.

Paolo

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Maciej W. Rozycki]

On Mon, 19 Jan 2015, Paolo Bonzini wrote:

> >> The reason I asked is simply because ISA devices never do MMIO (apart
> >> for the VGA window).
> > 
> > You mean in the QEMU world? At least physical SCSI and Ethernet
> > adapters had a MMIO space for the onboard ROM.
> 
> Uh right, ROMs count as MMIO too.

 Some ISA Ethernet cards also used MMIO for r/w access, probably to get at 
packet memory more efficiently (I don't remember the details offhand) as 
port I/O transactions were notoriously slow; in any case this is where the 
"Memory" field printed by `ifconfig' under Linux comes from.  I'm sure 
there was other ISA equipment too using MMIO for one purpose or another.

 On a PC/AT class x86 computer these resources would normally be allocated 
somehow to the memory space in the 0xd0000-0xeffff range, to work with 
real-mode software.  With "somehow" usually meaning jumpers, though newer 
cards may have had DOS configuration software available to set it up, in a 
similar manner to how ECU configured port I/O and MMIO resources for EISA 
equipment.

 BTW there were ISA DRAM expansion cards in existence too.

  Maciej

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Mark Cave-Ayland]

On 19/01/15 16:17, Artyom Tarasenko wrote:

>> Is there an "EBus bridge" PCI device similar to the PCI-to-ISA bridge?
> 
> As physical devices there are integrated SBus-to-EBus and PCI-to-EBus bridges.
> 
> But actually I may have been wrong about NVRAM always sitting on the
> EBus: looking at the page 28 of "UltraSPARC™-IIi User's Manual"
> (805-0087.pdf), I see that NVRAM, Serial and other controllers reside
> in a "PC compatible SuperIO" chip which sits on a PCI bus.

There's definitely a PCI device for a PCI-EBus bridge that we discover
during enumeration and attach to that (for example the EBus properties
in OpenBIOS can be found in the attach callback for the bridge PCI
device in drivers/pci.c).


ATB,

Mark.

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Mark Cave-Ayland]

On 19/01/15 15:31, Paolo Bonzini wrote:

> On 19/01/2015 16:22, Artyom Tarasenko wrote:
>>>>>>
>>>>>> On physical machines it's EBus, which is pretty much like 8-bit ISA.
>>>>>> So, I think modelling it as ISA is closer to to the reality.
>>>>>> But out of curiosity, would it be possible to have a sysbus device
>>>>>> somewhere in a middle of PCI space? [...]
>>>>
>>>> Why would you want to use a SysBusDevice in the first place?
>> Ask Paolo. :-) For me it's only important to have a MMIO device in the
>> proper address range.
> 
> The reason I asked is simply because ISA devices never do MMIO (apart
> for the VGA window).
> 
>>>> I previously discussed with Mark that it should be an EBusDevice, not an
>>>> ISADevice or SysBusDevice.
>> Interesting. I can't find this discussion in the list archive. Do you suggest to
>> create EBusDevices for all ISA devices (serial, parallel, keyboard,
>> floppy) used in sun4u, or only for m48t59?
>> What would be the advantage of using EBusDevice over ISADevice?
> 
> Is there a description of EBus and the sun4u memory map somewhere?

There are sample device trees from real hardware floating around, and
also snippets from various parts of Sun documentation (although sadly
Oracle have removed these now).

> Is there an "EBus bridge" PCI device similar to the PCI-to-ISA bridge?

Yes, that describes it pretty well. Also we can't just attach the NVRAM
to sysbus directly without some custom hacking to the OpenBIOS PCI
enumeration code as some OSs (particularly the *BSDs) locate the Ebus by
traversing down the PCI bus, locating the bridge device, and then
generating the physical addresses from calculating offsets from the
relevant address/reg properties.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Andreas Färber]

Am 19.01.2015 um 16:22 schrieb Artyom Tarasenko:
> On Mon, Jan 19, 2015 at 4:01 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 19.01.2015 um 13:57 schrieb Artyom Tarasenko:
>>> On Mon, Jan 19, 2015 at 1:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>> On 19/01/2015 12:35, Mark Cave-Ayland wrote:
>>>>> Similar to m48t59_init(), add a mem_base value so that NVRAM can be mapped via
>>>>> MMIO rather than ioport if required.
>>>>>
>>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>> ---
>>>>
>>>> Is it really ISA if it's MMIO?  In other words, why can't this be a
>>>> sysbus device?
>>>
>>> On physical machines it's EBus, which is pretty much like 8-bit ISA.
>>> So, I think modelling it as ISA is closer to to the reality.
>>> But out of curiosity, would it be possible to have a sysbus device
>>> somewhere in a middle of PCI space? [...]
>>
>> Why would you want to use a SysBusDevice in the first place?
> 
> Ask Paolo. :-) For me it's only important to have a MMIO device in the
> proper address range.
> 
>> I previously discussed with Mark that it should be an EBusDevice, not an
>> ISADevice or SysBusDevice.
> 
> Interesting. I can't find this discussion in the list archive.

Hm, am I mixing that up with SBus then? There were some helper functions
related to ROM loading being added as context to my suggestion that I
thought could become class fields.

> Do you suggest to
> create EBusDevices for all ISA devices (serial, parallel, keyboard,
> floppy) used in sun4u, or only for m48t59?
> What would be the advantage of using EBusDevice over ISADevice?

For all devices that are in fact EBus devices. The general idea is
encapsulation and abstraction - hiding the implementation detail of
whether it is internally an ISADevice or something else. Such a patch
should be quite trivial. Similarly it gives helper functions and
potential class methods a natural place to live.

Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Artyom Tarasenko]

On Mon, Jan 19, 2015 at 9:03 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 19.01.2015 um 16:22 schrieb Artyom Tarasenko:
>> On Mon, Jan 19, 2015 at 4:01 PM, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 19.01.2015 um 13:57 schrieb Artyom Tarasenko:
>>>> On Mon, Jan 19, 2015 at 1:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>> On 19/01/2015 12:35, Mark Cave-Ayland wrote:
>>>>>> Similar to m48t59_init(), add a mem_base value so that NVRAM can be mapped via
>>>>>> MMIO rather than ioport if required.
>>>>>>
>>>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>>> ---
>>>>>
>>>>> Is it really ISA if it's MMIO?  In other words, why can't this be a
>>>>> sysbus device?
>>>>
>>>> On physical machines it's EBus, which is pretty much like 8-bit ISA.
>>>> So, I think modelling it as ISA is closer to to the reality.
>>>> But out of curiosity, would it be possible to have a sysbus device
>>>> somewhere in a middle of PCI space? [...]
>>>
>>> Why would you want to use a SysBusDevice in the first place?
>>
>> Ask Paolo. :-) For me it's only important to have a MMIO device in the
>> proper address range.
>>
>>> I previously discussed with Mark that it should be an EBusDevice, not an
>>> ISADevice or SysBusDevice.
>>
>> Interesting. I can't find this discussion in the list archive.
>
> Hm, am I mixing that up with SBus then? There were some helper functions
> related to ROM loading being added as context to my suggestion that I
> thought could become class fields.

Yes, for SBus it totally makes sense.

>> Do you suggest to
>> create EBusDevices for all ISA devices (serial, parallel, keyboard,
>> floppy) used in sun4u, or only for m48t59?
>> What would be the advantage of using EBusDevice over ISADevice?
>
> For all devices that are in fact EBus devices. The general idea is
> encapsulation and abstraction - hiding the implementation detail of
> whether it is internally an ISADevice or something else. Such a patch
> should be quite trivial. Similarly it gives helper functions and
> potential class methods a natural place to live.

Yes, the patch is trivial. But EBus is nothing else but an ISA bus
with 8 data lines.
To me it looks like the bus width is an implementation detail we can
hide. (It's not documented what should happen if an EBus device is
accessed with a non 8-bit width. I tried 16 bit access on a physical
machine and it seemed to work).

Currently we can just use all the ISA devices unmodified if we like.
With EBus we would only be able to use a subset of ISA devices, no?

Artyom



-- 
Regards,
Artyom Tarasenko

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

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Mark Cave-Ayland]

On 19/01/15 15:01, Andreas Färber wrote:

> Am 19.01.2015 um 13:57 schrieb Artyom Tarasenko:
>> On Mon, Jan 19, 2015 at 1:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> On 19/01/2015 12:35, Mark Cave-Ayland wrote:
>>>> Similar to m48t59_init(), add a mem_base value so that NVRAM can be mapped via
>>>> MMIO rather than ioport if required.
>>>>
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> ---
>>>
>>> Is it really ISA if it's MMIO?  In other words, why can't this be a
>>> sysbus device?
>>
>> On physical machines it's EBus, which is pretty much like 8-bit ISA.
>> So, I think modelling it as ISA is closer to to the reality.
>> But out of curiosity, would it be possible to have a sysbus device
>> somewhere in a middle of PCI space? [...]
> 
> Why would you want to use a SysBusDevice in the first place? I
> previously discussed with Mark that it should be an EBusDevice, not an
> ISADevice or SysBusDevice. IndustryPack is an example of a custom bus
> that sits behind a PCI bridge and doesn't need a global variable.

I can see this makes logical sense - I guess the reason it hasn't been
done was to avoid having to write EBus-specific initialisation code for
every device which would only be used on one platform. So you're
suggesting that IndustryPack is a way of doing this?

> Also, wasn't Hervé's(?) plan to get rid of mem_base completely by always
> passing a pointer to ISADevice/ISABus around? It should only be needed
> when somewhere NULL is being passed, no?

That would definitely be better for wiring things up with -device but
wouldn't that involve changing all of the existing ISA devices?


ATB,

Mark.

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Hervé Poussineau]

Le 19/01/2015 16:01, Andreas Färber a écrit :
> Also, wasn't Hervé's(?) plan to get rid of mem_base completely by always
> passing a pointer to ISADevice/ISABus around? It should only be needed
> when somewhere NULL is being passed, no?

Yes, I've a patch series which is removing the isa_mem_base variable. I'll send it on ML.
ISA functions already take either a ISABus or a ISADevice, so they are good.
Next step would be to allow multiple ISA busses in hw/isa/isa-bus.c

Regards,

Hervé

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Peter Maydell]

On 19 January 2015 at 12:57, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
> But out of curiosity, would it be possible to have a sysbus device
> somewhere in a middle of PCI space? Do sysbus devices have higher
> priority if the address spaces overlap? Or do you mean that the PCI
> controller needs to be modified to have a hole for a sysbus device?

You can specify the priority when you map devices into a MemoryRegion,
so you can handle this by either making the sysbus device a positive
priority or by making the PCI window have a negative priority.

(We don't actually get this right on x86 currently,
which has resulted in some awkwardness for the PPC desire to
make PCI address 0 valid.)

-- PMM

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Mark Cave-Ayland]

On 19/01/15 15:04, Peter Maydell wrote:

> On 19 January 2015 at 12:57, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
>> But out of curiosity, would it be possible to have a sysbus device
>> somewhere in a middle of PCI space? Do sysbus devices have higher
>> priority if the address spaces overlap? Or do you mean that the PCI
>> controller needs to be modified to have a hole for a sysbus device?
> 
> You can specify the priority when you map devices into a MemoryRegion,
> so you can handle this by either making the sysbus device a positive
> priority or by making the PCI window have a negative priority.
> 
> (We don't actually get this right on x86 currently,
> which has resulted in some awkwardness for the PPC desire to
> make PCI address 0 valid.)

I'm not sure this would work for SPARC64 since potentially OpenBIOS can
program the I/O BAR for the ebus anywhere (and the NVRAM is located on
the ebus). At the moment we cheat by creating an alias to I/O space at
the top of memory so that OpenBIOS can always access it at a fixed address.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()

[Peter Maydell]

On 19 January 2015 at 16:48, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> I'm not sure this would work for SPARC64 since potentially OpenBIOS can
> program the I/O BAR for the ebus anywhere (and the NVRAM is located on
> the ebus). At the moment we cheat by creating an alias to I/O space at
> the top of memory so that OpenBIOS can always access it at a fixed address.

The MemoryRegion APIs should provide enough flexibility that you
can make QEMU do whatever the hardware does here (whether that is
"builtin devices higher priority than PCI window" or vice versa).
All you have to do is figure out what that actually is...

-- PMM

Re: [Qemu-devel] [PATCH 0/2] m48t59: add year offset and MMIO ISA mapping

[Artyom Tarasenko]

On Mon, Jan 19, 2015 at 12:35 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> This patch lays the groundwork for switching sun4u over from ioport NVRAM
> access to MMIO NVRAM access.
>
> Patch 1 introduces a new year_offset property which is the offset between the
> year value stored in hardware and the actual year. In particular, Sun hardware
> has a 68 year offset used to extend the date range of the IC. While the
> kernel sources I have viewed contain this offset within a #ifdef CONFIG_SPARC
> block, I've updated all the callers so that no change in behaviour will be
> seen across all platforms. PPC users may wish to alter the callers to change
> this behaviour as required.
>
> Patch 2 mimics the mem_base parameter from m48t59_init() to m48t59_init_isa()
> so that MMIO can be used for sun4u where the NVRAM is attached to the ebus
> which is essentially the same as an ISA bus.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

> Mark Cave-Ayland (2):
>   m48t59: introduce new year_offset qdev property
>   m48t59: add mem_base value to m48t59_init_isa()
>
>  hw/ppc/ppc405_boards.c    |    2 +-
>  hw/ppc/prep.c             |    2 +-
>  hw/sparc/sun4m.c          |    2 +-
>  hw/sparc64/sun4u.c        |    2 +-
>  hw/timer/m48t59.c         |   42 ++++++++++++++++++++++--------------------
>  include/hw/timer/m48t59.h |    7 ++++---
>  6 files changed, 30 insertions(+), 27 deletions(-)
>
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

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

Re: [Qemu-devel] [PATCH 0/2] m48t59: add year offset and MMIO ISA mapping

[Hervé Poussineau]

Le 19/01/2015 12:35, Mark Cave-Ayland a écrit :
> This patch lays the groundwork for switching sun4u over from ioport NVRAM
> access to MMIO NVRAM access.
>
> Patch 1 introduces a new year_offset property which is the offset between the
> year value stored in hardware and the actual year. In particular, Sun hardware
> has a 68 year offset used to extend the date range of the IC. While the
> kernel sources I have viewed contain this offset within a #ifdef CONFIG_SPARC
> block, I've updated all the callers so that no change in behaviour will be
> seen across all platforms. PPC users may wish to alter the callers to change
> this behaviour as required.
>
> Patch 2 mimics the mem_base parameter from m48t59_init() to m48t59_init_isa()
> so that MMIO can be used for sun4u where the NVRAM is attached to the ebus
> which is essentially the same as an ISA bus.

I've a patch which QOM'ifies m48t59, that I'll send to the list.
If you apply it, you'll be then able to create a sysbus-m48t02 device, and then to add it to ebus memory region.
IMO, there is no need to add a new parameter to m48t59_init_isa device.

Tell me what do you think of it.

Hervé

Re: [Qemu-devel] [PATCH 0/2] m48t59: add year offset and MMIO ISA mapping

[Artyom Tarasenko]

On Mon, Jan 19, 2015 at 10:59 PM, Hervé Poussineau <hpoussin@reactos.org> wrote:
> Le 19/01/2015 12:35, Mark Cave-Ayland a écrit :
>>
>> This patch lays the groundwork for switching sun4u over from ioport NVRAM
>> access to MMIO NVRAM access.
>>
>> Patch 1 introduces a new year_offset property which is the offset between
>> the
>> year value stored in hardware and the actual year. In particular, Sun
>> hardware
>> has a 68 year offset used to extend the date range of the IC. While the
>> kernel sources I have viewed contain this offset within a #ifdef
>> CONFIG_SPARC
>> block, I've updated all the callers so that no change in behaviour will be
>> seen across all platforms. PPC users may wish to alter the callers to
>> change
>> this behaviour as required.
>>
>> Patch 2 mimics the mem_base parameter from m48t59_init() to
>> m48t59_init_isa()
>> so that MMIO can be used for sun4u where the NVRAM is attached to the ebus
>> which is essentially the same as an ISA bus.
>
>
> I've a patch which QOM'ifies m48t59, that I'll send to the list.
> If you apply it, you'll be then able to create a sysbus-m48t02 device, and
> then to add it to ebus memory region.

Why m48t02? As discussed before, it shall be a m48t59 device:

http://lists.gnu.org/archive/html/qemu-devel/2013-04/msg05559.html

Artyom


-- 
Regards,
Artyom Tarasenko

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

Re: [Qemu-devel] [PATCH 0/2] m48t59: add year offset and MMIO ISA mapping

[Mark Cave-Ayland]

On 19/01/15 21:59, Hervé Poussineau wrote:

> Le 19/01/2015 12:35, Mark Cave-Ayland a écrit :
>> This patch lays the groundwork for switching sun4u over from ioport NVRAM
>> access to MMIO NVRAM access.
>>
>> Patch 1 introduces a new year_offset property which is the offset
>> between the
>> year value stored in hardware and the actual year. In particular, Sun
>> hardware
>> has a 68 year offset used to extend the date range of the IC. While the
>> kernel sources I have viewed contain this offset within a #ifdef
>> CONFIG_SPARC
>> block, I've updated all the callers so that no change in behaviour
>> will be
>> seen across all platforms. PPC users may wish to alter the callers to
>> change
>> this behaviour as required.
>>
>> Patch 2 mimics the mem_base parameter from m48t59_init() to
>> m48t59_init_isa()
>> so that MMIO can be used for sun4u where the NVRAM is attached to the
>> ebus
>> which is essentially the same as an ISA bus.
> 
> I've a patch which QOM'ifies m48t59, that I'll send to the list.
> If you apply it, you'll be then able to create a sysbus-m48t02 device,
> and then to add it to ebus memory region.
> IMO, there is no need to add a new parameter to m48t59_init_isa device.
> 
> Tell me what do you think of it.

It took me a while to go through these patches, but yes I think that
would work (i.e. create the sysbus-m48t59 device and then map it's
MemoryRegion directly into I/O space).

If these patches can be applied then I'm happy to rebase and resubmit my
patchset for the year_offset property. Who's tree should these changes
go through? Andreas' QOM tree?


ATB,

Mark.