Re: [PATCH] hw/acpi: Set memory regions to native endian as a work around

[BALATON Zoltan]

Hello,

Finally could get to write some test script and have somebody with a real 
machine run that so I'm coming back to this with those results.

On Tue, 7 Mar 2023, Paolo Bonzini wrote:
> On 3/7/23 11:01, BALATON Zoltan wrote:
> > > > I'm not sure I follow what you mean so I'd need a patch to see then I can
> > > > test it with the clients I run on pegasos2.
> > >
> > > Do you have a spec, or pointer to the morphos kernel sources, to figure 
> > > out how the hardware works?
> >
> > No, that's closed source and only available as a demo iso but it's known to 
> > work on real hardware and freely downloadable so is a good test. (AFAIK 
> > those who developed MorphOS had close connection to those who wrote the 
> > firmware for Pegasos II.) Maybe the VT8231 datasheet or similar parts (we 
> > only emulate VT82C686B and VT8231 in QEMU) has some info on this.
>
> I agree it's a good test, but it's not clear what it means to do sub-word 
> writes to the register.

Looking at the VT8231 data sheet I think it means to set bits related to 
soft-off when it writes 0x28 to upper byte of the control reg. This seems 
pretty obvious.

> For example, in the dump I posted you have:
>
> evt 3 1 write 1      // enable timer
> evt 0 2 read
> evt 0 2 write 1      // just writes again the same value, clearing sts
>
> It's important to note that the comments are just my guess.
>
> Before even looking at the effect of the write, the trace tells us that your 
> patch is incomplete.  With both current QEMU and your patch it would do 
> nothing because addr is not 0 or 2; but since the firmware of your machine 
> does use addr == 3, you need to handle it.  In other words, before looking at 
> this trace, I was wary of accepting your patch because it made no sense to 
> me; but I couldn't exclude that I was missing something.  Now, instead, I am 
> certain it shouldn't be accepted.

Testing on real machine also confirms that the patch was wrong as it 
breaks access to regs even if it happens to fix the unaligned write. Since 
we don't emulate most of it anyway and guests probably don't use it other 
than for power control the breakage does not appeared though.

> I am quite confident that the second guess is correct, because "write the 
> same value that was read" only makes sense for evt_sts and not for evt_en.

You don't have to guess which reg is which, check the data sheet instead:

Power Management I/O-Space Registers
Basic Power Management Control and Status
I/O Offset 1-0 - Power Management Status ................. RWC
I/O Offset 3-2 - Power Management Enable .................. RW
I/O Offset 5-4 - Power Management Control .................RW
I/O Offset 0B-08 - Power Management Timer............... RW

> We learnt something: no matter what bus this device sits on, it does not flip 
> bit 1 of the address for subword writes.  As I mentioned yesterday, we also 
> observe that the load and store use lhbrx and sthbrx.  Assuming this is not a 
> harmless bug in the firmware, this means the device is indeed little endian.
>
> This means that the first write is almost certainly to evt_en.  On a 
> little-endian machine the write would set bit 8 of evt.en (power button), but 
> what about a big-endian machine like yours?  Should it set bit 0 or bit 8? 
> If it's bit 0 (which resides at offset 2 according to the device), who flips 
> the low bit of the address?  Why is bit 0 flipped but not bit 1?
>
> You simply cannot fix the emulation of this machine until you can answer the 
> above questions.  If there is no source and no spec, the two ways to do it 
> are:
>
> - get a real machine, and figure out whether the write to offset 3 
> corresponds to the PM timer or the power button.
>
> - continue the trace up to the point the OS runs, and see if you get some 
> clues similar to the one above that placed evt_sts at offset 2.

So I wrote a script to dump the PM registers of VT8231 with different 
access sizes and compare that to QEMU. The script and results are attached here:
https://osdn.net/projects/qmiga/ticket/47971

The results I think show that:
- VIA chip is little endian and mapped as such on pegasos2
- proposed patch is indeed wrong
- unaligned access is not handled correctly in acpi_pm_cnt_write() which 
now I think is the place where this should be fixed
- QEMU runs the PM timer in 32 bit while the corresponding bit is set to 
24 bit mode (VT8231 has a bit to set this in PCI config reg 0x41), not 
sure if this causes any problem and if QEMU has a 24 bit mode

What do you think?

Regards,
BALATON Zoltan