Re: [PATCH v3] riscv: mm: Avoid spurious fault after hotplugging vmemmap

["David Hildenbrand (Arm)" <david@kernel.org>]

On 6/8/26 14:43, David Hildenbrand (Arm) wrote:
> On 6/8/26 04:24, Vivian Wang wrote:
>> On 6/5/26 17:16, David Hildenbrand (Arm) wrote:
>>> How does mark_new_valid_map() handle concurrent access?
>>>
>>> There is some comment in there, but I am not sure if that actually implies that
>>> mark_new_valid_map() can be called concurrently from arbitrary context?
>>
>> AFAICT, yes, it is intended to be used concurrently from arbitrary
>> context, and this is why I used them.
>>
>> I had not originally written the code, so some riscv maintainer can
>> maybe chime in on this, but as I understand it the idea is that we only
>> ever set bits in the bitmap in mark_new_valid_map() while "allocating",
>> whereas the assembly code in handle_exception in
>> arch/riscv/kernel/entry.S only ever clears individual bits. So at least
>> the access to the bitmap itself should be fine.
> 
> Right, flush_cache_vmap() looks like a bitmap_set() -- non-atomically.
> 
> .Lnew_vmalloc_kernel_address does an atomic bitmap clearing ("Atomically reset
> the current cpu bit in new_vmalloc").
> 
> I am primarily wondering whether we'd want atomic bit-setting as well?
> 
>>
>> This was added in commit 503638e0babf ("riscv: Stop emitting preventive
>> sfence.vma for new vmalloc mappings").
>>
>> Do you think the mark_new_valid_map() function needs extra
>> synchronization like smp_wmb()?
> 
> Right, I wonder about atomic access, but also about memory barriers.
> 

BTW, does riscv support a better mechanism to avoid such spurious faults?

E.g., on arm64 we have emit_pte_barriers() where we issue some barriers:

"The isb ensures that any previous speculative "invalid translation" marker that
is in the CPU's pipeline gets cleared, so that any access to that address after
setting the pte to valid won't cause a spurious fault."

I assume the riscv case is different, as it's about actual TLB entries, and not
just translation markers in the pipeline?

-- 
Cheers,

David