Re: [PATCH 14/21] mac_via: work around underflow in TimeDBRA timing loop in SETUPTIMEK

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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?