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-ayl...@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.


Reply via email to