linux-kernel - Re: [PATCH] riscv: Don't use va_pa

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <mhng-b28fa0cb-24ee-40d1-9f02-619605a4cc9a@palmerdabbelt-glaptop>
Date:   Sat, 23 Oct 2021 13:14:11 -0700 (PDT)
From:   Palmer Dabbelt <palmer@...belt.com>
To:     mick@....forth.gr
CC:     alexandre.ghiti@...onical.com, mick@....forth.gr,
        Paul Walmsley <paul.walmsley@...ive.com>,
        aou@...s.berkeley.edu, linux-riscv@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject:     Re: [PATCH] riscv: Don't use va_pa_offset on kdump

On Sat, 09 Oct 2021 06:18:48 PDT (-0700), mick@....forth.gr wrote:
> Στις 2021-10-06 14:13, Alexandre Ghiti έγραψε:
>>> +
>>> +       /* This will trigger a jump to CSR_STVEC anyway */
>>>         jalr    zero, a2, 0
>>
>> The last jump to a2 can be removed since the fault will be triggered
>> before even reaching this instruction.
>>
>
> Just switching SATP to zero doesn't generate a trap unless mstatus.TVM
> is set (for visualization purposes). The hart will try and execute the
> next instruction but it's not clear in the spec what happens in case the
> code is cached, I don't want to rely solely on STVEC. I prefer having
> this instruction there, note that some earlier QEMU versions also had
> this behavior (the original kdump patch didn't set STVEC and it worked
> fine after setting SATP to zero).

IIRC this came down to some very specific wording in the spec.  
Something along the lines of the 0 in SATP meaning "no translation", 
SFENCE.VMA ordering translations, and the general "if the spec doesn't 
mention it then it has to work" logic.  I thought I opened a spec issue 
about this for clarification, but I can't find it.

That said, I'm perfectly fine taking the safe approach here as it's not 
like the performance matters here.  Warrants a comment, though.

>
>>
>> This patch fixes a regression introduced when moving the kernel to the
>> end of the address space, so we should add:
>> Fixes: 2bfc6cd81bd1 ("riscv: Move kernel mapping outside of linear
>> mapping")
>>
>> And it should be backported to 5.13 and 5.14. It seems that the
>> following tags should be enough:
>>
>> Cc: <stable@...r.kernel.org> # 5.13
>> Cc: <stable@...r.kernel.org> # 5.14
>>
>> And finally, you can add:
>>
>> Reviewed-by: Alexandre Ghiti <alex@...ti.fr>
>>
>
> ACK, thanks ! I'll resend the patch with the tags you mentioned.

I don't have a v2 in my inbox, did I miss something?  Also, if it's just 
the tags then it's generally not necessary to re-send something.  The 
comment does, though.

LMK if you want me to deal with this, or if there's going to be a v2.

Thanks!