Re: [qemu-s390x] [Qemu-devel] [PATCH v2 15/28] s390x/tcg: Fault-safe memset

[David Hildenbrand]

On 11.09.19 17:29, Richard Henderson wrote:
> On 9/6/19 3:57 AM, David Hildenbrand wrote:
>> +    if (likely(desta.haddr1)) {
>> +        memset(desta.haddr1, byte, desta.size1);
>> +    } else {
>> +        for (i = 0; i < desta.size1; i++) {
>> +            helper_ret_stb_mmu(env, desta.vaddr1 + i, byte, oi, ra);
>> +        }
> 
> The only thing perhaps that we could do better here is to re-check
> tlb_vaddr_to_host after a singe access.  This would handle NOTDIRTY with a
> single iotlb access and then fall back to memset, without re-checking host
> address for real mmio after every store.

Yes - also thought about that :) - but I didn't find a clean way to do
that without uglifying that code (and especially access_memmove() and
friends).

> 
>> +    if (likely(desta.haddr2)) {
>> +            memset(desta.haddr2, byte, desta.size2);
> 
> Indentation.
> 
>> +    } else {
>> +        for (i = 0; i < desta.size2; i++) {
>> +            helper_ret_stb_mmu(env, desta.vaddr2 + i, byte, oi, ra);
>>          }
> 
> Likewise wrt NOTDIRTY, which suggests a subroutine to handle a single page?

I'll try to see if this turns out okay-ish. Maybe I'll add via a
separate patch on top.

> 
> That said, what's here looks correct so
> Reviewed-by: Richard Henderson <address@hidden>
> 

Thanks!


-- 

Thanks,

David / dhildenb