On Wed, 3 Jun 2020, BALATON Zoltan wrote:
On Wed, 3 Jun 2020, P J P wrote:
+-- On Wed, 3 Jun 2020, Gerd Hoffmann wrote --+
| Hmm, why modify mm_index? Shouldn't we just check it is non-zero
| before calling ati_mm_read/ati_mm_write?
if (s->regs.mm_index & BIT(31)) {
...
} else {
ati_mm_write(s, s->regs.mm_index + addr - MM_DATA, data, size);
}
Exit condition for recursion is to set (mm_index & BIT(31)), so recursion
would continue even with non-zero values I think.
MM_INDEX and MM_DATA allows access to registers (or memory if BIT(31) is set)
via only these two registers. This is called indexed access in docs. So it
does not really make sense to allow access to these registers as well thus we
can avoid infinite recursion. It's not meant to recurse until BIT(31) is set.
I think you can do:
if (s->regs.mm_index & BIT(31)) {
...
- } else {
+ } else if (s->regs.mm_index >= BIOS_0_SCRATCH) {
Actually this should be
+ } else if (s->regs.mm_index >= CLOCK_CNTL_INDEX) {
or even > MM_DATA + 3 may be best as that only refers to defines used in
that case. So maybe
+ } else if (s->regs.mm_index > MM_DATA + 3) {
ati_mm_write(s, s->regs.mm_index + addr - MM_DATA, data, size);
}
and do the same in ati_mm_read() as well.
Regards,
BALATON Zoltan
