All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device
@ 2018-02-09 18:51 Mark Cave-Ayland
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses Mark Cave-Ayland
                   ` (12 more replies)
  0 siblings, 13 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

The Mac CUDA device (also known as via-cuda) consists of two parts: a 6522 VIA
acting as an interface chip and the CUDA device itself.

Currently there are at least 2 other upcoming Mac devices that include their
own 6522 VIA implementations: Ben's via-pmu device and Laurent's mac_via
device with their own hacks based upon the existing CUDA device. So rather than
keep duplicating this same functionality with slightly different hacks, let's
move the 6522 into its own separate device.

Patches 1 converts CUDA away from using old_mmio accesses, whilst patches 2-3
came about from studying Laurent and Ben's implementations (and appear to have
no ill effects in my testing so far).

Patches 4-8 do the majority of the work which is to isolate the hacks required
by MacOS into separate functions so that a separate 6522 device can operate in
a standard way, yet allow a CUDA implementation to hook into the right places.

Patch 9 introduces a new mos6522 device for the shared functionality with the
relevant hook points for delayed SR interrupt delivery and MacOS timer
calibration hacks implemented as device class methods.

Patch 10 switches CUDA over from using its own 6522 implementation over
to using an embedded mos6522 device, allowing the duplicate code to be removed.

Finally patches 11-12 perform some additional tidy-up by moving the CUDA
definitions into a new cuda.h file and converting CUDA over to use trace-events.

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


v2:
- Add R-B tags from Laurent and Philippe
- Add David to CC as PPC maintainer and qemu-ppc for further comment
- Use DEVICE_BIG_ENDIAN in patch 1 as suggested by Laurent
- Rename *_INT constants in patch 9 to match those used in Ben's PMU patches
- Add patch 11 to move CUDA definitions into a new cuda.h file
- Add patch 12 to convert CUDA over to use trace-events


Mark Cave-Ayland (12):
  cuda: do not use old_mmio accesses
  cuda: don't allow writes to port output pins
  cuda: don't call cuda_update() when writing to ACR register
  cuda: introduce CUDAState parameter to get_counter()
  cuda: rename frequency property to tb_frequency
  cuda: minor cosmetic tidy-ups to get_next_irq_time()
  cuda: set timer 1 frequency property to CUDA_TIMER_FREQ
  cuda: factor out timebase-derived counter value and load time
  misc: introduce new mos6522 VIA device and enable it for ppc builds
  cuda: convert to use the shared mos6522 device
  ppc: move CUDAState and other CUDA-related definitions into separate
    cuda.h file
  cuda: convert to trace-events

 Makefile.objs                   |   1 +
 default-configs/ppc-softmmu.mak |   1 +
 hw/misc/Makefile.objs           |   3 +
 hw/misc/macio/cuda.c            | 666 +++++++++++-----------------------------
 hw/misc/macio/macio.c           |   3 +-
 hw/misc/macio/trace-events      |  11 +
 hw/misc/mos6522.c               | 505 ++++++++++++++++++++++++++++++
 hw/misc/trace-events            |   7 +
 hw/ppc/mac.h                    |  76 +----
 include/hw/misc/macio/cuda.h    | 107 +++++++
 include/hw/misc/mos6522.h       | 152 +++++++++
 11 files changed, 963 insertions(+), 569 deletions(-)
 create mode 100644 hw/misc/macio/trace-events
 create mode 100644 hw/misc/mos6522.c
 create mode 100644 include/hw/misc/macio/cuda.h
 create mode 100644 include/hw/misc/mos6522.h

-- 
2.11.0

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-09 20:05   ` Philippe Mathieu-Daudé
  2018-02-10  7:22   ` David Gibson
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 02/12] cuda: don't allow writes to port output pins Mark Cave-Ayland
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/cuda.c | 40 ++++++++--------------------------------
 1 file changed, 8 insertions(+), 32 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 008d8bd4d5..6631017ca2 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -275,7 +275,7 @@ static void cuda_delay_set_sr_int(CUDAState *s)
     timer_mod(s->sr_delay_timer, expire);
 }
 
-static uint32_t cuda_readb(void *opaque, hwaddr addr)
+static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size)
 {
     CUDAState *s = opaque;
     uint32_t val;
@@ -350,7 +350,7 @@ static uint32_t cuda_readb(void *opaque, hwaddr addr)
     return val;
 }
 
-static void cuda_writeb(void *opaque, hwaddr addr, uint32_t val)
+static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
 {
     CUDAState *s = opaque;
 
@@ -780,38 +780,14 @@ static void cuda_receive_packet_from_host(CUDAState *s,
     }
 }
 
-static void cuda_writew (void *opaque, hwaddr addr, uint32_t value)
-{
-}
-
-static void cuda_writel (void *opaque, hwaddr addr, uint32_t value)
-{
-}
-
-static uint32_t cuda_readw (void *opaque, hwaddr addr)
-{
-    return 0;
-}
-
-static uint32_t cuda_readl (void *opaque, hwaddr addr)
-{
-    return 0;
-}
-
 static const MemoryRegionOps cuda_ops = {
-    .old_mmio = {
-        .write = {
-            cuda_writeb,
-            cuda_writew,
-            cuda_writel,
-        },
-        .read = {
-            cuda_readb,
-            cuda_readw,
-            cuda_readl,
-        },
+    .read = cuda_read,
+    .write = cuda_write,
+    .endianness = DEVICE_BIG_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 1,
     },
-    .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static bool cuda_timer_exist(void *opaque, int version_id)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 02/12] cuda: don't allow writes to port output pins
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-10  7:23   ` David Gibson
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 03/12] cuda: don't call cuda_update() when writing to ACR register Mark Cave-Ayland
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

Use the direction registers as a mask to ensure that only input pins are
updated upon write.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/macio/cuda.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 6631017ca2..eaa8924f49 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -359,11 +359,11 @@ static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
 
     switch(addr) {
     case CUDA_REG_B:
-        s->b = val;
+        s->b = (s->b & ~s->dirb) | (val & s->dirb);
         cuda_update(s);
         break;
     case CUDA_REG_A:
-        s->a = val;
+        s->a = (s->a & ~s->dira) | (val & s->dira);
         break;
     case CUDA_REG_DIRB:
         s->dirb = val;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 03/12] cuda: don't call cuda_update() when writing to ACR register
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses Mark Cave-Ayland
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 02/12] cuda: don't allow writes to port output pins Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-10 22:35   ` David Gibson
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 04/12] cuda: introduce CUDAState parameter to get_counter() Mark Cave-Ayland
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

The wire protocol for reading data to/from the VIA is triggered by changing
inputs on port B rather than changing the timer configuration via the ACR.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/cuda.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index eaa8924f49..1d0f7e8289 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -406,7 +406,6 @@ static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
     case CUDA_REG_ACR:
         s->acr = val;
         cuda_timer_update(s, &s->timers[0], qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
-        cuda_update(s);
         break;
     case CUDA_REG_PCR:
         s->pcr = val;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 04/12] cuda: introduce CUDAState parameter to get_counter()
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
                   ` (2 preceding siblings ...)
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 03/12] cuda: don't call cuda_update() when writing to ACR register Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-10 22:31   ` David Gibson
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 05/12] cuda: rename frequency property to tb_frequency Mark Cave-Ayland
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

This will be required shortly and also happens to match nicely with the
corresponding signature for set_counter().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/macio/cuda.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 1d0f7e8289..a88535fa66 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -150,7 +150,7 @@ static uint64_t get_tb(uint64_t time, uint64_t freq)
     return muldiv64(time, freq, NANOSECONDS_PER_SECOND);
 }
 
-static unsigned int get_counter(CUDATimer *ti)
+static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
 {
     int64_t d;
     unsigned int counter;
@@ -295,12 +295,12 @@ static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size)
         val = s->dira;
         break;
     case CUDA_REG_T1CL:
-        val = get_counter(&s->timers[0]) & 0xff;
+        val = get_counter(s, &s->timers[0]) & 0xff;
         s->ifr &= ~T1_INT;
         cuda_update_irq(s);
         break;
     case CUDA_REG_T1CH:
-        val = get_counter(&s->timers[0]) >> 8;
+        val = get_counter(s, &s->timers[0]) >> 8;
         cuda_update_irq(s);
         break;
     case CUDA_REG_T1LL:
@@ -311,12 +311,12 @@ static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size)
         val = (s->timers[0].latch >> 8) & 0xff;
         break;
     case CUDA_REG_T2CL:
-        val = get_counter(&s->timers[1]) & 0xff;
+        val = get_counter(s, &s->timers[1]) & 0xff;
         s->ifr &= ~T2_INT;
         cuda_update_irq(s);
         break;
     case CUDA_REG_T2CH:
-        val = get_counter(&s->timers[1]) >> 8;
+        val = get_counter(s, &s->timers[1]) >> 8;
         break;
     case CUDA_REG_SR:
         val = s->sr;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 05/12] cuda: rename frequency property to tb_frequency
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
                   ` (3 preceding siblings ...)
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 04/12] cuda: introduce CUDAState parameter to get_counter() Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-10 22:32   ` David Gibson
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 06/12] cuda: minor cosmetic tidy-ups to get_next_irq_time() Mark Cave-Ayland
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

This allows us to more easily differentiate between the timebase frequency used
to calibrate the MacOS timers and the actual frequency of the hardware clock as
indicated by CUDA_TIMER_FREQ.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/macio/cuda.c  | 10 +++++-----
 hw/misc/macio/macio.c |  2 +-
 hw/ppc/mac.h          |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index a88535fa66..232b7f61aa 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -158,8 +158,8 @@ static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
     uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
     /* Reverse of the tb calculation algorithm that Mac OS X uses on bootup. */
-    tb_diff = get_tb(current_time, ti->frequency) - ti->load_time;
-    d = (tb_diff * 0xBF401675E5DULL) / (ti->frequency << 24);
+    tb_diff = get_tb(current_time, ti->tb_frequency) - ti->load_time;
+    d = (tb_diff * 0xBF401675E5DULL) / (ti->tb_frequency << 24);
 
     if (ti->index == 0) {
         /* the timer goes down from latch to -1 (period of latch + 2) */
@@ -179,7 +179,7 @@ static void set_counter(CUDAState *s, CUDATimer *ti, unsigned int val)
 {
     CUDA_DPRINTF("T%d.counter=%d\n", 1 + ti->index, val);
     ti->load_time = get_tb(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                           s->frequency);
+                           s->tb_frequency);
     ti->counter_value = val;
     cuda_timer_update(s, ti, ti->load_time);
 }
@@ -878,7 +878,7 @@ static void cuda_realizefn(DeviceState *dev, Error **errp)
     struct tm tm;
 
     s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer1, s);
-    s->timers[0].frequency = s->frequency;
+    s->timers[0].frequency = s->tb_frequency;
     s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer2, s);
     s->timers[1].frequency = (SCALE_US * 6000) / 4700;
 
@@ -909,7 +909,7 @@ static void cuda_initfn(Object *obj)
 }
 
 static Property cuda_properties[] = {
-    DEFINE_PROP_UINT64("frequency", CUDAState, frequency, 0),
+    DEFINE_PROP_UINT64("timebase-frequency", CUDAState, tb_frequency, 0),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 44f91d1e7f..a639b09e00 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -451,7 +451,7 @@ void macio_init(PCIDevice *d,
     macio_state->escc_mem = escc_mem;
     /* Note: this code is strongly inspirated from the corresponding code
        in PearPC */
-    qdev_prop_set_uint64(DEVICE(&macio_state->cuda), "frequency",
+    qdev_prop_set_uint64(DEVICE(&macio_state->cuda), "timebase-frequency",
                          macio_state->frequency);
 
     qdev_init_nofail(DEVICE(d));
diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index b501af1653..fa78115c95 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -99,7 +99,7 @@ typedef struct CUDAState {
     CUDATimer timers[2];
 
     uint32_t tick_offset;
-    uint64_t frequency;
+    uint64_t tb_frequency;
 
     uint8_t last_b;
     uint8_t last_acr;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 06/12] cuda: minor cosmetic tidy-ups to get_next_irq_time()
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
                   ` (4 preceding siblings ...)
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 05/12] cuda: rename frequency property to tb_frequency Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-10 22:33   ` David Gibson
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 07/12] cuda: set timer 1 frequency property to CUDA_TIMER_FREQ Mark Cave-Ayland
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/macio/cuda.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 232b7f61aa..408858e688 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -184,36 +184,37 @@ static void set_counter(CUDAState *s, CUDATimer *ti, unsigned int val)
     cuda_timer_update(s, ti, ti->load_time);
 }
 
-static int64_t get_next_irq_time(CUDATimer *s, int64_t current_time)
+static int64_t get_next_irq_time(CUDATimer *ti, int64_t current_time)
 {
     int64_t d, next_time;
     unsigned int counter;
 
     /* current counter value */
-    d = muldiv64(current_time - s->load_time,
+    d = muldiv64(current_time - ti->load_time,
                  CUDA_TIMER_FREQ, NANOSECONDS_PER_SECOND);
     /* the timer goes down from latch to -1 (period of latch + 2) */
-    if (d <= (s->counter_value + 1)) {
-        counter = (s->counter_value - d) & 0xffff;
+    if (d <= (ti->counter_value + 1)) {
+        counter = (ti->counter_value - d) & 0xffff;
     } else {
-        counter = (d - (s->counter_value + 1)) % (s->latch + 2);
-        counter = (s->latch - counter) & 0xffff;
+        counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
+        counter = (ti->latch - counter) & 0xffff;
     }
 
     /* Note: we consider the irq is raised on 0 */
     if (counter == 0xffff) {
-        next_time = d + s->latch + 1;
+        next_time = d + ti->latch + 1;
     } else if (counter == 0) {
-        next_time = d + s->latch + 2;
+        next_time = d + ti->latch + 2;
     } else {
         next_time = d + counter;
     }
     CUDA_DPRINTF("latch=%d counter=%" PRId64 " delta_next=%" PRId64 "\n",
-                 s->latch, d, next_time - d);
+                 ti->latch, d, next_time - d);
     next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, CUDA_TIMER_FREQ) +
-        s->load_time;
-    if (next_time <= current_time)
+                         ti->load_time;
+    if (next_time <= current_time) {
         next_time = current_time + 1;
+    }
     return next_time;
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 07/12] cuda: set timer 1 frequency property to CUDA_TIMER_FREQ
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
                   ` (5 preceding siblings ...)
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 06/12] cuda: minor cosmetic tidy-ups to get_next_irq_time() Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-10 23:15   ` David Gibson
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 08/12] cuda: factor out timebase-derived counter value and load time Mark Cave-Ayland
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

Now that we have successfully decoupled the timebase frequency and the hardware
timer frequency, set the timer 1 frequency property to CUDA_TIMER_FREQ and alter
get_next_irq_time() to use it rather than the hard-coded constant.

In addition to this we must now switch the tb_diff calculation over to use the
timebase frequency now that the hardware clock frequency and the timebase
frequency are different.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/cuda.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 408858e688..e00df4a21a 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -158,8 +158,8 @@ static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
     uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
     /* Reverse of the tb calculation algorithm that Mac OS X uses on bootup. */
-    tb_diff = get_tb(current_time, ti->tb_frequency) - ti->load_time;
-    d = (tb_diff * 0xBF401675E5DULL) / (ti->tb_frequency << 24);
+    tb_diff = get_tb(current_time, s->tb_frequency) - ti->load_time;
+    d = (tb_diff * 0xBF401675E5DULL) / (s->tb_frequency << 24);
 
     if (ti->index == 0) {
         /* the timer goes down from latch to -1 (period of latch + 2) */
@@ -191,7 +191,7 @@ static int64_t get_next_irq_time(CUDATimer *ti, int64_t current_time)
 
     /* current counter value */
     d = muldiv64(current_time - ti->load_time,
-                 CUDA_TIMER_FREQ, NANOSECONDS_PER_SECOND);
+                 ti->frequency, NANOSECONDS_PER_SECOND);
     /* the timer goes down from latch to -1 (period of latch + 2) */
     if (d <= (ti->counter_value + 1)) {
         counter = (ti->counter_value - d) & 0xffff;
@@ -210,7 +210,7 @@ static int64_t get_next_irq_time(CUDATimer *ti, int64_t current_time)
     }
     CUDA_DPRINTF("latch=%d counter=%" PRId64 " delta_next=%" PRId64 "\n",
                  ti->latch, d, next_time - d);
-    next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, CUDA_TIMER_FREQ) +
+    next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) +
                          ti->load_time;
     if (next_time <= current_time) {
         next_time = current_time + 1;
@@ -879,7 +879,7 @@ static void cuda_realizefn(DeviceState *dev, Error **errp)
     struct tm tm;
 
     s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer1, s);
-    s->timers[0].frequency = s->tb_frequency;
+    s->timers[0].frequency = CUDA_TIMER_FREQ;
     s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer2, s);
     s->timers[1].frequency = (SCALE_US * 6000) / 4700;
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 08/12] cuda: factor out timebase-derived counter value and load time
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
                   ` (6 preceding siblings ...)
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 07/12] cuda: set timer 1 frequency property to CUDA_TIMER_FREQ Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-10 23:18   ` David Gibson
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 09/12] misc: introduce new mos6522 VIA device and enable it for ppc builds Mark Cave-Ayland
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

Commit b981289c49 "PPC: Cuda: Use cuda timer to expose tbfreq to guest" altered
the timer calculations from those based upon the hardware CUDA clock frequency
to those based upon the CPU timebase frequency.

In fact we can isolate the differences to 2 simple changes: one to the counter
read value and another to the counter load time. Move these changes into
separate functions so the implementation can be swapped later.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/macio/cuda.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index e00df4a21a..a185252144 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -145,21 +145,29 @@ static void cuda_update_irq(CUDAState *s)
     }
 }
 
-static uint64_t get_tb(uint64_t time, uint64_t freq)
+static uint64_t get_counter_value(CUDAState *s, CUDATimer *ti)
 {
-    return muldiv64(time, freq, NANOSECONDS_PER_SECOND);
+    /* Reverse of the tb calculation algorithm that Mac OS X uses on bootup */
+    uint64_t tb_diff = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+                                s->tb_frequency, NANOSECONDS_PER_SECOND) -
+                           ti->load_time;
+
+    return (tb_diff * 0xBF401675E5DULL) / (s->tb_frequency << 24);
+}
+
+static uint64_t get_counter_load_time(CUDAState *s, CUDATimer *ti)
+{
+    uint64_t load_time = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+                                  s->tb_frequency, NANOSECONDS_PER_SECOND);
+    return load_time;
 }
 
 static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
 {
     int64_t d;
     unsigned int counter;
-    uint64_t tb_diff;
-    uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
-    /* Reverse of the tb calculation algorithm that Mac OS X uses on bootup. */
-    tb_diff = get_tb(current_time, s->tb_frequency) - ti->load_time;
-    d = (tb_diff * 0xBF401675E5DULL) / (s->tb_frequency << 24);
+    d = get_counter_value(s, ti);
 
     if (ti->index == 0) {
         /* the timer goes down from latch to -1 (period of latch + 2) */
@@ -178,8 +186,7 @@ static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
 static void set_counter(CUDAState *s, CUDATimer *ti, unsigned int val)
 {
     CUDA_DPRINTF("T%d.counter=%d\n", 1 + ti->index, val);
-    ti->load_time = get_tb(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                           s->tb_frequency);
+    ti->load_time = get_counter_load_time(s, ti);
     ti->counter_value = val;
     cuda_timer_update(s, ti, ti->load_time);
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 09/12] misc: introduce new mos6522 VIA device and enable it for ppc builds
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
                   ` (7 preceding siblings ...)
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 08/12] cuda: factor out timebase-derived counter value and load time Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-10 23:20   ` David Gibson
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 10/12] cuda: convert to use the shared mos6522 device Mark Cave-Ayland
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

The MOS6522 VIA forms the bridge part of several Mac devices, including the
Mac via-cuda and via-pmu devices. Introduce a standard mos6522 device that
can be shared amongst multiple implementations.

This is effectively taking the 6522 parts out of cuda.c and turning them
into a separate device whilst also applying some style tidy-ups and including
a conversion to trace-events.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 default-configs/ppc-softmmu.mak |   1 +
 hw/misc/Makefile.objs           |   3 +
 hw/misc/mos6522.c               | 505 ++++++++++++++++++++++++++++++++++++++++
 hw/misc/trace-events            |   7 +
 include/hw/misc/mos6522.h       | 152 ++++++++++++
 5 files changed, 668 insertions(+)
 create mode 100644 hw/misc/mos6522.c
 create mode 100644 include/hw/misc/mos6522.h

diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 65680d85bc..76e29cfa14 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -30,6 +30,7 @@ CONFIG_MAC=y
 CONFIG_ESCC=y
 CONFIG_MACIO=y
 CONFIG_SUNGEM=y
+CONFIG_MOS6522=y
 CONFIG_CUDA=y
 CONFIG_ADB=y
 CONFIG_MAC_NVRAM=y
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index fce426eb75..f33b37a8e5 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -17,6 +17,9 @@ common-obj-$(CONFIG_INTEGRATOR_DEBUG) += arm_integrator_debug.o
 common-obj-$(CONFIG_A9SCU) += a9scu.o
 common-obj-$(CONFIG_ARM11SCU) += arm11scu.o
 
+# Mac devices
+common-obj-$(CONFIG_MOS6522) += mos6522.o
+
 # PKUnity SoC devices
 common-obj-$(CONFIG_PUV3) += puv3_pm.o
 
diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
new file mode 100644
index 0000000000..8ad9fc831e
--- /dev/null
+++ b/hw/misc/mos6522.c
@@ -0,0 +1,505 @@
+/*
+ * QEMU MOS6522 VIA emulation
+ *
+ * Copyright (c) 2004-2007 Fabrice Bellard
+ * Copyright (c) 2007 Jocelyn Mayer
+ * Copyright (c) 2018 Mark Cave-Ayland
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+#include "hw/hw.h"
+#include "hw/input/adb.h"
+#include "hw/misc/mos6522.h"
+#include "qemu/timer.h"
+#include "sysemu/sysemu.h"
+#include "qemu/cutils.h"
+#include "qemu/log.h"
+#include "trace.h"
+
+/* XXX: implement all timer modes */
+
+static void mos6522_timer_update(MOS6522State *s, MOS6522Timer *ti,
+                                 int64_t current_time);
+
+static void mos6522_update_irq(MOS6522State *s)
+{
+    if (s->ifr & s->ier & (SR_INT | T1_INT | T2_INT)) {
+        qemu_irq_raise(s->irq);
+    } else {
+        qemu_irq_lower(s->irq);
+    }
+}
+
+static uint64_t get_counter_value(MOS6522State *s, MOS6522Timer *ti)
+{
+    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(s);
+
+    if (ti->index == 0) {
+        return mdc->get_timer1_counter_value(s, ti);
+    } else {
+        return mdc->get_timer2_counter_value(s, ti);
+    }
+}
+
+static uint64_t get_load_time(MOS6522State *s, MOS6522Timer *ti)
+{
+    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(s);
+
+    if (ti->index == 0) {
+        return mdc->get_timer1_load_time(s, ti);
+    } else {
+        return mdc->get_timer2_load_time(s, ti);
+    }
+}
+
+static unsigned int get_counter(MOS6522State *s, MOS6522Timer *ti)
+{
+    int64_t d;
+    unsigned int counter;
+
+    d = get_counter_value(s, ti);
+
+    if (ti->index == 0) {
+        /* the timer goes down from latch to -1 (period of latch + 2) */
+        if (d <= (ti->counter_value + 1)) {
+            counter = (ti->counter_value - d) & 0xffff;
+        } else {
+            counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
+            counter = (ti->latch - counter) & 0xffff;
+        }
+    } else {
+        counter = (ti->counter_value - d) & 0xffff;
+    }
+    return counter;
+}
+
+static void set_counter(MOS6522State *s, MOS6522Timer *ti, unsigned int val)
+{
+    trace_mos6522_set_counter(1 + ti->index, val);
+    ti->load_time = get_load_time(s, ti);
+    ti->counter_value = val;
+    mos6522_timer_update(s, ti, ti->load_time);
+}
+
+static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti,
+                                 int64_t current_time)
+{
+    int64_t d, next_time;
+    unsigned int counter;
+
+    /* current counter value */
+    d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time,
+                 ti->frequency, NANOSECONDS_PER_SECOND);
+
+    /* the timer goes down from latch to -1 (period of latch + 2) */
+    if (d <= (ti->counter_value + 1)) {
+        counter = (ti->counter_value - d) & 0xffff;
+    } else {
+        counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
+        counter = (ti->latch - counter) & 0xffff;
+    }
+
+    /* Note: we consider the irq is raised on 0 */
+    if (counter == 0xffff) {
+        next_time = d + ti->latch + 1;
+    } else if (counter == 0) {
+        next_time = d + ti->latch + 2;
+    } else {
+        next_time = d + counter;
+    }
+    trace_mos6522_get_next_irq_time(ti->latch, d, next_time - d);
+    next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) +
+                         ti->load_time;
+    if (next_time <= current_time) {
+        next_time = current_time + 1;
+    }
+    return next_time;
+}
+
+static void mos6522_timer_update(MOS6522State *s, MOS6522Timer *ti,
+                                 int64_t current_time)
+{
+    if (!ti->timer) {
+        return;
+    }
+    if (ti->index == 0 && (s->acr & T1MODE) != T1MODE_CONT) {
+        timer_del(ti->timer);
+    } else {
+        ti->next_irq_time = get_next_irq_time(s, ti, current_time);
+        timer_mod(ti->timer, ti->next_irq_time);
+    }
+}
+
+static void mos6522_timer1(void *opaque)
+{
+    MOS6522State *s = opaque;
+    MOS6522Timer *ti = &s->timers[0];
+
+    mos6522_timer_update(s, ti, ti->next_irq_time);
+    s->ifr |= T1_INT;
+    mos6522_update_irq(s);
+}
+
+static void mos6522_timer2(void *opaque)
+{
+    MOS6522State *s = opaque;
+    MOS6522Timer *ti = &s->timers[1];
+
+    mos6522_timer_update(s, ti, ti->next_irq_time);
+    s->ifr |= T2_INT;
+    mos6522_update_irq(s);
+}
+
+static void mos6522_set_sr_int(MOS6522State *s)
+{
+    trace_mos6522_set_sr_int();
+    s->ifr |= SR_INT;
+    mos6522_update_irq(s);
+}
+
+static uint64_t mos6522_get_counter_value(MOS6522State *s, MOS6522Timer *ti)
+{
+    uint64_t d;
+
+    d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time,
+                 ti->frequency, NANOSECONDS_PER_SECOND);
+
+    return d;
+}
+
+static uint64_t mos6522_get_load_time(MOS6522State *s, MOS6522Timer *ti)
+{
+    uint64_t load_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+
+    return load_time;
+}
+
+static void mos6522_portA_write(MOS6522State *s)
+{
+    qemu_log_mask(LOG_UNIMP, "portA_write unimplemented");
+}
+
+static void mos6522_portB_write(MOS6522State *s)
+{
+    qemu_log_mask(LOG_UNIMP, "portB_write unimplemented");
+}
+
+uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size)
+{
+    MOS6522State *s = opaque;
+    uint32_t val;
+
+    switch (addr) {
+    case VIA_REG_B:
+        val = s->b;
+        break;
+    case VIA_REG_A:
+        val = s->a;
+        break;
+    case VIA_REG_DIRB:
+        val = s->dirb;
+        break;
+    case VIA_REG_DIRA:
+        val = s->dira;
+        break;
+    case VIA_REG_T1CL:
+        val = get_counter(s, &s->timers[0]) & 0xff;
+        s->ifr &= ~T1_INT;
+        mos6522_update_irq(s);
+        break;
+    case VIA_REG_T1CH:
+        val = get_counter(s, &s->timers[0]) >> 8;
+        mos6522_update_irq(s);
+        break;
+    case VIA_REG_T1LL:
+        val = s->timers[0].latch & 0xff;
+        break;
+    case VIA_REG_T1LH:
+        /* XXX: check this */
+        val = (s->timers[0].latch >> 8) & 0xff;
+        break;
+    case VIA_REG_T2CL:
+        val = get_counter(s, &s->timers[1]) & 0xff;
+        s->ifr &= ~T2_INT;
+        mos6522_update_irq(s);
+        break;
+    case VIA_REG_T2CH:
+        val = get_counter(s, &s->timers[1]) >> 8;
+        break;
+    case VIA_REG_SR:
+        val = s->sr;
+        s->ifr &= ~(SR_INT | CB1_INT | CB2_INT);
+        mos6522_update_irq(s);
+        break;
+    case VIA_REG_ACR:
+        val = s->acr;
+        break;
+    case VIA_REG_PCR:
+        val = s->pcr;
+        break;
+    case VIA_REG_IFR:
+        val = s->ifr;
+        if (s->ifr & s->ier) {
+            val |= 0x80;
+        }
+        break;
+    case VIA_REG_IER:
+        val = s->ier | 0x80;
+        break;
+    default:
+    case VIA_REG_ANH:
+        val = s->anh;
+        break;
+    }
+
+    if (addr != VIA_REG_IFR || val != 0) {
+        trace_mos6522_read(addr, val);
+    }
+
+    return val;
+}
+
+void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+    MOS6522State *s = opaque;
+    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(s);
+
+    trace_mos6522_write(addr, val);
+
+    switch (addr) {
+    case VIA_REG_B:
+        s->b = (s->b & ~s->dirb) | (val & s->dirb);
+        mdc->portB_write(s);
+        break;
+    case VIA_REG_A:
+        s->a = (s->a & ~s->dira) | (val & s->dira);
+        mdc->portA_write(s);
+        break;
+    case VIA_REG_DIRB:
+        s->dirb = val;
+        break;
+    case VIA_REG_DIRA:
+        s->dira = val;
+        break;
+    case VIA_REG_T1CL:
+        s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
+        mos6522_timer_update(s, &s->timers[0],
+                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+        break;
+    case VIA_REG_T1CH:
+        s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
+        s->ifr &= ~T1_INT;
+        set_counter(s, &s->timers[0], s->timers[0].latch);
+        break;
+    case VIA_REG_T1LL:
+        s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
+        mos6522_timer_update(s, &s->timers[0],
+                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+        break;
+    case VIA_REG_T1LH:
+        s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
+        s->ifr &= ~T1_INT;
+        mos6522_timer_update(s, &s->timers[0],
+                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+        break;
+    case VIA_REG_T2CL:
+        s->timers[1].latch = (s->timers[1].latch & 0xff00) | val;
+        break;
+    case VIA_REG_T2CH:
+        /* To ensure T2 generates an interrupt on zero crossing with the
+           common timer code, write the value directly from the latch to
+           the counter */
+        s->timers[1].latch = (s->timers[1].latch & 0xff) | (val << 8);
+        s->ifr &= ~T2_INT;
+        set_counter(s, &s->timers[1], s->timers[1].latch);
+        break;
+    case VIA_REG_SR:
+        s->sr = val;
+        break;
+    case VIA_REG_ACR:
+        s->acr = val;
+        mos6522_timer_update(s, &s->timers[0],
+                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+        break;
+    case VIA_REG_PCR:
+        s->pcr = val;
+        break;
+    case VIA_REG_IFR:
+        /* reset bits */
+        s->ifr &= ~val;
+        mos6522_update_irq(s);
+        break;
+    case VIA_REG_IER:
+        if (val & IER_SET) {
+            /* set bits */
+            s->ier |= val & 0x7f;
+        } else {
+            /* reset bits */
+            s->ier &= ~val;
+        }
+        mos6522_update_irq(s);
+        break;
+    default:
+    case VIA_REG_ANH:
+        s->anh = val;
+        break;
+    }
+}
+
+static const MemoryRegionOps mos6522_ops = {
+    .read = mos6522_read,
+    .write = mos6522_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+static bool mos6522_timer_exist(void *opaque, int version_id)
+{
+    MOS6522Timer *s = opaque;
+
+    return s->timer != NULL;
+}
+
+static const VMStateDescription vmstate_mos6522_timer = {
+    .name = "mos6522_timer",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT16(latch, MOS6522Timer),
+        VMSTATE_UINT16(counter_value, MOS6522Timer),
+        VMSTATE_INT64(load_time, MOS6522Timer),
+        VMSTATE_INT64(next_irq_time, MOS6522Timer),
+        VMSTATE_TIMER_PTR_TEST(timer, MOS6522Timer, mos6522_timer_exist),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_mos6522 = {
+    .name = "mos6522",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(a, MOS6522State),
+        VMSTATE_UINT8(b, MOS6522State),
+        VMSTATE_UINT8(dira, MOS6522State),
+        VMSTATE_UINT8(dirb, MOS6522State),
+        VMSTATE_UINT8(sr, MOS6522State),
+        VMSTATE_UINT8(acr, MOS6522State),
+        VMSTATE_UINT8(pcr, MOS6522State),
+        VMSTATE_UINT8(ifr, MOS6522State),
+        VMSTATE_UINT8(ier, MOS6522State),
+        VMSTATE_UINT8(anh, MOS6522State),
+        VMSTATE_STRUCT_ARRAY(timers, MOS6522State, 2, 1,
+                             vmstate_mos6522_timer, MOS6522Timer),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void mos6522_reset(DeviceState *dev)
+{
+    MOS6522State *s = MOS6522(dev);
+
+    s->b = 0;
+    s->a = 0;
+    s->dirb = 0xff;
+    s->dira = 0;
+    s->sr = 0;
+    s->acr = 0;
+    s->pcr = 0;
+    s->ifr = 0;
+    s->ier = 0;
+    /* s->ier = T1_INT | SR_INT; */
+    s->anh = 0;
+
+    s->timers[0].latch = 0xffff;
+    set_counter(s, &s->timers[0], 0xffff);
+
+    s->timers[1].latch = 0xffff;
+}
+
+static void mos6522_realize(DeviceState *dev, Error **errp)
+{
+    MOS6522State *s = MOS6522(dev);
+
+    s->timers[0].frequency = s->frequency;
+    s->timers[1].frequency = s->frequency;
+}
+
+static void mos6522_init(Object *obj)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    MOS6522State *s = MOS6522(obj);
+    int i;
+
+    memory_region_init_io(&s->mem, obj, &mos6522_ops, s, "mos6522", 0x10);
+    sysbus_init_mmio(sbd, &s->mem);
+    sysbus_init_irq(sbd, &s->irq);
+
+    for (i = 0; i < ARRAY_SIZE(s->timers); i++) {
+        s->timers[i].index = i;
+    }
+
+    s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer1, s);
+    s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer2, s);
+}
+
+static Property mos6522_properties[] = {
+    DEFINE_PROP_UINT64("frequency", MOS6522State, frequency, 0),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void mos6522_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    MOS6522DeviceClass *mdc = MOS6522_DEVICE_CLASS(oc);
+
+    dc->realize = mos6522_realize;
+    dc->reset = mos6522_reset;
+    dc->vmsd = &vmstate_mos6522;
+    dc->props = mos6522_properties;
+    mdc->parent_realize = dc->realize;
+    mdc->set_sr_int = mos6522_set_sr_int;
+    mdc->portB_write = mos6522_portB_write;
+    mdc->portA_write = mos6522_portA_write;
+    mdc->get_timer1_counter_value = mos6522_get_counter_value;
+    mdc->get_timer2_counter_value = mos6522_get_counter_value;
+    mdc->get_timer1_load_time = mos6522_get_load_time;
+    mdc->get_timer2_load_time = mos6522_get_load_time;
+}
+
+static const TypeInfo mos6522_type_info = {
+    .name = TYPE_MOS6522,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(MOS6522State),
+    .instance_init = mos6522_init,
+    .abstract = true,
+    .class_size = sizeof(MOS6522DeviceClass),
+    .class_init = mos6522_class_init,
+};
+
+static void mos6522_register_types(void)
+{
+    type_register_static(&mos6522_type_info);
+}
+
+type_init(mos6522_register_types)
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index e6070f280d..b340d4e81c 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -70,3 +70,10 @@ msf2_sysreg_write_pll_status(void) "Invalid write to read only PLL status regist
 #hw/misc/imx7_gpr.c
 imx7_gpr_read(uint64_t offset) "addr 0x%08" HWADDR_PRIx
 imx7_gpr_write(uint64_t offset, uint64_t value) "addr 0x%08" HWADDR_PRIx "value 0x%08" HWADDR_PRIx
+
+# hw/misc/mos6522.c
+mos6522_set_counter(int index, unsigned int val) "T%d.counter=%d"
+mos6522_get_next_irq_time(uint16_t latch, int64_t d, int64_t delta) "latch=%d counter=0x%"PRId64 " delta_next=0x%"PRId64
+mos6522_set_sr_int(void) "set sr_int"
+mos6522_write(uint64_t addr, uint64_t val) "reg=0x%"PRIx64 " val=0x%"PRIx64
+mos6522_read(uint64_t addr, unsigned val) "reg=0x%"PRIx64 " val=0x%x"
diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
new file mode 100644
index 0000000000..a53c161b00
--- /dev/null
+++ b/include/hw/misc/mos6522.h
@@ -0,0 +1,152 @@
+/*
+ * QEMU MOS6522 VIA emulation
+ *
+ * Copyright (c) 2004-2007 Fabrice Bellard
+ * Copyright (c) 2007 Jocelyn Mayer
+ * Copyright (c) 2018 Mark Cave-Ayland
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef MOS6522_H
+#define MOS6522_H
+
+#include "exec/memory.h"
+#include "hw/sysbus.h"
+#include "hw/ide/internal.h"
+#include "hw/input/adb.h"
+
+/* Bits in ACR */
+#define SR_CTRL            0x1c    /* Shift register control bits */
+#define SR_EXT             0x0c    /* Shift on external clock */
+#define SR_OUT             0x10    /* Shift out if 1 */
+
+/* Bits in IFR and IER */
+#define IER_SET            0x80    /* set bits in IER */
+#define IER_CLR            0       /* clear bits in IER */
+
+#define CA2_INT            0x01
+#define CA1_INT            0x02
+#define SR_INT             0x04    /* Shift register full/empty */
+#define CB2_INT            0x08
+#define CB1_INT            0x10
+#define T2_INT             0x20    /* Timer 2 interrupt */
+#define T1_INT             0x40    /* Timer 1 interrupt */
+
+/* Bits in ACR */
+#define T1MODE             0xc0    /* Timer 1 mode */
+#define T1MODE_CONT        0x40    /*  continuous interrupts */
+
+/* VIA registers */
+#define VIA_REG_B       0x00
+#define VIA_REG_A       0x01
+#define VIA_REG_DIRB    0x02
+#define VIA_REG_DIRA    0x03
+#define VIA_REG_T1CL    0x04
+#define VIA_REG_T1CH    0x05
+#define VIA_REG_T1LL    0x06
+#define VIA_REG_T1LH    0x07
+#define VIA_REG_T2CL    0x08
+#define VIA_REG_T2CH    0x09
+#define VIA_REG_SR      0x0a
+#define VIA_REG_ACR     0x0b
+#define VIA_REG_PCR     0x0c
+#define VIA_REG_IFR     0x0d
+#define VIA_REG_IER     0x0e
+#define VIA_REG_ANH     0x0f
+
+/**
+ * MOS6522Timer:
+ * @counter_value: counter value at load time
+ */
+typedef struct MOS6522Timer {
+    int index;
+    uint16_t latch;
+    uint16_t counter_value;
+    int64_t load_time;
+    int64_t next_irq_time;
+    uint64_t frequency;
+    QEMUTimer *timer;
+} MOS6522Timer;
+
+/**
+ * MOS6522State:
+ * @b: B-side data
+ * @a: A-side data
+ * @dirb: B-side direction (1=output)
+ * @dira: A-side direction (1=output)
+ * @sr: Shift register
+ * @acr: Auxiliary control register
+ * @pcr: Peripheral control register
+ * @ifr: Interrupt flag register
+ * @ier: Interrupt enable register
+ * @anh: A-side data, no handshake
+ * @last_b: last value of B register
+ * @last_acr: last value of ACR register
+ */
+typedef struct MOS6522State {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    MemoryRegion mem;
+    /* VIA registers */
+    uint8_t b;
+    uint8_t a;
+    uint8_t dirb;
+    uint8_t dira;
+    uint8_t sr;
+    uint8_t acr;
+    uint8_t pcr;
+    uint8_t ifr;
+    uint8_t ier;
+    uint8_t anh;
+
+    MOS6522Timer timers[2];
+    uint64_t frequency;
+
+    qemu_irq irq;
+} MOS6522State;
+
+#define TYPE_MOS6522 "mos6522"
+#define MOS6522(obj) OBJECT_CHECK(MOS6522State, (obj), TYPE_MOS6522)
+
+typedef struct MOS6522DeviceClass {
+    DeviceClass parent_class;
+
+    DeviceRealize parent_realize;
+    void (*set_sr_int)(MOS6522State *dev);
+    void (*portB_write)(MOS6522State *dev);
+    void (*portA_write)(MOS6522State *dev);
+    /* These are used to influence the CUDA MacOS timebase calibration */
+    uint64_t (*get_timer1_counter_value)(MOS6522State *dev, MOS6522Timer *ti);
+    uint64_t (*get_timer2_counter_value)(MOS6522State *dev, MOS6522Timer *ti);
+    uint64_t (*get_timer1_load_time)(MOS6522State *dev, MOS6522Timer *ti);
+    uint64_t (*get_timer2_load_time)(MOS6522State *dev, MOS6522Timer *ti);
+} MOS6522DeviceClass;
+
+#define MOS6522_DEVICE_CLASS(cls) \
+    OBJECT_CLASS_CHECK(MOS6522DeviceClass, (cls), TYPE_MOS6522)
+#define MOS6522_DEVICE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(MOS6522DeviceClass, (obj), TYPE_MOS6522)
+
+uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size);
+void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size);
+
+#endif /* MOS6522_H */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 10/12] cuda: convert to use the shared mos6522 device
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
                   ` (8 preceding siblings ...)
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 09/12] misc: introduce new mos6522 VIA device and enable it for ppc builds Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-12  7:49   ` David Gibson
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 11/12] ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file Mark Cave-Ayland
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

Add the relevant hooks as required for the MacOS timer calibration and delayed
SR interrupt.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/cuda.c | 606 ++++++++++++++-------------------------------------
 hw/ppc/mac.h         |  87 ++++----
 2 files changed, 204 insertions(+), 489 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index a185252144..54c02aeffb 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -26,15 +26,14 @@
 #include "hw/hw.h"
 #include "hw/ppc/mac.h"
 #include "hw/input/adb.h"
+#include "hw/misc/mos6522.h"
 #include "qemu/timer.h"
 #include "sysemu/sysemu.h"
 #include "qemu/cutils.h"
 #include "qemu/log.h"
 
-/* XXX: implement all timer modes */
-
-/* debug CUDA */
-//#define DEBUG_CUDA
+/* debug CUDA packets */
+//#define DEBUG_CUDA_PACKET
 
 /* debug CUDA packets */
 //#define DEBUG_CUDA_PACKET
@@ -47,426 +46,114 @@
 #endif
 
 /* Bits in B data register: all active low */
-#define TREQ		0x08		/* Transfer request (input) */
-#define TACK		0x10		/* Transfer acknowledge (output) */
-#define TIP		0x20		/* Transfer in progress (output) */
-
-/* Bits in ACR */
-#define SR_CTRL		0x1c		/* Shift register control bits */
-#define SR_EXT		0x0c		/* Shift on external clock */
-#define SR_OUT		0x10		/* Shift out if 1 */
-
-/* Bits in IFR and IER */
-#define IER_SET		0x80		/* set bits in IER */
-#define IER_CLR		0		/* clear bits in IER */
-#define SR_INT		0x04		/* Shift register full/empty */
-#define SR_DATA_INT	0x08
-#define SR_CLOCK_INT	0x10
-#define T1_INT          0x40            /* Timer 1 interrupt */
-#define T2_INT          0x20            /* Timer 2 interrupt */
-
-/* Bits in ACR */
-#define T1MODE          0xc0            /* Timer 1 mode */
-#define T1MODE_CONT     0x40            /*  continuous interrupts */
+#define TREQ            0x08    /* Transfer request (input) */
+#define TACK            0x10    /* Transfer acknowledge (output) */
+#define TIP             0x20    /* Transfer in progress (output) */
 
 /* commands (1st byte) */
-#define ADB_PACKET	0
-#define CUDA_PACKET	1
-#define ERROR_PACKET	2
-#define TIMER_PACKET	3
-#define POWER_PACKET	4
-#define MACIIC_PACKET	5
-#define PMU_PACKET	6
-
-
-/* CUDA commands (2nd byte) */
-#define CUDA_WARM_START			0x0
-#define CUDA_AUTOPOLL			0x1
-#define CUDA_GET_6805_ADDR		0x2
-#define CUDA_GET_TIME			0x3
-#define CUDA_GET_PRAM			0x7
-#define CUDA_SET_6805_ADDR		0x8
-#define CUDA_SET_TIME			0x9
-#define CUDA_POWERDOWN			0xa
-#define CUDA_POWERUP_TIME		0xb
-#define CUDA_SET_PRAM			0xc
-#define CUDA_MS_RESET			0xd
-#define CUDA_SEND_DFAC			0xe
-#define CUDA_BATTERY_SWAP_SENSE		0x10
-#define CUDA_RESET_SYSTEM		0x11
-#define CUDA_SET_IPL			0x12
-#define CUDA_FILE_SERVER_FLAG		0x13
-#define CUDA_SET_AUTO_RATE		0x14
-#define CUDA_GET_AUTO_RATE		0x16
-#define CUDA_SET_DEVICE_LIST		0x19
-#define CUDA_GET_DEVICE_LIST		0x1a
-#define CUDA_SET_ONE_SECOND_MODE	0x1b
-#define CUDA_SET_POWER_MESSAGES		0x21
-#define CUDA_GET_SET_IIC		0x22
-#define CUDA_WAKEUP			0x23
-#define CUDA_TIMER_TICKLE		0x24
-#define CUDA_COMBINED_FORMAT_IIC	0x25
+#define ADB_PACKET      0
+#define CUDA_PACKET     1
+#define ERROR_PACKET    2
+#define TIMER_PACKET    3
+#define POWER_PACKET    4
+#define MACIIC_PACKET   5
+#define PMU_PACKET      6
 
 #define CUDA_TIMER_FREQ (4700000 / 6)
 
 /* CUDA returns time_t's offset from Jan 1, 1904, not 1970 */
 #define RTC_OFFSET                      2082844800
 
-/* CUDA registers */
-#define CUDA_REG_B       0x00
-#define CUDA_REG_A       0x01
-#define CUDA_REG_DIRB    0x02
-#define CUDA_REG_DIRA    0x03
-#define CUDA_REG_T1CL    0x04
-#define CUDA_REG_T1CH    0x05
-#define CUDA_REG_T1LL    0x06
-#define CUDA_REG_T1LH    0x07
-#define CUDA_REG_T2CL    0x08
-#define CUDA_REG_T2CH    0x09
-#define CUDA_REG_SR      0x0a
-#define CUDA_REG_ACR     0x0b
-#define CUDA_REG_PCR     0x0c
-#define CUDA_REG_IFR     0x0d
-#define CUDA_REG_IER     0x0e
-#define CUDA_REG_ANH     0x0f
-
-static void cuda_update(CUDAState *s);
 static void cuda_receive_packet_from_host(CUDAState *s,
                                           const uint8_t *data, int len);
-static void cuda_timer_update(CUDAState *s, CUDATimer *ti,
-                              int64_t current_time);
 
-static void cuda_update_irq(CUDAState *s)
-{
-    if (s->ifr & s->ier & (SR_INT | T1_INT | T2_INT)) {
-        qemu_irq_raise(s->irq);
-    } else {
-        qemu_irq_lower(s->irq);
-    }
-}
+/* MacOS uses timer 1 for calibration on startup, so we use
+ * the timebase frequency and cuda_get_counter_value() with
+ * cuda_get_load_time() to steer MacOS to calculate calibrate its timers
+ * correctly for both TCG and KVM (see commit b981289c49 "PPC: Cuda: Use cuda
+ * timer to expose tbfreq to guest" for more information) */
 
-static uint64_t get_counter_value(CUDAState *s, CUDATimer *ti)
+static uint64_t cuda_get_counter_value(MOS6522State *s, MOS6522Timer *ti)
 {
+    MOS6522CUDAState *mcs = container_of(s, MOS6522CUDAState, parent_obj);
+    CUDAState *cs = mcs->cuda;
+
     /* Reverse of the tb calculation algorithm that Mac OS X uses on bootup */
     uint64_t tb_diff = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                                s->tb_frequency, NANOSECONDS_PER_SECOND) -
+                                cs->tb_frequency, NANOSECONDS_PER_SECOND) -
                            ti->load_time;
 
-    return (tb_diff * 0xBF401675E5DULL) / (s->tb_frequency << 24);
+    return (tb_diff * 0xBF401675E5DULL) / (cs->tb_frequency << 24);
 }
 
-static uint64_t get_counter_load_time(CUDAState *s, CUDATimer *ti)
+static uint64_t cuda_get_load_time(MOS6522State *s, MOS6522Timer *ti)
 {
+    MOS6522CUDAState *mcs = container_of(s, MOS6522CUDAState, parent_obj);
+    CUDAState *cs = mcs->cuda;
+
     uint64_t load_time = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                                  s->tb_frequency, NANOSECONDS_PER_SECOND);
+                                  cs->tb_frequency, NANOSECONDS_PER_SECOND);
     return load_time;
 }
 
-static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
-{
-    int64_t d;
-    unsigned int counter;
-
-    d = get_counter_value(s, ti);
-
-    if (ti->index == 0) {
-        /* the timer goes down from latch to -1 (period of latch + 2) */
-        if (d <= (ti->counter_value + 1)) {
-            counter = (ti->counter_value - d) & 0xffff;
-        } else {
-            counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
-            counter = (ti->latch - counter) & 0xffff;
-        }
-    } else {
-        counter = (ti->counter_value - d) & 0xffff;
-    }
-    return counter;
-}
-
-static void set_counter(CUDAState *s, CUDATimer *ti, unsigned int val)
-{
-    CUDA_DPRINTF("T%d.counter=%d\n", 1 + ti->index, val);
-    ti->load_time = get_counter_load_time(s, ti);
-    ti->counter_value = val;
-    cuda_timer_update(s, ti, ti->load_time);
-}
-
-static int64_t get_next_irq_time(CUDATimer *ti, int64_t current_time)
-{
-    int64_t d, next_time;
-    unsigned int counter;
-
-    /* current counter value */
-    d = muldiv64(current_time - ti->load_time,
-                 ti->frequency, NANOSECONDS_PER_SECOND);
-    /* the timer goes down from latch to -1 (period of latch + 2) */
-    if (d <= (ti->counter_value + 1)) {
-        counter = (ti->counter_value - d) & 0xffff;
-    } else {
-        counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
-        counter = (ti->latch - counter) & 0xffff;
-    }
-
-    /* Note: we consider the irq is raised on 0 */
-    if (counter == 0xffff) {
-        next_time = d + ti->latch + 1;
-    } else if (counter == 0) {
-        next_time = d + ti->latch + 2;
-    } else {
-        next_time = d + counter;
-    }
-    CUDA_DPRINTF("latch=%d counter=%" PRId64 " delta_next=%" PRId64 "\n",
-                 ti->latch, d, next_time - d);
-    next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) +
-                         ti->load_time;
-    if (next_time <= current_time) {
-        next_time = current_time + 1;
-    }
-    return next_time;
-}
-
-static void cuda_timer_update(CUDAState *s, CUDATimer *ti,
-                              int64_t current_time)
-{
-    if (!ti->timer)
-        return;
-    if (ti->index == 0 && (s->acr & T1MODE) != T1MODE_CONT) {
-        timer_del(ti->timer);
-    } else {
-        ti->next_irq_time = get_next_irq_time(ti, current_time);
-        timer_mod(ti->timer, ti->next_irq_time);
-    }
-}
-
-static void cuda_timer1(void *opaque)
-{
-    CUDAState *s = opaque;
-    CUDATimer *ti = &s->timers[0];
-
-    cuda_timer_update(s, ti, ti->next_irq_time);
-    s->ifr |= T1_INT;
-    cuda_update_irq(s);
-}
-
-static void cuda_timer2(void *opaque)
-{
-    CUDAState *s = opaque;
-    CUDATimer *ti = &s->timers[1];
-
-    cuda_timer_update(s, ti, ti->next_irq_time);
-    s->ifr |= T2_INT;
-    cuda_update_irq(s);
-}
-
 static void cuda_set_sr_int(void *opaque)
 {
     CUDAState *s = opaque;
+    MOS6522CUDAState *mcs = s->mos6522_cuda;
+    MOS6522State *ms = MOS6522(mcs);
+    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(ms);
 
-    CUDA_DPRINTF("CUDA: %s:%d\n", __func__, __LINE__);
-    s->ifr |= SR_INT;
-    cuda_update_irq(s);
+    mdc->set_sr_int(ms);
 }
 
 static void cuda_delay_set_sr_int(CUDAState *s)
 {
+    MOS6522CUDAState *mcs = s->mos6522_cuda;
+    MOS6522State *ms = MOS6522(mcs);
+    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(ms);
     int64_t expire;
 
-    if (s->dirb == 0xff) {
-        /* Not in Mac OS, fire the IRQ directly */
-        cuda_set_sr_int(s);
+    if (ms->dirb == 0xff || s->sr_delay_ns == 0) {
+        /* Disabled or not in Mac OS, fire the IRQ directly */
+        mdc->set_sr_int(ms);
         return;
     }
 
     CUDA_DPRINTF("CUDA: %s:%d\n", __func__, __LINE__);
 
-    expire = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 300 * SCALE_US;
+    expire = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->sr_delay_ns;
     timer_mod(s->sr_delay_timer, expire);
 }
 
-static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size)
-{
-    CUDAState *s = opaque;
-    uint32_t val;
-
-    addr = (addr >> 9) & 0xf;
-    switch(addr) {
-    case CUDA_REG_B:
-        val = s->b;
-        break;
-    case CUDA_REG_A:
-        val = s->a;
-        break;
-    case CUDA_REG_DIRB:
-        val = s->dirb;
-        break;
-    case CUDA_REG_DIRA:
-        val = s->dira;
-        break;
-    case CUDA_REG_T1CL:
-        val = get_counter(s, &s->timers[0]) & 0xff;
-        s->ifr &= ~T1_INT;
-        cuda_update_irq(s);
-        break;
-    case CUDA_REG_T1CH:
-        val = get_counter(s, &s->timers[0]) >> 8;
-        cuda_update_irq(s);
-        break;
-    case CUDA_REG_T1LL:
-        val = s->timers[0].latch & 0xff;
-        break;
-    case CUDA_REG_T1LH:
-        /* XXX: check this */
-        val = (s->timers[0].latch >> 8) & 0xff;
-        break;
-    case CUDA_REG_T2CL:
-        val = get_counter(s, &s->timers[1]) & 0xff;
-        s->ifr &= ~T2_INT;
-        cuda_update_irq(s);
-        break;
-    case CUDA_REG_T2CH:
-        val = get_counter(s, &s->timers[1]) >> 8;
-        break;
-    case CUDA_REG_SR:
-        val = s->sr;
-        s->ifr &= ~(SR_INT | SR_CLOCK_INT | SR_DATA_INT);
-        cuda_update_irq(s);
-        break;
-    case CUDA_REG_ACR:
-        val = s->acr;
-        break;
-    case CUDA_REG_PCR:
-        val = s->pcr;
-        break;
-    case CUDA_REG_IFR:
-        val = s->ifr;
-        if (s->ifr & s->ier) {
-            val |= 0x80;
-        }
-        break;
-    case CUDA_REG_IER:
-        val = s->ier | 0x80;
-        break;
-    default:
-    case CUDA_REG_ANH:
-        val = s->anh;
-        break;
-    }
-    if (addr != CUDA_REG_IFR || val != 0) {
-        CUDA_DPRINTF("read: reg=0x%x val=%02x\n", (int)addr, val);
-    }
-
-    return val;
-}
-
-static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
-{
-    CUDAState *s = opaque;
-
-    addr = (addr >> 9) & 0xf;
-    CUDA_DPRINTF("write: reg=0x%x val=%02x\n", (int)addr, val);
-
-    switch(addr) {
-    case CUDA_REG_B:
-        s->b = (s->b & ~s->dirb) | (val & s->dirb);
-        cuda_update(s);
-        break;
-    case CUDA_REG_A:
-        s->a = (s->a & ~s->dira) | (val & s->dira);
-        break;
-    case CUDA_REG_DIRB:
-        s->dirb = val;
-        break;
-    case CUDA_REG_DIRA:
-        s->dira = val;
-        break;
-    case CUDA_REG_T1CL:
-        s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
-        cuda_timer_update(s, &s->timers[0], qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
-        break;
-    case CUDA_REG_T1CH:
-        s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
-        s->ifr &= ~T1_INT;
-        set_counter(s, &s->timers[0], s->timers[0].latch);
-        break;
-    case CUDA_REG_T1LL:
-        s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
-        cuda_timer_update(s, &s->timers[0], qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
-        break;
-    case CUDA_REG_T1LH:
-        s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
-        s->ifr &= ~T1_INT;
-        cuda_timer_update(s, &s->timers[0], qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
-        break;
-    case CUDA_REG_T2CL:
-        s->timers[1].latch = (s->timers[1].latch & 0xff00) | val;
-        break;
-    case CUDA_REG_T2CH:
-        /* To ensure T2 generates an interrupt on zero crossing with the
-           common timer code, write the value directly from the latch to
-           the counter */
-        s->timers[1].latch = (s->timers[1].latch & 0xff) | (val << 8);
-        s->ifr &= ~T2_INT;
-        set_counter(s, &s->timers[1], s->timers[1].latch);
-        break;
-    case CUDA_REG_SR:
-        s->sr = val;
-        break;
-    case CUDA_REG_ACR:
-        s->acr = val;
-        cuda_timer_update(s, &s->timers[0], qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
-        break;
-    case CUDA_REG_PCR:
-        s->pcr = val;
-        break;
-    case CUDA_REG_IFR:
-        /* reset bits */
-        s->ifr &= ~val;
-        cuda_update_irq(s);
-        break;
-    case CUDA_REG_IER:
-        if (val & IER_SET) {
-            /* set bits */
-            s->ier |= val & 0x7f;
-        } else {
-            /* reset bits */
-            s->ier &= ~val;
-        }
-        cuda_update_irq(s);
-        break;
-    default:
-    case CUDA_REG_ANH:
-        s->anh = val;
-        break;
-    }
-}
-
 /* NOTE: TIP and TREQ are negated */
 static void cuda_update(CUDAState *s)
 {
+    MOS6522CUDAState *mcs = s->mos6522_cuda;
+    MOS6522State *ms = MOS6522(mcs);
     int packet_received, len;
 
     packet_received = 0;
-    if (!(s->b & TIP)) {
+    if (!(ms->b & TIP)) {
         /* transfer requested from host */
 
-        if (s->acr & SR_OUT) {
+        if (ms->acr & SR_OUT) {
             /* data output */
-            if ((s->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
+            if ((ms->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
                 if (s->data_out_index < sizeof(s->data_out)) {
-                    CUDA_DPRINTF("send: %02x\n", s->sr);
-                    s->data_out[s->data_out_index++] = s->sr;
+                    CUDA_DPRINTF("send: %02x\n", ms->sr);
+                    s->data_out[s->data_out_index++] = ms->sr;
                     cuda_delay_set_sr_int(s);
                 }
             }
         } else {
             if (s->data_in_index < s->data_in_size) {
                 /* data input */
-                if ((s->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
-                    s->sr = s->data_in[s->data_in_index++];
-                    CUDA_DPRINTF("recv: %02x\n", s->sr);
+                if ((ms->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
+                    ms->sr = s->data_in[s->data_in_index++];
+                    CUDA_DPRINTF("recv: %02x\n", ms->sr);
                     /* indicate end of transfer */
                     if (s->data_in_index >= s->data_in_size) {
-                        s->b = (s->b | TREQ);
+                        ms->b = (ms->b | TREQ);
                     }
                     cuda_delay_set_sr_int(s);
                 }
@@ -474,12 +161,13 @@ static void cuda_update(CUDAState *s)
         }
     } else {
         /* no transfer requested: handle sync case */
-        if ((s->last_b & TIP) && (s->b & TACK) != (s->last_b & TACK)) {
+        if ((s->last_b & TIP) && (ms->b & TACK) != (s->last_b & TACK)) {
             /* update TREQ state each time TACK change state */
-            if (s->b & TACK)
-                s->b = (s->b | TREQ);
-            else
-                s->b = (s->b & ~TREQ);
+            if (ms->b & TACK) {
+                ms->b = (ms->b | TREQ);
+            } else {
+                ms->b = (ms->b & ~TREQ);
+            }
             cuda_delay_set_sr_int(s);
         } else {
             if (!(s->last_b & TIP)) {
@@ -490,13 +178,13 @@ static void cuda_update(CUDAState *s)
             }
             /* signal if there is data to read */
             if (s->data_in_index < s->data_in_size) {
-                s->b = (s->b & ~TREQ);
+                ms->b = (ms->b & ~TREQ);
             }
         }
     }
 
-    s->last_acr = s->acr;
-    s->last_b = s->b;
+    s->last_acr = ms->acr;
+    s->last_b = ms->b;
 
     /* NOTE: cuda_receive_packet_from_host() can call cuda_update()
        recursively */
@@ -538,9 +226,8 @@ static void cuda_adb_poll(void *opaque)
         obuf[1] = 0x40; /* polled data */
         cuda_send_packet_to_host(s, obuf, olen + 2);
     }
-    timer_mod(s->adb_poll_timer,
-                   qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-                   (NANOSECONDS_PER_SECOND / (1000 / s->autopoll_rate_ms)));
+    timer_mod(s->adb_poll_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
+              (NANOSECONDS_PER_SECOND / (1000 / s->autopoll_rate_ms)));
 }
 
 /* description of commands */
@@ -787,35 +474,35 @@ static void cuda_receive_packet_from_host(CUDAState *s,
     }
 }
 
-static const MemoryRegionOps cuda_ops = {
-    .read = cuda_read,
-    .write = cuda_write,
-    .endianness = DEVICE_BIG_ENDIAN,
-    .valid = {
-        .min_access_size = 1,
-        .max_access_size = 1,
-    },
-};
+static uint64_t mos6522_cuda_read(void *opaque, hwaddr addr, unsigned size)
+{
+    CUDAState *s = opaque;
+    MOS6522CUDAState *mcs = s->mos6522_cuda;
+    MOS6522State *ms = MOS6522(mcs);
 
-static bool cuda_timer_exist(void *opaque, int version_id)
+    addr = (addr >> 9) & 0xf;
+    return mos6522_read(ms, addr, size);
+}
+
+static void mos6522_cuda_write(void *opaque, hwaddr addr, uint64_t val,
+                               unsigned size)
 {
-    CUDATimer *s = opaque;
+    CUDAState *s = opaque;
+    MOS6522CUDAState *mcs = s->mos6522_cuda;
+    MOS6522State *ms = MOS6522(mcs);
 
-    return s->timer != NULL;
+    addr = (addr >> 9) & 0xf;
+    mos6522_write(ms, addr, val, size);
 }
 
-static const VMStateDescription vmstate_cuda_timer = {
-    .name = "cuda_timer",
-    .version_id = 0,
-    .minimum_version_id = 0,
-    .fields = (VMStateField[]) {
-        VMSTATE_UINT16(latch, CUDATimer),
-        VMSTATE_UINT16(counter_value, CUDATimer),
-        VMSTATE_INT64(load_time, CUDATimer),
-        VMSTATE_INT64(next_irq_time, CUDATimer),
-        VMSTATE_TIMER_PTR_TEST(timer, CUDATimer, cuda_timer_exist),
-        VMSTATE_END_OF_LIST()
-    }
+static const MemoryRegionOps mos6522_cuda_ops = {
+    .read = mos6522_cuda_read,
+    .write = mos6522_cuda_write,
+    .endianness = DEVICE_BIG_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
 };
 
 static const VMStateDescription vmstate_cuda = {
@@ -823,18 +510,8 @@ static const VMStateDescription vmstate_cuda = {
     .version_id = 4,
     .minimum_version_id = 4,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT8(a, CUDAState),
-        VMSTATE_UINT8(b, CUDAState),
         VMSTATE_UINT8(last_b, CUDAState),
-        VMSTATE_UINT8(dira, CUDAState),
-        VMSTATE_UINT8(dirb, CUDAState),
-        VMSTATE_UINT8(sr, CUDAState),
-        VMSTATE_UINT8(acr, CUDAState),
         VMSTATE_UINT8(last_acr, CUDAState),
-        VMSTATE_UINT8(pcr, CUDAState),
-        VMSTATE_UINT8(ifr, CUDAState),
-        VMSTATE_UINT8(ier, CUDAState),
-        VMSTATE_UINT8(anh, CUDAState),
         VMSTATE_INT32(data_in_size, CUDAState),
         VMSTATE_INT32(data_in_index, CUDAState),
         VMSTATE_INT32(data_out_index, CUDAState),
@@ -844,8 +521,6 @@ static const VMStateDescription vmstate_cuda = {
         VMSTATE_BUFFER(data_in, CUDAState),
         VMSTATE_BUFFER(data_out, CUDAState),
         VMSTATE_UINT32(tick_offset, CUDAState),
-        VMSTATE_STRUCT_ARRAY(timers, CUDAState, 2, 1,
-                             vmstate_cuda_timer, CUDATimer),
         VMSTATE_TIMER_PTR(adb_poll_timer, CUDAState),
         VMSTATE_TIMER_PTR(sr_delay_timer, CUDAState),
         VMSTATE_END_OF_LIST()
@@ -856,61 +531,48 @@ static void cuda_reset(DeviceState *dev)
 {
     CUDAState *s = CUDA(dev);
 
-    s->b = 0;
-    s->a = 0;
-    s->dirb = 0xff;
-    s->dira = 0;
-    s->sr = 0;
-    s->acr = 0;
-    s->pcr = 0;
-    s->ifr = 0;
-    s->ier = 0;
-    //    s->ier = T1_INT | SR_INT;
-    s->anh = 0;
     s->data_in_size = 0;
     s->data_in_index = 0;
     s->data_out_index = 0;
     s->autopoll = 0;
-
-    s->timers[0].latch = 0xffff;
-    set_counter(s, &s->timers[0], 0xffff);
-
-    s->timers[1].latch = 0xffff;
-
-    s->sr_delay_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_set_sr_int, s);
 }
 
-static void cuda_realizefn(DeviceState *dev, Error **errp)
+static void cuda_realize(DeviceState *dev, Error **errp)
 {
     CUDAState *s = CUDA(dev);
+    SysBusDevice *sbd;
+    MOS6522State *ms;
+    DeviceState *d;
     struct tm tm;
 
-    s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer1, s);
-    s->timers[0].frequency = CUDA_TIMER_FREQ;
-    s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer2, s);
-    s->timers[1].frequency = (SCALE_US * 6000) / 4700;
+    d = qdev_create(NULL, TYPE_MOS6522_CUDA);
+    object_property_set_link(OBJECT(d), OBJECT(s), "cuda", errp);
+    qdev_init_nofail(d);
+    s->mos6522_cuda = MOS6522_CUDA(d);
+
+    /* Pass IRQ from 6522 */
+    ms = MOS6522(d);
+    sbd = SYS_BUS_DEVICE(s);
+    sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms));
 
     qemu_get_timedate(&tm, 0);
     s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
 
+    s->sr_delay_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_set_sr_int, s);
+    s->sr_delay_ns = 300 * SCALE_US;
+
     s->adb_poll_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_adb_poll, s);
-    s->autopoll_rate_ms = 20;
     s->adb_poll_mask = 0xffff;
+    s->autopoll_rate_ms = 20;
 }
 
-static void cuda_initfn(Object *obj)
+static void cuda_init(Object *obj)
 {
-    SysBusDevice *d = SYS_BUS_DEVICE(obj);
     CUDAState *s = CUDA(obj);
-    int i;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
-    memory_region_init_io(&s->mem, obj, &cuda_ops, s, "cuda", 0x2000);
-    sysbus_init_mmio(d, &s->mem);
-    sysbus_init_irq(d, &s->irq);
-
-    for (i = 0; i < ARRAY_SIZE(s->timers); i++) {
-        s->timers[i].index = i;
-    }
+    memory_region_init_io(&s->mem, obj, &mos6522_cuda_ops, s, "cuda", 0x2000);
+    sysbus_init_mmio(sbd, &s->mem);
 
     qbus_create_inplace(&s->adb_bus, sizeof(s->adb_bus), TYPE_ADB_BUS,
                         DEVICE(obj), "adb.0");
@@ -925,7 +587,7 @@ static void cuda_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
 
-    dc->realize = cuda_realizefn;
+    dc->realize = cuda_realize;
     dc->reset = cuda_reset;
     dc->vmsd = &vmstate_cuda;
     dc->props = cuda_properties;
@@ -936,12 +598,62 @@ static const TypeInfo cuda_type_info = {
     .name = TYPE_CUDA,
     .parent = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(CUDAState),
-    .instance_init = cuda_initfn,
+    .instance_init = cuda_init,
     .class_init = cuda_class_init,
 };
 
+static void mos6522_cuda_portB_write(MOS6522State *s)
+{
+    MOS6522CUDAState *mcs = container_of(s, MOS6522CUDAState, parent_obj);
+
+    cuda_update(mcs->cuda);
+}
+
+static void mos6522_cuda_realize(DeviceState *dev, Error **errp)
+{
+    MOS6522State *ms = MOS6522(dev);
+    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(ms);
+
+    mdc->parent_realize(dev, errp);
+
+    ms->timers[0].frequency = CUDA_TIMER_FREQ;
+    ms->timers[1].frequency = (SCALE_US * 6000) / 4700;
+}
+
+static void mos6522_cuda_init(Object *obj)
+{
+    MOS6522CUDAState *s = MOS6522_CUDA(obj);
+
+    object_property_add_link(obj, "cuda", TYPE_CUDA,
+                             (Object **) &s->cuda,
+                             qdev_prop_allow_set_link_before_realize,
+                             0, NULL);
+}
+
+static void mos6522_cuda_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    MOS6522DeviceClass *mdc = MOS6522_DEVICE_CLASS(oc);
+
+    dc->realize = mos6522_cuda_realize;
+    mdc->portB_write = mos6522_cuda_portB_write;
+    mdc->get_timer1_counter_value = cuda_get_counter_value;
+    mdc->get_timer2_counter_value = cuda_get_counter_value;
+    mdc->get_timer1_load_time = cuda_get_load_time;
+    mdc->get_timer2_load_time = cuda_get_load_time;
+}
+
+static const TypeInfo mos6522_cuda_type_info = {
+    .name = TYPE_MOS6522_CUDA,
+    .parent = TYPE_MOS6522,
+    .instance_size = sizeof(MOS6522CUDAState),
+    .instance_init = mos6522_cuda_init,
+    .class_init = mos6522_cuda_class_init,
+};
+
 static void cuda_register_types(void)
 {
+    type_register_static(&mos6522_cuda_type_info);
     type_register_static(&cuda_type_info);
 }
 
diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index fa78115c95..3e9f13d9b4 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -30,6 +30,7 @@
 #include "hw/sysbus.h"
 #include "hw/ide/internal.h"
 #include "hw/input/adb.h"
+#include "hw/misc/mos6522.h"
 
 /* SMP is not enabled, for now */
 #define MAX_CPUS 1
@@ -44,59 +45,48 @@
 
 #define ESCC_CLOCK 3686400
 
+/* CUDA commands (2nd byte) */
+#define CUDA_WARM_START                0x0
+#define CUDA_AUTOPOLL                  0x1
+#define CUDA_GET_6805_ADDR             0x2
+#define CUDA_GET_TIME                  0x3
+#define CUDA_GET_PRAM                  0x7
+#define CUDA_SET_6805_ADDR             0x8
+#define CUDA_SET_TIME                  0x9
+#define CUDA_POWERDOWN                 0xa
+#define CUDA_POWERUP_TIME              0xb
+#define CUDA_SET_PRAM                  0xc
+#define CUDA_MS_RESET                  0xd
+#define CUDA_SEND_DFAC                 0xe
+#define CUDA_BATTERY_SWAP_SENSE        0x10
+#define CUDA_RESET_SYSTEM              0x11
+#define CUDA_SET_IPL                   0x12
+#define CUDA_FILE_SERVER_FLAG          0x13
+#define CUDA_SET_AUTO_RATE             0x14
+#define CUDA_GET_AUTO_RATE             0x16
+#define CUDA_SET_DEVICE_LIST           0x19
+#define CUDA_GET_DEVICE_LIST           0x1a
+#define CUDA_SET_ONE_SECOND_MODE       0x1b
+#define CUDA_SET_POWER_MESSAGES        0x21
+#define CUDA_GET_SET_IIC               0x22
+#define CUDA_WAKEUP                    0x23
+#define CUDA_TIMER_TICKLE              0x24
+#define CUDA_COMBINED_FORMAT_IIC       0x25
+
 /* Cuda */
 #define TYPE_CUDA "cuda"
 #define CUDA(obj) OBJECT_CHECK(CUDAState, (obj), TYPE_CUDA)
 
-/**
- * CUDATimer:
- * @counter_value: counter value at load time
- */
-typedef struct CUDATimer {
-    int index;
-    uint16_t latch;
-    uint16_t counter_value;
-    int64_t load_time;
-    int64_t next_irq_time;
-    uint64_t frequency;
-    QEMUTimer *timer;
-} CUDATimer;
-
-/**
- * CUDAState:
- * @b: B-side data
- * @a: A-side data
- * @dirb: B-side direction (1=output)
- * @dira: A-side direction (1=output)
- * @sr: Shift register
- * @acr: Auxiliary control register
- * @pcr: Peripheral control register
- * @ifr: Interrupt flag register
- * @ier: Interrupt enable register
- * @anh: A-side data, no handshake
- * @last_b: last value of B register
- * @last_acr: last value of ACR register
- */
+typedef struct MOS6522CUDAState MOS6522CUDAState;
+
 typedef struct CUDAState {
     /*< private >*/
     SysBusDevice parent_obj;
     /*< public >*/
-
     MemoryRegion mem;
-    /* cuda registers */
-    uint8_t b;
-    uint8_t a;
-    uint8_t dirb;
-    uint8_t dira;
-    uint8_t sr;
-    uint8_t acr;
-    uint8_t pcr;
-    uint8_t ifr;
-    uint8_t ier;
-    uint8_t anh;
 
     ADBBusState adb_bus;
-    CUDATimer timers[2];
+    MOS6522CUDAState *mos6522_cuda;
 
     uint32_t tick_offset;
     uint64_t tb_frequency;
@@ -105,6 +95,7 @@ typedef struct CUDAState {
     uint8_t last_acr;
 
     /* MacOS 9 is racy and requires a delay upon setting the SR_INT bit */
+    uint64_t sr_delay_ns;
     QEMUTimer *sr_delay_timer;
 
     int data_in_size;
@@ -120,6 +111,18 @@ typedef struct CUDAState {
     QEMUTimer *adb_poll_timer;
 } CUDAState;
 
+/* MOS6522 CUDA */
+typedef struct MOS6522CUDAState {
+    /*< private >*/
+    MOS6522State parent_obj;
+
+    CUDAState *cuda;
+} MOS6522CUDAState;
+
+#define TYPE_MOS6522_CUDA "mos6522-cuda"
+#define MOS6522_CUDA(obj) OBJECT_CHECK(MOS6522CUDAState, (obj), \
+                                       TYPE_MOS6522_CUDA)
+
 /* MacIO */
 #define TYPE_OLDWORLD_MACIO "macio-oldworld"
 #define TYPE_NEWWORLD_MACIO "macio-newworld"
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 11/12] ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
                   ` (9 preceding siblings ...)
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 10/12] cuda: convert to use the shared mos6522 device Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-09 19:58   ` Philippe Mathieu-Daudé
  2018-02-12  7:59   ` David Gibson
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 12/12] cuda: convert to trace-events Mark Cave-Ayland
  2018-02-09 19:12 ` [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device no-reply
  12 siblings, 2 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/cuda.c         |   1 +
 hw/misc/macio/macio.c        |   1 +
 hw/ppc/mac.h                 |  77 -------------------------------
 include/hw/misc/macio/cuda.h | 107 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 109 insertions(+), 77 deletions(-)
 create mode 100644 include/hw/misc/macio/cuda.h

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 54c02aeffb..377b91d266 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -27,6 +27,7 @@
 #include "hw/ppc/mac.h"
 #include "hw/input/adb.h"
 #include "hw/misc/mos6522.h"
+#include "hw/misc/macio/cuda.h"
 #include "qemu/timer.h"
 #include "sysemu/sysemu.h"
 #include "qemu/cutils.h"
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index a639b09e00..024f8557ab 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -26,6 +26,7 @@
 #include "qapi/error.h"
 #include "hw/hw.h"
 #include "hw/ppc/mac.h"
+#include "hw/misc/macio/cuda.h"
 #include "hw/pci/pci.h"
 #include "hw/ppc/mac_dbdma.h"
 #include "hw/char/escc.h"
diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index 3e9f13d9b4..4702194f3f 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -45,83 +45,6 @@
 
 #define ESCC_CLOCK 3686400
 
-/* CUDA commands (2nd byte) */
-#define CUDA_WARM_START                0x0
-#define CUDA_AUTOPOLL                  0x1
-#define CUDA_GET_6805_ADDR             0x2
-#define CUDA_GET_TIME                  0x3
-#define CUDA_GET_PRAM                  0x7
-#define CUDA_SET_6805_ADDR             0x8
-#define CUDA_SET_TIME                  0x9
-#define CUDA_POWERDOWN                 0xa
-#define CUDA_POWERUP_TIME              0xb
-#define CUDA_SET_PRAM                  0xc
-#define CUDA_MS_RESET                  0xd
-#define CUDA_SEND_DFAC                 0xe
-#define CUDA_BATTERY_SWAP_SENSE        0x10
-#define CUDA_RESET_SYSTEM              0x11
-#define CUDA_SET_IPL                   0x12
-#define CUDA_FILE_SERVER_FLAG          0x13
-#define CUDA_SET_AUTO_RATE             0x14
-#define CUDA_GET_AUTO_RATE             0x16
-#define CUDA_SET_DEVICE_LIST           0x19
-#define CUDA_GET_DEVICE_LIST           0x1a
-#define CUDA_SET_ONE_SECOND_MODE       0x1b
-#define CUDA_SET_POWER_MESSAGES        0x21
-#define CUDA_GET_SET_IIC               0x22
-#define CUDA_WAKEUP                    0x23
-#define CUDA_TIMER_TICKLE              0x24
-#define CUDA_COMBINED_FORMAT_IIC       0x25
-
-/* Cuda */
-#define TYPE_CUDA "cuda"
-#define CUDA(obj) OBJECT_CHECK(CUDAState, (obj), TYPE_CUDA)
-
-typedef struct MOS6522CUDAState MOS6522CUDAState;
-
-typedef struct CUDAState {
-    /*< private >*/
-    SysBusDevice parent_obj;
-    /*< public >*/
-    MemoryRegion mem;
-
-    ADBBusState adb_bus;
-    MOS6522CUDAState *mos6522_cuda;
-
-    uint32_t tick_offset;
-    uint64_t tb_frequency;
-
-    uint8_t last_b;
-    uint8_t last_acr;
-
-    /* MacOS 9 is racy and requires a delay upon setting the SR_INT bit */
-    uint64_t sr_delay_ns;
-    QEMUTimer *sr_delay_timer;
-
-    int data_in_size;
-    int data_in_index;
-    int data_out_index;
-
-    qemu_irq irq;
-    uint16_t adb_poll_mask;
-    uint8_t autopoll_rate_ms;
-    uint8_t autopoll;
-    uint8_t data_in[128];
-    uint8_t data_out[16];
-    QEMUTimer *adb_poll_timer;
-} CUDAState;
-
-/* MOS6522 CUDA */
-typedef struct MOS6522CUDAState {
-    /*< private >*/
-    MOS6522State parent_obj;
-
-    CUDAState *cuda;
-} MOS6522CUDAState;
-
-#define TYPE_MOS6522_CUDA "mos6522-cuda"
-#define MOS6522_CUDA(obj) OBJECT_CHECK(MOS6522CUDAState, (obj), \
-                                       TYPE_MOS6522_CUDA)
 
 /* MacIO */
 #define TYPE_OLDWORLD_MACIO "macio-oldworld"
diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h
new file mode 100644
index 0000000000..6afbdd13ee
--- /dev/null
+++ b/include/hw/misc/macio/cuda.h
@@ -0,0 +1,107 @@
+/*
+ * QEMU PowerMac CUDA device support
+ *
+ * Copyright (c) 2004-2007 Fabrice Bellard
+ * Copyright (c) 2007 Jocelyn Mayer
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef CUDA_H
+#define CUDA_H
+
+/* CUDA commands (2nd byte) */
+#define CUDA_WARM_START                0x0
+#define CUDA_AUTOPOLL                  0x1
+#define CUDA_GET_6805_ADDR             0x2
+#define CUDA_GET_TIME                  0x3
+#define CUDA_GET_PRAM                  0x7
+#define CUDA_SET_6805_ADDR             0x8
+#define CUDA_SET_TIME                  0x9
+#define CUDA_POWERDOWN                 0xa
+#define CUDA_POWERUP_TIME              0xb
+#define CUDA_SET_PRAM                  0xc
+#define CUDA_MS_RESET                  0xd
+#define CUDA_SEND_DFAC                 0xe
+#define CUDA_BATTERY_SWAP_SENSE        0x10
+#define CUDA_RESET_SYSTEM              0x11
+#define CUDA_SET_IPL                   0x12
+#define CUDA_FILE_SERVER_FLAG          0x13
+#define CUDA_SET_AUTO_RATE             0x14
+#define CUDA_GET_AUTO_RATE             0x16
+#define CUDA_SET_DEVICE_LIST           0x19
+#define CUDA_GET_DEVICE_LIST           0x1a
+#define CUDA_SET_ONE_SECOND_MODE       0x1b
+#define CUDA_SET_POWER_MESSAGES        0x21
+#define CUDA_GET_SET_IIC               0x22
+#define CUDA_WAKEUP                    0x23
+#define CUDA_TIMER_TICKLE              0x24
+#define CUDA_COMBINED_FORMAT_IIC       0x25
+
+/* Cuda */
+#define TYPE_CUDA "cuda"
+#define CUDA(obj) OBJECT_CHECK(CUDAState, (obj), TYPE_CUDA)
+
+typedef struct MOS6522CUDAState MOS6522CUDAState;
+
+typedef struct CUDAState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+    MemoryRegion mem;
+
+    ADBBusState adb_bus;
+    MOS6522CUDAState *mos6522_cuda;
+
+    uint32_t tick_offset;
+    uint64_t tb_frequency;
+
+    uint8_t last_b;
+    uint8_t last_acr;
+
+    /* MacOS 9 is racy and requires a delay upon setting the SR_INT bit */
+    uint64_t sr_delay_ns;
+    QEMUTimer *sr_delay_timer;
+
+    int data_in_size;
+    int data_in_index;
+    int data_out_index;
+
+    qemu_irq irq;
+    uint16_t adb_poll_mask;
+    uint8_t autopoll_rate_ms;
+    uint8_t autopoll;
+    uint8_t data_in[128];
+    uint8_t data_out[16];
+    QEMUTimer *adb_poll_timer;
+} CUDAState;
+
+/* MOS6522 CUDA */
+typedef struct MOS6522CUDAState {
+    /*< private >*/
+    MOS6522State parent_obj;
+
+    CUDAState *cuda;
+} MOS6522CUDAState;
+
+#define TYPE_MOS6522_CUDA "mos6522-cuda"
+#define MOS6522_CUDA(obj) OBJECT_CHECK(MOS6522CUDAState, (obj), \
+                                       TYPE_MOS6522_CUDA)
+
+#endif /* CUDA_H */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Qemu-devel] [PATCHv2 12/12] cuda: convert to trace-events
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
                   ` (10 preceding siblings ...)
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 11/12] ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file Mark Cave-Ayland
@ 2018-02-09 18:51 ` Mark Cave-Ayland
  2018-02-09 19:56   ` Philippe Mathieu-Daudé
  2018-02-12  9:25   ` David Gibson
  2018-02-09 19:12 ` [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device no-reply
  12 siblings, 2 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 18:51 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, lvivier

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 Makefile.objs              |  1 +
 hw/misc/macio/cuda.c       | 50 ++++++++++++++++------------------------------
 hw/misc/macio/trace-events | 11 ++++++++++
 3 files changed, 29 insertions(+), 33 deletions(-)
 create mode 100644 hw/misc/macio/trace-events

diff --git a/Makefile.objs b/Makefile.objs
index 2efba6d768..3f1a1b674d 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -133,6 +133,7 @@ trace-events-subdirs += hw/net
 trace-events-subdirs += hw/virtio
 trace-events-subdirs += hw/audio
 trace-events-subdirs += hw/misc
+trace-events-subdirs += hw/misc/macio
 trace-events-subdirs += hw/usb
 trace-events-subdirs += hw/scsi
 trace-events-subdirs += hw/nvram
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 377b91d266..bd9b862034 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -32,19 +32,7 @@
 #include "sysemu/sysemu.h"
 #include "qemu/cutils.h"
 #include "qemu/log.h"
-
-/* debug CUDA packets */
-//#define DEBUG_CUDA_PACKET
-
-/* debug CUDA packets */
-//#define DEBUG_CUDA_PACKET
-
-#ifdef DEBUG_CUDA
-#define CUDA_DPRINTF(fmt, ...)                                  \
-    do { printf("CUDA: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define CUDA_DPRINTF(fmt, ...)
-#endif
+#include "trace.h"
 
 /* Bits in B data register: all active low */
 #define TREQ            0x08    /* Transfer request (input) */
@@ -120,7 +108,7 @@ static void cuda_delay_set_sr_int(CUDAState *s)
         return;
     }
 
-    CUDA_DPRINTF("CUDA: %s:%d\n", __func__, __LINE__);
+    trace_cuda_delay_set_sr_int();
 
     expire = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->sr_delay_ns;
     timer_mod(s->sr_delay_timer, expire);
@@ -141,7 +129,7 @@ static void cuda_update(CUDAState *s)
             /* data output */
             if ((ms->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
                 if (s->data_out_index < sizeof(s->data_out)) {
-                    CUDA_DPRINTF("send: %02x\n", ms->sr);
+                    trace_cuda_data_send(ms->sr);
                     s->data_out[s->data_out_index++] = ms->sr;
                     cuda_delay_set_sr_int(s);
                 }
@@ -151,7 +139,7 @@ static void cuda_update(CUDAState *s)
                 /* data input */
                 if ((ms->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
                     ms->sr = s->data_in[s->data_in_index++];
-                    CUDA_DPRINTF("recv: %02x\n", ms->sr);
+                    trace_cuda_data_recv(ms->sr);
                     /* indicate end of transfer */
                     if (s->data_in_index >= s->data_in_size) {
                         ms->b = (ms->b | TREQ);
@@ -199,15 +187,13 @@ static void cuda_update(CUDAState *s)
 static void cuda_send_packet_to_host(CUDAState *s,
                                      const uint8_t *data, int len)
 {
-#ifdef DEBUG_CUDA_PACKET
-    {
-        int i;
-        printf("cuda_send_packet_to_host:\n");
-        for(i = 0; i < len; i++)
-            printf(" %02x", data[i]);
-        printf("\n");
+    int i;
+
+    trace_cuda_packet_send(len);
+    for (i = 0; i < len; i++) {
+        trace_cuda_packet_send_data(i, data[i]);
     }
-#endif
+
     memcpy(s->data_in, data, len);
     s->data_in_size = len;
     s->data_in_index = 0;
@@ -411,7 +397,7 @@ static void cuda_receive_packet(CUDAState *s,
     for (i = 0; i < ARRAY_SIZE(handlers); i++) {
         const CudaCommand *desc = &handlers[i];
         if (desc->command == data[0]) {
-            CUDA_DPRINTF("handling command %s\n", desc->name);
+            trace_cuda_receive_packet_cmd(desc->name);
             out_len = 0;
             if (desc->handler(s, data + 1, len - 1, obuf + 3, &out_len)) {
                 cuda_send_packet_to_host(s, obuf, 3 + out_len);
@@ -440,15 +426,13 @@ static void cuda_receive_packet(CUDAState *s,
 static void cuda_receive_packet_from_host(CUDAState *s,
                                           const uint8_t *data, int len)
 {
-#ifdef DEBUG_CUDA_PACKET
-    {
-        int i;
-        printf("cuda_receive_packet_from_host:\n");
-        for(i = 0; i < len; i++)
-            printf(" %02x", data[i]);
-        printf("\n");
+    int i;
+
+    trace_cuda_packet_receive(len);
+    for (i = 0; i < len; i++) {
+        trace_cuda_packet_receive_data(i, data[i]);
     }
-#endif
+
     switch(data[0]) {
     case ADB_PACKET:
         {
diff --git a/hw/misc/macio/trace-events b/hw/misc/macio/trace-events
new file mode 100644
index 0000000000..24c0a36824
--- /dev/null
+++ b/hw/misc/macio/trace-events
@@ -0,0 +1,11 @@
+# See docs/devel/tracing.txt for syntax documentation.
+
+# hw/misc/macio/cuda.c
+cuda_delay_set_sr_int(void) ""
+cuda_data_send(uint8_t data) "send: 0x%02x"
+cuda_data_recv(uint8_t data) "recv: 0x%02x"
+cuda_receive_packet_cmd(const char *cmd) "handling command %s"
+cuda_packet_receive(int len) "length %d"
+cuda_packet_receive_data(int i, const uint8_t data) "[%d] 0x%02x"
+cuda_packet_send(int len) "length %d"
+cuda_packet_send_data(int i, const uint8_t data) "[%d] 0x%02x"
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device
  2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
                   ` (11 preceding siblings ...)
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 12/12] cuda: convert to trace-events Mark Cave-Ayland
@ 2018-02-09 19:12 ` no-reply
  2018-02-09 19:15   ` Mark Cave-Ayland
  12 siblings, 1 reply; 34+ messages in thread
From: no-reply @ 2018-02-09 19:12 UTC (permalink / raw)
  To: mark.cave-ayland; +Cc: famz, qemu-devel, qemu-ppc, david, lvivier

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180209185142.17151-1-mark.cave-ayland@ilande.co.uk
Subject: [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20180209185142.17151-1-mark.cave-ayland@ilande.co.uk -> patchew/20180209185142.17151-1-mark.cave-ayland@ilande.co.uk
Switched to a new branch 'test'
8ffc09de07 cuda: convert to trace-events
e2e6817bf3 ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file
a49d2800aa cuda: convert to use the shared mos6522 device
783d299115 misc: introduce new mos6522 VIA device and enable it for ppc builds
467b07e22d cuda: factor out timebase-derived counter value and load time
1bf294c5a4 cuda: set timer 1 frequency property to CUDA_TIMER_FREQ
fdd2899c88 cuda: minor cosmetic tidy-ups to get_next_irq_time()
1b9a70ecf8 cuda: rename frequency property to tb_frequency
f80a0ed8ee cuda: introduce CUDAState parameter to get_counter()
1d814a15da cuda: don't call cuda_update() when writing to ACR register
c37f165538 cuda: don't allow writes to port output pins
167b6d50f8 cuda: do not use old_mmio accesses

=== OUTPUT BEGIN ===
Checking PATCH 1/12: cuda: do not use old_mmio accesses...
Checking PATCH 2/12: cuda: don't allow writes to port output pins...
Checking PATCH 3/12: cuda: don't call cuda_update() when writing to ACR register...
Checking PATCH 4/12: cuda: introduce CUDAState parameter to get_counter()...
Checking PATCH 5/12: cuda: rename frequency property to tb_frequency...
Checking PATCH 6/12: cuda: minor cosmetic tidy-ups to get_next_irq_time()...
Checking PATCH 7/12: cuda: set timer 1 frequency property to CUDA_TIMER_FREQ...
Checking PATCH 8/12: cuda: factor out timebase-derived counter value and load time...
Checking PATCH 9/12: misc: introduce new mos6522 VIA device and enable it for ppc builds...
Checking PATCH 10/12: cuda: convert to use the shared mos6522 device...
ERROR: do not use C99 // comments
#31: FILE: hw/misc/macio/cuda.c:36:
+//#define DEBUG_CUDA_PACKET

total: 1 errors, 0 warnings, 897 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 11/12: ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file...
Checking PATCH 12/12: cuda: convert to trace-events...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device
  2018-02-09 19:12 ` [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device no-reply
@ 2018-02-09 19:15   ` Mark Cave-Ayland
  0 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-09 19:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: famz, qemu-ppc, david, lvivier

On 09/02/18 19:12, no-reply@patchew.org wrote:

> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: 20180209185142.17151-1-mark.cave-ayland@ilande.co.uk
> Subject: [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> 
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
> 
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> 
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
>      echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
>      if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
>          failed=1
>          echo
>      fi
>      n=$((n+1))
> done
> 
> exit $failed
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
>  From https://github.com/patchew-project/qemu
>   * [new tag]               patchew/20180209185142.17151-1-mark.cave-ayland@ilande.co.uk -> patchew/20180209185142.17151-1-mark.cave-ayland@ilande.co.uk
> Switched to a new branch 'test'
> 8ffc09de07 cuda: convert to trace-events
> e2e6817bf3 ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file
> a49d2800aa cuda: convert to use the shared mos6522 device
> 783d299115 misc: introduce new mos6522 VIA device and enable it for ppc builds
> 467b07e22d cuda: factor out timebase-derived counter value and load time
> 1bf294c5a4 cuda: set timer 1 frequency property to CUDA_TIMER_FREQ
> fdd2899c88 cuda: minor cosmetic tidy-ups to get_next_irq_time()
> 1b9a70ecf8 cuda: rename frequency property to tb_frequency
> f80a0ed8ee cuda: introduce CUDAState parameter to get_counter()
> 1d814a15da cuda: don't call cuda_update() when writing to ACR register
> c37f165538 cuda: don't allow writes to port output pins
> 167b6d50f8 cuda: do not use old_mmio accesses
> 
> === OUTPUT BEGIN ===
> Checking PATCH 1/12: cuda: do not use old_mmio accesses...
> Checking PATCH 2/12: cuda: don't allow writes to port output pins...
> Checking PATCH 3/12: cuda: don't call cuda_update() when writing to ACR register...
> Checking PATCH 4/12: cuda: introduce CUDAState parameter to get_counter()...
> Checking PATCH 5/12: cuda: rename frequency property to tb_frequency...
> Checking PATCH 6/12: cuda: minor cosmetic tidy-ups to get_next_irq_time()...
> Checking PATCH 7/12: cuda: set timer 1 frequency property to CUDA_TIMER_FREQ...
> Checking PATCH 8/12: cuda: factor out timebase-derived counter value and load time...
> Checking PATCH 9/12: misc: introduce new mos6522 VIA device and enable it for ppc builds...
> Checking PATCH 10/12: cuda: convert to use the shared mos6522 device...
> ERROR: do not use C99 // comments
> #31: FILE: hw/misc/macio/cuda.c:36:
> +//#define DEBUG_CUDA_PACKET
> 
> total: 1 errors, 0 warnings, 897 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 11/12: ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file...
> Checking PATCH 12/12: cuda: convert to trace-events...
> === OUTPUT END ===
> 
> Test command exited with code: 1

As before, this is a false positive (especially since v2 patch 12 
converts CUDA to use trace-events and so these lines disappear anyhow).


ATB,

Mark.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 12/12] cuda: convert to trace-events
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 12/12] cuda: convert to trace-events Mark Cave-Ayland
@ 2018-02-09 19:56   ` Philippe Mathieu-Daudé
  2018-02-12  9:25   ` David Gibson
  1 sibling, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-09 19:56 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, qemu-ppc, david, lvivier, Stefan Hajnoczi

On 02/09/2018 03:51 PM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  Makefile.objs              |  1 +
>  hw/misc/macio/cuda.c       | 50 ++++++++++++++++------------------------------
>  hw/misc/macio/trace-events | 11 ++++++++++
>  3 files changed, 29 insertions(+), 33 deletions(-)
>  create mode 100644 hw/misc/macio/trace-events
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 2efba6d768..3f1a1b674d 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -133,6 +133,7 @@ trace-events-subdirs += hw/net
>  trace-events-subdirs += hw/virtio
>  trace-events-subdirs += hw/audio
>  trace-events-subdirs += hw/misc
> +trace-events-subdirs += hw/misc/macio
>  trace-events-subdirs += hw/usb
>  trace-events-subdirs += hw/scsi
>  trace-events-subdirs += hw/nvram
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 377b91d266..bd9b862034 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -32,19 +32,7 @@
>  #include "sysemu/sysemu.h"
>  #include "qemu/cutils.h"
>  #include "qemu/log.h"
> -
> -/* debug CUDA packets */
> -//#define DEBUG_CUDA_PACKET
> -
> -/* debug CUDA packets */
> -//#define DEBUG_CUDA_PACKET
> -
> -#ifdef DEBUG_CUDA
> -#define CUDA_DPRINTF(fmt, ...)                                  \
> -    do { printf("CUDA: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define CUDA_DPRINTF(fmt, ...)
> -#endif
> +#include "trace.h"
>  
>  /* Bits in B data register: all active low */
>  #define TREQ            0x08    /* Transfer request (input) */
> @@ -120,7 +108,7 @@ static void cuda_delay_set_sr_int(CUDAState *s)
>          return;
>      }
>  
> -    CUDA_DPRINTF("CUDA: %s:%d\n", __func__, __LINE__);
> +    trace_cuda_delay_set_sr_int();
>  
>      expire = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->sr_delay_ns;
>      timer_mod(s->sr_delay_timer, expire);
> @@ -141,7 +129,7 @@ static void cuda_update(CUDAState *s)
>              /* data output */
>              if ((ms->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
>                  if (s->data_out_index < sizeof(s->data_out)) {
> -                    CUDA_DPRINTF("send: %02x\n", ms->sr);
> +                    trace_cuda_data_send(ms->sr);
>                      s->data_out[s->data_out_index++] = ms->sr;
>                      cuda_delay_set_sr_int(s);
>                  }
> @@ -151,7 +139,7 @@ static void cuda_update(CUDAState *s)
>                  /* data input */
>                  if ((ms->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
>                      ms->sr = s->data_in[s->data_in_index++];
> -                    CUDA_DPRINTF("recv: %02x\n", ms->sr);
> +                    trace_cuda_data_recv(ms->sr);
>                      /* indicate end of transfer */
>                      if (s->data_in_index >= s->data_in_size) {
>                          ms->b = (ms->b | TREQ);
> @@ -199,15 +187,13 @@ static void cuda_update(CUDAState *s)
>  static void cuda_send_packet_to_host(CUDAState *s,
>                                       const uint8_t *data, int len)
>  {
> -#ifdef DEBUG_CUDA_PACKET
> -    {
> -        int i;
> -        printf("cuda_send_packet_to_host:\n");
> -        for(i = 0; i < len; i++)
> -            printf(" %02x", data[i]);
> -        printf("\n");
> +    int i;
> +
> +    trace_cuda_packet_send(len);
> +    for (i = 0; i < len; i++) {
> +        trace_cuda_packet_send_data(i, data[i]);

Stefan said the trace framework is not meant to dump packet,
but I'm fine with your patch, so:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>      }
> -#endif
> +
>      memcpy(s->data_in, data, len);
>      s->data_in_size = len;
>      s->data_in_index = 0;
> @@ -411,7 +397,7 @@ static void cuda_receive_packet(CUDAState *s,
>      for (i = 0; i < ARRAY_SIZE(handlers); i++) {
>          const CudaCommand *desc = &handlers[i];
>          if (desc->command == data[0]) {
> -            CUDA_DPRINTF("handling command %s\n", desc->name);
> +            trace_cuda_receive_packet_cmd(desc->name);
>              out_len = 0;
>              if (desc->handler(s, data + 1, len - 1, obuf + 3, &out_len)) {
>                  cuda_send_packet_to_host(s, obuf, 3 + out_len);
> @@ -440,15 +426,13 @@ static void cuda_receive_packet(CUDAState *s,
>  static void cuda_receive_packet_from_host(CUDAState *s,
>                                            const uint8_t *data, int len)
>  {
> -#ifdef DEBUG_CUDA_PACKET
> -    {
> -        int i;
> -        printf("cuda_receive_packet_from_host:\n");
> -        for(i = 0; i < len; i++)
> -            printf(" %02x", data[i]);
> -        printf("\n");
> +    int i;
> +
> +    trace_cuda_packet_receive(len);
> +    for (i = 0; i < len; i++) {
> +        trace_cuda_packet_receive_data(i, data[i]);
>      }
> -#endif
> +
>      switch(data[0]) {
>      case ADB_PACKET:
>          {
> diff --git a/hw/misc/macio/trace-events b/hw/misc/macio/trace-events
> new file mode 100644
> index 0000000000..24c0a36824
> --- /dev/null
> +++ b/hw/misc/macio/trace-events
> @@ -0,0 +1,11 @@
> +# See docs/devel/tracing.txt for syntax documentation.
> +
> +# hw/misc/macio/cuda.c
> +cuda_delay_set_sr_int(void) ""
> +cuda_data_send(uint8_t data) "send: 0x%02x"
> +cuda_data_recv(uint8_t data) "recv: 0x%02x"
> +cuda_receive_packet_cmd(const char *cmd) "handling command %s"
> +cuda_packet_receive(int len) "length %d"
> +cuda_packet_receive_data(int i, const uint8_t data) "[%d] 0x%02x"
> +cuda_packet_send(int len) "length %d"
> +cuda_packet_send_data(int i, const uint8_t data) "[%d] 0x%02x"
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 11/12] ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 11/12] ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file Mark Cave-Ayland
@ 2018-02-09 19:58   ` Philippe Mathieu-Daudé
  2018-02-12  7:59   ` David Gibson
  1 sibling, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-09 19:58 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, qemu-ppc, david, lvivier

On 02/09/2018 03:51 PM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/misc/macio/cuda.c         |   1 +
>  hw/misc/macio/macio.c        |   1 +
>  hw/ppc/mac.h                 |  77 -------------------------------
>  include/hw/misc/macio/cuda.h | 107 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 109 insertions(+), 77 deletions(-)
>  create mode 100644 include/hw/misc/macio/cuda.h
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 54c02aeffb..377b91d266 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -27,6 +27,7 @@
>  #include "hw/ppc/mac.h"
>  #include "hw/input/adb.h"
>  #include "hw/misc/mos6522.h"
> +#include "hw/misc/macio/cuda.h"
>  #include "qemu/timer.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/cutils.h"
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index a639b09e00..024f8557ab 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -26,6 +26,7 @@
>  #include "qapi/error.h"
>  #include "hw/hw.h"
>  #include "hw/ppc/mac.h"
> +#include "hw/misc/macio/cuda.h"
>  #include "hw/pci/pci.h"
>  #include "hw/ppc/mac_dbdma.h"
>  #include "hw/char/escc.h"
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index 3e9f13d9b4..4702194f3f 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -45,83 +45,6 @@
>  
>  #define ESCC_CLOCK 3686400
>  
> -/* CUDA commands (2nd byte) */
> -#define CUDA_WARM_START                0x0
> -#define CUDA_AUTOPOLL                  0x1
> -#define CUDA_GET_6805_ADDR             0x2
> -#define CUDA_GET_TIME                  0x3
> -#define CUDA_GET_PRAM                  0x7
> -#define CUDA_SET_6805_ADDR             0x8
> -#define CUDA_SET_TIME                  0x9
> -#define CUDA_POWERDOWN                 0xa
> -#define CUDA_POWERUP_TIME              0xb
> -#define CUDA_SET_PRAM                  0xc
> -#define CUDA_MS_RESET                  0xd
> -#define CUDA_SEND_DFAC                 0xe
> -#define CUDA_BATTERY_SWAP_SENSE        0x10
> -#define CUDA_RESET_SYSTEM              0x11
> -#define CUDA_SET_IPL                   0x12
> -#define CUDA_FILE_SERVER_FLAG          0x13
> -#define CUDA_SET_AUTO_RATE             0x14
> -#define CUDA_GET_AUTO_RATE             0x16
> -#define CUDA_SET_DEVICE_LIST           0x19
> -#define CUDA_GET_DEVICE_LIST           0x1a
> -#define CUDA_SET_ONE_SECOND_MODE       0x1b
> -#define CUDA_SET_POWER_MESSAGES        0x21
> -#define CUDA_GET_SET_IIC               0x22
> -#define CUDA_WAKEUP                    0x23
> -#define CUDA_TIMER_TICKLE              0x24
> -#define CUDA_COMBINED_FORMAT_IIC       0x25
> -
> -/* Cuda */
> -#define TYPE_CUDA "cuda"
> -#define CUDA(obj) OBJECT_CHECK(CUDAState, (obj), TYPE_CUDA)
> -
> -typedef struct MOS6522CUDAState MOS6522CUDAState;
> -
> -typedef struct CUDAState {
> -    /*< private >*/
> -    SysBusDevice parent_obj;
> -    /*< public >*/
> -    MemoryRegion mem;
> -
> -    ADBBusState adb_bus;
> -    MOS6522CUDAState *mos6522_cuda;
> -
> -    uint32_t tick_offset;
> -    uint64_t tb_frequency;
> -
> -    uint8_t last_b;
> -    uint8_t last_acr;
> -
> -    /* MacOS 9 is racy and requires a delay upon setting the SR_INT bit */
> -    uint64_t sr_delay_ns;
> -    QEMUTimer *sr_delay_timer;
> -
> -    int data_in_size;
> -    int data_in_index;
> -    int data_out_index;
> -
> -    qemu_irq irq;
> -    uint16_t adb_poll_mask;
> -    uint8_t autopoll_rate_ms;
> -    uint8_t autopoll;
> -    uint8_t data_in[128];
> -    uint8_t data_out[16];
> -    QEMUTimer *adb_poll_timer;
> -} CUDAState;
> -
> -/* MOS6522 CUDA */
> -typedef struct MOS6522CUDAState {
> -    /*< private >*/
> -    MOS6522State parent_obj;
> -
> -    CUDAState *cuda;
> -} MOS6522CUDAState;
> -
> -#define TYPE_MOS6522_CUDA "mos6522-cuda"
> -#define MOS6522_CUDA(obj) OBJECT_CHECK(MOS6522CUDAState, (obj), \
> -                                       TYPE_MOS6522_CUDA)
>  
>  /* MacIO */
>  #define TYPE_OLDWORLD_MACIO "macio-oldworld"
> diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h
> new file mode 100644
> index 0000000000..6afbdd13ee
> --- /dev/null
> +++ b/include/hw/misc/macio/cuda.h
> @@ -0,0 +1,107 @@
> +/*
> + * QEMU PowerMac CUDA device support
> + *
> + * Copyright (c) 2004-2007 Fabrice Bellard
> + * Copyright (c) 2007 Jocelyn Mayer
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef CUDA_H
> +#define CUDA_H
> +
> +/* CUDA commands (2nd byte) */
> +#define CUDA_WARM_START                0x0
> +#define CUDA_AUTOPOLL                  0x1
> +#define CUDA_GET_6805_ADDR             0x2
> +#define CUDA_GET_TIME                  0x3
> +#define CUDA_GET_PRAM                  0x7
> +#define CUDA_SET_6805_ADDR             0x8
> +#define CUDA_SET_TIME                  0x9
> +#define CUDA_POWERDOWN                 0xa
> +#define CUDA_POWERUP_TIME              0xb
> +#define CUDA_SET_PRAM                  0xc
> +#define CUDA_MS_RESET                  0xd
> +#define CUDA_SEND_DFAC                 0xe
> +#define CUDA_BATTERY_SWAP_SENSE        0x10
> +#define CUDA_RESET_SYSTEM              0x11
> +#define CUDA_SET_IPL                   0x12
> +#define CUDA_FILE_SERVER_FLAG          0x13
> +#define CUDA_SET_AUTO_RATE             0x14
> +#define CUDA_GET_AUTO_RATE             0x16
> +#define CUDA_SET_DEVICE_LIST           0x19
> +#define CUDA_GET_DEVICE_LIST           0x1a
> +#define CUDA_SET_ONE_SECOND_MODE       0x1b
> +#define CUDA_SET_POWER_MESSAGES        0x21
> +#define CUDA_GET_SET_IIC               0x22
> +#define CUDA_WAKEUP                    0x23
> +#define CUDA_TIMER_TICKLE              0x24
> +#define CUDA_COMBINED_FORMAT_IIC       0x25
> +
> +/* Cuda */
> +#define TYPE_CUDA "cuda"
> +#define CUDA(obj) OBJECT_CHECK(CUDAState, (obj), TYPE_CUDA)
> +
> +typedef struct MOS6522CUDAState MOS6522CUDAState;
> +
> +typedef struct CUDAState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +    MemoryRegion mem;
> +
> +    ADBBusState adb_bus;
> +    MOS6522CUDAState *mos6522_cuda;
> +
> +    uint32_t tick_offset;
> +    uint64_t tb_frequency;
> +
> +    uint8_t last_b;
> +    uint8_t last_acr;
> +
> +    /* MacOS 9 is racy and requires a delay upon setting the SR_INT bit */
> +    uint64_t sr_delay_ns;
> +    QEMUTimer *sr_delay_timer;
> +
> +    int data_in_size;
> +    int data_in_index;
> +    int data_out_index;
> +
> +    qemu_irq irq;
> +    uint16_t adb_poll_mask;
> +    uint8_t autopoll_rate_ms;
> +    uint8_t autopoll;
> +    uint8_t data_in[128];
> +    uint8_t data_out[16];
> +    QEMUTimer *adb_poll_timer;
> +} CUDAState;
> +
> +/* MOS6522 CUDA */
> +typedef struct MOS6522CUDAState {
> +    /*< private >*/
> +    MOS6522State parent_obj;
> +
> +    CUDAState *cuda;
> +} MOS6522CUDAState;
> +
> +#define TYPE_MOS6522_CUDA "mos6522-cuda"
> +#define MOS6522_CUDA(obj) OBJECT_CHECK(MOS6522CUDAState, (obj), \
> +                                       TYPE_MOS6522_CUDA)
> +
> +#endif /* CUDA_H */
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses Mark Cave-Ayland
@ 2018-02-09 20:05   ` Philippe Mathieu-Daudé
  2018-02-10  7:18     ` David Gibson
  2018-02-10  7:22   ` David Gibson
  1 sibling, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-09 20:05 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, qemu-ppc, david, lvivier

Hi Mark,

On 02/09/2018 03:51 PM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/misc/macio/cuda.c | 40 ++++++++--------------------------------
>  1 file changed, 8 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 008d8bd4d5..6631017ca2 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -275,7 +275,7 @@ static void cuda_delay_set_sr_int(CUDAState *s)
>      timer_mod(s->sr_delay_timer, expire);
>  }
>  
> -static uint32_t cuda_readb(void *opaque, hwaddr addr)
> +static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      CUDAState *s = opaque;
>      uint32_t val;
> @@ -350,7 +350,7 @@ static uint32_t cuda_readb(void *opaque, hwaddr addr)
>      return val;
>  }
>  
> -static void cuda_writeb(void *opaque, hwaddr addr, uint32_t val)
> +static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>  {
>      CUDAState *s = opaque;
>  
> @@ -780,38 +780,14 @@ static void cuda_receive_packet_from_host(CUDAState *s,
>      }
>  }
>  
> -static void cuda_writew (void *opaque, hwaddr addr, uint32_t value)
> -{
> -}
> -
> -static void cuda_writel (void *opaque, hwaddr addr, uint32_t value)
> -{
> -}
> -
> -static uint32_t cuda_readw (void *opaque, hwaddr addr)
> -{
> -    return 0;
> -}
> -
> -static uint32_t cuda_readl (void *opaque, hwaddr addr)
> -{
> -    return 0;
> -}
> -
>  static const MemoryRegionOps cuda_ops = {
> -    .old_mmio = {
> -        .write = {
> -            cuda_writeb,
> -            cuda_writew,
> -            cuda_writel,
> -        },
> -        .read = {
> -            cuda_readb,
> -            cuda_readw,
> -            cuda_readl,
> -        },
> +    .read = cuda_read,
> +    .write = cuda_write,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +    .valid = {

This change the device behavior, previously 16/32bit access were
ignored, now they are illegal.

Using ".impl" instead also change the behavior, since 16/32bits access
would be processed (as 2x or 4x 8bit access).

A comment "this change is safe because ..." would reassure me ;)

> +        .min_access_size = 1,
> +        .max_access_size = 1,
>      },
> -    .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
>  static bool cuda_timer_exist(void *opaque, int version_id)
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses
  2018-02-09 20:05   ` Philippe Mathieu-Daudé
@ 2018-02-10  7:18     ` David Gibson
  0 siblings, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-10  7:18 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Mark Cave-Ayland, qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 3209 bytes --]

On Fri, Feb 09, 2018 at 05:05:23PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Mark,
> 
> On 02/09/2018 03:51 PM, Mark Cave-Ayland wrote:
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> >  hw/misc/macio/cuda.c | 40 ++++++++--------------------------------
> >  1 file changed, 8 insertions(+), 32 deletions(-)
> > 
> > diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> > index 008d8bd4d5..6631017ca2 100644
> > --- a/hw/misc/macio/cuda.c
> > +++ b/hw/misc/macio/cuda.c
> > @@ -275,7 +275,7 @@ static void cuda_delay_set_sr_int(CUDAState *s)
> >      timer_mod(s->sr_delay_timer, expire);
> >  }
> >  
> > -static uint32_t cuda_readb(void *opaque, hwaddr addr)
> > +static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size)
> >  {
> >      CUDAState *s = opaque;
> >      uint32_t val;
> > @@ -350,7 +350,7 @@ static uint32_t cuda_readb(void *opaque, hwaddr addr)
> >      return val;
> >  }
> >  
> > -static void cuda_writeb(void *opaque, hwaddr addr, uint32_t val)
> > +static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> >  {
> >      CUDAState *s = opaque;
> >  
> > @@ -780,38 +780,14 @@ static void cuda_receive_packet_from_host(CUDAState *s,
> >      }
> >  }
> >  
> > -static void cuda_writew (void *opaque, hwaddr addr, uint32_t value)
> > -{
> > -}
> > -
> > -static void cuda_writel (void *opaque, hwaddr addr, uint32_t value)
> > -{
> > -}
> > -
> > -static uint32_t cuda_readw (void *opaque, hwaddr addr)
> > -{
> > -    return 0;
> > -}
> > -
> > -static uint32_t cuda_readl (void *opaque, hwaddr addr)
> > -{
> > -    return 0;
> > -}
> > -
> >  static const MemoryRegionOps cuda_ops = {
> > -    .old_mmio = {
> > -        .write = {
> > -            cuda_writeb,
> > -            cuda_writew,
> > -            cuda_writel,
> > -        },
> > -        .read = {
> > -            cuda_readb,
> > -            cuda_readw,
> > -            cuda_readl,
> > -        },
> > +    .read = cuda_read,
> > +    .write = cuda_write,
> > +    .endianness = DEVICE_BIG_ENDIAN,
> > +    .valid = {
> 
> This change the device behavior, previously 16/32bit access were
> ignored, now they are illegal.

This change looks correct to me.  As you note >1 byte accesses were
previously ignored - not broken or handled in any other way.  That
means there's no possibility using >1byte accesses could ever
accomplish, so it reads to me as being invalid.  I'd say the fact that
it previously didn't generate some sort of error is the bug.

> Using ".impl" instead also change the behavior, since 16/32bits access
> would be processed (as 2x or 4x 8bit access).
> 
> A comment "this change is safe because ..." would reassure me ;)
> 
> > +        .min_access_size = 1,
> > +        .max_access_size = 1,
> >      },
> > -    .endianness = DEVICE_NATIVE_ENDIAN,
> >  };
> >  
> >  static bool cuda_timer_exist(void *opaque, int version_id)
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses Mark Cave-Ayland
  2018-02-09 20:05   ` Philippe Mathieu-Daudé
@ 2018-02-10  7:22   ` David Gibson
  1 sibling, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-10  7:22 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 2361 bytes --]

On Fri, Feb 09, 2018 at 06:51:31PM +0000, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Applied, thanks.

> ---
>  hw/misc/macio/cuda.c | 40 ++++++++--------------------------------
>  1 file changed, 8 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 008d8bd4d5..6631017ca2 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -275,7 +275,7 @@ static void cuda_delay_set_sr_int(CUDAState *s)
>      timer_mod(s->sr_delay_timer, expire);
>  }
>  
> -static uint32_t cuda_readb(void *opaque, hwaddr addr)
> +static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      CUDAState *s = opaque;
>      uint32_t val;
> @@ -350,7 +350,7 @@ static uint32_t cuda_readb(void *opaque, hwaddr addr)
>      return val;
>  }
>  
> -static void cuda_writeb(void *opaque, hwaddr addr, uint32_t val)
> +static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>  {
>      CUDAState *s = opaque;
>  
> @@ -780,38 +780,14 @@ static void cuda_receive_packet_from_host(CUDAState *s,
>      }
>  }
>  
> -static void cuda_writew (void *opaque, hwaddr addr, uint32_t value)
> -{
> -}
> -
> -static void cuda_writel (void *opaque, hwaddr addr, uint32_t value)
> -{
> -}
> -
> -static uint32_t cuda_readw (void *opaque, hwaddr addr)
> -{
> -    return 0;
> -}
> -
> -static uint32_t cuda_readl (void *opaque, hwaddr addr)
> -{
> -    return 0;
> -}
> -
>  static const MemoryRegionOps cuda_ops = {
> -    .old_mmio = {
> -        .write = {
> -            cuda_writeb,
> -            cuda_writew,
> -            cuda_writel,
> -        },
> -        .read = {
> -            cuda_readb,
> -            cuda_readw,
> -            cuda_readl,
> -        },
> +    .read = cuda_read,
> +    .write = cuda_write,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
>      },
> -    .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
>  static bool cuda_timer_exist(void *opaque, int version_id)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 02/12] cuda: don't allow writes to port output pins
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 02/12] cuda: don't allow writes to port output pins Mark Cave-Ayland
@ 2018-02-10  7:23   ` David Gibson
  0 siblings, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-10  7:23 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

On Fri, Feb 09, 2018 at 06:51:32PM +0000, Mark Cave-Ayland wrote:
> Use the direction registers as a mask to ensure that only input pins are
> updated upon write.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Applied, thanks.

> ---
>  hw/misc/macio/cuda.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 6631017ca2..eaa8924f49 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -359,11 +359,11 @@ static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>  
>      switch(addr) {
>      case CUDA_REG_B:
> -        s->b = val;
> +        s->b = (s->b & ~s->dirb) | (val & s->dirb);
>          cuda_update(s);
>          break;
>      case CUDA_REG_A:
> -        s->a = val;
> +        s->a = (s->a & ~s->dira) | (val & s->dira);
>          break;
>      case CUDA_REG_DIRB:
>          s->dirb = val;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 04/12] cuda: introduce CUDAState parameter to get_counter()
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 04/12] cuda: introduce CUDAState parameter to get_counter() Mark Cave-Ayland
@ 2018-02-10 22:31   ` David Gibson
  0 siblings, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-10 22:31 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]

On Fri, Feb 09, 2018 at 06:51:34PM +0000, Mark Cave-Ayland wrote:
> This will be required shortly and also happens to match nicely with the
> corresponding signature for set_counter().
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Applied, thanks.

> ---
>  hw/misc/macio/cuda.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 1d0f7e8289..a88535fa66 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -150,7 +150,7 @@ static uint64_t get_tb(uint64_t time, uint64_t freq)
>      return muldiv64(time, freq, NANOSECONDS_PER_SECOND);
>  }
>  
> -static unsigned int get_counter(CUDATimer *ti)
> +static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
>  {
>      int64_t d;
>      unsigned int counter;
> @@ -295,12 +295,12 @@ static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size)
>          val = s->dira;
>          break;
>      case CUDA_REG_T1CL:
> -        val = get_counter(&s->timers[0]) & 0xff;
> +        val = get_counter(s, &s->timers[0]) & 0xff;
>          s->ifr &= ~T1_INT;
>          cuda_update_irq(s);
>          break;
>      case CUDA_REG_T1CH:
> -        val = get_counter(&s->timers[0]) >> 8;
> +        val = get_counter(s, &s->timers[0]) >> 8;
>          cuda_update_irq(s);
>          break;
>      case CUDA_REG_T1LL:
> @@ -311,12 +311,12 @@ static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size)
>          val = (s->timers[0].latch >> 8) & 0xff;
>          break;
>      case CUDA_REG_T2CL:
> -        val = get_counter(&s->timers[1]) & 0xff;
> +        val = get_counter(s, &s->timers[1]) & 0xff;
>          s->ifr &= ~T2_INT;
>          cuda_update_irq(s);
>          break;
>      case CUDA_REG_T2CH:
> -        val = get_counter(&s->timers[1]) >> 8;
> +        val = get_counter(s, &s->timers[1]) >> 8;
>          break;
>      case CUDA_REG_SR:
>          val = s->sr;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 05/12] cuda: rename frequency property to tb_frequency
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 05/12] cuda: rename frequency property to tb_frequency Mark Cave-Ayland
@ 2018-02-10 22:32   ` David Gibson
  2018-02-10 23:11     ` David Gibson
  0 siblings, 1 reply; 34+ messages in thread
From: David Gibson @ 2018-02-10 22:32 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 3685 bytes --]

On Fri, Feb 09, 2018 at 06:51:35PM +0000, Mark Cave-Ayland wrote:
> This allows us to more easily differentiate between the timebase frequency used
> to calibrate the MacOS timers and the actual frequency of the hardware clock as
> indicated by CUDA_TIMER_FREQ.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Applied, thanks.

> ---
>  hw/misc/macio/cuda.c  | 10 +++++-----
>  hw/misc/macio/macio.c |  2 +-
>  hw/ppc/mac.h          |  2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index a88535fa66..232b7f61aa 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -158,8 +158,8 @@ static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
>      uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>  
>      /* Reverse of the tb calculation algorithm that Mac OS X uses on bootup. */
> -    tb_diff = get_tb(current_time, ti->frequency) - ti->load_time;
> -    d = (tb_diff * 0xBF401675E5DULL) / (ti->frequency << 24);
> +    tb_diff = get_tb(current_time, ti->tb_frequency) - ti->load_time;
> +    d = (tb_diff * 0xBF401675E5DULL) / (ti->tb_frequency << 24);
>  
>      if (ti->index == 0) {
>          /* the timer goes down from latch to -1 (period of latch + 2) */
> @@ -179,7 +179,7 @@ static void set_counter(CUDAState *s, CUDATimer *ti, unsigned int val)
>  {
>      CUDA_DPRINTF("T%d.counter=%d\n", 1 + ti->index, val);
>      ti->load_time = get_tb(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                           s->frequency);
> +                           s->tb_frequency);
>      ti->counter_value = val;
>      cuda_timer_update(s, ti, ti->load_time);
>  }
> @@ -878,7 +878,7 @@ static void cuda_realizefn(DeviceState *dev, Error **errp)
>      struct tm tm;
>  
>      s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer1, s);
> -    s->timers[0].frequency = s->frequency;
> +    s->timers[0].frequency = s->tb_frequency;
>      s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer2, s);
>      s->timers[1].frequency = (SCALE_US * 6000) / 4700;
>  
> @@ -909,7 +909,7 @@ static void cuda_initfn(Object *obj)
>  }
>  
>  static Property cuda_properties[] = {
> -    DEFINE_PROP_UINT64("frequency", CUDAState, frequency, 0),
> +    DEFINE_PROP_UINT64("timebase-frequency", CUDAState, tb_frequency, 0),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 44f91d1e7f..a639b09e00 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -451,7 +451,7 @@ void macio_init(PCIDevice *d,
>      macio_state->escc_mem = escc_mem;
>      /* Note: this code is strongly inspirated from the corresponding code
>         in PearPC */
> -    qdev_prop_set_uint64(DEVICE(&macio_state->cuda), "frequency",
> +    qdev_prop_set_uint64(DEVICE(&macio_state->cuda), "timebase-frequency",
>                           macio_state->frequency);
>  
>      qdev_init_nofail(DEVICE(d));
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index b501af1653..fa78115c95 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -99,7 +99,7 @@ typedef struct CUDAState {
>      CUDATimer timers[2];
>  
>      uint32_t tick_offset;
> -    uint64_t frequency;
> +    uint64_t tb_frequency;
>  
>      uint8_t last_b;
>      uint8_t last_acr;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 06/12] cuda: minor cosmetic tidy-ups to get_next_irq_time()
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 06/12] cuda: minor cosmetic tidy-ups to get_next_irq_time() Mark Cave-Ayland
@ 2018-02-10 22:33   ` David Gibson
  0 siblings, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-10 22:33 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 2669 bytes --]

On Fri, Feb 09, 2018 at 06:51:36PM +0000, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Applied, thanks.

> ---
>  hw/misc/macio/cuda.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 232b7f61aa..408858e688 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -184,36 +184,37 @@ static void set_counter(CUDAState *s, CUDATimer *ti, unsigned int val)
>      cuda_timer_update(s, ti, ti->load_time);
>  }
>  
> -static int64_t get_next_irq_time(CUDATimer *s, int64_t current_time)
> +static int64_t get_next_irq_time(CUDATimer *ti, int64_t current_time)
>  {
>      int64_t d, next_time;
>      unsigned int counter;
>  
>      /* current counter value */
> -    d = muldiv64(current_time - s->load_time,
> +    d = muldiv64(current_time - ti->load_time,
>                   CUDA_TIMER_FREQ, NANOSECONDS_PER_SECOND);
>      /* the timer goes down from latch to -1 (period of latch + 2) */
> -    if (d <= (s->counter_value + 1)) {
> -        counter = (s->counter_value - d) & 0xffff;
> +    if (d <= (ti->counter_value + 1)) {
> +        counter = (ti->counter_value - d) & 0xffff;
>      } else {
> -        counter = (d - (s->counter_value + 1)) % (s->latch + 2);
> -        counter = (s->latch - counter) & 0xffff;
> +        counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
> +        counter = (ti->latch - counter) & 0xffff;
>      }
>  
>      /* Note: we consider the irq is raised on 0 */
>      if (counter == 0xffff) {
> -        next_time = d + s->latch + 1;
> +        next_time = d + ti->latch + 1;
>      } else if (counter == 0) {
> -        next_time = d + s->latch + 2;
> +        next_time = d + ti->latch + 2;
>      } else {
>          next_time = d + counter;
>      }
>      CUDA_DPRINTF("latch=%d counter=%" PRId64 " delta_next=%" PRId64 "\n",
> -                 s->latch, d, next_time - d);
> +                 ti->latch, d, next_time - d);
>      next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, CUDA_TIMER_FREQ) +
> -        s->load_time;
> -    if (next_time <= current_time)
> +                         ti->load_time;
> +    if (next_time <= current_time) {
>          next_time = current_time + 1;
> +    }
>      return next_time;
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 03/12] cuda: don't call cuda_update() when writing to ACR register
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 03/12] cuda: don't call cuda_update() when writing to ACR register Mark Cave-Ayland
@ 2018-02-10 22:35   ` David Gibson
  0 siblings, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-10 22:35 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]

On Fri, Feb 09, 2018 at 06:51:33PM +0000, Mark Cave-Ayland wrote:
> The wire protocol for reading data to/from the VIA is triggered by changing
> inputs on port B rather than changing the timer configuration via the ACR.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Applied, thanks.

> ---
>  hw/misc/macio/cuda.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index eaa8924f49..1d0f7e8289 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -406,7 +406,6 @@ static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>      case CUDA_REG_ACR:
>          s->acr = val;
>          cuda_timer_update(s, &s->timers[0], qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> -        cuda_update(s);
>          break;
>      case CUDA_REG_PCR:
>          s->pcr = val;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 05/12] cuda: rename frequency property to tb_frequency
  2018-02-10 22:32   ` David Gibson
@ 2018-02-10 23:11     ` David Gibson
  2018-02-11 10:59       ` Mark Cave-Ayland
  0 siblings, 1 reply; 34+ messages in thread
From: David Gibson @ 2018-02-10 23:11 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 4106 bytes --]

On Sun, Feb 11, 2018 at 09:32:14AM +1100, David Gibson wrote:
> On Fri, Feb 09, 2018 at 06:51:35PM +0000, Mark Cave-Ayland wrote:
> > This allows us to more easily differentiate between the timebase frequency used
> > to calibrate the MacOS timers and the actual frequency of the hardware clock as
> > indicated by CUDA_TIMER_FREQ.
> > 
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Applied, thanks.

Actually, this patch doesn't compile, because you've changed
ti->frequency in a couple of places where you should only be changing
s->frequency.  I've fixed it up in my tree.

> 
> > ---
> >  hw/misc/macio/cuda.c  | 10 +++++-----
> >  hw/misc/macio/macio.c |  2 +-
> >  hw/ppc/mac.h          |  2 +-
> >  3 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> > index a88535fa66..232b7f61aa 100644
> > --- a/hw/misc/macio/cuda.c
> > +++ b/hw/misc/macio/cuda.c
> > @@ -158,8 +158,8 @@ static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
> >      uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> >  
> >      /* Reverse of the tb calculation algorithm that Mac OS X uses on bootup. */
> > -    tb_diff = get_tb(current_time, ti->frequency) - ti->load_time;
> > -    d = (tb_diff * 0xBF401675E5DULL) / (ti->frequency << 24);
> > +    tb_diff = get_tb(current_time, ti->tb_frequency) - ti->load_time;
> > +    d = (tb_diff * 0xBF401675E5DULL) / (ti->tb_frequency << 24);
> >  
> >      if (ti->index == 0) {
> >          /* the timer goes down from latch to -1 (period of latch + 2) */
> > @@ -179,7 +179,7 @@ static void set_counter(CUDAState *s, CUDATimer *ti, unsigned int val)
> >  {
> >      CUDA_DPRINTF("T%d.counter=%d\n", 1 + ti->index, val);
> >      ti->load_time = get_tb(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> > -                           s->frequency);
> > +                           s->tb_frequency);
> >      ti->counter_value = val;
> >      cuda_timer_update(s, ti, ti->load_time);
> >  }
> > @@ -878,7 +878,7 @@ static void cuda_realizefn(DeviceState *dev, Error **errp)
> >      struct tm tm;
> >  
> >      s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer1, s);
> > -    s->timers[0].frequency = s->frequency;
> > +    s->timers[0].frequency = s->tb_frequency;
> >      s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer2, s);
> >      s->timers[1].frequency = (SCALE_US * 6000) / 4700;
> >  
> > @@ -909,7 +909,7 @@ static void cuda_initfn(Object *obj)
> >  }
> >  
> >  static Property cuda_properties[] = {
> > -    DEFINE_PROP_UINT64("frequency", CUDAState, frequency, 0),
> > +    DEFINE_PROP_UINT64("timebase-frequency", CUDAState, tb_frequency, 0),
> >      DEFINE_PROP_END_OF_LIST()
> >  };
> >  
> > diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> > index 44f91d1e7f..a639b09e00 100644
> > --- a/hw/misc/macio/macio.c
> > +++ b/hw/misc/macio/macio.c
> > @@ -451,7 +451,7 @@ void macio_init(PCIDevice *d,
> >      macio_state->escc_mem = escc_mem;
> >      /* Note: this code is strongly inspirated from the corresponding code
> >         in PearPC */
> > -    qdev_prop_set_uint64(DEVICE(&macio_state->cuda), "frequency",
> > +    qdev_prop_set_uint64(DEVICE(&macio_state->cuda), "timebase-frequency",
> >                           macio_state->frequency);
> >  
> >      qdev_init_nofail(DEVICE(d));
> > diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> > index b501af1653..fa78115c95 100644
> > --- a/hw/ppc/mac.h
> > +++ b/hw/ppc/mac.h
> > @@ -99,7 +99,7 @@ typedef struct CUDAState {
> >      CUDATimer timers[2];
> >  
> >      uint32_t tick_offset;
> > -    uint64_t frequency;
> > +    uint64_t tb_frequency;
> >  
> >      uint8_t last_b;
> >      uint8_t last_acr;
> 



-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 07/12] cuda: set timer 1 frequency property to CUDA_TIMER_FREQ
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 07/12] cuda: set timer 1 frequency property to CUDA_TIMER_FREQ Mark Cave-Ayland
@ 2018-02-10 23:15   ` David Gibson
  0 siblings, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-10 23:15 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 3214 bytes --]

On Fri, Feb 09, 2018 at 06:51:37PM +0000, Mark Cave-Ayland wrote:
> Now that we have successfully decoupled the timebase frequency and the hardware
> timer frequency, set the timer 1 frequency property to CUDA_TIMER_FREQ and alter
> get_next_irq_time() to use it rather than the hard-coded constant.
> 
> In addition to this we must now switch the tb_diff calculation over to use the
> timebase frequency now that the hardware clock frequency and the timebase
> frequency are different.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/misc/macio/cuda.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 408858e688..e00df4a21a 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -158,8 +158,8 @@ static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
>      uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>  
>      /* Reverse of the tb calculation algorithm that Mac OS X uses on bootup. */
> -    tb_diff = get_tb(current_time, ti->tb_frequency) - ti->load_time;
> -    d = (tb_diff * 0xBF401675E5DULL) / (ti->tb_frequency << 24);
> +    tb_diff = get_tb(current_time, s->tb_frequency) - ti->load_time;
> +    d = (tb_diff * 0xBF401675E5DULL) / (s->tb_frequency << 24);

This requires an update due to the bug in the earlier patch which
touched this.  I've fixed this up in my tree.

>      if (ti->index == 0) {
>          /* the timer goes down from latch to -1 (period of latch + 2) */
> @@ -191,7 +191,7 @@ static int64_t get_next_irq_time(CUDATimer *ti, int64_t current_time)
>  
>      /* current counter value */
>      d = muldiv64(current_time - ti->load_time,
> -                 CUDA_TIMER_FREQ, NANOSECONDS_PER_SECOND);
> +                 ti->frequency, NANOSECONDS_PER_SECOND);
>      /* the timer goes down from latch to -1 (period of latch + 2) */
>      if (d <= (ti->counter_value + 1)) {
>          counter = (ti->counter_value - d) & 0xffff;
> @@ -210,7 +210,7 @@ static int64_t get_next_irq_time(CUDATimer *ti, int64_t current_time)
>      }
>      CUDA_DPRINTF("latch=%d counter=%" PRId64 " delta_next=%" PRId64 "\n",
>                   ti->latch, d, next_time - d);
> -    next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, CUDA_TIMER_FREQ) +
> +    next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) +
>                           ti->load_time;
>      if (next_time <= current_time) {
>          next_time = current_time + 1;
> @@ -879,7 +879,7 @@ static void cuda_realizefn(DeviceState *dev, Error **errp)
>      struct tm tm;
>  
>      s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer1, s);
> -    s->timers[0].frequency = s->tb_frequency;
> +    s->timers[0].frequency = CUDA_TIMER_FREQ;
>      s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer2, s);
>      s->timers[1].frequency = (SCALE_US * 6000) / 4700;
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 08/12] cuda: factor out timebase-derived counter value and load time
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 08/12] cuda: factor out timebase-derived counter value and load time Mark Cave-Ayland
@ 2018-02-10 23:18   ` David Gibson
  0 siblings, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-10 23:18 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 3141 bytes --]

On Fri, Feb 09, 2018 at 06:51:38PM +0000, Mark Cave-Ayland wrote:
> Commit b981289c49 "PPC: Cuda: Use cuda timer to expose tbfreq to guest" altered
> the timer calculations from those based upon the hardware CUDA clock frequency
> to those based upon the CPU timebase frequency.
> 
> In fact we can isolate the differences to 2 simple changes: one to the counter
> read value and another to the counter load time. Move these changes into
> separate functions so the implementation can be swapped later.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Applied, thanks.

> ---
>  hw/misc/macio/cuda.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index e00df4a21a..a185252144 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -145,21 +145,29 @@ static void cuda_update_irq(CUDAState *s)
>      }
>  }
>  
> -static uint64_t get_tb(uint64_t time, uint64_t freq)
> +static uint64_t get_counter_value(CUDAState *s, CUDATimer *ti)
>  {
> -    return muldiv64(time, freq, NANOSECONDS_PER_SECOND);
> +    /* Reverse of the tb calculation algorithm that Mac OS X uses on bootup */
> +    uint64_t tb_diff = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> +                                s->tb_frequency, NANOSECONDS_PER_SECOND) -
> +                           ti->load_time;
> +
> +    return (tb_diff * 0xBF401675E5DULL) / (s->tb_frequency << 24);
> +}
> +
> +static uint64_t get_counter_load_time(CUDAState *s, CUDATimer *ti)
> +{
> +    uint64_t load_time = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> +                                  s->tb_frequency, NANOSECONDS_PER_SECOND);
> +    return load_time;
>  }
>  
>  static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
>  {
>      int64_t d;
>      unsigned int counter;
> -    uint64_t tb_diff;
> -    uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>  
> -    /* Reverse of the tb calculation algorithm that Mac OS X uses on bootup. */
> -    tb_diff = get_tb(current_time, s->tb_frequency) - ti->load_time;
> -    d = (tb_diff * 0xBF401675E5DULL) / (s->tb_frequency << 24);
> +    d = get_counter_value(s, ti);
>  
>      if (ti->index == 0) {
>          /* the timer goes down from latch to -1 (period of latch + 2) */
> @@ -178,8 +186,7 @@ static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
>  static void set_counter(CUDAState *s, CUDATimer *ti, unsigned int val)
>  {
>      CUDA_DPRINTF("T%d.counter=%d\n", 1 + ti->index, val);
> -    ti->load_time = get_tb(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                           s->tb_frequency);
> +    ti->load_time = get_counter_load_time(s, ti);
>      ti->counter_value = val;
>      cuda_timer_update(s, ti, ti->load_time);
>  }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 09/12] misc: introduce new mos6522 VIA device and enable it for ppc builds
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 09/12] misc: introduce new mos6522 VIA device and enable it for ppc builds Mark Cave-Ayland
@ 2018-02-10 23:20   ` David Gibson
  0 siblings, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-10 23:20 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 25042 bytes --]

On Fri, Feb 09, 2018 at 06:51:39PM +0000, Mark Cave-Ayland wrote:
> The MOS6522 VIA forms the bridge part of several Mac devices, including the
> Mac via-cuda and via-pmu devices. Introduce a standard mos6522 device that
> can be shared amongst multiple implementations.
> 
> This is effectively taking the 6522 parts out of cuda.c and turning them
> into a separate device whilst also applying some style tidy-ups and including
> a conversion to trace-events.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Applied, thanks.

> ---
>  default-configs/ppc-softmmu.mak |   1 +
>  hw/misc/Makefile.objs           |   3 +
>  hw/misc/mos6522.c               | 505 ++++++++++++++++++++++++++++++++++++++++
>  hw/misc/trace-events            |   7 +
>  include/hw/misc/mos6522.h       | 152 ++++++++++++
>  5 files changed, 668 insertions(+)
>  create mode 100644 hw/misc/mos6522.c
>  create mode 100644 include/hw/misc/mos6522.h
> 
> diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
> index 65680d85bc..76e29cfa14 100644
> --- a/default-configs/ppc-softmmu.mak
> +++ b/default-configs/ppc-softmmu.mak
> @@ -30,6 +30,7 @@ CONFIG_MAC=y
>  CONFIG_ESCC=y
>  CONFIG_MACIO=y
>  CONFIG_SUNGEM=y
> +CONFIG_MOS6522=y
>  CONFIG_CUDA=y
>  CONFIG_ADB=y
>  CONFIG_MAC_NVRAM=y
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index fce426eb75..f33b37a8e5 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -17,6 +17,9 @@ common-obj-$(CONFIG_INTEGRATOR_DEBUG) += arm_integrator_debug.o
>  common-obj-$(CONFIG_A9SCU) += a9scu.o
>  common-obj-$(CONFIG_ARM11SCU) += arm11scu.o
>  
> +# Mac devices
> +common-obj-$(CONFIG_MOS6522) += mos6522.o
> +
>  # PKUnity SoC devices
>  common-obj-$(CONFIG_PUV3) += puv3_pm.o
>  
> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
> new file mode 100644
> index 0000000000..8ad9fc831e
> --- /dev/null
> +++ b/hw/misc/mos6522.c
> @@ -0,0 +1,505 @@
> +/*
> + * QEMU MOS6522 VIA emulation
> + *
> + * Copyright (c) 2004-2007 Fabrice Bellard
> + * Copyright (c) 2007 Jocelyn Mayer
> + * Copyright (c) 2018 Mark Cave-Ayland
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include "qemu/osdep.h"
> +#include "hw/hw.h"
> +#include "hw/input/adb.h"
> +#include "hw/misc/mos6522.h"
> +#include "qemu/timer.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/cutils.h"
> +#include "qemu/log.h"
> +#include "trace.h"
> +
> +/* XXX: implement all timer modes */
> +
> +static void mos6522_timer_update(MOS6522State *s, MOS6522Timer *ti,
> +                                 int64_t current_time);
> +
> +static void mos6522_update_irq(MOS6522State *s)
> +{
> +    if (s->ifr & s->ier & (SR_INT | T1_INT | T2_INT)) {
> +        qemu_irq_raise(s->irq);
> +    } else {
> +        qemu_irq_lower(s->irq);
> +    }
> +}
> +
> +static uint64_t get_counter_value(MOS6522State *s, MOS6522Timer *ti)
> +{
> +    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(s);
> +
> +    if (ti->index == 0) {
> +        return mdc->get_timer1_counter_value(s, ti);
> +    } else {
> +        return mdc->get_timer2_counter_value(s, ti);
> +    }
> +}
> +
> +static uint64_t get_load_time(MOS6522State *s, MOS6522Timer *ti)
> +{
> +    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(s);
> +
> +    if (ti->index == 0) {
> +        return mdc->get_timer1_load_time(s, ti);
> +    } else {
> +        return mdc->get_timer2_load_time(s, ti);
> +    }
> +}
> +
> +static unsigned int get_counter(MOS6522State *s, MOS6522Timer *ti)
> +{
> +    int64_t d;
> +    unsigned int counter;
> +
> +    d = get_counter_value(s, ti);
> +
> +    if (ti->index == 0) {
> +        /* the timer goes down from latch to -1 (period of latch + 2) */
> +        if (d <= (ti->counter_value + 1)) {
> +            counter = (ti->counter_value - d) & 0xffff;
> +        } else {
> +            counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
> +            counter = (ti->latch - counter) & 0xffff;
> +        }
> +    } else {
> +        counter = (ti->counter_value - d) & 0xffff;
> +    }
> +    return counter;
> +}
> +
> +static void set_counter(MOS6522State *s, MOS6522Timer *ti, unsigned int val)
> +{
> +    trace_mos6522_set_counter(1 + ti->index, val);
> +    ti->load_time = get_load_time(s, ti);
> +    ti->counter_value = val;
> +    mos6522_timer_update(s, ti, ti->load_time);
> +}
> +
> +static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti,
> +                                 int64_t current_time)
> +{
> +    int64_t d, next_time;
> +    unsigned int counter;
> +
> +    /* current counter value */
> +    d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time,
> +                 ti->frequency, NANOSECONDS_PER_SECOND);
> +
> +    /* the timer goes down from latch to -1 (period of latch + 2) */
> +    if (d <= (ti->counter_value + 1)) {
> +        counter = (ti->counter_value - d) & 0xffff;
> +    } else {
> +        counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
> +        counter = (ti->latch - counter) & 0xffff;
> +    }
> +
> +    /* Note: we consider the irq is raised on 0 */
> +    if (counter == 0xffff) {
> +        next_time = d + ti->latch + 1;
> +    } else if (counter == 0) {
> +        next_time = d + ti->latch + 2;
> +    } else {
> +        next_time = d + counter;
> +    }
> +    trace_mos6522_get_next_irq_time(ti->latch, d, next_time - d);
> +    next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) +
> +                         ti->load_time;
> +    if (next_time <= current_time) {
> +        next_time = current_time + 1;
> +    }
> +    return next_time;
> +}
> +
> +static void mos6522_timer_update(MOS6522State *s, MOS6522Timer *ti,
> +                                 int64_t current_time)
> +{
> +    if (!ti->timer) {
> +        return;
> +    }
> +    if (ti->index == 0 && (s->acr & T1MODE) != T1MODE_CONT) {
> +        timer_del(ti->timer);
> +    } else {
> +        ti->next_irq_time = get_next_irq_time(s, ti, current_time);
> +        timer_mod(ti->timer, ti->next_irq_time);
> +    }
> +}
> +
> +static void mos6522_timer1(void *opaque)
> +{
> +    MOS6522State *s = opaque;
> +    MOS6522Timer *ti = &s->timers[0];
> +
> +    mos6522_timer_update(s, ti, ti->next_irq_time);
> +    s->ifr |= T1_INT;
> +    mos6522_update_irq(s);
> +}
> +
> +static void mos6522_timer2(void *opaque)
> +{
> +    MOS6522State *s = opaque;
> +    MOS6522Timer *ti = &s->timers[1];
> +
> +    mos6522_timer_update(s, ti, ti->next_irq_time);
> +    s->ifr |= T2_INT;
> +    mos6522_update_irq(s);
> +}
> +
> +static void mos6522_set_sr_int(MOS6522State *s)
> +{
> +    trace_mos6522_set_sr_int();
> +    s->ifr |= SR_INT;
> +    mos6522_update_irq(s);
> +}
> +
> +static uint64_t mos6522_get_counter_value(MOS6522State *s, MOS6522Timer *ti)
> +{
> +    uint64_t d;
> +
> +    d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time,
> +                 ti->frequency, NANOSECONDS_PER_SECOND);
> +
> +    return d;
> +}
> +
> +static uint64_t mos6522_get_load_time(MOS6522State *s, MOS6522Timer *ti)
> +{
> +    uint64_t load_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +
> +    return load_time;
> +}
> +
> +static void mos6522_portA_write(MOS6522State *s)
> +{
> +    qemu_log_mask(LOG_UNIMP, "portA_write unimplemented");
> +}
> +
> +static void mos6522_portB_write(MOS6522State *s)
> +{
> +    qemu_log_mask(LOG_UNIMP, "portB_write unimplemented");
> +}
> +
> +uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    MOS6522State *s = opaque;
> +    uint32_t val;
> +
> +    switch (addr) {
> +    case VIA_REG_B:
> +        val = s->b;
> +        break;
> +    case VIA_REG_A:
> +        val = s->a;
> +        break;
> +    case VIA_REG_DIRB:
> +        val = s->dirb;
> +        break;
> +    case VIA_REG_DIRA:
> +        val = s->dira;
> +        break;
> +    case VIA_REG_T1CL:
> +        val = get_counter(s, &s->timers[0]) & 0xff;
> +        s->ifr &= ~T1_INT;
> +        mos6522_update_irq(s);
> +        break;
> +    case VIA_REG_T1CH:
> +        val = get_counter(s, &s->timers[0]) >> 8;
> +        mos6522_update_irq(s);
> +        break;
> +    case VIA_REG_T1LL:
> +        val = s->timers[0].latch & 0xff;
> +        break;
> +    case VIA_REG_T1LH:
> +        /* XXX: check this */
> +        val = (s->timers[0].latch >> 8) & 0xff;
> +        break;
> +    case VIA_REG_T2CL:
> +        val = get_counter(s, &s->timers[1]) & 0xff;
> +        s->ifr &= ~T2_INT;
> +        mos6522_update_irq(s);
> +        break;
> +    case VIA_REG_T2CH:
> +        val = get_counter(s, &s->timers[1]) >> 8;
> +        break;
> +    case VIA_REG_SR:
> +        val = s->sr;
> +        s->ifr &= ~(SR_INT | CB1_INT | CB2_INT);
> +        mos6522_update_irq(s);
> +        break;
> +    case VIA_REG_ACR:
> +        val = s->acr;
> +        break;
> +    case VIA_REG_PCR:
> +        val = s->pcr;
> +        break;
> +    case VIA_REG_IFR:
> +        val = s->ifr;
> +        if (s->ifr & s->ier) {
> +            val |= 0x80;
> +        }
> +        break;
> +    case VIA_REG_IER:
> +        val = s->ier | 0x80;
> +        break;
> +    default:
> +    case VIA_REG_ANH:
> +        val = s->anh;
> +        break;
> +    }
> +
> +    if (addr != VIA_REG_IFR || val != 0) {
> +        trace_mos6522_read(addr, val);
> +    }
> +
> +    return val;
> +}
> +
> +void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +    MOS6522State *s = opaque;
> +    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(s);
> +
> +    trace_mos6522_write(addr, val);
> +
> +    switch (addr) {
> +    case VIA_REG_B:
> +        s->b = (s->b & ~s->dirb) | (val & s->dirb);
> +        mdc->portB_write(s);
> +        break;
> +    case VIA_REG_A:
> +        s->a = (s->a & ~s->dira) | (val & s->dira);
> +        mdc->portA_write(s);
> +        break;
> +    case VIA_REG_DIRB:
> +        s->dirb = val;
> +        break;
> +    case VIA_REG_DIRA:
> +        s->dira = val;
> +        break;
> +    case VIA_REG_T1CL:
> +        s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
> +        mos6522_timer_update(s, &s->timers[0],
> +                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> +        break;
> +    case VIA_REG_T1CH:
> +        s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
> +        s->ifr &= ~T1_INT;
> +        set_counter(s, &s->timers[0], s->timers[0].latch);
> +        break;
> +    case VIA_REG_T1LL:
> +        s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
> +        mos6522_timer_update(s, &s->timers[0],
> +                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> +        break;
> +    case VIA_REG_T1LH:
> +        s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
> +        s->ifr &= ~T1_INT;
> +        mos6522_timer_update(s, &s->timers[0],
> +                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> +        break;
> +    case VIA_REG_T2CL:
> +        s->timers[1].latch = (s->timers[1].latch & 0xff00) | val;
> +        break;
> +    case VIA_REG_T2CH:
> +        /* To ensure T2 generates an interrupt on zero crossing with the
> +           common timer code, write the value directly from the latch to
> +           the counter */
> +        s->timers[1].latch = (s->timers[1].latch & 0xff) | (val << 8);
> +        s->ifr &= ~T2_INT;
> +        set_counter(s, &s->timers[1], s->timers[1].latch);
> +        break;
> +    case VIA_REG_SR:
> +        s->sr = val;
> +        break;
> +    case VIA_REG_ACR:
> +        s->acr = val;
> +        mos6522_timer_update(s, &s->timers[0],
> +                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> +        break;
> +    case VIA_REG_PCR:
> +        s->pcr = val;
> +        break;
> +    case VIA_REG_IFR:
> +        /* reset bits */
> +        s->ifr &= ~val;
> +        mos6522_update_irq(s);
> +        break;
> +    case VIA_REG_IER:
> +        if (val & IER_SET) {
> +            /* set bits */
> +            s->ier |= val & 0x7f;
> +        } else {
> +            /* reset bits */
> +            s->ier &= ~val;
> +        }
> +        mos6522_update_irq(s);
> +        break;
> +    default:
> +    case VIA_REG_ANH:
> +        s->anh = val;
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps mos6522_ops = {
> +    .read = mos6522_read,
> +    .write = mos6522_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
> +    },
> +};
> +
> +static bool mos6522_timer_exist(void *opaque, int version_id)
> +{
> +    MOS6522Timer *s = opaque;
> +
> +    return s->timer != NULL;
> +}
> +
> +static const VMStateDescription vmstate_mos6522_timer = {
> +    .name = "mos6522_timer",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT16(latch, MOS6522Timer),
> +        VMSTATE_UINT16(counter_value, MOS6522Timer),
> +        VMSTATE_INT64(load_time, MOS6522Timer),
> +        VMSTATE_INT64(next_irq_time, MOS6522Timer),
> +        VMSTATE_TIMER_PTR_TEST(timer, MOS6522Timer, mos6522_timer_exist),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_mos6522 = {
> +    .name = "mos6522",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(a, MOS6522State),
> +        VMSTATE_UINT8(b, MOS6522State),
> +        VMSTATE_UINT8(dira, MOS6522State),
> +        VMSTATE_UINT8(dirb, MOS6522State),
> +        VMSTATE_UINT8(sr, MOS6522State),
> +        VMSTATE_UINT8(acr, MOS6522State),
> +        VMSTATE_UINT8(pcr, MOS6522State),
> +        VMSTATE_UINT8(ifr, MOS6522State),
> +        VMSTATE_UINT8(ier, MOS6522State),
> +        VMSTATE_UINT8(anh, MOS6522State),
> +        VMSTATE_STRUCT_ARRAY(timers, MOS6522State, 2, 1,
> +                             vmstate_mos6522_timer, MOS6522Timer),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void mos6522_reset(DeviceState *dev)
> +{
> +    MOS6522State *s = MOS6522(dev);
> +
> +    s->b = 0;
> +    s->a = 0;
> +    s->dirb = 0xff;
> +    s->dira = 0;
> +    s->sr = 0;
> +    s->acr = 0;
> +    s->pcr = 0;
> +    s->ifr = 0;
> +    s->ier = 0;
> +    /* s->ier = T1_INT | SR_INT; */
> +    s->anh = 0;
> +
> +    s->timers[0].latch = 0xffff;
> +    set_counter(s, &s->timers[0], 0xffff);
> +
> +    s->timers[1].latch = 0xffff;
> +}
> +
> +static void mos6522_realize(DeviceState *dev, Error **errp)
> +{
> +    MOS6522State *s = MOS6522(dev);
> +
> +    s->timers[0].frequency = s->frequency;
> +    s->timers[1].frequency = s->frequency;
> +}
> +
> +static void mos6522_init(Object *obj)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    MOS6522State *s = MOS6522(obj);
> +    int i;
> +
> +    memory_region_init_io(&s->mem, obj, &mos6522_ops, s, "mos6522", 0x10);
> +    sysbus_init_mmio(sbd, &s->mem);
> +    sysbus_init_irq(sbd, &s->irq);
> +
> +    for (i = 0; i < ARRAY_SIZE(s->timers); i++) {
> +        s->timers[i].index = i;
> +    }
> +
> +    s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer1, s);
> +    s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer2, s);
> +}
> +
> +static Property mos6522_properties[] = {
> +    DEFINE_PROP_UINT64("frequency", MOS6522State, frequency, 0),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void mos6522_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    MOS6522DeviceClass *mdc = MOS6522_DEVICE_CLASS(oc);
> +
> +    dc->realize = mos6522_realize;
> +    dc->reset = mos6522_reset;
> +    dc->vmsd = &vmstate_mos6522;
> +    dc->props = mos6522_properties;
> +    mdc->parent_realize = dc->realize;
> +    mdc->set_sr_int = mos6522_set_sr_int;
> +    mdc->portB_write = mos6522_portB_write;
> +    mdc->portA_write = mos6522_portA_write;
> +    mdc->get_timer1_counter_value = mos6522_get_counter_value;
> +    mdc->get_timer2_counter_value = mos6522_get_counter_value;
> +    mdc->get_timer1_load_time = mos6522_get_load_time;
> +    mdc->get_timer2_load_time = mos6522_get_load_time;
> +}
> +
> +static const TypeInfo mos6522_type_info = {
> +    .name = TYPE_MOS6522,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(MOS6522State),
> +    .instance_init = mos6522_init,
> +    .abstract = true,
> +    .class_size = sizeof(MOS6522DeviceClass),
> +    .class_init = mos6522_class_init,
> +};
> +
> +static void mos6522_register_types(void)
> +{
> +    type_register_static(&mos6522_type_info);
> +}
> +
> +type_init(mos6522_register_types)
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index e6070f280d..b340d4e81c 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -70,3 +70,10 @@ msf2_sysreg_write_pll_status(void) "Invalid write to read only PLL status regist
>  #hw/misc/imx7_gpr.c
>  imx7_gpr_read(uint64_t offset) "addr 0x%08" HWADDR_PRIx
>  imx7_gpr_write(uint64_t offset, uint64_t value) "addr 0x%08" HWADDR_PRIx "value 0x%08" HWADDR_PRIx
> +
> +# hw/misc/mos6522.c
> +mos6522_set_counter(int index, unsigned int val) "T%d.counter=%d"
> +mos6522_get_next_irq_time(uint16_t latch, int64_t d, int64_t delta) "latch=%d counter=0x%"PRId64 " delta_next=0x%"PRId64
> +mos6522_set_sr_int(void) "set sr_int"
> +mos6522_write(uint64_t addr, uint64_t val) "reg=0x%"PRIx64 " val=0x%"PRIx64
> +mos6522_read(uint64_t addr, unsigned val) "reg=0x%"PRIx64 " val=0x%x"
> diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
> new file mode 100644
> index 0000000000..a53c161b00
> --- /dev/null
> +++ b/include/hw/misc/mos6522.h
> @@ -0,0 +1,152 @@
> +/*
> + * QEMU MOS6522 VIA emulation
> + *
> + * Copyright (c) 2004-2007 Fabrice Bellard
> + * Copyright (c) 2007 Jocelyn Mayer
> + * Copyright (c) 2018 Mark Cave-Ayland
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef MOS6522_H
> +#define MOS6522_H
> +
> +#include "exec/memory.h"
> +#include "hw/sysbus.h"
> +#include "hw/ide/internal.h"
> +#include "hw/input/adb.h"
> +
> +/* Bits in ACR */
> +#define SR_CTRL            0x1c    /* Shift register control bits */
> +#define SR_EXT             0x0c    /* Shift on external clock */
> +#define SR_OUT             0x10    /* Shift out if 1 */
> +
> +/* Bits in IFR and IER */
> +#define IER_SET            0x80    /* set bits in IER */
> +#define IER_CLR            0       /* clear bits in IER */
> +
> +#define CA2_INT            0x01
> +#define CA1_INT            0x02
> +#define SR_INT             0x04    /* Shift register full/empty */
> +#define CB2_INT            0x08
> +#define CB1_INT            0x10
> +#define T2_INT             0x20    /* Timer 2 interrupt */
> +#define T1_INT             0x40    /* Timer 1 interrupt */
> +
> +/* Bits in ACR */
> +#define T1MODE             0xc0    /* Timer 1 mode */
> +#define T1MODE_CONT        0x40    /*  continuous interrupts */
> +
> +/* VIA registers */
> +#define VIA_REG_B       0x00
> +#define VIA_REG_A       0x01
> +#define VIA_REG_DIRB    0x02
> +#define VIA_REG_DIRA    0x03
> +#define VIA_REG_T1CL    0x04
> +#define VIA_REG_T1CH    0x05
> +#define VIA_REG_T1LL    0x06
> +#define VIA_REG_T1LH    0x07
> +#define VIA_REG_T2CL    0x08
> +#define VIA_REG_T2CH    0x09
> +#define VIA_REG_SR      0x0a
> +#define VIA_REG_ACR     0x0b
> +#define VIA_REG_PCR     0x0c
> +#define VIA_REG_IFR     0x0d
> +#define VIA_REG_IER     0x0e
> +#define VIA_REG_ANH     0x0f
> +
> +/**
> + * MOS6522Timer:
> + * @counter_value: counter value at load time
> + */
> +typedef struct MOS6522Timer {
> +    int index;
> +    uint16_t latch;
> +    uint16_t counter_value;
> +    int64_t load_time;
> +    int64_t next_irq_time;
> +    uint64_t frequency;
> +    QEMUTimer *timer;
> +} MOS6522Timer;
> +
> +/**
> + * MOS6522State:
> + * @b: B-side data
> + * @a: A-side data
> + * @dirb: B-side direction (1=output)
> + * @dira: A-side direction (1=output)
> + * @sr: Shift register
> + * @acr: Auxiliary control register
> + * @pcr: Peripheral control register
> + * @ifr: Interrupt flag register
> + * @ier: Interrupt enable register
> + * @anh: A-side data, no handshake
> + * @last_b: last value of B register
> + * @last_acr: last value of ACR register
> + */
> +typedef struct MOS6522State {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    MemoryRegion mem;
> +    /* VIA registers */
> +    uint8_t b;
> +    uint8_t a;
> +    uint8_t dirb;
> +    uint8_t dira;
> +    uint8_t sr;
> +    uint8_t acr;
> +    uint8_t pcr;
> +    uint8_t ifr;
> +    uint8_t ier;
> +    uint8_t anh;
> +
> +    MOS6522Timer timers[2];
> +    uint64_t frequency;
> +
> +    qemu_irq irq;
> +} MOS6522State;
> +
> +#define TYPE_MOS6522 "mos6522"
> +#define MOS6522(obj) OBJECT_CHECK(MOS6522State, (obj), TYPE_MOS6522)
> +
> +typedef struct MOS6522DeviceClass {
> +    DeviceClass parent_class;
> +
> +    DeviceRealize parent_realize;
> +    void (*set_sr_int)(MOS6522State *dev);
> +    void (*portB_write)(MOS6522State *dev);
> +    void (*portA_write)(MOS6522State *dev);
> +    /* These are used to influence the CUDA MacOS timebase calibration */
> +    uint64_t (*get_timer1_counter_value)(MOS6522State *dev, MOS6522Timer *ti);
> +    uint64_t (*get_timer2_counter_value)(MOS6522State *dev, MOS6522Timer *ti);
> +    uint64_t (*get_timer1_load_time)(MOS6522State *dev, MOS6522Timer *ti);
> +    uint64_t (*get_timer2_load_time)(MOS6522State *dev, MOS6522Timer *ti);
> +} MOS6522DeviceClass;
> +
> +#define MOS6522_DEVICE_CLASS(cls) \
> +    OBJECT_CLASS_CHECK(MOS6522DeviceClass, (cls), TYPE_MOS6522)
> +#define MOS6522_DEVICE_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(MOS6522DeviceClass, (obj), TYPE_MOS6522)
> +
> +uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size);
> +void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size);
> +
> +#endif /* MOS6522_H */

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 05/12] cuda: rename frequency property to tb_frequency
  2018-02-10 23:11     ` David Gibson
@ 2018-02-11 10:59       ` Mark Cave-Ayland
  2018-02-11 11:22         ` David Gibson
  0 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2018-02-11 10:59 UTC (permalink / raw)
  To: David Gibson; +Cc: lvivier, qemu-ppc, qemu-devel

On 10/02/18 23:11, David Gibson wrote:

> On Sun, Feb 11, 2018 at 09:32:14AM +1100, David Gibson wrote:
>> On Fri, Feb 09, 2018 at 06:51:35PM +0000, Mark Cave-Ayland wrote:
>>> This allows us to more easily differentiate between the timebase frequency used
>>> to calibrate the MacOS timers and the actual frequency of the hardware clock as
>>> indicated by CUDA_TIMER_FREQ.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> Applied, thanks.
> 
> Actually, this patch doesn't compile, because you've changed
> ti->frequency in a couple of places where you should only be changing
> s->frequency.  I've fixed it up in my tree.

Ah apologies there - this was a mistake I made quite early on in the 
patchset which I must have accidentally brought back in during a 
subsequent rebase :(

I've just done a check on patch 7 ("cuda: set timer 1 frequency property 
to CUDA_TIMER_FREQ") and confirmed that the cuda.c version in your 
ppc-for-2.12 branch matches that in my local branch, so your fixups are 
good.

Any thoughts on the last few patches? I know Philippe had a question for 
Stefan re: the trace-events patch, however it would be useful for 
patches 10/11 to be applied as they complete the transition from CUDA 
over to using the mos6522 device (also providing an initial example as 
to how it can be used).


ATB,

Mark.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 05/12] cuda: rename frequency property to tb_frequency
  2018-02-11 10:59       ` Mark Cave-Ayland
@ 2018-02-11 11:22         ` David Gibson
  0 siblings, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-11 11:22 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: lvivier, qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1819 bytes --]

On Sun, Feb 11, 2018 at 10:59:05AM +0000, Mark Cave-Ayland wrote:
> On 10/02/18 23:11, David Gibson wrote:
> 
> > On Sun, Feb 11, 2018 at 09:32:14AM +1100, David Gibson wrote:
> > > On Fri, Feb 09, 2018 at 06:51:35PM +0000, Mark Cave-Ayland wrote:
> > > > This allows us to more easily differentiate between the timebase frequency used
> > > > to calibrate the MacOS timers and the actual frequency of the hardware clock as
> > > > indicated by CUDA_TIMER_FREQ.
> > > > 
> > > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > > 
> > > Applied, thanks.
> > 
> > Actually, this patch doesn't compile, because you've changed
> > ti->frequency in a couple of places where you should only be changing
> > s->frequency.  I've fixed it up in my tree.
> 
> Ah apologies there - this was a mistake I made quite early on in the
> patchset which I must have accidentally brought back in during a subsequent
> rebase :(
> 
> I've just done a check on patch 7 ("cuda: set timer 1 frequency property to
> CUDA_TIMER_FREQ") and confirmed that the cuda.c version in your ppc-for-2.12
> branch matches that in my local branch, so your fixups are good.

Ok, great.

> Any thoughts on the last few patches? I know Philippe had a question for
> Stefan re: the trace-events patch, however it would be useful for patches
> 10/11 to be applied as they complete the transition from CUDA over to using
> the mos6522 device (also providing an initial example as to how it can be
> used).

Haven't had a chance to look at them yet.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 10/12] cuda: convert to use the shared mos6522 device
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 10/12] cuda: convert to use the shared mos6522 device Mark Cave-Ayland
@ 2018-02-12  7:49   ` David Gibson
  0 siblings, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-12  7:49 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 32442 bytes --]

On Fri, Feb 09, 2018 at 06:51:40PM +0000, Mark Cave-Ayland wrote:
> Add the relevant hooks as required for the MacOS timer calibration and delayed
> SR interrupt.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Applied, thanks.

> ---
>  hw/misc/macio/cuda.c | 606 ++++++++++++++-------------------------------------
>  hw/ppc/mac.h         |  87 ++++----
>  2 files changed, 204 insertions(+), 489 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index a185252144..54c02aeffb 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -26,15 +26,14 @@
>  #include "hw/hw.h"
>  #include "hw/ppc/mac.h"
>  #include "hw/input/adb.h"
> +#include "hw/misc/mos6522.h"
>  #include "qemu/timer.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/cutils.h"
>  #include "qemu/log.h"
>  
> -/* XXX: implement all timer modes */
> -
> -/* debug CUDA */
> -//#define DEBUG_CUDA
> +/* debug CUDA packets */
> +//#define DEBUG_CUDA_PACKET
>  
>  /* debug CUDA packets */
>  //#define DEBUG_CUDA_PACKET
> @@ -47,426 +46,114 @@
>  #endif
>  
>  /* Bits in B data register: all active low */
> -#define TREQ		0x08		/* Transfer request (input) */
> -#define TACK		0x10		/* Transfer acknowledge (output) */
> -#define TIP		0x20		/* Transfer in progress (output) */
> -
> -/* Bits in ACR */
> -#define SR_CTRL		0x1c		/* Shift register control bits */
> -#define SR_EXT		0x0c		/* Shift on external clock */
> -#define SR_OUT		0x10		/* Shift out if 1 */
> -
> -/* Bits in IFR and IER */
> -#define IER_SET		0x80		/* set bits in IER */
> -#define IER_CLR		0		/* clear bits in IER */
> -#define SR_INT		0x04		/* Shift register full/empty */
> -#define SR_DATA_INT	0x08
> -#define SR_CLOCK_INT	0x10
> -#define T1_INT          0x40            /* Timer 1 interrupt */
> -#define T2_INT          0x20            /* Timer 2 interrupt */
> -
> -/* Bits in ACR */
> -#define T1MODE          0xc0            /* Timer 1 mode */
> -#define T1MODE_CONT     0x40            /*  continuous interrupts */
> +#define TREQ            0x08    /* Transfer request (input) */
> +#define TACK            0x10    /* Transfer acknowledge (output) */
> +#define TIP             0x20    /* Transfer in progress (output) */
>  
>  /* commands (1st byte) */
> -#define ADB_PACKET	0
> -#define CUDA_PACKET	1
> -#define ERROR_PACKET	2
> -#define TIMER_PACKET	3
> -#define POWER_PACKET	4
> -#define MACIIC_PACKET	5
> -#define PMU_PACKET	6
> -
> -
> -/* CUDA commands (2nd byte) */
> -#define CUDA_WARM_START			0x0
> -#define CUDA_AUTOPOLL			0x1
> -#define CUDA_GET_6805_ADDR		0x2
> -#define CUDA_GET_TIME			0x3
> -#define CUDA_GET_PRAM			0x7
> -#define CUDA_SET_6805_ADDR		0x8
> -#define CUDA_SET_TIME			0x9
> -#define CUDA_POWERDOWN			0xa
> -#define CUDA_POWERUP_TIME		0xb
> -#define CUDA_SET_PRAM			0xc
> -#define CUDA_MS_RESET			0xd
> -#define CUDA_SEND_DFAC			0xe
> -#define CUDA_BATTERY_SWAP_SENSE		0x10
> -#define CUDA_RESET_SYSTEM		0x11
> -#define CUDA_SET_IPL			0x12
> -#define CUDA_FILE_SERVER_FLAG		0x13
> -#define CUDA_SET_AUTO_RATE		0x14
> -#define CUDA_GET_AUTO_RATE		0x16
> -#define CUDA_SET_DEVICE_LIST		0x19
> -#define CUDA_GET_DEVICE_LIST		0x1a
> -#define CUDA_SET_ONE_SECOND_MODE	0x1b
> -#define CUDA_SET_POWER_MESSAGES		0x21
> -#define CUDA_GET_SET_IIC		0x22
> -#define CUDA_WAKEUP			0x23
> -#define CUDA_TIMER_TICKLE		0x24
> -#define CUDA_COMBINED_FORMAT_IIC	0x25
> +#define ADB_PACKET      0
> +#define CUDA_PACKET     1
> +#define ERROR_PACKET    2
> +#define TIMER_PACKET    3
> +#define POWER_PACKET    4
> +#define MACIIC_PACKET   5
> +#define PMU_PACKET      6
>  
>  #define CUDA_TIMER_FREQ (4700000 / 6)
>  
>  /* CUDA returns time_t's offset from Jan 1, 1904, not 1970 */
>  #define RTC_OFFSET                      2082844800
>  
> -/* CUDA registers */
> -#define CUDA_REG_B       0x00
> -#define CUDA_REG_A       0x01
> -#define CUDA_REG_DIRB    0x02
> -#define CUDA_REG_DIRA    0x03
> -#define CUDA_REG_T1CL    0x04
> -#define CUDA_REG_T1CH    0x05
> -#define CUDA_REG_T1LL    0x06
> -#define CUDA_REG_T1LH    0x07
> -#define CUDA_REG_T2CL    0x08
> -#define CUDA_REG_T2CH    0x09
> -#define CUDA_REG_SR      0x0a
> -#define CUDA_REG_ACR     0x0b
> -#define CUDA_REG_PCR     0x0c
> -#define CUDA_REG_IFR     0x0d
> -#define CUDA_REG_IER     0x0e
> -#define CUDA_REG_ANH     0x0f
> -
> -static void cuda_update(CUDAState *s);
>  static void cuda_receive_packet_from_host(CUDAState *s,
>                                            const uint8_t *data, int len);
> -static void cuda_timer_update(CUDAState *s, CUDATimer *ti,
> -                              int64_t current_time);
>  
> -static void cuda_update_irq(CUDAState *s)
> -{
> -    if (s->ifr & s->ier & (SR_INT | T1_INT | T2_INT)) {
> -        qemu_irq_raise(s->irq);
> -    } else {
> -        qemu_irq_lower(s->irq);
> -    }
> -}
> +/* MacOS uses timer 1 for calibration on startup, so we use
> + * the timebase frequency and cuda_get_counter_value() with
> + * cuda_get_load_time() to steer MacOS to calculate calibrate its timers
> + * correctly for both TCG and KVM (see commit b981289c49 "PPC: Cuda: Use cuda
> + * timer to expose tbfreq to guest" for more information) */
>  
> -static uint64_t get_counter_value(CUDAState *s, CUDATimer *ti)
> +static uint64_t cuda_get_counter_value(MOS6522State *s, MOS6522Timer *ti)
>  {
> +    MOS6522CUDAState *mcs = container_of(s, MOS6522CUDAState, parent_obj);
> +    CUDAState *cs = mcs->cuda;
> +
>      /* Reverse of the tb calculation algorithm that Mac OS X uses on bootup */
>      uint64_t tb_diff = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                                s->tb_frequency, NANOSECONDS_PER_SECOND) -
> +                                cs->tb_frequency, NANOSECONDS_PER_SECOND) -
>                             ti->load_time;
>  
> -    return (tb_diff * 0xBF401675E5DULL) / (s->tb_frequency << 24);
> +    return (tb_diff * 0xBF401675E5DULL) / (cs->tb_frequency << 24);
>  }
>  
> -static uint64_t get_counter_load_time(CUDAState *s, CUDATimer *ti)
> +static uint64_t cuda_get_load_time(MOS6522State *s, MOS6522Timer *ti)
>  {
> +    MOS6522CUDAState *mcs = container_of(s, MOS6522CUDAState, parent_obj);
> +    CUDAState *cs = mcs->cuda;
> +
>      uint64_t load_time = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                                  s->tb_frequency, NANOSECONDS_PER_SECOND);
> +                                  cs->tb_frequency, NANOSECONDS_PER_SECOND);
>      return load_time;
>  }
>  
> -static unsigned int get_counter(CUDAState *s, CUDATimer *ti)
> -{
> -    int64_t d;
> -    unsigned int counter;
> -
> -    d = get_counter_value(s, ti);
> -
> -    if (ti->index == 0) {
> -        /* the timer goes down from latch to -1 (period of latch + 2) */
> -        if (d <= (ti->counter_value + 1)) {
> -            counter = (ti->counter_value - d) & 0xffff;
> -        } else {
> -            counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
> -            counter = (ti->latch - counter) & 0xffff;
> -        }
> -    } else {
> -        counter = (ti->counter_value - d) & 0xffff;
> -    }
> -    return counter;
> -}
> -
> -static void set_counter(CUDAState *s, CUDATimer *ti, unsigned int val)
> -{
> -    CUDA_DPRINTF("T%d.counter=%d\n", 1 + ti->index, val);
> -    ti->load_time = get_counter_load_time(s, ti);
> -    ti->counter_value = val;
> -    cuda_timer_update(s, ti, ti->load_time);
> -}
> -
> -static int64_t get_next_irq_time(CUDATimer *ti, int64_t current_time)
> -{
> -    int64_t d, next_time;
> -    unsigned int counter;
> -
> -    /* current counter value */
> -    d = muldiv64(current_time - ti->load_time,
> -                 ti->frequency, NANOSECONDS_PER_SECOND);
> -    /* the timer goes down from latch to -1 (period of latch + 2) */
> -    if (d <= (ti->counter_value + 1)) {
> -        counter = (ti->counter_value - d) & 0xffff;
> -    } else {
> -        counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
> -        counter = (ti->latch - counter) & 0xffff;
> -    }
> -
> -    /* Note: we consider the irq is raised on 0 */
> -    if (counter == 0xffff) {
> -        next_time = d + ti->latch + 1;
> -    } else if (counter == 0) {
> -        next_time = d + ti->latch + 2;
> -    } else {
> -        next_time = d + counter;
> -    }
> -    CUDA_DPRINTF("latch=%d counter=%" PRId64 " delta_next=%" PRId64 "\n",
> -                 ti->latch, d, next_time - d);
> -    next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) +
> -                         ti->load_time;
> -    if (next_time <= current_time) {
> -        next_time = current_time + 1;
> -    }
> -    return next_time;
> -}
> -
> -static void cuda_timer_update(CUDAState *s, CUDATimer *ti,
> -                              int64_t current_time)
> -{
> -    if (!ti->timer)
> -        return;
> -    if (ti->index == 0 && (s->acr & T1MODE) != T1MODE_CONT) {
> -        timer_del(ti->timer);
> -    } else {
> -        ti->next_irq_time = get_next_irq_time(ti, current_time);
> -        timer_mod(ti->timer, ti->next_irq_time);
> -    }
> -}
> -
> -static void cuda_timer1(void *opaque)
> -{
> -    CUDAState *s = opaque;
> -    CUDATimer *ti = &s->timers[0];
> -
> -    cuda_timer_update(s, ti, ti->next_irq_time);
> -    s->ifr |= T1_INT;
> -    cuda_update_irq(s);
> -}
> -
> -static void cuda_timer2(void *opaque)
> -{
> -    CUDAState *s = opaque;
> -    CUDATimer *ti = &s->timers[1];
> -
> -    cuda_timer_update(s, ti, ti->next_irq_time);
> -    s->ifr |= T2_INT;
> -    cuda_update_irq(s);
> -}
> -
>  static void cuda_set_sr_int(void *opaque)
>  {
>      CUDAState *s = opaque;
> +    MOS6522CUDAState *mcs = s->mos6522_cuda;
> +    MOS6522State *ms = MOS6522(mcs);
> +    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(ms);
>  
> -    CUDA_DPRINTF("CUDA: %s:%d\n", __func__, __LINE__);
> -    s->ifr |= SR_INT;
> -    cuda_update_irq(s);
> +    mdc->set_sr_int(ms);
>  }
>  
>  static void cuda_delay_set_sr_int(CUDAState *s)
>  {
> +    MOS6522CUDAState *mcs = s->mos6522_cuda;
> +    MOS6522State *ms = MOS6522(mcs);
> +    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(ms);
>      int64_t expire;
>  
> -    if (s->dirb == 0xff) {
> -        /* Not in Mac OS, fire the IRQ directly */
> -        cuda_set_sr_int(s);
> +    if (ms->dirb == 0xff || s->sr_delay_ns == 0) {
> +        /* Disabled or not in Mac OS, fire the IRQ directly */
> +        mdc->set_sr_int(ms);
>          return;
>      }
>  
>      CUDA_DPRINTF("CUDA: %s:%d\n", __func__, __LINE__);
>  
> -    expire = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 300 * SCALE_US;
> +    expire = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->sr_delay_ns;
>      timer_mod(s->sr_delay_timer, expire);
>  }
>  
> -static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size)
> -{
> -    CUDAState *s = opaque;
> -    uint32_t val;
> -
> -    addr = (addr >> 9) & 0xf;
> -    switch(addr) {
> -    case CUDA_REG_B:
> -        val = s->b;
> -        break;
> -    case CUDA_REG_A:
> -        val = s->a;
> -        break;
> -    case CUDA_REG_DIRB:
> -        val = s->dirb;
> -        break;
> -    case CUDA_REG_DIRA:
> -        val = s->dira;
> -        break;
> -    case CUDA_REG_T1CL:
> -        val = get_counter(s, &s->timers[0]) & 0xff;
> -        s->ifr &= ~T1_INT;
> -        cuda_update_irq(s);
> -        break;
> -    case CUDA_REG_T1CH:
> -        val = get_counter(s, &s->timers[0]) >> 8;
> -        cuda_update_irq(s);
> -        break;
> -    case CUDA_REG_T1LL:
> -        val = s->timers[0].latch & 0xff;
> -        break;
> -    case CUDA_REG_T1LH:
> -        /* XXX: check this */
> -        val = (s->timers[0].latch >> 8) & 0xff;
> -        break;
> -    case CUDA_REG_T2CL:
> -        val = get_counter(s, &s->timers[1]) & 0xff;
> -        s->ifr &= ~T2_INT;
> -        cuda_update_irq(s);
> -        break;
> -    case CUDA_REG_T2CH:
> -        val = get_counter(s, &s->timers[1]) >> 8;
> -        break;
> -    case CUDA_REG_SR:
> -        val = s->sr;
> -        s->ifr &= ~(SR_INT | SR_CLOCK_INT | SR_DATA_INT);
> -        cuda_update_irq(s);
> -        break;
> -    case CUDA_REG_ACR:
> -        val = s->acr;
> -        break;
> -    case CUDA_REG_PCR:
> -        val = s->pcr;
> -        break;
> -    case CUDA_REG_IFR:
> -        val = s->ifr;
> -        if (s->ifr & s->ier) {
> -            val |= 0x80;
> -        }
> -        break;
> -    case CUDA_REG_IER:
> -        val = s->ier | 0x80;
> -        break;
> -    default:
> -    case CUDA_REG_ANH:
> -        val = s->anh;
> -        break;
> -    }
> -    if (addr != CUDA_REG_IFR || val != 0) {
> -        CUDA_DPRINTF("read: reg=0x%x val=%02x\n", (int)addr, val);
> -    }
> -
> -    return val;
> -}
> -
> -static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> -{
> -    CUDAState *s = opaque;
> -
> -    addr = (addr >> 9) & 0xf;
> -    CUDA_DPRINTF("write: reg=0x%x val=%02x\n", (int)addr, val);
> -
> -    switch(addr) {
> -    case CUDA_REG_B:
> -        s->b = (s->b & ~s->dirb) | (val & s->dirb);
> -        cuda_update(s);
> -        break;
> -    case CUDA_REG_A:
> -        s->a = (s->a & ~s->dira) | (val & s->dira);
> -        break;
> -    case CUDA_REG_DIRB:
> -        s->dirb = val;
> -        break;
> -    case CUDA_REG_DIRA:
> -        s->dira = val;
> -        break;
> -    case CUDA_REG_T1CL:
> -        s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
> -        cuda_timer_update(s, &s->timers[0], qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> -        break;
> -    case CUDA_REG_T1CH:
> -        s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
> -        s->ifr &= ~T1_INT;
> -        set_counter(s, &s->timers[0], s->timers[0].latch);
> -        break;
> -    case CUDA_REG_T1LL:
> -        s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
> -        cuda_timer_update(s, &s->timers[0], qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> -        break;
> -    case CUDA_REG_T1LH:
> -        s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
> -        s->ifr &= ~T1_INT;
> -        cuda_timer_update(s, &s->timers[0], qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> -        break;
> -    case CUDA_REG_T2CL:
> -        s->timers[1].latch = (s->timers[1].latch & 0xff00) | val;
> -        break;
> -    case CUDA_REG_T2CH:
> -        /* To ensure T2 generates an interrupt on zero crossing with the
> -           common timer code, write the value directly from the latch to
> -           the counter */
> -        s->timers[1].latch = (s->timers[1].latch & 0xff) | (val << 8);
> -        s->ifr &= ~T2_INT;
> -        set_counter(s, &s->timers[1], s->timers[1].latch);
> -        break;
> -    case CUDA_REG_SR:
> -        s->sr = val;
> -        break;
> -    case CUDA_REG_ACR:
> -        s->acr = val;
> -        cuda_timer_update(s, &s->timers[0], qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> -        break;
> -    case CUDA_REG_PCR:
> -        s->pcr = val;
> -        break;
> -    case CUDA_REG_IFR:
> -        /* reset bits */
> -        s->ifr &= ~val;
> -        cuda_update_irq(s);
> -        break;
> -    case CUDA_REG_IER:
> -        if (val & IER_SET) {
> -            /* set bits */
> -            s->ier |= val & 0x7f;
> -        } else {
> -            /* reset bits */
> -            s->ier &= ~val;
> -        }
> -        cuda_update_irq(s);
> -        break;
> -    default:
> -    case CUDA_REG_ANH:
> -        s->anh = val;
> -        break;
> -    }
> -}
> -
>  /* NOTE: TIP and TREQ are negated */
>  static void cuda_update(CUDAState *s)
>  {
> +    MOS6522CUDAState *mcs = s->mos6522_cuda;
> +    MOS6522State *ms = MOS6522(mcs);
>      int packet_received, len;
>  
>      packet_received = 0;
> -    if (!(s->b & TIP)) {
> +    if (!(ms->b & TIP)) {
>          /* transfer requested from host */
>  
> -        if (s->acr & SR_OUT) {
> +        if (ms->acr & SR_OUT) {
>              /* data output */
> -            if ((s->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
> +            if ((ms->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
>                  if (s->data_out_index < sizeof(s->data_out)) {
> -                    CUDA_DPRINTF("send: %02x\n", s->sr);
> -                    s->data_out[s->data_out_index++] = s->sr;
> +                    CUDA_DPRINTF("send: %02x\n", ms->sr);
> +                    s->data_out[s->data_out_index++] = ms->sr;
>                      cuda_delay_set_sr_int(s);
>                  }
>              }
>          } else {
>              if (s->data_in_index < s->data_in_size) {
>                  /* data input */
> -                if ((s->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
> -                    s->sr = s->data_in[s->data_in_index++];
> -                    CUDA_DPRINTF("recv: %02x\n", s->sr);
> +                if ((ms->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
> +                    ms->sr = s->data_in[s->data_in_index++];
> +                    CUDA_DPRINTF("recv: %02x\n", ms->sr);
>                      /* indicate end of transfer */
>                      if (s->data_in_index >= s->data_in_size) {
> -                        s->b = (s->b | TREQ);
> +                        ms->b = (ms->b | TREQ);
>                      }
>                      cuda_delay_set_sr_int(s);
>                  }
> @@ -474,12 +161,13 @@ static void cuda_update(CUDAState *s)
>          }
>      } else {
>          /* no transfer requested: handle sync case */
> -        if ((s->last_b & TIP) && (s->b & TACK) != (s->last_b & TACK)) {
> +        if ((s->last_b & TIP) && (ms->b & TACK) != (s->last_b & TACK)) {
>              /* update TREQ state each time TACK change state */
> -            if (s->b & TACK)
> -                s->b = (s->b | TREQ);
> -            else
> -                s->b = (s->b & ~TREQ);
> +            if (ms->b & TACK) {
> +                ms->b = (ms->b | TREQ);
> +            } else {
> +                ms->b = (ms->b & ~TREQ);
> +            }
>              cuda_delay_set_sr_int(s);
>          } else {
>              if (!(s->last_b & TIP)) {
> @@ -490,13 +178,13 @@ static void cuda_update(CUDAState *s)
>              }
>              /* signal if there is data to read */
>              if (s->data_in_index < s->data_in_size) {
> -                s->b = (s->b & ~TREQ);
> +                ms->b = (ms->b & ~TREQ);
>              }
>          }
>      }
>  
> -    s->last_acr = s->acr;
> -    s->last_b = s->b;
> +    s->last_acr = ms->acr;
> +    s->last_b = ms->b;
>  
>      /* NOTE: cuda_receive_packet_from_host() can call cuda_update()
>         recursively */
> @@ -538,9 +226,8 @@ static void cuda_adb_poll(void *opaque)
>          obuf[1] = 0x40; /* polled data */
>          cuda_send_packet_to_host(s, obuf, olen + 2);
>      }
> -    timer_mod(s->adb_poll_timer,
> -                   qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> -                   (NANOSECONDS_PER_SECOND / (1000 / s->autopoll_rate_ms)));
> +    timer_mod(s->adb_poll_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> +              (NANOSECONDS_PER_SECOND / (1000 / s->autopoll_rate_ms)));
>  }
>  
>  /* description of commands */
> @@ -787,35 +474,35 @@ static void cuda_receive_packet_from_host(CUDAState *s,
>      }
>  }
>  
> -static const MemoryRegionOps cuda_ops = {
> -    .read = cuda_read,
> -    .write = cuda_write,
> -    .endianness = DEVICE_BIG_ENDIAN,
> -    .valid = {
> -        .min_access_size = 1,
> -        .max_access_size = 1,
> -    },
> -};
> +static uint64_t mos6522_cuda_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    CUDAState *s = opaque;
> +    MOS6522CUDAState *mcs = s->mos6522_cuda;
> +    MOS6522State *ms = MOS6522(mcs);
>  
> -static bool cuda_timer_exist(void *opaque, int version_id)
> +    addr = (addr >> 9) & 0xf;
> +    return mos6522_read(ms, addr, size);
> +}
> +
> +static void mos6522_cuda_write(void *opaque, hwaddr addr, uint64_t val,
> +                               unsigned size)
>  {
> -    CUDATimer *s = opaque;
> +    CUDAState *s = opaque;
> +    MOS6522CUDAState *mcs = s->mos6522_cuda;
> +    MOS6522State *ms = MOS6522(mcs);
>  
> -    return s->timer != NULL;
> +    addr = (addr >> 9) & 0xf;
> +    mos6522_write(ms, addr, val, size);
>  }
>  
> -static const VMStateDescription vmstate_cuda_timer = {
> -    .name = "cuda_timer",
> -    .version_id = 0,
> -    .minimum_version_id = 0,
> -    .fields = (VMStateField[]) {
> -        VMSTATE_UINT16(latch, CUDATimer),
> -        VMSTATE_UINT16(counter_value, CUDATimer),
> -        VMSTATE_INT64(load_time, CUDATimer),
> -        VMSTATE_INT64(next_irq_time, CUDATimer),
> -        VMSTATE_TIMER_PTR_TEST(timer, CUDATimer, cuda_timer_exist),
> -        VMSTATE_END_OF_LIST()
> -    }
> +static const MemoryRegionOps mos6522_cuda_ops = {
> +    .read = mos6522_cuda_read,
> +    .write = mos6522_cuda_write,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
> +    },
>  };
>  
>  static const VMStateDescription vmstate_cuda = {
> @@ -823,18 +510,8 @@ static const VMStateDescription vmstate_cuda = {
>      .version_id = 4,
>      .minimum_version_id = 4,
>      .fields = (VMStateField[]) {
> -        VMSTATE_UINT8(a, CUDAState),
> -        VMSTATE_UINT8(b, CUDAState),
>          VMSTATE_UINT8(last_b, CUDAState),
> -        VMSTATE_UINT8(dira, CUDAState),
> -        VMSTATE_UINT8(dirb, CUDAState),
> -        VMSTATE_UINT8(sr, CUDAState),
> -        VMSTATE_UINT8(acr, CUDAState),
>          VMSTATE_UINT8(last_acr, CUDAState),
> -        VMSTATE_UINT8(pcr, CUDAState),
> -        VMSTATE_UINT8(ifr, CUDAState),
> -        VMSTATE_UINT8(ier, CUDAState),
> -        VMSTATE_UINT8(anh, CUDAState),
>          VMSTATE_INT32(data_in_size, CUDAState),
>          VMSTATE_INT32(data_in_index, CUDAState),
>          VMSTATE_INT32(data_out_index, CUDAState),
> @@ -844,8 +521,6 @@ static const VMStateDescription vmstate_cuda = {
>          VMSTATE_BUFFER(data_in, CUDAState),
>          VMSTATE_BUFFER(data_out, CUDAState),
>          VMSTATE_UINT32(tick_offset, CUDAState),
> -        VMSTATE_STRUCT_ARRAY(timers, CUDAState, 2, 1,
> -                             vmstate_cuda_timer, CUDATimer),
>          VMSTATE_TIMER_PTR(adb_poll_timer, CUDAState),
>          VMSTATE_TIMER_PTR(sr_delay_timer, CUDAState),
>          VMSTATE_END_OF_LIST()
> @@ -856,61 +531,48 @@ static void cuda_reset(DeviceState *dev)
>  {
>      CUDAState *s = CUDA(dev);
>  
> -    s->b = 0;
> -    s->a = 0;
> -    s->dirb = 0xff;
> -    s->dira = 0;
> -    s->sr = 0;
> -    s->acr = 0;
> -    s->pcr = 0;
> -    s->ifr = 0;
> -    s->ier = 0;
> -    //    s->ier = T1_INT | SR_INT;
> -    s->anh = 0;
>      s->data_in_size = 0;
>      s->data_in_index = 0;
>      s->data_out_index = 0;
>      s->autopoll = 0;
> -
> -    s->timers[0].latch = 0xffff;
> -    set_counter(s, &s->timers[0], 0xffff);
> -
> -    s->timers[1].latch = 0xffff;
> -
> -    s->sr_delay_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_set_sr_int, s);
>  }
>  
> -static void cuda_realizefn(DeviceState *dev, Error **errp)
> +static void cuda_realize(DeviceState *dev, Error **errp)
>  {
>      CUDAState *s = CUDA(dev);
> +    SysBusDevice *sbd;
> +    MOS6522State *ms;
> +    DeviceState *d;
>      struct tm tm;
>  
> -    s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer1, s);
> -    s->timers[0].frequency = CUDA_TIMER_FREQ;
> -    s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer2, s);
> -    s->timers[1].frequency = (SCALE_US * 6000) / 4700;
> +    d = qdev_create(NULL, TYPE_MOS6522_CUDA);
> +    object_property_set_link(OBJECT(d), OBJECT(s), "cuda", errp);
> +    qdev_init_nofail(d);
> +    s->mos6522_cuda = MOS6522_CUDA(d);
> +
> +    /* Pass IRQ from 6522 */
> +    ms = MOS6522(d);
> +    sbd = SYS_BUS_DEVICE(s);
> +    sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms));
>  
>      qemu_get_timedate(&tm, 0);
>      s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
>  
> +    s->sr_delay_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_set_sr_int, s);
> +    s->sr_delay_ns = 300 * SCALE_US;
> +
>      s->adb_poll_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_adb_poll, s);
> -    s->autopoll_rate_ms = 20;
>      s->adb_poll_mask = 0xffff;
> +    s->autopoll_rate_ms = 20;
>  }
>  
> -static void cuda_initfn(Object *obj)
> +static void cuda_init(Object *obj)
>  {
> -    SysBusDevice *d = SYS_BUS_DEVICE(obj);
>      CUDAState *s = CUDA(obj);
> -    int i;
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>  
> -    memory_region_init_io(&s->mem, obj, &cuda_ops, s, "cuda", 0x2000);
> -    sysbus_init_mmio(d, &s->mem);
> -    sysbus_init_irq(d, &s->irq);
> -
> -    for (i = 0; i < ARRAY_SIZE(s->timers); i++) {
> -        s->timers[i].index = i;
> -    }
> +    memory_region_init_io(&s->mem, obj, &mos6522_cuda_ops, s, "cuda", 0x2000);
> +    sysbus_init_mmio(sbd, &s->mem);
>  
>      qbus_create_inplace(&s->adb_bus, sizeof(s->adb_bus), TYPE_ADB_BUS,
>                          DEVICE(obj), "adb.0");
> @@ -925,7 +587,7 @@ static void cuda_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
>  
> -    dc->realize = cuda_realizefn;
> +    dc->realize = cuda_realize;
>      dc->reset = cuda_reset;
>      dc->vmsd = &vmstate_cuda;
>      dc->props = cuda_properties;
> @@ -936,12 +598,62 @@ static const TypeInfo cuda_type_info = {
>      .name = TYPE_CUDA,
>      .parent = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(CUDAState),
> -    .instance_init = cuda_initfn,
> +    .instance_init = cuda_init,
>      .class_init = cuda_class_init,
>  };
>  
> +static void mos6522_cuda_portB_write(MOS6522State *s)
> +{
> +    MOS6522CUDAState *mcs = container_of(s, MOS6522CUDAState, parent_obj);
> +
> +    cuda_update(mcs->cuda);
> +}
> +
> +static void mos6522_cuda_realize(DeviceState *dev, Error **errp)
> +{
> +    MOS6522State *ms = MOS6522(dev);
> +    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(ms);
> +
> +    mdc->parent_realize(dev, errp);
> +
> +    ms->timers[0].frequency = CUDA_TIMER_FREQ;
> +    ms->timers[1].frequency = (SCALE_US * 6000) / 4700;
> +}
> +
> +static void mos6522_cuda_init(Object *obj)
> +{
> +    MOS6522CUDAState *s = MOS6522_CUDA(obj);
> +
> +    object_property_add_link(obj, "cuda", TYPE_CUDA,
> +                             (Object **) &s->cuda,
> +                             qdev_prop_allow_set_link_before_realize,
> +                             0, NULL);
> +}
> +
> +static void mos6522_cuda_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    MOS6522DeviceClass *mdc = MOS6522_DEVICE_CLASS(oc);
> +
> +    dc->realize = mos6522_cuda_realize;
> +    mdc->portB_write = mos6522_cuda_portB_write;
> +    mdc->get_timer1_counter_value = cuda_get_counter_value;
> +    mdc->get_timer2_counter_value = cuda_get_counter_value;
> +    mdc->get_timer1_load_time = cuda_get_load_time;
> +    mdc->get_timer2_load_time = cuda_get_load_time;
> +}
> +
> +static const TypeInfo mos6522_cuda_type_info = {
> +    .name = TYPE_MOS6522_CUDA,
> +    .parent = TYPE_MOS6522,
> +    .instance_size = sizeof(MOS6522CUDAState),
> +    .instance_init = mos6522_cuda_init,
> +    .class_init = mos6522_cuda_class_init,
> +};
> +
>  static void cuda_register_types(void)
>  {
> +    type_register_static(&mos6522_cuda_type_info);
>      type_register_static(&cuda_type_info);
>  }
>  
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index fa78115c95..3e9f13d9b4 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -30,6 +30,7 @@
>  #include "hw/sysbus.h"
>  #include "hw/ide/internal.h"
>  #include "hw/input/adb.h"
> +#include "hw/misc/mos6522.h"
>  
>  /* SMP is not enabled, for now */
>  #define MAX_CPUS 1
> @@ -44,59 +45,48 @@
>  
>  #define ESCC_CLOCK 3686400
>  
> +/* CUDA commands (2nd byte) */
> +#define CUDA_WARM_START                0x0
> +#define CUDA_AUTOPOLL                  0x1
> +#define CUDA_GET_6805_ADDR             0x2
> +#define CUDA_GET_TIME                  0x3
> +#define CUDA_GET_PRAM                  0x7
> +#define CUDA_SET_6805_ADDR             0x8
> +#define CUDA_SET_TIME                  0x9
> +#define CUDA_POWERDOWN                 0xa
> +#define CUDA_POWERUP_TIME              0xb
> +#define CUDA_SET_PRAM                  0xc
> +#define CUDA_MS_RESET                  0xd
> +#define CUDA_SEND_DFAC                 0xe
> +#define CUDA_BATTERY_SWAP_SENSE        0x10
> +#define CUDA_RESET_SYSTEM              0x11
> +#define CUDA_SET_IPL                   0x12
> +#define CUDA_FILE_SERVER_FLAG          0x13
> +#define CUDA_SET_AUTO_RATE             0x14
> +#define CUDA_GET_AUTO_RATE             0x16
> +#define CUDA_SET_DEVICE_LIST           0x19
> +#define CUDA_GET_DEVICE_LIST           0x1a
> +#define CUDA_SET_ONE_SECOND_MODE       0x1b
> +#define CUDA_SET_POWER_MESSAGES        0x21
> +#define CUDA_GET_SET_IIC               0x22
> +#define CUDA_WAKEUP                    0x23
> +#define CUDA_TIMER_TICKLE              0x24
> +#define CUDA_COMBINED_FORMAT_IIC       0x25
> +
>  /* Cuda */
>  #define TYPE_CUDA "cuda"
>  #define CUDA(obj) OBJECT_CHECK(CUDAState, (obj), TYPE_CUDA)
>  
> -/**
> - * CUDATimer:
> - * @counter_value: counter value at load time
> - */
> -typedef struct CUDATimer {
> -    int index;
> -    uint16_t latch;
> -    uint16_t counter_value;
> -    int64_t load_time;
> -    int64_t next_irq_time;
> -    uint64_t frequency;
> -    QEMUTimer *timer;
> -} CUDATimer;
> -
> -/**
> - * CUDAState:
> - * @b: B-side data
> - * @a: A-side data
> - * @dirb: B-side direction (1=output)
> - * @dira: A-side direction (1=output)
> - * @sr: Shift register
> - * @acr: Auxiliary control register
> - * @pcr: Peripheral control register
> - * @ifr: Interrupt flag register
> - * @ier: Interrupt enable register
> - * @anh: A-side data, no handshake
> - * @last_b: last value of B register
> - * @last_acr: last value of ACR register
> - */
> +typedef struct MOS6522CUDAState MOS6522CUDAState;
> +
>  typedef struct CUDAState {
>      /*< private >*/
>      SysBusDevice parent_obj;
>      /*< public >*/
> -
>      MemoryRegion mem;
> -    /* cuda registers */
> -    uint8_t b;
> -    uint8_t a;
> -    uint8_t dirb;
> -    uint8_t dira;
> -    uint8_t sr;
> -    uint8_t acr;
> -    uint8_t pcr;
> -    uint8_t ifr;
> -    uint8_t ier;
> -    uint8_t anh;
>  
>      ADBBusState adb_bus;
> -    CUDATimer timers[2];
> +    MOS6522CUDAState *mos6522_cuda;
>  
>      uint32_t tick_offset;
>      uint64_t tb_frequency;
> @@ -105,6 +95,7 @@ typedef struct CUDAState {
>      uint8_t last_acr;
>  
>      /* MacOS 9 is racy and requires a delay upon setting the SR_INT bit */
> +    uint64_t sr_delay_ns;
>      QEMUTimer *sr_delay_timer;
>  
>      int data_in_size;
> @@ -120,6 +111,18 @@ typedef struct CUDAState {
>      QEMUTimer *adb_poll_timer;
>  } CUDAState;
>  
> +/* MOS6522 CUDA */
> +typedef struct MOS6522CUDAState {
> +    /*< private >*/
> +    MOS6522State parent_obj;
> +
> +    CUDAState *cuda;
> +} MOS6522CUDAState;
> +
> +#define TYPE_MOS6522_CUDA "mos6522-cuda"
> +#define MOS6522_CUDA(obj) OBJECT_CHECK(MOS6522CUDAState, (obj), \
> +                                       TYPE_MOS6522_CUDA)
> +
>  /* MacIO */
>  #define TYPE_OLDWORLD_MACIO "macio-oldworld"
>  #define TYPE_NEWWORLD_MACIO "macio-newworld"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 11/12] ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 11/12] ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file Mark Cave-Ayland
  2018-02-09 19:58   ` Philippe Mathieu-Daudé
@ 2018-02-12  7:59   ` David Gibson
  1 sibling, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-12  7:59 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 8545 bytes --]

On Fri, Feb 09, 2018 at 06:51:41PM +0000, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Applied, thanks.

> ---
>  hw/misc/macio/cuda.c         |   1 +
>  hw/misc/macio/macio.c        |   1 +
>  hw/ppc/mac.h                 |  77 -------------------------------
>  include/hw/misc/macio/cuda.h | 107 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 109 insertions(+), 77 deletions(-)
>  create mode 100644 include/hw/misc/macio/cuda.h
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 54c02aeffb..377b91d266 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -27,6 +27,7 @@
>  #include "hw/ppc/mac.h"
>  #include "hw/input/adb.h"
>  #include "hw/misc/mos6522.h"
> +#include "hw/misc/macio/cuda.h"
>  #include "qemu/timer.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/cutils.h"
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index a639b09e00..024f8557ab 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -26,6 +26,7 @@
>  #include "qapi/error.h"
>  #include "hw/hw.h"
>  #include "hw/ppc/mac.h"
> +#include "hw/misc/macio/cuda.h"
>  #include "hw/pci/pci.h"
>  #include "hw/ppc/mac_dbdma.h"
>  #include "hw/char/escc.h"
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index 3e9f13d9b4..4702194f3f 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -45,83 +45,6 @@
>  
>  #define ESCC_CLOCK 3686400
>  
> -/* CUDA commands (2nd byte) */
> -#define CUDA_WARM_START                0x0
> -#define CUDA_AUTOPOLL                  0x1
> -#define CUDA_GET_6805_ADDR             0x2
> -#define CUDA_GET_TIME                  0x3
> -#define CUDA_GET_PRAM                  0x7
> -#define CUDA_SET_6805_ADDR             0x8
> -#define CUDA_SET_TIME                  0x9
> -#define CUDA_POWERDOWN                 0xa
> -#define CUDA_POWERUP_TIME              0xb
> -#define CUDA_SET_PRAM                  0xc
> -#define CUDA_MS_RESET                  0xd
> -#define CUDA_SEND_DFAC                 0xe
> -#define CUDA_BATTERY_SWAP_SENSE        0x10
> -#define CUDA_RESET_SYSTEM              0x11
> -#define CUDA_SET_IPL                   0x12
> -#define CUDA_FILE_SERVER_FLAG          0x13
> -#define CUDA_SET_AUTO_RATE             0x14
> -#define CUDA_GET_AUTO_RATE             0x16
> -#define CUDA_SET_DEVICE_LIST           0x19
> -#define CUDA_GET_DEVICE_LIST           0x1a
> -#define CUDA_SET_ONE_SECOND_MODE       0x1b
> -#define CUDA_SET_POWER_MESSAGES        0x21
> -#define CUDA_GET_SET_IIC               0x22
> -#define CUDA_WAKEUP                    0x23
> -#define CUDA_TIMER_TICKLE              0x24
> -#define CUDA_COMBINED_FORMAT_IIC       0x25
> -
> -/* Cuda */
> -#define TYPE_CUDA "cuda"
> -#define CUDA(obj) OBJECT_CHECK(CUDAState, (obj), TYPE_CUDA)
> -
> -typedef struct MOS6522CUDAState MOS6522CUDAState;
> -
> -typedef struct CUDAState {
> -    /*< private >*/
> -    SysBusDevice parent_obj;
> -    /*< public >*/
> -    MemoryRegion mem;
> -
> -    ADBBusState adb_bus;
> -    MOS6522CUDAState *mos6522_cuda;
> -
> -    uint32_t tick_offset;
> -    uint64_t tb_frequency;
> -
> -    uint8_t last_b;
> -    uint8_t last_acr;
> -
> -    /* MacOS 9 is racy and requires a delay upon setting the SR_INT bit */
> -    uint64_t sr_delay_ns;
> -    QEMUTimer *sr_delay_timer;
> -
> -    int data_in_size;
> -    int data_in_index;
> -    int data_out_index;
> -
> -    qemu_irq irq;
> -    uint16_t adb_poll_mask;
> -    uint8_t autopoll_rate_ms;
> -    uint8_t autopoll;
> -    uint8_t data_in[128];
> -    uint8_t data_out[16];
> -    QEMUTimer *adb_poll_timer;
> -} CUDAState;
> -
> -/* MOS6522 CUDA */
> -typedef struct MOS6522CUDAState {
> -    /*< private >*/
> -    MOS6522State parent_obj;
> -
> -    CUDAState *cuda;
> -} MOS6522CUDAState;
> -
> -#define TYPE_MOS6522_CUDA "mos6522-cuda"
> -#define MOS6522_CUDA(obj) OBJECT_CHECK(MOS6522CUDAState, (obj), \
> -                                       TYPE_MOS6522_CUDA)
>  
>  /* MacIO */
>  #define TYPE_OLDWORLD_MACIO "macio-oldworld"
> diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h
> new file mode 100644
> index 0000000000..6afbdd13ee
> --- /dev/null
> +++ b/include/hw/misc/macio/cuda.h
> @@ -0,0 +1,107 @@
> +/*
> + * QEMU PowerMac CUDA device support
> + *
> + * Copyright (c) 2004-2007 Fabrice Bellard
> + * Copyright (c) 2007 Jocelyn Mayer
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef CUDA_H
> +#define CUDA_H
> +
> +/* CUDA commands (2nd byte) */
> +#define CUDA_WARM_START                0x0
> +#define CUDA_AUTOPOLL                  0x1
> +#define CUDA_GET_6805_ADDR             0x2
> +#define CUDA_GET_TIME                  0x3
> +#define CUDA_GET_PRAM                  0x7
> +#define CUDA_SET_6805_ADDR             0x8
> +#define CUDA_SET_TIME                  0x9
> +#define CUDA_POWERDOWN                 0xa
> +#define CUDA_POWERUP_TIME              0xb
> +#define CUDA_SET_PRAM                  0xc
> +#define CUDA_MS_RESET                  0xd
> +#define CUDA_SEND_DFAC                 0xe
> +#define CUDA_BATTERY_SWAP_SENSE        0x10
> +#define CUDA_RESET_SYSTEM              0x11
> +#define CUDA_SET_IPL                   0x12
> +#define CUDA_FILE_SERVER_FLAG          0x13
> +#define CUDA_SET_AUTO_RATE             0x14
> +#define CUDA_GET_AUTO_RATE             0x16
> +#define CUDA_SET_DEVICE_LIST           0x19
> +#define CUDA_GET_DEVICE_LIST           0x1a
> +#define CUDA_SET_ONE_SECOND_MODE       0x1b
> +#define CUDA_SET_POWER_MESSAGES        0x21
> +#define CUDA_GET_SET_IIC               0x22
> +#define CUDA_WAKEUP                    0x23
> +#define CUDA_TIMER_TICKLE              0x24
> +#define CUDA_COMBINED_FORMAT_IIC       0x25
> +
> +/* Cuda */
> +#define TYPE_CUDA "cuda"
> +#define CUDA(obj) OBJECT_CHECK(CUDAState, (obj), TYPE_CUDA)
> +
> +typedef struct MOS6522CUDAState MOS6522CUDAState;
> +
> +typedef struct CUDAState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +    MemoryRegion mem;
> +
> +    ADBBusState adb_bus;
> +    MOS6522CUDAState *mos6522_cuda;
> +
> +    uint32_t tick_offset;
> +    uint64_t tb_frequency;
> +
> +    uint8_t last_b;
> +    uint8_t last_acr;
> +
> +    /* MacOS 9 is racy and requires a delay upon setting the SR_INT bit */
> +    uint64_t sr_delay_ns;
> +    QEMUTimer *sr_delay_timer;
> +
> +    int data_in_size;
> +    int data_in_index;
> +    int data_out_index;
> +
> +    qemu_irq irq;
> +    uint16_t adb_poll_mask;
> +    uint8_t autopoll_rate_ms;
> +    uint8_t autopoll;
> +    uint8_t data_in[128];
> +    uint8_t data_out[16];
> +    QEMUTimer *adb_poll_timer;
> +} CUDAState;
> +
> +/* MOS6522 CUDA */
> +typedef struct MOS6522CUDAState {
> +    /*< private >*/
> +    MOS6522State parent_obj;
> +
> +    CUDAState *cuda;
> +} MOS6522CUDAState;
> +
> +#define TYPE_MOS6522_CUDA "mos6522-cuda"
> +#define MOS6522_CUDA(obj) OBJECT_CHECK(MOS6522CUDAState, (obj), \
> +                                       TYPE_MOS6522_CUDA)
> +
> +#endif /* CUDA_H */

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Qemu-devel] [PATCHv2 12/12] cuda: convert to trace-events
  2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 12/12] cuda: convert to trace-events Mark Cave-Ayland
  2018-02-09 19:56   ` Philippe Mathieu-Daudé
@ 2018-02-12  9:25   ` David Gibson
  1 sibling, 0 replies; 34+ messages in thread
From: David Gibson @ 2018-02-12  9:25 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, lvivier

[-- Attachment #1: Type: text/plain, Size: 5787 bytes --]

On Fri, Feb 09, 2018 at 06:51:42PM +0000, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Applied, thanks.

> ---
>  Makefile.objs              |  1 +
>  hw/misc/macio/cuda.c       | 50 ++++++++++++++++------------------------------
>  hw/misc/macio/trace-events | 11 ++++++++++
>  3 files changed, 29 insertions(+), 33 deletions(-)
>  create mode 100644 hw/misc/macio/trace-events
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 2efba6d768..3f1a1b674d 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -133,6 +133,7 @@ trace-events-subdirs += hw/net
>  trace-events-subdirs += hw/virtio
>  trace-events-subdirs += hw/audio
>  trace-events-subdirs += hw/misc
> +trace-events-subdirs += hw/misc/macio
>  trace-events-subdirs += hw/usb
>  trace-events-subdirs += hw/scsi
>  trace-events-subdirs += hw/nvram
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 377b91d266..bd9b862034 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -32,19 +32,7 @@
>  #include "sysemu/sysemu.h"
>  #include "qemu/cutils.h"
>  #include "qemu/log.h"
> -
> -/* debug CUDA packets */
> -//#define DEBUG_CUDA_PACKET
> -
> -/* debug CUDA packets */
> -//#define DEBUG_CUDA_PACKET
> -
> -#ifdef DEBUG_CUDA
> -#define CUDA_DPRINTF(fmt, ...)                                  \
> -    do { printf("CUDA: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define CUDA_DPRINTF(fmt, ...)
> -#endif
> +#include "trace.h"
>  
>  /* Bits in B data register: all active low */
>  #define TREQ            0x08    /* Transfer request (input) */
> @@ -120,7 +108,7 @@ static void cuda_delay_set_sr_int(CUDAState *s)
>          return;
>      }
>  
> -    CUDA_DPRINTF("CUDA: %s:%d\n", __func__, __LINE__);
> +    trace_cuda_delay_set_sr_int();
>  
>      expire = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->sr_delay_ns;
>      timer_mod(s->sr_delay_timer, expire);
> @@ -141,7 +129,7 @@ static void cuda_update(CUDAState *s)
>              /* data output */
>              if ((ms->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
>                  if (s->data_out_index < sizeof(s->data_out)) {
> -                    CUDA_DPRINTF("send: %02x\n", ms->sr);
> +                    trace_cuda_data_send(ms->sr);
>                      s->data_out[s->data_out_index++] = ms->sr;
>                      cuda_delay_set_sr_int(s);
>                  }
> @@ -151,7 +139,7 @@ static void cuda_update(CUDAState *s)
>                  /* data input */
>                  if ((ms->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
>                      ms->sr = s->data_in[s->data_in_index++];
> -                    CUDA_DPRINTF("recv: %02x\n", ms->sr);
> +                    trace_cuda_data_recv(ms->sr);
>                      /* indicate end of transfer */
>                      if (s->data_in_index >= s->data_in_size) {
>                          ms->b = (ms->b | TREQ);
> @@ -199,15 +187,13 @@ static void cuda_update(CUDAState *s)
>  static void cuda_send_packet_to_host(CUDAState *s,
>                                       const uint8_t *data, int len)
>  {
> -#ifdef DEBUG_CUDA_PACKET
> -    {
> -        int i;
> -        printf("cuda_send_packet_to_host:\n");
> -        for(i = 0; i < len; i++)
> -            printf(" %02x", data[i]);
> -        printf("\n");
> +    int i;
> +
> +    trace_cuda_packet_send(len);
> +    for (i = 0; i < len; i++) {
> +        trace_cuda_packet_send_data(i, data[i]);
>      }
> -#endif
> +
>      memcpy(s->data_in, data, len);
>      s->data_in_size = len;
>      s->data_in_index = 0;
> @@ -411,7 +397,7 @@ static void cuda_receive_packet(CUDAState *s,
>      for (i = 0; i < ARRAY_SIZE(handlers); i++) {
>          const CudaCommand *desc = &handlers[i];
>          if (desc->command == data[0]) {
> -            CUDA_DPRINTF("handling command %s\n", desc->name);
> +            trace_cuda_receive_packet_cmd(desc->name);
>              out_len = 0;
>              if (desc->handler(s, data + 1, len - 1, obuf + 3, &out_len)) {
>                  cuda_send_packet_to_host(s, obuf, 3 + out_len);
> @@ -440,15 +426,13 @@ static void cuda_receive_packet(CUDAState *s,
>  static void cuda_receive_packet_from_host(CUDAState *s,
>                                            const uint8_t *data, int len)
>  {
> -#ifdef DEBUG_CUDA_PACKET
> -    {
> -        int i;
> -        printf("cuda_receive_packet_from_host:\n");
> -        for(i = 0; i < len; i++)
> -            printf(" %02x", data[i]);
> -        printf("\n");
> +    int i;
> +
> +    trace_cuda_packet_receive(len);
> +    for (i = 0; i < len; i++) {
> +        trace_cuda_packet_receive_data(i, data[i]);
>      }
> -#endif
> +
>      switch(data[0]) {
>      case ADB_PACKET:
>          {
> diff --git a/hw/misc/macio/trace-events b/hw/misc/macio/trace-events
> new file mode 100644
> index 0000000000..24c0a36824
> --- /dev/null
> +++ b/hw/misc/macio/trace-events
> @@ -0,0 +1,11 @@
> +# See docs/devel/tracing.txt for syntax documentation.
> +
> +# hw/misc/macio/cuda.c
> +cuda_delay_set_sr_int(void) ""
> +cuda_data_send(uint8_t data) "send: 0x%02x"
> +cuda_data_recv(uint8_t data) "recv: 0x%02x"
> +cuda_receive_packet_cmd(const char *cmd) "handling command %s"
> +cuda_packet_receive(int len) "length %d"
> +cuda_packet_receive_data(int i, const uint8_t data) "[%d] 0x%02x"
> +cuda_packet_send(int len) "length %d"
> +cuda_packet_send_data(int i, const uint8_t data) "[%d] 0x%02x"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2018-02-12  9:26 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-09 18:51 [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device Mark Cave-Ayland
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses Mark Cave-Ayland
2018-02-09 20:05   ` Philippe Mathieu-Daudé
2018-02-10  7:18     ` David Gibson
2018-02-10  7:22   ` David Gibson
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 02/12] cuda: don't allow writes to port output pins Mark Cave-Ayland
2018-02-10  7:23   ` David Gibson
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 03/12] cuda: don't call cuda_update() when writing to ACR register Mark Cave-Ayland
2018-02-10 22:35   ` David Gibson
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 04/12] cuda: introduce CUDAState parameter to get_counter() Mark Cave-Ayland
2018-02-10 22:31   ` David Gibson
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 05/12] cuda: rename frequency property to tb_frequency Mark Cave-Ayland
2018-02-10 22:32   ` David Gibson
2018-02-10 23:11     ` David Gibson
2018-02-11 10:59       ` Mark Cave-Ayland
2018-02-11 11:22         ` David Gibson
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 06/12] cuda: minor cosmetic tidy-ups to get_next_irq_time() Mark Cave-Ayland
2018-02-10 22:33   ` David Gibson
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 07/12] cuda: set timer 1 frequency property to CUDA_TIMER_FREQ Mark Cave-Ayland
2018-02-10 23:15   ` David Gibson
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 08/12] cuda: factor out timebase-derived counter value and load time Mark Cave-Ayland
2018-02-10 23:18   ` David Gibson
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 09/12] misc: introduce new mos6522 VIA device and enable it for ppc builds Mark Cave-Ayland
2018-02-10 23:20   ` David Gibson
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 10/12] cuda: convert to use the shared mos6522 device Mark Cave-Ayland
2018-02-12  7:49   ` David Gibson
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 11/12] ppc: move CUDAState and other CUDA-related definitions into separate cuda.h file Mark Cave-Ayland
2018-02-09 19:58   ` Philippe Mathieu-Daudé
2018-02-12  7:59   ` David Gibson
2018-02-09 18:51 ` [Qemu-devel] [PATCHv2 12/12] cuda: convert to trace-events Mark Cave-Ayland
2018-02-09 19:56   ` Philippe Mathieu-Daudé
2018-02-12  9:25   ` David Gibson
2018-02-09 19:12 ` [Qemu-devel] [PATCHv2 00/12] cuda: various fixes, tidy-ups, and move 6522 to separate device no-reply
2018-02-09 19:15   ` Mark Cave-Ayland

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.