Re: [PATCH 04/17] target/arm: Raise alignment fault for DC_ZVA on Device memory

On 7/28/26 07:21, Peter Maydell wrote:
>> +    flags = probe_access_full(env, addr, 0, MMU_DATA_STORE, mmu_idx,
>> +                              false, &mem, &full, ra);
> 
> Is 0 really the right size to pass in here? Looking at
> probe_access_full(), if you pass in a 0 size then it will
> call notdirty_write() with a dirtysize parameter of 1,
> which will then not invalidate or set the VGA/migration
> dirty bits on the full range of memory we're about to clear.
> This would be OK if we then went through the slowpath and
> did a set of byte accesses, but probe_access_full() will
> clear TLB_NOTDIRTY from the flags it returns to us if it
> called notdirty_write(), so we will take the fastpath if
> that was the only flag set.
> 
> Maybe probe_access_full(), probe_access_full_mmu() and
> probe_access_flags() should not call notdirty_write() and
> clear TLB_NOTDIRTY from the flags they return if the passed
> in size is 0 ?
> 
> The notdirty_write() API also wants the base address of the
> DC ZVA block (since that's where we will be writing from),
> not the address from the guest.
> 
> (We may be getting away with some of this because of various
> functions rounding up/down to full page addresses, but none
> of these functions seem to document that, so we shouldn't
> rely on it.)

That's exactly what's happening: physical_memory_set_dirty_range takes a range, but then 
widens it to the surrounding page boundaries as does tb_invalidate_phys_range_fast.

I'm certainly open to rearranging the (entire) API here, but it's not immediately clear 
what form that should take.

Perhaps instead of the (addr, size) pair, we should have (addr, first, last) triple, where

     first <= addr <= last

Normally, addr == first.  But for these cases like DC_ZVA, addr would be what is reported 
to when faulting, but the surrounding [first, last] would be used for 
watchpoints/notdirty, etc.  With this api, the rather confusing size == 0 is not a thing.

Hmm, if we remove size, what to do when passing size down to tlb_fill?  Do we need to 
expand that API too for the benefit of the range-based protection mechanisms (Arm PMSA, 
RISC-V PMP).

Thoughts?


r~