| Message ID | 20230702154838.722809-15-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | q800: add support for booting MacOS Classic - part 2 | expand |
On 2/7/23 17:48, Mark Cave-Ayland wrote: > The MacOS toolbox ROM calculates the number of branches that can be executed > per millisecond as part of its timer calibration. Since modern hosts are > considerably quicker than original hardware, the negative counter reaches zero > before the calibration completes leading to division by zero later in > CALCULATESLOD. > > Instead of trying to fudge the timing loop (which won't work for TimeDBRA/TimeSCCDB > anyhow), use the pattern of access to the VIA1 registers to detect when SETUPTIMEK > has finished executing and write some well-known good timer values to TimeDBRA > and TimeSCCDB taken from real hardware with a suitable scaling factor. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/misc/mac_via.c | 115 ++++++++++++++++++++++++++++++++++++++ > hw/misc/trace-events | 1 + > include/hw/misc/mac_via.h | 3 + > 3 files changed, 119 insertions(+) > > diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c > index baeb73eeb3..766a32a95d 100644 > --- a/hw/misc/mac_via.c > +++ b/hw/misc/mac_via.c > @@ -16,6 +16,7 @@ > */ > > #include "qemu/osdep.h" > +#include "exec/address-spaces.h" > #include "migration/vmstate.h" > #include "hw/sysbus.h" > #include "hw/irq.h" > +/* > + * Addresses and real values for TimeDBRA/TimeSCCB to allow timer calibration > + * to succeed (NOTE: both values have been multiplied by 3 to cope with the > + * speed of QEMU execution on a modern host > + */ > +#define MACOS_TIMEDBRA 0xd00 > +#define MACOS_TIMESCCB 0xd02 > + > +#define MACOS_TIMEDBRA_VALUE (0x2a00 * 3) > +#define MACOS_TIMESCCB_VALUE (0x079d * 3) > + > +static bool via1_is_toolbox_timer_calibrated(void) > +{ > + /* > + * Indicate whether the MacOS toolbox has been calibrated by checking > + * for the value of our magic constants > + */ > + uint16_t timedbra = lduw_be_phys(&address_space_memory, MACOS_TIMEDBRA); > + uint16_t timesccdb = lduw_be_phys(&address_space_memory, MACOS_TIMESCCB); Rather than using the global address_space_memory (which we secretly try to remove entirely), could we pass a MemoryRegion link property to the VIA1 device?
On 03/07/2023 09:30, Philippe Mathieu-Daudé wrote: > On 2/7/23 17:48, Mark Cave-Ayland wrote: >> The MacOS toolbox ROM calculates the number of branches that can be executed >> per millisecond as part of its timer calibration. Since modern hosts are >> considerably quicker than original hardware, the negative counter reaches zero >> before the calibration completes leading to division by zero later in >> CALCULATESLOD. >> >> Instead of trying to fudge the timing loop (which won't work for TimeDBRA/TimeSCCDB >> anyhow), use the pattern of access to the VIA1 registers to detect when SETUPTIMEK >> has finished executing and write some well-known good timer values to TimeDBRA >> and TimeSCCDB taken from real hardware with a suitable scaling factor. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> hw/misc/mac_via.c | 115 ++++++++++++++++++++++++++++++++++++++ >> hw/misc/trace-events | 1 + >> include/hw/misc/mac_via.h | 3 + >> 3 files changed, 119 insertions(+) >> >> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c >> index baeb73eeb3..766a32a95d 100644 >> --- a/hw/misc/mac_via.c >> +++ b/hw/misc/mac_via.c >> @@ -16,6 +16,7 @@ >> */ >> #include "qemu/osdep.h" >> +#include "exec/address-spaces.h" >> #include "migration/vmstate.h" >> #include "hw/sysbus.h" >> #include "hw/irq.h" > > >> +/* >> + * Addresses and real values for TimeDBRA/TimeSCCB to allow timer calibration >> + * to succeed (NOTE: both values have been multiplied by 3 to cope with the >> + * speed of QEMU execution on a modern host >> + */ >> +#define MACOS_TIMEDBRA 0xd00 >> +#define MACOS_TIMESCCB 0xd02 >> + >> +#define MACOS_TIMEDBRA_VALUE (0x2a00 * 3) >> +#define MACOS_TIMESCCB_VALUE (0x079d * 3) >> + >> +static bool via1_is_toolbox_timer_calibrated(void) >> +{ >> + /* >> + * Indicate whether the MacOS toolbox has been calibrated by checking >> + * for the value of our magic constants >> + */ >> + uint16_t timedbra = lduw_be_phys(&address_space_memory, MACOS_TIMEDBRA); >> + uint16_t timesccdb = lduw_be_phys(&address_space_memory, MACOS_TIMESCCB); > > Rather than using the global address_space_memory (which we secretly > try to remove entirely), could we pass a MemoryRegion link property > to the VIA1 device? Hmmm good question. It seems to me that we're dispatching a write to the default address space (which includes all RAM and MMIO etc.) rather than a particular MemoryRegion so it feels as if AddressSpace is the right approach here. Unfortunately since AddressSpace is not a QOM type then it isn't possible to pass it as a link property. There are existing examples in qtest that use first_cpu->as which seems a better option unless we want to move away from using first_cpu in the codebase? ATB, Mark.
On 5/7/23 21:49, Mark Cave-Ayland wrote: > On 03/07/2023 09:30, Philippe Mathieu-Daudé wrote: > >> On 2/7/23 17:48, Mark Cave-Ayland wrote: >>> The MacOS toolbox ROM calculates the number of branches that can be >>> executed >>> per millisecond as part of its timer calibration. Since modern hosts are >>> considerably quicker than original hardware, the negative counter >>> reaches zero >>> before the calibration completes leading to division by zero later in >>> CALCULATESLOD. >>> >>> Instead of trying to fudge the timing loop (which won't work for >>> TimeDBRA/TimeSCCDB >>> anyhow), use the pattern of access to the VIA1 registers to detect >>> when SETUPTIMEK >>> has finished executing and write some well-known good timer values to >>> TimeDBRA >>> and TimeSCCDB taken from real hardware with a suitable scaling factor. >>> >>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >>> --- >>> hw/misc/mac_via.c | 115 ++++++++++++++++++++++++++++++++++++++ >>> hw/misc/trace-events | 1 + >>> include/hw/misc/mac_via.h | 3 + >>> 3 files changed, 119 insertions(+) >>> >>> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c >>> index baeb73eeb3..766a32a95d 100644 >>> --- a/hw/misc/mac_via.c >>> +++ b/hw/misc/mac_via.c >>> @@ -16,6 +16,7 @@ >>> */ >>> #include "qemu/osdep.h" >>> +#include "exec/address-spaces.h" >>> #include "migration/vmstate.h" >>> #include "hw/sysbus.h" >>> #include "hw/irq.h" >> >> >>> +/* >>> + * Addresses and real values for TimeDBRA/TimeSCCB to allow timer >>> calibration >>> + * to succeed (NOTE: both values have been multiplied by 3 to cope >>> with the >>> + * speed of QEMU execution on a modern host >>> + */ >>> +#define MACOS_TIMEDBRA 0xd00 >>> +#define MACOS_TIMESCCB 0xd02 >>> + >>> +#define MACOS_TIMEDBRA_VALUE (0x2a00 * 3) >>> +#define MACOS_TIMESCCB_VALUE (0x079d * 3) >>> + >>> +static bool via1_is_toolbox_timer_calibrated(void) >>> +{ >>> + /* >>> + * Indicate whether the MacOS toolbox has been calibrated by >>> checking >>> + * for the value of our magic constants >>> + */ >>> + uint16_t timedbra = lduw_be_phys(&address_space_memory, >>> MACOS_TIMEDBRA); >>> + uint16_t timesccdb = lduw_be_phys(&address_space_memory, >>> MACOS_TIMESCCB); >> >> Rather than using the global address_space_memory (which we secretly >> try to remove entirely), could we pass a MemoryRegion link property >> to the VIA1 device? > > Hmmm good question. It seems to me that we're dispatching a write to the > default address space (which includes all RAM and MMIO etc.) rather than > a particular MemoryRegion so it feels as if AddressSpace is the right > approach here. Unfortunately since AddressSpace is not a QOM type then > it isn't possible to pass it as a link property. We pass a MR link. > There are existing examples in qtest that use first_cpu->as which seems > a better option unless we want to move away from using first_cpu in the > codebase? See: $ git grep -E "(PROP.*LINK.*MEMORY_REGION|dma-mr)" Anyway I don't object to your patch, we can rework this &address_space_memory use later when we'll have discussed the whole design.
On 06/07/2023 11:10, Philippe Mathieu-Daudé wrote: > On 5/7/23 21:49, Mark Cave-Ayland wrote: >> On 03/07/2023 09:30, Philippe Mathieu-Daudé wrote: >> >>> On 2/7/23 17:48, Mark Cave-Ayland wrote: >>>> The MacOS toolbox ROM calculates the number of branches that can be executed >>>> per millisecond as part of its timer calibration. Since modern hosts are >>>> considerably quicker than original hardware, the negative counter reaches zero >>>> before the calibration completes leading to division by zero later in >>>> CALCULATESLOD. >>>> >>>> Instead of trying to fudge the timing loop (which won't work for TimeDBRA/TimeSCCDB >>>> anyhow), use the pattern of access to the VIA1 registers to detect when SETUPTIMEK >>>> has finished executing and write some well-known good timer values to TimeDBRA >>>> and TimeSCCDB taken from real hardware with a suitable scaling factor. >>>> >>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >>>> --- >>>> hw/misc/mac_via.c | 115 ++++++++++++++++++++++++++++++++++++++ >>>> hw/misc/trace-events | 1 + >>>> include/hw/misc/mac_via.h | 3 + >>>> 3 files changed, 119 insertions(+) >>>> >>>> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c >>>> index baeb73eeb3..766a32a95d 100644 >>>> --- a/hw/misc/mac_via.c >>>> +++ b/hw/misc/mac_via.c >>>> @@ -16,6 +16,7 @@ >>>> */ >>>> #include "qemu/osdep.h" >>>> +#include "exec/address-spaces.h" >>>> #include "migration/vmstate.h" >>>> #include "hw/sysbus.h" >>>> #include "hw/irq.h" >>> >>> >>>> +/* >>>> + * Addresses and real values for TimeDBRA/TimeSCCB to allow timer calibration >>>> + * to succeed (NOTE: both values have been multiplied by 3 to cope with the >>>> + * speed of QEMU execution on a modern host >>>> + */ >>>> +#define MACOS_TIMEDBRA 0xd00 >>>> +#define MACOS_TIMESCCB 0xd02 >>>> + >>>> +#define MACOS_TIMEDBRA_VALUE (0x2a00 * 3) >>>> +#define MACOS_TIMESCCB_VALUE (0x079d * 3) >>>> + >>>> +static bool via1_is_toolbox_timer_calibrated(void) >>>> +{ >>>> + /* >>>> + * Indicate whether the MacOS toolbox has been calibrated by checking >>>> + * for the value of our magic constants >>>> + */ >>>> + uint16_t timedbra = lduw_be_phys(&address_space_memory, MACOS_TIMEDBRA); >>>> + uint16_t timesccdb = lduw_be_phys(&address_space_memory, MACOS_TIMESCCB); >>> >>> Rather than using the global address_space_memory (which we secretly >>> try to remove entirely), could we pass a MemoryRegion link property >>> to the VIA1 device? >> >> Hmmm good question. It seems to me that we're dispatching a write to the default >> address space (which includes all RAM and MMIO etc.) rather than a particular >> MemoryRegion so it feels as if AddressSpace is the right approach here. >> Unfortunately since AddressSpace is not a QOM type then it isn't possible to pass >> it as a link property. > > We pass a MR link. That could work, but then we'd have to grab the host pointer via memory_region_get_ram_ptr() first and switch to using lduw_be_p(). And if we did have an MR property then what should it be set to? The obvious candidate would be get_system_memory() but should we also be avoiding that for similar reasons? If we did use it, then we might as well use get_system_memory() directly rather than having to pass in a separate link property. >> There are existing examples in qtest that use first_cpu->as which seems a better >> option unless we want to move away from using first_cpu in the codebase? > > See: > > $ git grep -E "(PROP.*LINK.*MEMORY_REGION|dma-mr)" > > Anyway I don't object to your patch, we can rework this > &address_space_memory use later when we'll have discussed the whole > design. Agreed. It feels like there are still some discussions needed to understand exactly what needs to be done in order to move forward with this (maybe start a separate thread?). ATB, Mark.
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c index baeb73eeb3..766a32a95d 100644 --- a/hw/misc/mac_via.c +++ b/hw/misc/mac_via.c @@ -16,6 +16,7 @@ */ #include "qemu/osdep.h" +#include "exec/address-spaces.h" #include "migration/vmstate.h" #include "hw/sysbus.h" #include "hw/irq.h" @@ -871,6 +872,112 @@ static void via1_auxmode_update(MOS6522Q800VIA1State *v1s) } } +/* + * Addresses and real values for TimeDBRA/TimeSCCB to allow timer calibration + * to succeed (NOTE: both values have been multiplied by 3 to cope with the + * speed of QEMU execution on a modern host + */ +#define MACOS_TIMEDBRA 0xd00 +#define MACOS_TIMESCCB 0xd02 + +#define MACOS_TIMEDBRA_VALUE (0x2a00 * 3) +#define MACOS_TIMESCCB_VALUE (0x079d * 3) + +static bool via1_is_toolbox_timer_calibrated(void) +{ + /* + * Indicate whether the MacOS toolbox has been calibrated by checking + * for the value of our magic constants + */ + uint16_t timedbra = lduw_be_phys(&address_space_memory, MACOS_TIMEDBRA); + uint16_t timesccdb = lduw_be_phys(&address_space_memory, MACOS_TIMESCCB); + + return (timedbra == MACOS_TIMEDBRA_VALUE && + timesccdb == MACOS_TIMESCCB_VALUE); +} + +static void via1_timer_calibration_hack(MOS6522Q800VIA1State *v1s, int addr, + uint64_t val, int size) +{ + /* + * Work around timer calibration to ensure we that we have non-zero and + * known good values for TIMEDRBA and TIMESCCDB. + * + * This works by attempting to detect the reset and calibration sequence + * of writes to VIA1 + */ + int old_timer_hack_state = v1s->timer_hack_state; + + switch (v1s->timer_hack_state) { + case 0: + if (addr == VIA_REG_PCR && val == 0x22) { + /* VIA_REG_PCR: configure VIA1 edge triggering */ + v1s->timer_hack_state = 1; + } + break; + case 1: + if (addr == VIA_REG_T2CL && val == 0xc) { + /* VIA_REG_T2CL: low byte of 1ms counter */ + if (!via1_is_toolbox_timer_calibrated()) { + v1s->timer_hack_state = 2; + } else { + v1s->timer_hack_state = 0; + } + } + break; + case 2: + if (addr == VIA_REG_T2CH && val == 0x3) { + /* + * VIA_REG_T2CH: high byte of 1ms counter (very likely at the + * start of SETUPTIMEK) + */ + if (!via1_is_toolbox_timer_calibrated()) { + v1s->timer_hack_state = 3; + } else { + v1s->timer_hack_state = 0; + } + } + break; + case 3: + if (addr == VIA_REG_IER && val == 0x20) { + /* + * VIA_REG_IER: update at end of SETUPTIMEK + * + * Timer calibration has finished: unfortunately the values in + * TIMEDBRA (0xd00) and TIMESCCDB (0xd02) are so far out they + * cause divide by zero errors. + * + * Update them with values obtained from a real Q800 but with + * a x3 scaling factor which seems to work well + */ + stw_be_phys(&address_space_memory, MACOS_TIMEDBRA, + MACOS_TIMEDBRA_VALUE); + stw_be_phys(&address_space_memory, MACOS_TIMESCCB, + MACOS_TIMESCCB_VALUE); + + v1s->timer_hack_state = 4; + } + break; + case 4: + /* + * This is the normal post-calibration timer state: we should + * generally remain here unless we detect the A/UX calibration + * loop, or a write to VIA_REG_PCR suggesting a reset + */ + if (addr == VIA_REG_PCR && val == 0x22) { + /* Looks like there has been a reset? */ + v1s->timer_hack_state = 1; + } + break; + default: + g_assert_not_reached(); + } + + if (old_timer_hack_state != v1s->timer_hack_state) { + trace_via1_timer_hack_state(v1s->timer_hack_state); + } +} + static uint64_t mos6522_q800_via1_read(void *opaque, hwaddr addr, unsigned size) { MOS6522Q800VIA1State *s = MOS6522_Q800_VIA1(opaque); @@ -896,6 +1003,9 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr addr, uint64_t val, MOS6522State *ms = MOS6522(v1s); addr = (addr >> 9) & 0xf; + + via1_timer_calibration_hack(v1s, addr, val, size); + mos6522_write(ms, addr, val, size); switch (addr) { @@ -1008,6 +1118,9 @@ static void mos6522_q800_via1_reset_hold(Object *obj) adb_set_autopoll_enabled(adb_bus, true); v1s->cmd = REG_EMPTY; v1s->alt = REG_EMPTY; + + /* Timer calibration hack */ + v1s->timer_hack_state = 0; } static void mos6522_q800_via1_realize(DeviceState *dev, Error **errp) @@ -1100,6 +1213,8 @@ static const VMStateDescription vmstate_q800_via1 = { VMSTATE_INT64(next_second, MOS6522Q800VIA1State), VMSTATE_TIMER_PTR(sixty_hz_timer, MOS6522Q800VIA1State), VMSTATE_INT64(next_sixty_hz, MOS6522Q800VIA1State), + /* Timer hack */ + VMSTATE_INT32(timer_hack_state, MOS6522Q800VIA1State), VMSTATE_END_OF_LIST() } }; diff --git a/hw/misc/trace-events b/hw/misc/trace-events index d5711dcff2..c717090659 100644 --- a/hw/misc/trace-events +++ b/hw/misc/trace-events @@ -268,6 +268,7 @@ via1_adb_send(const char *state, uint8_t data, const char *vadbint) "state %s da via1_adb_receive(const char *state, uint8_t data, const char *vadbint, int status, int index, int size) "state %s data=0x%02x vADBInt=%s status=0x%x index=%d size=%d" via1_adb_poll(uint8_t data, const char *vadbint, int status, int index, int size) "data=0x%02x vADBInt=%s status=0x%x index=%d size=%d" via1_auxmode(int mode) "setting auxmode to %d" +via1_timer_hack_state(int state) "setting timer_hack_state to %d" # grlib_ahb_apb_pnp.c grlib_ahb_pnp_read(uint64_t addr, unsigned size, uint32_t value) "AHB PnP read addr:0x%03"PRIx64" size:%u data:0x%08x" diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h index 422da43bf9..63cdcf7c69 100644 --- a/include/hw/misc/mac_via.h +++ b/include/hw/misc/mac_via.h @@ -74,6 +74,9 @@ struct MOS6522Q800VIA1State { int64_t next_second; QEMUTimer *sixty_hz_timer; int64_t next_sixty_hz; + + /* SETUPTIMEK hack */ + int timer_hack_state; };
The MacOS toolbox ROM calculates the number of branches that can be executed per millisecond as part of its timer calibration. Since modern hosts are considerably quicker than original hardware, the negative counter reaches zero before the calibration completes leading to division by zero later in CALCULATESLOD. Instead of trying to fudge the timing loop (which won't work for TimeDBRA/TimeSCCDB anyhow), use the pattern of access to the VIA1 registers to detect when SETUPTIMEK has finished executing and write some well-known good timer values to TimeDBRA and TimeSCCDB taken from real hardware with a suitable scaling factor. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/misc/mac_via.c | 115 ++++++++++++++++++++++++++++++++++++++ hw/misc/trace-events | 1 + include/hw/misc/mac_via.h | 3 + 3 files changed, 119 insertions(+)