* [Qemu-devel] [PATCH] ide: Enable byte&word access to DMA address register
@ 2008-05-05 8:20 Jan Kiszka
2008-05-05 21:20 ` Aurelien Jarno
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kiszka @ 2008-05-05 8:20 UTC (permalink / raw)
To: qemu-devel
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 <jan.kiszka@siemens.com>
---
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;
+ 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;
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] ide: Enable byte&word access to DMA address register
2008-05-05 8:20 [Qemu-devel] [PATCH] ide: Enable byte&word access to DMA address register Jan Kiszka
@ 2008-05-05 21:20 ` Aurelien Jarno
2008-05-05 22:39 ` Jan Kiszka
0 siblings, 1 reply; 3+ messages in thread
From: Aurelien Jarno @ 2008-05-05 21:20 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel
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 <jan.kiszka@siemens.com>
> ---
> 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
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] ide: Enable byte&word access to DMA address register
2008-05-05 21:20 ` Aurelien Jarno
@ 2008-05-05 22:39 ` Jan Kiszka
0 siblings, 0 replies; 3+ messages in thread
From: Jan Kiszka @ 2008-05-05 22:39 UTC (permalink / raw)
To: qemu-devel
Aurelien Jarno wrote:
> 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 <jan.kiszka@siemens.com>
>> ---
>> 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;
Oh, oh, oh. Of course. I just wonder why my colleague was able to work
with it despite of this bug. Looks like we have been lucky. Find
corrected patch below.
>
>> + 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.
Yes, word access is supported, the default handler will simply call
twice into our byte handler.
Thanks!
Jan
---
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 & 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;
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-05-05 22:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-05 8:20 [Qemu-devel] [PATCH] ide: Enable byte&word access to DMA address register Jan Kiszka
2008-05-05 21:20 ` Aurelien Jarno
2008-05-05 22:39 ` Jan Kiszka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).