Re: [Qemu-devel] [PATCH 13/13] cuda.c: add delay to setting of SR_INT bit

[David Gibson <david@gibson.dropbear.id.au>]

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

On Fri, Oct 23, 2015 at 02:56:38PM +0100, Mark Cave-Ayland wrote:
> MacOS 9 is racy when it comes to accessing the shift register. Fix this by
> introducing a small delay between data accesses and raising the SR_INT
> interrupt bit.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/misc/macio/cuda.c |   44 +++++++++++++++++++++++++++++++++-----------
>  hw/ppc/mac.h         |    3 +++
>  2 files changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index d864b24..5156c72 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -248,6 +248,31 @@ static void cuda_timer2(void *opaque)
>      cuda_update_irq(s);
>  }
>  
> +static void cuda_set_sr_int(void *opaque)
> +{
> +    CUDAState *s = opaque;
> +
> +    CUDA_DPRINTF("CUDA: %s:%d\n", __func__, __LINE__);
> +    s->ifr |= SR_INT;
> +    cuda_update_irq(s);
> +}
> +
> +static void cuda_delay_set_sr_int(CUDAState *s)
> +{
> +    int64_t expire;
> +
> +    if (s->dirb == 0xff) {
> +        /* Not in Mac OS, fire the IRQ directly */
> +        cuda_set_sr_int(s);
> +        return;
> +    }
> +
> +    CUDA_DPRINTF("CUDA: %s:%d\n", __func__, __LINE__);
> +
> +    expire = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 300 * SCALE_US;
> +    timer_mod(s->sr_delay_timer, expire);
> +}
> +
>  static uint32_t cuda_readb(void *opaque, hwaddr addr)
>  {
>      CUDAState *s = opaque;
> @@ -418,8 +443,7 @@ static void cuda_update(CUDAState *s)
>                  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;
> -                    s->ifr |= SR_INT;
> -                    cuda_update_irq(s);
> +                    cuda_delay_set_sr_int(s);
>                  }
>              }
>          } else {
> @@ -432,8 +456,7 @@ static void cuda_update(CUDAState *s)
>                      if (s->data_in_index >= s->data_in_size) {
>                          s->b = (s->b | TREQ);
>                      }
> -                    s->ifr |= SR_INT;
> -                    cuda_update_irq(s);
> +                    cuda_delay_set_sr_int(s);
>                  }
>              }
>          }
> @@ -445,15 +468,13 @@ static void cuda_update(CUDAState *s)
>                  s->b = (s->b | TREQ);
>              else
>                  s->b = (s->b & ~TREQ);
> -            s->ifr |= SR_INT;
> -            cuda_update_irq(s);
> +            cuda_delay_set_sr_int(s);
>          } else {
>              if (!(s->last_b & TIP)) {
>                  /* handle end of host to cuda transfer */
>                  packet_received = (s->data_out_index > 0);
>                  /* always an IRQ at the end of transfer */
> -                s->ifr |= SR_INT;
> -                cuda_update_irq(s);
> +                cuda_delay_set_sr_int(s);
>              }
>              /* signal if there is data to read */
>              if (s->data_in_index < s->data_in_size) {
> @@ -490,8 +511,7 @@ static void cuda_send_packet_to_host(CUDAState *s,
>      s->data_in_size = len;
>      s->data_in_index = 0;
>      cuda_update(s);
> -    s->ifr |= SR_INT;
> -    cuda_update_irq(s);
> +    cuda_delay_set_sr_int(s);
>  }
>  
>  static void cuda_adb_poll(void *opaque)
> @@ -714,7 +734,7 @@ static void cuda_reset(DeviceState *dev)
>  
>      s->b = 0;
>      s->a = 0;
> -    s->dirb = 0;
> +    s->dirb = 0xff;
>      s->dira = 0;
>      s->sr = 0;
>      s->acr = 0;
> @@ -732,6 +752,8 @@ static void cuda_reset(DeviceState *dev)
>      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)
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index 8bdba30..e375ed2 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -103,6 +103,9 @@ typedef struct CUDAState {
>      uint8_t last_b;
>      uint8_t last_acr;
>  
> +    /* MacOS 9 is racy and requires a delay upon setting the SR_INT bit */
> +    QEMUTimer *sr_delay_timer;
> +
>      int data_in_size;
>      int data_in_index;
>      int data_out_index;

-- 
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: 819 bytes --]