On Mon, Apr 16, 2012 at 16:52, Alexander Graf <ag...@suse.de> wrote:
>
> On 31.03.2012, at 18:32, Blue Swirl wrote:
>
>> Add an explicit CPUPPCState parameter instead of relying on AREG0
>> and rename op_helper.c (which only contains load and store helpers)
>> to mem_helper.c. Remove AREG0 swapping in
>> tlb_fill().
>>
>> Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation
>> and interrupt handling.
>>
>> Signed-off-by: Blue Swirl <blauwir...@gmail.com>
>> ---
>> Makefile.target | 6 +-
>> configure | 2 +-
>> target-ppc/excp_helper.c | 3 +-
>> target-ppc/helper.h | 30 ++++----
>> target-ppc/{op_helper.c => mem_helper.c} | 117
>> ++++++++++++++++--------------
>> target-ppc/translate.c | 30 ++++----
>> 6 files changed, 100 insertions(+), 88 deletions(-)
>> rename target-ppc/{op_helper.c => mem_helper.c} (66%)
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index 96b4c05..b45b773 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -82,10 +82,12 @@ libobj-y += tcg/tcg.o tcg/optimize.o
>> libobj-$(CONFIG_TCG_INTERPRETER) += tci.o
>> libobj-y += fpu/softfloat.o
>> ifneq ($(TARGET_BASE_ARCH), sparc)
>> +ifneq ($(TARGET_BASE_ARCH), ppc)
>> ifneq ($(TARGET_BASE_ARCH), alpha)
>> libobj-y += op_helper.o
>> endif
>> endif
>> +endif
>> libobj-y += helper.o
>> ifeq ($(TARGET_BASE_ARCH), i386)
>> libobj-y += cpuid.o
>> @@ -104,7 +106,7 @@ libobj-$(TARGET_UNICORE32) += cpu.o
>> libobj-$(TARGET_ALPHA) += int_helper.o fpu_helper.o sys_helper.o mem_helper.o
>> ifeq ($(TARGET_BASE_ARCH), ppc)
>> libobj-y += fpu_helper.o int_helper.o mmu_helper.o
>> -libobj-y += mmu_helper.o excp_helper.o timebase_helper.o misc_helper.o
>> +libobj-y += mmu_helper.o excp_helper.o timebase_helper.o
>> misc_helper.o mem_helper.o
>> endif
>>
>> libobj-y += disas.o
>> @@ -116,7 +118,7 @@ $(libobj-y): $(GENERATED_HEADERS)
>>
>> # HELPER_CFLAGS is used for all the legacy code compiled with static register
>> # variables
>> -ifneq ($(TARGET_BASE_ARCH), sparc)
>> +ifndef CONFIG_TCG_PASS_AREG0
>> op_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
>> endif
>> user-exec.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
>> diff --git a/configure b/configure
>> index b51a749..de19ca4 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3616,7 +3616,7 @@ case "$target_arch2" in
>> esac
>>
>> case "$target_arch2" in
>> - alpha | sparc*)
>> + alpha | ppc* | sparc*)
>> echo "CONFIG_TCG_PASS_AREG0=y" >> $config_target_mak
>> ;;
>> esac
>> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
>> index 945cd66..0f2ad4e 100644
>> --- a/target-ppc/excp_helper.c
>> +++ b/target-ppc/excp_helper.c
>> @@ -179,7 +179,8 @@ static inline void powerpc_excp(CPUPPCState *env,
>> int excp_model, int excp)
>> }
>> /* XXX: this is false */
>> /* Get rS/rD and rA from faulting opcode */
>> - env->spr[SPR_DSISR] |= (ldl_code((env->nip - 4)) & 0x03FF0000) >>
>> 16;
>> + env->spr[SPR_DSISR] |= (cpu_ldl_code(env, (env->nip - 4))
>> + & 0x03FF0000) >> 16;
>
> Mind to explain why we need the change from ldl to cpu_ldl?
Function signatures are different, ldl_code() does not take any
CPUState *env argument but uses AREG0 directly.
>> goto store_current;
>> case POWERPC_EXCP_PROGRAM: /* Program exception
>> */
>> switch (env->error_code & ~0xF) {
>> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
>> index 1939c66..a4f033b 100644
>> --- a/target-ppc/helper.h
>> +++ b/target-ppc/helper.h
>> @@ -20,15 +20,15 @@ DEF_HELPER_1(hrfid, void, env)
>> #endif
>> #endif
>>
>> -DEF_HELPER_2(lmw, void, tl, i32)
>> -DEF_HELPER_2(stmw, void, tl, i32)
>> -DEF_HELPER_3(lsw, void, tl, i32, i32)
>> -DEF_HELPER_4(lswx, void, tl, i32, i32, i32)
>> -DEF_HELPER_3(stsw, void, tl, i32, i32)
>> -DEF_HELPER_1(dcbz, void, tl)
>> -DEF_HELPER_1(dcbz_970, void, tl)
>> -DEF_HELPER_1(icbi, void, tl)
>> -DEF_HELPER_4(lscbx, tl, tl, i32, i32, i32)
>> +DEF_HELPER_3(lmw, void, env, tl, i32)
>> +DEF_HELPER_3(stmw, void, env, tl, i32)
>> +DEF_HELPER_4(lsw, void, env, tl, i32, i32)
>> +DEF_HELPER_5(lswx, void, env, tl, i32, i32, i32)
>> +DEF_HELPER_4(stsw, void, env, tl, i32, i32)
>> +DEF_HELPER_2(dcbz, void, env, tl)
>> +DEF_HELPER_2(dcbz_970, void, env, tl)
>> +DEF_HELPER_2(icbi, void, env, tl)
>> +DEF_HELPER_5(lscbx, tl, env, tl, i32, i32, i32)
>>
>> #if defined(TARGET_PPC64)
>> DEF_HELPER_FLAGS_2(mulhd, TCG_CALL_CONST | TCG_CALL_PURE, i64, i64, i64)
>> @@ -226,12 +226,12 @@ DEF_HELPER_5(vmsumshm, void, env, avr, avr, avr, avr)
>> DEF_HELPER_5(vmsumshs, void, env, avr, avr, avr, avr)
>> DEF_HELPER_4(vmladduhm, void, avr, avr, avr, avr)
>> DEF_HELPER_2(mtvscr, void, env, avr);
>> -DEF_HELPER_2(lvebx, void, avr, tl)
>> -DEF_HELPER_2(lvehx, void, avr, tl)
>> -DEF_HELPER_2(lvewx, void, avr, tl)
>> -DEF_HELPER_2(stvebx, void, avr, tl)
>> -DEF_HELPER_2(stvehx, void, avr, tl)
>> -DEF_HELPER_2(stvewx, void, avr, tl)
>> +DEF_HELPER_3(lvebx, void, env, avr, tl)
>> +DEF_HELPER_3(lvehx, void, env, avr, tl)
>> +DEF_HELPER_3(lvewx, void, env, avr, tl)
>> +DEF_HELPER_3(stvebx, void, env, avr, tl)
>> +DEF_HELPER_3(stvehx, void, env, avr, tl)
>> +DEF_HELPER_3(stvewx, void, env, avr, tl)
>> DEF_HELPER_4(vsumsws, void, env, avr, avr, avr)
>> DEF_HELPER_4(vsum2sws, void, env, avr, avr, avr)
>> DEF_HELPER_4(vsum4sbs, void, env, avr, avr, avr)
>> diff --git a/target-ppc/op_helper.c b/target-ppc/mem_helper.c
>> similarity index 66%
>> rename from target-ppc/op_helper.c
>> rename to target-ppc/mem_helper.c
>> index dcdbf5f..11eca68 100644
>> --- a/target-ppc/op_helper.c
>> +++ b/target-ppc/mem_helper.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * PowerPC emulation helpers for QEMU.
>> + * PowerPC memory access emulation helpers for QEMU.
>> *
>> * Copyright (c) 2003-2007 Jocelyn Mayer
>> *
>> @@ -16,9 +16,7 @@
>> * You should have received a copy of the GNU Lesser General Public
>> * License along with this library; if not, see
>> <http://www.gnu.org/licenses/>.
>> */
>> -#include <string.h>
>> #include "cpu.h"
>> -#include "dyngen-exec.h"
>> #include "host-utils.h"
>> #include "helper.h"
>>
>> @@ -26,12 +24,21 @@
>>
>> #if !defined(CONFIG_USER_ONLY)
>> #include "softmmu_exec.h"
>> +#else
>> +/* ??? Put these somewhere else? */
>> +#define cpu_ldub_data(env, addr) ldub_raw(addr)
>> +#define cpu_lduw_data(env, addr) lduw_raw(addr)
>> +#define cpu_ldl_data(env, addr) ldl_raw(addr)
>> +#define cpu_stb_data(env, addr, data) stb_raw(addr, data)
>> +#define cpu_stw_data(env, addr, data) stw_raw(addr, data)
>> +#define cpu_stl_data(env, addr, data) stl_raw(addr, data)
>
> Please avoid double negation - just use
>
> #ifdef CONFIG_USER_ONLY
> #else
> #endif
>
> which makes the code more readable.
OK.
>
> Also, why not use ldl and friends here?
This is for user emulation case. In softmmu_exec.h, ldl() is defined
as ldl_data(). Some other similar defines are in cpu-all.h. Actually I
think that would be a better place than here.
>
>
> Alex
>
