| Message ID | 1505851374-6408-1-git-send-email-mark.cave-ayland@ilande.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 19, 2017 at 09:02:54PM +0100, Mark Cave-Ayland wrote: > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> This didn't apply for me. Can you rebase on top of ppc-for-2.11, please. > --- > hw/ide/macio.c | 181 +++++++++++++++++++++++--------------------------------- > 1 file changed, 75 insertions(+), 106 deletions(-) > > diff --git a/hw/ide/macio.c b/hw/ide/macio.c > index db5db39..18ae952 100644 > --- a/hw/ide/macio.c > +++ b/hw/ide/macio.c > @@ -255,131 +255,100 @@ static void pmac_ide_flush(DBDMA_io *io) > } > > /* PowerMac IDE memory IO */ > -static void pmac_ide_writeb (void *opaque, > - hwaddr addr, uint32_t val) > +static uint64_t pmac_ide_read(void *opaque, hwaddr addr, unsigned size) > { > MACIOIDEState *d = opaque; > - > - addr = (addr & 0xFFF) >> 4; > - switch (addr) { > - case 1 ... 7: > - ide_ioport_write(&d->bus, addr, val); > - break; > - case 8: > - case 22: > - ide_cmd_write(&d->bus, 0, val); > + uint64_t retval = 0xffffffff; > + int reg = addr >> 4; > + > + switch (reg) { > + case 0x0: > + if (size == 2) { > + retval = ide_data_readw(&d->bus, 0); > + } else if (size == 4) { > + retval = ide_data_readl(&d->bus, 0); > + } > break; > - default: > + case 0x1 ... 0x7: > + if (size == 1) { > + retval = ide_ioport_read(&d->bus, reg); > + } > break; > - } > -} > - > -static uint32_t pmac_ide_readb (void *opaque,hwaddr addr) > -{ > - uint8_t retval; > - MACIOIDEState *d = opaque; > - > - addr = (addr & 0xFFF) >> 4; > - switch (addr) { > - case 1 ... 7: > - retval = ide_ioport_read(&d->bus, addr); > + case 0x8: > + case 0x16: > + if (size == 1) { > + retval = ide_status_read(&d->bus, 0); > + } > break; > - case 8: > - case 22: > - retval = ide_status_read(&d->bus, 0); > + case 0x20: > + if (size == 4) { > + retval = d->timing_reg; > + } > break; > - default: > - retval = 0xFF; > + case 0x30: > + /* This is an interrupt state register that only exists > + * in the KeyLargo and later variants. Bit 0x8000_0000 > + * latches the DMA interrupt and has to be written to > + * clear. Bit 0x4000_0000 is an image of the disk > + * interrupt. MacOS X relies on this and will hang if > + * we don't provide at least the disk interrupt > + */ > + if (size == 4) { > + retval = d->irq_reg; > + } > break; > } > - return retval; > -} > > -static void pmac_ide_writew (void *opaque, > - hwaddr addr, uint32_t val) > -{ > - MACIOIDEState *d = opaque; > - > - addr = (addr & 0xFFF) >> 4; > - val = bswap16(val); > - if (addr == 0) { > - ide_data_writew(&d->bus, 0, val); > - } > -} > - > -static uint32_t pmac_ide_readw (void *opaque,hwaddr addr) > -{ > - uint16_t retval; > - MACIOIDEState *d = opaque; > - > - addr = (addr & 0xFFF) >> 4; > - if (addr == 0) { > - retval = ide_data_readw(&d->bus, 0); > - } else { > - retval = 0xFFFF; > - } > - retval = bswap16(retval); > return retval; > } > > -static void pmac_ide_writel (void *opaque, > - hwaddr addr, uint32_t val) > -{ > - MACIOIDEState *d = opaque; > > - addr = (addr & 0xFFF) >> 4; > - val = bswap32(val); > - if (addr == 0) { > - ide_data_writel(&d->bus, 0, val); > - } else if (addr == 0x20) { > - d->timing_reg = val; > - } else if (addr == 0x30) { > - if (val & 0x80000000u) { > - d->irq_reg &= 0x7fffffff; > - } > - } > -} > - > -static uint32_t pmac_ide_readl (void *opaque,hwaddr addr) > +static void pmac_ide_write(void *opaque, hwaddr addr, uint64_t val, > + unsigned size) > { > - uint32_t retval; > MACIOIDEState *d = opaque; > - > - addr = (addr & 0xFFF) >> 4; > - if (addr == 0) { > - retval = ide_data_readl(&d->bus, 0); > - } else if (addr == 0x20) { > - retval = d->timing_reg; > - } else if (addr == 0x30) { > - /* This is an interrupt state register that only exists > - * in the KeyLargo and later variants. Bit 0x8000_0000 > - * latches the DMA interrupt and has to be written to > - * clear. Bit 0x4000_0000 is an image of the disk > - * interrupt. MacOS X relies on this and will hang if > - * we don't provide at least the disk interrupt > - */ > - retval = d->irq_reg; > - } else { > - retval = 0xFFFFFFFF; > + int reg = addr >> 4; > + > + switch (reg) { > + case 0x0: > + if (size == 2) { > + ide_data_writew(&d->bus, 0, val); > + } else if (size == 4) { > + ide_data_writel(&d->bus, 0, val); > + } > + break; > + case 0x1 ... 0x7: > + if (size == 1) { > + ide_ioport_write(&d->bus, reg, val); > + } > + break; > + case 0x8: > + case 0x16: > + if (size == 1) { > + ide_cmd_write(&d->bus, 0, val); > + } > + break; > + case 0x20: > + if (size == 4) { > + d->timing_reg = val; > + } > + break; > + case 0x30: > + if (size == 4) { > + if (val & 0x80000000u) { > + d->irq_reg &= 0x7fffffff; > + } > + } > + break; > } > - retval = bswap32(retval); > - return retval; > } > > static const MemoryRegionOps pmac_ide_ops = { > - .old_mmio = { > - .write = { > - pmac_ide_writeb, > - pmac_ide_writew, > - pmac_ide_writel, > - }, > - .read = { > - pmac_ide_readb, > - pmac_ide_readw, > - pmac_ide_readl, > - }, > - }, > - .endianness = DEVICE_NATIVE_ENDIAN, > + .read = pmac_ide_read, > + .write = pmac_ide_write, > + .valid.min_access_size = 1, > + .valid.max_access_size = 4, > + .endianness = DEVICE_LITTLE_ENDIAN, > }; > > static const VMStateDescription vmstate_pmac = {
On 20/09/17 05:29, David Gibson wrote: > On Tue, Sep 19, 2017 at 09:02:54PM +0100, Mark Cave-Ayland wrote: >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > > This didn't apply for me. Can you rebase on top of ppc-for-2.11, please. (goes and looks) Okay looks like the issue is that you're missing "[PATCH 3/8] ppc/ide/macio: Add missing registers" from your queue - I see you replied with an applied to ppc-2.11 email so I bet this got dropped on rebase somewhere. Can you try again with that patch added and then let me know if it still doesn't work? ATB, Mark.
On Wed, Sep 20, 2017 at 06:56:37AM +0100, Mark Cave-Ayland wrote: > On 20/09/17 05:29, David Gibson wrote: > > > On Tue, Sep 19, 2017 at 09:02:54PM +0100, Mark Cave-Ayland wrote: > >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > > > > This didn't apply for me. Can you rebase on top of ppc-for-2.11, please. > > (goes and looks) > > Okay looks like the issue is that you're missing "[PATCH 3/8] > ppc/ide/macio: Add missing registers" from your queue - I see you > replied with an applied to ppc-2.11 email so I bet this got dropped on > rebase somewhere. Oh, sorry. Not sure how that happened. > Can you try again with that patch added and then let me know if it still > doesn't work? Uh.. I've lost track of it, I'm afraid. Can you resend it and the ide_ops one as a series.
On 20/09/17 07:00, David Gibson wrote: > On Wed, Sep 20, 2017 at 06:56:37AM +0100, Mark Cave-Ayland wrote: >> On 20/09/17 05:29, David Gibson wrote: >> >>> On Tue, Sep 19, 2017 at 09:02:54PM +0100, Mark Cave-Ayland wrote: >>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >>> >>> This didn't apply for me. Can you rebase on top of ppc-for-2.11, please. >> >> (goes and looks) >> >> Okay looks like the issue is that you're missing "[PATCH 3/8] >> ppc/ide/macio: Add missing registers" from your queue - I see you >> replied with an applied to ppc-2.11 email so I bet this got dropped on >> rebase somewhere. > > Oh, sorry. Not sure how that happened. > >> Can you try again with that patch added and then let me know if it still >> doesn't work? > > Uh.. I've lost track of it, I'm afraid. Can you resend it and the > ide_ops one as a series. Done. BTW if possible can you keep them in a similar order, i.e. ensure that "ppc/ide/macio: Add missing registers" comes before "ppc: Fix OpenPIC model"? (See https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg04397.html for the full set). This is because in my testing Linux will see the change in OpenPIC model and try to access the new macio registers, and so if that patch isn't there first then you end up with a hang on boot. Keeping the order will help make things much easier for bisection. ATB, Mark.
On Wed, Sep 20, 2017 at 07:28:27AM +0100, Mark Cave-Ayland wrote: > On 20/09/17 07:00, David Gibson wrote: > > > On Wed, Sep 20, 2017 at 06:56:37AM +0100, Mark Cave-Ayland wrote: > >> On 20/09/17 05:29, David Gibson wrote: > >> > >>> On Tue, Sep 19, 2017 at 09:02:54PM +0100, Mark Cave-Ayland wrote: > >>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > >>> > >>> This didn't apply for me. Can you rebase on top of ppc-for-2.11, please. > >> > >> (goes and looks) > >> > >> Okay looks like the issue is that you're missing "[PATCH 3/8] > >> ppc/ide/macio: Add missing registers" from your queue - I see you > >> replied with an applied to ppc-2.11 email so I bet this got dropped on > >> rebase somewhere. > > > > Oh, sorry. Not sure how that happened. > > > >> Can you try again with that patch added and then let me know if it still > >> doesn't work? > > > > Uh.. I've lost track of it, I'm afraid. Can you resend it and the > > ide_ops one as a series. > > Done. BTW if possible can you keep them in a similar order, i.e. ensure > that "ppc/ide/macio: Add missing registers" comes before "ppc: Fix > OpenPIC model"? (See > https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg04397.html for > the full set). Done. > This is because in my testing Linux will see the change in OpenPIC model > and try to access the new macio registers, and so if that patch isn't > there first then you end up with a hang on boot. Keeping the order will > help make things much easier for bisection. > > > ATB, > > Mark. >
diff --git a/hw/ide/macio.c b/hw/ide/macio.c index db5db39..18ae952 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -255,131 +255,100 @@ static void pmac_ide_flush(DBDMA_io *io) } /* PowerMac IDE memory IO */ -static void pmac_ide_writeb (void *opaque, - hwaddr addr, uint32_t val) +static uint64_t pmac_ide_read(void *opaque, hwaddr addr, unsigned size) { MACIOIDEState *d = opaque; - - addr = (addr & 0xFFF) >> 4; - switch (addr) { - case 1 ... 7: - ide_ioport_write(&d->bus, addr, val); - break; - case 8: - case 22: - ide_cmd_write(&d->bus, 0, val); + uint64_t retval = 0xffffffff; + int reg = addr >> 4; + + switch (reg) { + case 0x0: + if (size == 2) { + retval = ide_data_readw(&d->bus, 0); + } else if (size == 4) { + retval = ide_data_readl(&d->bus, 0); + } break; - default: + case 0x1 ... 0x7: + if (size == 1) { + retval = ide_ioport_read(&d->bus, reg); + } break; - } -} - -static uint32_t pmac_ide_readb (void *opaque,hwaddr addr) -{ - uint8_t retval; - MACIOIDEState *d = opaque; - - addr = (addr & 0xFFF) >> 4; - switch (addr) { - case 1 ... 7: - retval = ide_ioport_read(&d->bus, addr); + case 0x8: + case 0x16: + if (size == 1) { + retval = ide_status_read(&d->bus, 0); + } break; - case 8: - case 22: - retval = ide_status_read(&d->bus, 0); + case 0x20: + if (size == 4) { + retval = d->timing_reg; + } break; - default: - retval = 0xFF; + case 0x30: + /* This is an interrupt state register that only exists + * in the KeyLargo and later variants. Bit 0x8000_0000 + * latches the DMA interrupt and has to be written to + * clear. Bit 0x4000_0000 is an image of the disk + * interrupt. MacOS X relies on this and will hang if + * we don't provide at least the disk interrupt + */ + if (size == 4) { + retval = d->irq_reg; + } break; } - return retval; -} -static void pmac_ide_writew (void *opaque, - hwaddr addr, uint32_t val) -{ - MACIOIDEState *d = opaque; - - addr = (addr & 0xFFF) >> 4; - val = bswap16(val); - if (addr == 0) { - ide_data_writew(&d->bus, 0, val); - } -} - -static uint32_t pmac_ide_readw (void *opaque,hwaddr addr) -{ - uint16_t retval; - MACIOIDEState *d = opaque; - - addr = (addr & 0xFFF) >> 4; - if (addr == 0) { - retval = ide_data_readw(&d->bus, 0); - } else { - retval = 0xFFFF; - } - retval = bswap16(retval); return retval; } -static void pmac_ide_writel (void *opaque, - hwaddr addr, uint32_t val) -{ - MACIOIDEState *d = opaque; - addr = (addr & 0xFFF) >> 4; - val = bswap32(val); - if (addr == 0) { - ide_data_writel(&d->bus, 0, val); - } else if (addr == 0x20) { - d->timing_reg = val; - } else if (addr == 0x30) { - if (val & 0x80000000u) { - d->irq_reg &= 0x7fffffff; - } - } -} - -static uint32_t pmac_ide_readl (void *opaque,hwaddr addr) +static void pmac_ide_write(void *opaque, hwaddr addr, uint64_t val, + unsigned size) { - uint32_t retval; MACIOIDEState *d = opaque; - - addr = (addr & 0xFFF) >> 4; - if (addr == 0) { - retval = ide_data_readl(&d->bus, 0); - } else if (addr == 0x20) { - retval = d->timing_reg; - } else if (addr == 0x30) { - /* This is an interrupt state register that only exists - * in the KeyLargo and later variants. Bit 0x8000_0000 - * latches the DMA interrupt and has to be written to - * clear. Bit 0x4000_0000 is an image of the disk - * interrupt. MacOS X relies on this and will hang if - * we don't provide at least the disk interrupt - */ - retval = d->irq_reg; - } else { - retval = 0xFFFFFFFF; + int reg = addr >> 4; + + switch (reg) { + case 0x0: + if (size == 2) { + ide_data_writew(&d->bus, 0, val); + } else if (size == 4) { + ide_data_writel(&d->bus, 0, val); + } + break; + case 0x1 ... 0x7: + if (size == 1) { + ide_ioport_write(&d->bus, reg, val); + } + break; + case 0x8: + case 0x16: + if (size == 1) { + ide_cmd_write(&d->bus, 0, val); + } + break; + case 0x20: + if (size == 4) { + d->timing_reg = val; + } + break; + case 0x30: + if (size == 4) { + if (val & 0x80000000u) { + d->irq_reg &= 0x7fffffff; + } + } + break; } - retval = bswap32(retval); - return retval; } static const MemoryRegionOps pmac_ide_ops = { - .old_mmio = { - .write = { - pmac_ide_writeb, - pmac_ide_writew, - pmac_ide_writel, - }, - .read = { - pmac_ide_readb, - pmac_ide_readw, - pmac_ide_readl, - }, - }, - .endianness = DEVICE_NATIVE_ENDIAN, + .read = pmac_ide_read, + .write = pmac_ide_write, + .valid.min_access_size = 1, + .valid.max_access_size = 4, + .endianness = DEVICE_LITTLE_ENDIAN, }; static const VMStateDescription vmstate_pmac = {
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/ide/macio.c | 181 +++++++++++++++++++++++--------------------------------- 1 file changed, 75 insertions(+), 106 deletions(-)