On 10/19/2016 05:50 PM, Richard Henderson wrote:
> On 10/18/2016 08:23 PM, Marek Vasut wrote:
>>> The documentation appears less than clear about whether or not loads
>>> into r0 recognize exceptions from the load, as opposed to simply not
>>> modifying r0.
>>
>> What about [1] page 10, quote:
>>
>> The Nios II architecture provides thirty-two 32-bit general-purpose
>> registers, r0 through r31. Some registers have names recognized by the
>> assembler. For example, the zero register (r0) always returns the
>> value zero, and writing to zero has no effect.
>
> I read that, but is that "... writing to zero has no effect [on the zero
> register]" but loads still take mmu faults, or, as other architecture
> manuals usually call out explicitly, "loads to the zero register are
> prefetch instructions and do not fault".
>
> Other architectures are split on this issue. For instance HPPA, Sparc,
> MIPS all fault when loading to the zero register from a protected page.
> Alpha *initially* reported such faults, but later changed the spec to be
> a prefetch (and therefore kernels supporting ev4 are supposed to
> recognize such loads and return without reporting the SIGSEGV).
>
> Do you have hardware that you can test this against? If a load to zero
> is a prefetch, the nios2 gcc port is missing the prefetch pattern. ;-)
Ha, I see, thanks for explaining :) I have a real system, so I tried
doing the following:
int main()
{
asm volatile("ldb r0, 0x10(r0)");
}
This will trigger a segfault:
test: unhandled page fault (11) at 0x00000000, cause 14
CPU: 0 PID: 1133 Comm: test Not tainted
4.8.0-rc2-next-20160816-00017-g40615ac #2
task: cfcef160 task.stack: cfcbe000
r1: 00000000 r2: 00000000 r3: 000023cc r4: 00000001
r5: 7f8d7e64 r6: 7f8d7e6c r7: 991f2697 r8: 001263cc
r9: e6925b67 r10: 2aac8ca0 r11: 00000003 r12: 00000000
r13: 2aabcc9c r14: 2aac8428 r15: 00000000
ra: 2aaeee94 fp: 00000000 sp: 7f8d7df0 gp: 0000c000
ea: 000023cc estatus: 00000003
Segmentation fault
> Either way, a comment would clear this up for future maintainers.
Will add one.
>>>> +/* Branch instructions */
>>>> +static void br(DisasContext *dc, uint32_t code, uint32_t flags)
>>>> +{
>>>> + I_TYPE(instr, code);
>>>> +
>>>> + gen_goto_tb(dc, 0, dc->pc + 4 + (int16_t)(instr.imm16 & 0xFFFC));
>>>
>>> Probably clearer as pc + 4 + (instr.imm16s & -4)
>>
>> I disagree, the 0xfffc in my opinion clearly states it removes bottom
>> two bits. Doing bitops with signed number looks real iffy.
>
> It also removes the top 16 bits, requiring you to sign-extend again.
>
> You might like 0xfffffffc better, but that does require that you count
> f's appropriately for the type. That's why I like -4: it's obvious (or
> should be) that it masks to a multiple of 4, and type promotion extends
> bits to the left as needed for the type of the other AND argument.
But then you can also use ~3 , right ? In which case, you'd avoid doing
bitwise operations with negative numbers, which is nice.
--
Best regards,
Marek Vasut
