On 23/10/15 15:56, Mark Cave-Ayland wrote:
> From: Alexander Graf <ag...@suse.de>
>
> The lsxw instruction checks whether the desired string actually fits
> into all defined registers. Unfortunately it does the calculation wrong,
> resulting in illegal instruction traps for loads that really should fit.
s/lsxw/lswx/ in the above text and in the title ... but I guess this
could also be done when this patch gets picked up.
> Fix it up, making Mac OS happier.
>
> Signed-off-by: Alexander Graf <ag...@suse.de>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
> ---
> target-ppc/mem_helper.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
> index 6d37dae..7e1f234 100644
> --- a/target-ppc/mem_helper.c
> +++ b/target-ppc/mem_helper.c
> @@ -100,8 +100,9 @@ void helper_lswx(CPUPPCState *env, target_ulong addr,
> uint32_t reg,
> uint32_t ra, uint32_t rb)
> {
> if (likely(xer_bc != 0)) {
> - if (unlikely((ra != 0 && reg < ra && (reg + xer_bc) > ra) ||
> - (reg < rb && (reg + xer_bc) > rb))) {
> + int num_used_regs = (xer_bc + 3) / 4;
> + if (unlikely((ra != 0 && reg < ra && (reg + num_used_regs) > ra) ||
> + (reg < rb && (reg + num_used_regs) > rb))) {
> helper_raise_exception_err(env, POWERPC_EXCP_PROGRAM,
> POWERPC_EXCP_INVAL |
> POWERPC_EXCP_INVAL_LSWX);
The calculation of num_used_regs looks fine to me ... but is the
remaining part of the condition really right already?
According to the PowerISA:
If RA or RB is in the range of registers to be loaded,
including the case in which RA=0, the instruction is
treated as if the instruction form were invalid. If RT=RA
or RT=RB, the instruction form is invalid.
So I wonder whether the check for "ra != 0" is really necessary here?
Also, shouldn't the code rather check for "reg <= ra" instead of "reg <
ra"? And "reg <= rb", too, of course?
Also this code seems to completely ignore the case of the register
wrap-around, where the sequence of registers jumps back to r0 ...
So I'm basically fine with the num_used_regs fix for now, but I think
this needs a big "FIXME" comment so that the remaining issues get
cleaned up later?
Thomas
