[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH] accel/tcg/translate-all: expand cpu_restore_state
From: |
Richard Henderson |
Subject: |
Re: [Qemu-arm] [PATCH] accel/tcg/translate-all: expand cpu_restore_state retaddr check |
Date: |
Wed, 8 Nov 2017 09:52:22 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 11/07/2017 07:53 PM, Peter Maydell wrote:
>> Then why call cpu_restore_state at all? We should be consistent as there
>> are plenty of places that do things like:
>>
>> if (pc) {
>> /* now we have a real cpu fault */
>> cpu_restore_state(cs, pc);
>> }
>>
>> I'm happy to make a 0 retaddr officially valid and actually document it
>> in exec-all.h. It's not like most callers even bother checking the
>> return code.
This is exactly the discussion that we had last time, and we did just that.
> Hmm, there's more places than I expected that do that "don't call
> if 0" check than I thought. Overall it seems better to me to officially
> allow the zero, rather than having lots of callsites that all have
> to remember to check.
... what we didn't do is go through and change all of the call sites to remove
the check for zero.
> Incidentally if retaddr is zero then
> (retaddr < (uintptr_t) tcg_init_ctx.code_gen_buffer)
> is always true and you don't need to explicitly check for zero, though
> it might be clearer to do so if we think we might change the rest
> of the condition in future.
Indeed, I was thinking
retaddr - code_gen_buffer < code_gen_buffer_size
which works well with unsigned arithmetic. And a large comment re zero.
r~