[Qemu-devel] target-sparc has inverse cwp logic for SAVE/RESTORE?

[Qemu-devel] target-sparc has inverse cwp logic for SAVE/RESTORE?

[Mark Cave-Ayland]

Hi all,

I've been working on debugging a window-related OpenBIOS issue and 
noticed that the cwp register logic in QEMU appears to be backwards 
according to the SPARCv9 specification. From sections 6.3.6.1 and 6.3.6.2:

"The SAVE instruction allocates a new register window and saves the 
caller’s register window by incrementing the CWP register."

"The RESTORE instruction restores the previous register window by 
decrementing the CWP register."

In target-sparc/win_helper.c the logic in helper_save() and 
helper_restore() is inverted, i.e. executing SAVE decrements cwp while 
executing RESTORE increments cwp.

The surprise here was that executing SAVE when cwp == 0 changed cwp to 7 
rather than 1. AFAICT there should be no functional difference, but it 
would make things less confusing when debugging window traps if the 
logic from the specification was followed. Does anyone know why this is 
currently done this way?


ATB,

Mark.

Re: [Qemu-devel] target-sparc has inverse cwp logic for SAVE/RESTORE?

[Olivier Danet]

On 18/05/2014 14:48, Mark Cave-Ayland wrote:
> Hi all,
> 
> I've been working on debugging a window-related OpenBIOS issue and noticed that the cwp register logic in QEMU appears to be backwards according to the SPARCv9 specification. From sections 6.3.6.1 and 6.3.6.2:
> 
> "The SAVE instruction allocates a new register window and saves the caller’s register window by incrementing the CWP register."
> 
> "The RESTORE instruction restores the previous register window by decrementing the CWP register."
> 
> In target-sparc/win_helper.c the logic in helper_save() and helper_restore() is inverted, i.e. executing SAVE decrements cwp while executing RESTORE increments cwp.
> 
> The surprise here was that executing SAVE when cwp == 0 changed cwp to 7 rather than 1. AFAICT there should be no functional difference, but it would make things less confusing when debugging window traps if the logic from the specification was followed. Does anyone know why this is currently done this way?
> 
> 
> ATB,
> 
> Mark.
> 
The problem may be related to the fact that the 32bits SPARCv8 and 64bits SPARCv9 work in opposite directions !

SparcV9 standard, page 360/399 :
The SPARC-V9 CWP register is incremented during a SAVE instruction and decremented during
a RESTORE instruction. Although this is the opposite of PSR.CWP’s behavior in SPARC-V8, the
only software it should affect is a few trap handlers that operate in privileged mode, and that must
be rewritten for SPARC-V9 anyway. This change will have no effect on nonprivileged software.


Olivier

Re: [Qemu-devel] target-sparc has inverse cwp logic for SAVE/RESTORE?

[Mark Cave-Ayland]

On 18/05/14 17:06, Olivier Danet wrote:

> The problem may be related to the fact that the 32bits SPARCv8 and 64bits SPARCv9 work in opposite directions !
>
> SparcV9 standard, page 360/399 :
> The SPARC-V9 CWP register is incremented during a SAVE instruction and decremented during
> a RESTORE instruction. Although this is the opposite of PSR.CWP’s behavior in SPARC-V8, the
> only software it should affect is a few trap handlers that operate in privileged mode, and that must
> be rewritten for SPARC-V9 anyway. This change will have no effect on nonprivileged software.

Ah I wonder if that could be it? I did try swapping the 
increment/decrement operators around in the helpers, however OpenBIOS 
crashed fairly early on so it's obviously not as simple as that. I 
suspect that because a lot of the code is shared been v8/v9 then there 
are some other assumptions that would need to be corrected for this to 
work :/


ATB,

Mark.

Re: [Qemu-devel] target-sparc has inverse cwp logic for SAVE/RESTORE?

[Blue Swirl]

On Mon, May 19, 2014 at 1:56 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 18/05/14 17:06, Olivier Danet wrote:
>
>> The problem may be related to the fact that the 32bits SPARCv8 and 64bits
>> SPARCv9 work in opposite directions !
>>
>> SparcV9 standard, page 360/399 :
>> The SPARC-V9 CWP register is incremented during a SAVE instruction and
>> decremented during
>> a RESTORE instruction. Although this is the opposite of PSR.CWP’s behavior
>> in SPARC-V8, the
>> only software it should affect is a few trap handlers that operate in
>> privileged mode, and that must
>> be rewritten for SPARC-V9 anyway. This change will have no effect on
>> nonprivileged software.
>
>
> Ah I wonder if that could be it? I did try swapping the increment/decrement
> operators around in the helpers, however OpenBIOS crashed fairly early on so
> it's obviously not as simple as that. I suspect that because a lot of the
> code is shared been v8/v9 then there are some other assumptions that would
> need to be corrected for this to work :/

Yes, QEMU uses v8 register window layout internally also for v9, guest
only sees fixed up CWP. IIRC using v9 layout internally would mean
adjusting a lot of places for marginal performance gains.
Alternatively v9 layout could be used, fixing up CWP for v8.