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

[BALATON Zoltan]

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

Well at least that question is cleared then.

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

It could also make sense if the guest is trying to flip a bit that's 
already set or cleared. It could be that the reset value on QEMU is not 
the same as on hardware or the firmware just runs the same routine on 
reset and cold start and wants to make sure a bit is in a state it wants 
it even if it already is that way so it could read value, unconditionally 
apply mask which in our case does not change the value and write it back.

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

AFAIU the device is little endian (VT8231 is a PCI device and primarily a 
PC part so it's expected to be little endian) but the guest also knows 
this and writes byte swaped values to it. But since the memory region is 
set to LITTLE_ENDIAN and we're on a big endian CPU QEMU will byte swap it 
again which it should not do as the guest already did that. Setting it to 
NATIVE_ENDIAN solves this and it oes not break anything for little endian 
machines on little endian hosts for sure. The only part I'm not sure about 
is big endian hosts which I don't have any to test.

> 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?

I think the guest already writes byte swapped value so it should work the 
same as on little endian machine but the QEMU memory layer gets in the 
way.

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

I don't have real machine but know somebody who does but I'm not sure what 
to ast to test on it. Can you describe it what you want to see or maybe 
write a sctipt for the fimrware to test it (if you're familiar enough with 
Forth for that). I can try to find some more info on this but so far I was 
concentrating on other bigger issues. This is a minor annoyance but would 
be nice to fix eventually.

Regards,
BALATON Zoltan

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