[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] sparc32 protect read-only bits in DMA CSR regis
From: |
Blue Swirl |
Subject: |
[Qemu-devel] Re: [PATCH] sparc32 protect read-only bits in DMA CSR registers |
Date: |
Sat, 22 May 2010 07:26:05 +0000 |
On Fri, May 21, 2010 at 9:53 PM, Artyom Tarasenko
<address@hidden> wrote:
> On a real hardware changing read-only bits has no effect
> Use a mask common for SCSI and Ethernet registers. The crucial
> bit is DMA_INTR, because setting or clearing it may produce
> spurious interrupts.
>
> This patch allows booting Solaris 2.3
Great!
> Signed-off-by: Artyom Tarasenko <address@hidden>
> ---
> hw/sparc32_dma.c | 11 +++++++----
> 1 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/hw/sparc32_dma.c b/hw/sparc32_dma.c
> index 3ceb851..d54e165 100644
> --- a/hw/sparc32_dma.c
> +++ b/hw/sparc32_dma.c
> @@ -62,6 +62,9 @@
> #define DMA_DRAIN_FIFO 0x40
> #define DMA_RESET 0x80
>
> +/* XXX SCSI and ethernet should have different read-only bit masks */
> +#define DMA_CSR_RO_MASK 0xfe000007
I'm preparing (again) some generic DMA patches, it looks like I have
to make Lance and ESP DMA controllers separate. Your patch highlights
yet another problem with the current shared design. This part of the
patch is fine.
> +
> typedef struct DMAState DMAState;
>
> struct DMAState {
> @@ -187,7 +190,7 @@ static void dma_mem_writel(void *opaque,
> target_phys_addr_t addr, uint32_t val)
> switch (saddr) {
> case 0:
> if (val & DMA_INTREN) {
> - if (val & DMA_INTR) {
> + if (s->dmaregs[0] & DMA_INTR) {
Doesn't this change the way irqs are raised so that a pending irq is
only generated on the write access _after_ the access that enables the
irq. Currently we check for pending irqs immediately when the irq is
enabled.
> DPRINTF("Raise IRQ\n");
> qemu_irq_raise(s->irq);
> }
> @@ -204,16 +207,16 @@ static void dma_mem_writel(void *opaque,
> target_phys_addr_t addr, uint32_t val)
> val &= ~DMA_DRAIN_FIFO;
> } else if (val == 0)
> val = DMA_DRAIN_FIFO;
> - val &= 0x0fffffff;
> + val &= ~DMA_CSR_RO_MASK;
> val |= DMA_VER;
> + s->dmaregs[0] = (s->dmaregs[0] & DMA_CSR_RO_MASK) | val;
> break;
> case 1:
> s->dmaregs[0] |= DMA_LOADED;
> - break;
A comment about fall through should be added.
> default:
> + s->dmaregs[saddr] = val;
> break;
> }
> - s->dmaregs[saddr] = val;
> }
>
> static CPUReadMemoryFunc * const dma_mem_read[3] = {
> --
> 1.6.2.5
>
>