On 22.09.19 05:54, Richard Henderson wrote: > It does not require going through the whole I/O path > in order to discard a write. > > 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 1ebd1b59ab..9f0b17802e 100644 > --- a/include/exec/cpu-all.h > +++ b/include/exec/cpu-all.h > @@ -348,12 +348,15 @@ CPUArchState *cpu_copy(CPUArchState *env); > #define TLB_WATCHPOINT (1 << (TARGET_PAGE_BITS_MIN - 4)) > /* Set if TLB entry requires byte swap. */ > #define TLB_BSWAP (1 << (TARGET_PAGE_BITS_MIN - 5)) > +/* Set if TLB entry writes ignored. */ > +#define TLB_ROM (1 << (TARGET_PAGE_BITS_MIN - 6))
I was wondering if TLB_DISCARD_WRITE/TLB_IGNORE_WRITE/TLB_READONLY would make it clearer what's actually happening here. E.g., it isn't used for memory_region_is_romd(section->mr) but only for memory_region_is_ram(section->mr) && section->readonly. But anyhow, changes look fine to me Reviewed-by: David Hildenbrand <da...@redhat.com> > > /* Use this mask to check interception with an alignment mask > * in a TCG backend. > */ > #define TLB_FLAGS_MASK \ > - (TLB_INVALID_MASK | TLB_NOTDIRTY | TLB_MMIO | TLB_WATCHPOINT | TLB_BSWAP) > + (TLB_INVALID_MASK | TLB_NOTDIRTY | TLB_MMIO \ > + | TLB_WATCHPOINT | TLB_BSWAP | TLB_ROM) > > /** > * tlb_hit_page: return true if page aligned @addr is a hit against the > diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h > index f7dbe75fbc..1c0e03ddc2 100644 > --- a/include/exec/cpu-common.h > +++ b/include/exec/cpu-common.h > @@ -100,7 +100,6 @@ void qemu_flush_coalesced_mmio_buffer(void); > > void cpu_flush_icache_range(hwaddr start, hwaddr len); > > -extern struct MemoryRegion io_mem_rom; > extern struct MemoryRegion io_mem_notdirty; > > typedef int (RAMBlockIterFunc)(RAMBlock *rb, void *opaque); > diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c > index cb603917a2..7ab523d7ec 100644 > --- a/accel/tcg/cputlb.c > +++ b/accel/tcg/cputlb.c > @@ -577,7 +577,7 @@ static void tlb_reset_dirty_range_locked(CPUTLBEntry > *tlb_entry, > { > uintptr_t addr = tlb_entry->addr_write; > > - if ((addr & (TLB_INVALID_MASK | TLB_MMIO | TLB_NOTDIRTY)) == 0) { > + if ((addr & (TLB_INVALID_MASK | TLB_MMIO | TLB_ROM | TLB_NOTDIRTY)) == > 0) { > addr &= TARGET_PAGE_MASK; > addr += tlb_entry->addend; > if ((addr - start) < length) { > @@ -745,7 +745,6 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong > vaddr, > address |= TLB_MMIO; > addend = 0; > } else { > - /* TLB_MMIO for rom/romd handled below */ > addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) + xlat; > } > > @@ -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; > + } > } > if (prot & PAGE_WRITE_INV) { > tn.addr_write |= TLB_INVALID_MASK; > @@ -904,7 +904,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry > *iotlbentry, > mr = section->mr; > mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr; > cpu->mem_io_pc = retaddr; > - if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) { > + if (mr != &io_mem_notdirty && !cpu->can_do_io) { > cpu_io_recompile(cpu, retaddr); > } > > @@ -945,7 +945,7 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry > *iotlbentry, > section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs); > mr = section->mr; > mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr; > - if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) { > + if (mr != &io_mem_notdirty && !cpu->can_do_io) { > cpu_io_recompile(cpu, retaddr); > } > cpu->mem_io_vaddr = addr; > @@ -1125,7 +1125,7 @@ void *probe_access(CPUArchState *env, target_ulong > addr, int size, > } > > /* Reject I/O access, or other required slow-path. */ > - if (tlb_addr & (TLB_NOTDIRTY | TLB_MMIO | TLB_BSWAP)) { > + if (tlb_addr & (TLB_NOTDIRTY | TLB_MMIO | TLB_BSWAP | TLB_ROM)) { > return NULL; > } > > @@ -1612,6 +1612,11 @@ store_helper(CPUArchState *env, target_ulong addr, > uint64_t val, > return; > } > > + /* Ignore writes to ROM. */ > + if (unlikely(tlb_addr & TLB_ROM)) { > + return; > + } > + > haddr = (void *)((uintptr_t)addr + entry->addend); > > if (unlikely(tlb_addr & TLB_BSWAP)) { > diff --git a/exec.c b/exec.c > index 7ce0515635..e21e068535 100644 > --- a/exec.c > +++ b/exec.c > @@ -88,7 +88,7 @@ static MemoryRegion *system_io; > AddressSpace address_space_io; > AddressSpace address_space_memory; > > -MemoryRegion io_mem_rom, io_mem_notdirty; > +MemoryRegion io_mem_notdirty; > static MemoryRegion io_mem_unassigned; > #endif > > @@ -158,7 +158,6 @@ typedef struct subpage_t { > > #define PHYS_SECTION_UNASSIGNED 0 > #define PHYS_SECTION_NOTDIRTY 1 > -#define PHYS_SECTION_ROM 2 > > static void io_mem_init(void); > static void memory_map_init(void); > @@ -1441,8 +1440,6 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu, > iotlb = memory_region_get_ram_addr(section->mr) + xlat; > if (!section->readonly) { > iotlb |= PHYS_SECTION_NOTDIRTY; > - } else { > - iotlb |= PHYS_SECTION_ROM; > } > } else { > AddressSpaceDispatch *d; > @@ -2968,38 +2965,6 @@ static uint16_t dummy_section(PhysPageMap *map, > FlatView *fv, MemoryRegion *mr) > return phys_section_add(map, §ion); > } > > -static void readonly_mem_write(void *opaque, hwaddr addr, > - uint64_t val, unsigned size) > -{ > - /* Ignore any write to ROM. */ > -} > - > -static bool readonly_mem_accepts(void *opaque, hwaddr addr, > - unsigned size, bool is_write, > - MemTxAttrs attrs) > -{ > - return is_write; > -} > - > -/* This will only be used for writes, because reads are special cased > - * to directly access the underlying host ram. > - */ > -static const MemoryRegionOps readonly_mem_ops = { > - .write = readonly_mem_write, > - .valid.accepts = readonly_mem_accepts, > - .endianness = DEVICE_NATIVE_ENDIAN, > - .valid = { > - .min_access_size = 1, > - .max_access_size = 8, > - .unaligned = false, > - }, > - .impl = { > - .min_access_size = 1, > - .max_access_size = 8, > - .unaligned = false, > - }, > -}; > - > MemoryRegionSection *iotlb_to_section(CPUState *cpu, > hwaddr index, MemTxAttrs attrs) > { > @@ -3013,8 +2978,6 @@ MemoryRegionSection *iotlb_to_section(CPUState *cpu, > > static void io_mem_init(void) > { > - memory_region_init_io(&io_mem_rom, NULL, &readonly_mem_ops, > - NULL, NULL, UINT64_MAX); > memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, > NULL, > NULL, UINT64_MAX); > > @@ -3035,8 +2998,6 @@ AddressSpaceDispatch > *address_space_dispatch_new(FlatView *fv) > assert(n == PHYS_SECTION_UNASSIGNED); > n = dummy_section(&d->map, fv, &io_mem_notdirty); > assert(n == PHYS_SECTION_NOTDIRTY); > - n = dummy_section(&d->map, fv, &io_mem_rom); > - assert(n == PHYS_SECTION_ROM); > > d->phys_map = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 }; > > -- Thanks, David / dhildenb