On Fri, May 21, 2010 at 9:53 PM, Artyom Tarasenko <atar4q...@googlemail.com> 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 <atar4q...@gmail.com> > --- > 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 > >