On 25.09.19 02:16, Alex Bennée wrote:
>
> Richard Henderson <richard.hender...@linaro.org> writes:
>
>> It does not require going through the whole I/O path
>> in order to discard a write.
>>
>> Reviewed-by: David Hildenbrand <da...@redhat.com>
>> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>> ---
>> include/exec/cpu-all.h | 5 ++++-
>> include/exec/cpu-common.h | 1 -
>> accel/tcg/cputlb.c | 35 +++++++++++++++++++--------------
>> exec.c | 41 +--------------------------------------
>> 4 files changed, 25 insertions(+), 57 deletions(-)
>>
>> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
>> index d148bded35..26547cd6dd 100644
>> --- a/include/exec/cpu-all.h
>> +++ b/include/exec/cpu-all.h
> <snip>
>> @@ -822,16 +821,17 @@ void tlb_set_page_with_attrs(CPUState *cpu,
>> target_ulong vaddr,
>>
>> tn.addr_write = -1;
>> if (prot & PAGE_WRITE) {
>> - if ((memory_region_is_ram(section->mr) && section->readonly)
>> - || memory_region_is_romd(section->mr)) {
>> - /* Write access calls the I/O callback. */
>> - tn.addr_write = address | TLB_MMIO;
>> - } else if (memory_region_is_ram(section->mr)
>> - && cpu_physical_memory_is_clean(
>> - memory_region_get_ram_addr(section->mr) + xlat)) {
>> - tn.addr_write = address | TLB_NOTDIRTY;
>> - } else {
>> - tn.addr_write = address;
>> + tn.addr_write = address;
>> + if (memory_region_is_romd(section->mr)) {
>> + /* Use the MMIO path so that the device can switch states. */
>> + tn.addr_write |= TLB_MMIO;
>> + } else if (memory_region_is_ram(section->mr)) {
>> + if (section->readonly) {
>> + tn.addr_write |= TLB_ROM;
>> + } else if (cpu_physical_memory_is_clean(
>> + memory_region_get_ram_addr(section->mr) + xlat)) {
>> + tn.addr_write |= TLB_NOTDIRTY;
>> + }
>
> This reads a bit weird because we are saying romd isn't a ROM but
> something that identifies as RAM can be ROM rather than just a memory
> protected piece of RAM.
>
I proposed a bunch of alternatives as reply to v3 (e.g.,
TLB_DISCARD_WRITES), either Richard missed them or I missed his reply :)
>> }
>> if (prot & PAGE_WRITE_INV) {
>> tn.addr_write |= TLB_INVALID_MASK;
>
> So at the moment I don't see what the TLB_ROM flag gives us that setting
> TLB_INVALID doesn't - either way we won't make the write to our
> ram-not-ram-rom.
TLB_INVALID will trigger a new MMU translation on every access to fill
the TLB. TLB_ROM states that we have a valid entry, but that writes are
to be discarded.
--
Thanks,
David / dhildenb
