Re: [PATCH] [HVM] Prevent usb driver crashes in Windows

[Keir Fraser <keir@xensource.com>]

What precisely is the race? Qemu-dm is single threaded. Memcpy() probably
ends up doing int copies anyway if things are appropriately aligned
(although we might not want to rely on that).

 -- Keir

On 6/6/07 17:00, "Ben Guthro" <bguthro@virtualiron.com> wrote:

> qemu-word-tearing.patch:
> Use atomic updates to read/write usb controller data.
> This can be done because:
>     a) word copies on x86 are atomic
>     b) The USB spec requires word alignment
> 
> This will need to be enhanced once USB 1.2 is supported.
> 
> Signed-off-by: Steve Ofsthun <sofsthun@virtualiron.com>
> 
> diff -r 86fa3e4277f6 tools/ioemu/target-i386-dm/exec-dm.c
> --- a/tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:26:30 2007 -0400
> +++ b/tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:29:03 2007 -0400
> @@ -434,6 +434,28 @@ extern unsigned long *logdirty_bitmap;
>  extern unsigned long *logdirty_bitmap;
>  extern unsigned long logdirty_bitmap_size;
>  
> +/*
> + * Replace the standard byte memcpy with a int memcpy for appropriately sized
> + * memory copy operations.  Some users (USB-UHCI) can not tolerate the
> possible
> + * word tearing that can result from a guest concurrently writing a memory
> + * structure while the qemu device model is modifying the same location.
> + * Forcing a int sized read/write prevents the guest from seeing a partially
> + * written int sized atom.
> + */
> +void memcpy32(void *dst, void *src, size_t n)
> +{
> +    if ((n % sizeof(int)) != 0) {
> +        memcpy(dst, src, n);
> +        return;
> +    }
> +    n /= sizeof(int);
> +    while(n--) {
> +        *(int *)dst = *(int *)src;
> +        dst += sizeof(int);
> +        src += sizeof(int);
> +    }
> +}
> +
>  void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>                              int len, int is_write)
>  {
> @@ -470,7 +492,7 @@ void cpu_physical_memory_rw(target_phys_
>                  }
>              } else if ((ptr = phys_ram_addr(addr)) != NULL) {
>                  /* Writing to RAM */
> -                memcpy(ptr, buf, l);
> +                memcpy32(ptr, buf, l);
>                  if (logdirty_bitmap != NULL) {
>                      /* Record that we have dirtied this frame */
>                      unsigned long pfn = addr >> TARGET_PAGE_BITS;
> @@ -506,7 +528,7 @@ void cpu_physical_memory_rw(target_phys_
>                  }
>              } else if ((ptr = phys_ram_addr(addr)) != NULL) {
>                  /* Reading from RAM */
> -                memcpy(buf, ptr, l);
> +                memcpy32(buf, ptr, l);
>              } else {
>                  /* Neither RAM nor known MMIO space */
>                  memset(buf, 0xff, len);
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel