Re: [Qemu-devel] [PATCH] ide: Enable byte&word access to DMA address register

[Aurelien Jarno]

On Mon, May 05, 2008 at 10:20:39AM +0200, Jan Kiszka wrote:
> According to the specs, also byte- and word-wise access to the busmaster
> DMA address register is allowed. Patch below fixes the IDE emulation
> in this regard (avoiding to touch the existing common case of 32-bit
> access) and makes our guest happy.
> 
> Signed-off-by: Jan Kiszka <address@hidden>
> ---
>  hw/ide.c |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> Index: b/hw/ide.c
> ===================================================================
> --- a/hw/ide.c
> +++ b/hw/ide.c
> @@ -2838,6 +2838,29 @@ static void bmdma_writeb(void *opaque, u
>      }
>  }
>  
> +static uint32_t bmdma_addr_readb(void *opaque, uint32_t addr)
> +{
> +    BMDMAState *bm = opaque;
> +    uint32_t val;
> +    val = (bm->addr >> ((addr & 3) * 8)) & 0xff;
> +#ifdef DEBUG_IDE
> +    printf("%s: 0x%08x\n", __func__, val);
> +#endif
> +    return val;
> +}
> +
> +static void bmdma_addr_writeb(void *opaque, uint32_t addr, uint32_t val)
> +{
> +    BMDMAState *bm = opaque;
> +    int shift = (addr & 3) * 8;
> +#ifdef DEBUG_IDE
> +    printf("%s: 0x%08x\n", __func__, val);
> +#endif
> +    bm->addr &= ~(0xFF << shift);
> +    bm->addr |= (val & 0xfc) << shift;

Are you sure it is correct? If you want to make sure the 2 lowest bits
are 0, it should be instead:

       bm->addr |= ((val & 0xFF) << shift) & ~3;

> +    bm->cur_addr = bm->addr;
> +}
> +
>  static uint32_t bmdma_addr_readl(void *opaque, uint32_t addr)
>  {
>      BMDMAState *bm = opaque;
> @@ -2876,6 +2899,8 @@ static void bmdma_map(PCIDevice *pci_dev
>          register_ioport_write(addr + 1, 3, 1, bmdma_writeb, bm);
>          register_ioport_read(addr, 4, 1, bmdma_readb, bm);
>  
> +        register_ioport_write(addr + 4, 4, 1, bmdma_addr_writeb, bm);
> +        register_ioport_read(addr + 4, 4, 1, bmdma_addr_readb, bm);
>          register_ioport_write(addr + 4, 4, 4, bmdma_addr_writel, bm);
>          register_ioport_read(addr + 4, 4, 4, bmdma_addr_readl, bm);
>          addr += 8;
> 

Otherwise, looks ok. Are word accesses supported? If yes it may be nice
to implement bmdma_addr_writew and bmdma_addr_readw at the same time.


-- 
  .''`.  Aurelien Jarno             | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   address@hidden         | address@hidden
   `-    people.debian.org/~aurel32 | www.aurel32.net