[PATCH v3 20/20] cputlb: Pass retaddr to tb_check_watchpoint

[Richard Henderson]

Fixes the previous TLB_WATCHPOINT patches because we are currently
failing to set cpu->mem_io_pc with the call to cpu_check_watchpoint.
Pass down the retaddr directly because it's readily available.

Fixes: 50b107c5d61
Signed-off-by: Richard Henderson 
---
 accel/tcg/translate-all.h | 2 +-
 accel/tcg/translate-all.c | 6 +++---
 exec.c| 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/accel/tcg/translate-all.h b/accel/tcg/translate-all.h
index 135c1ea96a..a557b4e2bb 100644
--- a/accel/tcg/translate-all.h
+++ b/accel/tcg/translate-all.h
@@ -30,7 +30,7 @@ void tb_invalidate_phys_page_fast(struct page_collection 
*pages,
   tb_page_addr_t start, int len,
   uintptr_t retaddr);
 void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end);
-void tb_check_watchpoint(CPUState *cpu);
+void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr);
 
 #ifdef CONFIG_USER_ONLY
 int page_unprotect(target_ulong address, uintptr_t pc);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index db77fb221b..66d4bc4341 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2142,16 +2142,16 @@ static bool tb_invalidate_phys_page(tb_page_addr_t 
addr, uintptr_t pc)
 #endif
 
 /* user-mode: call with mmap_lock held */
-void tb_check_watchpoint(CPUState *cpu)
+void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr)
 {
 TranslationBlock *tb;
 
 assert_memory_lock();
 
-tb = tcg_tb_lookup(cpu->mem_io_pc);
+tb = tcg_tb_lookup(retaddr);
 if (tb) {
 /* We can use retranslation to find the PC.  */
-cpu_restore_state_from_tb(cpu, tb, cpu->mem_io_pc, true);
+cpu_restore_state_from_tb(cpu, tb, retaddr, true);
 tb_phys_invalidate(tb, -1);
 } else {
 /* The exception probably happened in a helper.  The CPU state should
diff --git a/exec.c b/exec.c
index fed25d029b..ceeef4cd4b 100644
--- a/exec.c
+++ b/exec.c
@@ -2724,7 +2724,7 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, 
vaddr len,
 cpu->watchpoint_hit = wp;
 
 mmap_lock();
-tb_check_watchpoint(cpu);
+tb_check_watchpoint(cpu, ra);
 if (wp->flags & BP_STOP_BEFORE_ACCESS) {
 cpu->exception_index = EXCP_DEBUG;
 mmap_unlock();
-- 
2.17.1

[PATCH v3 18/20] cputlb: Remove tb_invalidate_phys_page_range is_cpu_write_access

[Richard Henderson]

All callers pass false to this argument.  Remove it and pass the
constant on to tb_invalidate_phys_page_range__locked.

Signed-off-by: Richard Henderson 
---
 accel/tcg/translate-all.h | 3 +--
 accel/tcg/translate-all.c | 6 ++
 exec.c| 4 ++--
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/accel/tcg/translate-all.h b/accel/tcg/translate-all.h
index 64f5fd9a05..31f2117188 100644
--- a/accel/tcg/translate-all.h
+++ b/accel/tcg/translate-all.h
@@ -28,8 +28,7 @@ struct page_collection *page_collection_lock(tb_page_addr_t 
start,
 void page_collection_unlock(struct page_collection *set);
 void tb_invalidate_phys_page_fast(struct page_collection *pages,
   tb_page_addr_t start, int len);
-void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
-   int is_cpu_write_access);
+void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end);
 void tb_check_watchpoint(CPUState *cpu);
 
 #ifdef CONFIG_USER_ONLY
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 5d1e08b169..de4b697163 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1983,8 +1983,7 @@ tb_invalidate_phys_page_range__locked(struct 
page_collection *pages,
  *
  * Called with mmap_lock held for user-mode emulation
  */
-void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
-   int is_cpu_write_access)
+void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end)
 {
 struct page_collection *pages;
 PageDesc *p;
@@ -1996,8 +1995,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, 
tb_page_addr_t end,
 return;
 }
 pages = page_collection_lock(start, end);
-tb_invalidate_phys_page_range__locked(pages, p, start, end,
-  is_cpu_write_access);
+tb_invalidate_phys_page_range__locked(pages, p, start, end, 0);
 page_collection_unlock(pages);
 }
 
diff --git a/exec.c b/exec.c
index 090bcc05da..fed25d029b 100644
--- a/exec.c
+++ b/exec.c
@@ -978,7 +978,7 @@ const char *parse_cpu_option(const char *cpu_option)
 void tb_invalidate_phys_addr(target_ulong addr)
 {
 mmap_lock();
-tb_invalidate_phys_page_range(addr, addr + 1, 0);
+tb_invalidate_phys_page_range(addr, addr + 1);
 mmap_unlock();
 }
 
@@ -1005,7 +1005,7 @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr 
addr, MemTxAttrs attrs)
 return;
 }
 ram_addr = memory_region_get_ram_addr(mr) + addr;
-tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0);
+tb_invalidate_phys_page_range(ram_addr, ram_addr + 1);
 rcu_read_unlock();
 }
 
-- 
2.17.1

Re: [PATCH v3 00/20] Move rom and notdirty handling to cputlb

[Richard Henderson]

On 9/21/19 8:54 PM, Richard Henderson wrote:
> Richard Henderson (20):
>   exec: Use TARGET_PAGE_BITS_MIN for TLB flags
>   exec: Split out variable page size support to exec-vary.c
>   exec: Use const alias for TARGET_PAGE_BITS_VARY
>   exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG
>   exec: Promote TARGET_PAGE_MASK to target_long
>   exec: Tidy TARGET_PAGE_ALIGN
>   exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY

I didn't mean to include the TARGET_PAGE_BITS_VARY patches.
Ignore the first 7, or review them at your pleasure.  ;-)


r~

[PATCH v3 19/20] cputlb: Pass retaddr to tb_invalidate_phys_page_fast

[Richard Henderson]

Rather than rely on cpu->mem_io_pc, pass retaddr down directly.

Within tb_invalidate_phys_page_range__locked, the is_cpu_write_access
parameter is non-zero exactly when retaddr would be non-zero, so that
is a simple replacement.

Recognize that current_tb_not_found is true only when mem_io_pc
(and now retaddr) are also non-zero, so remove a redundant test.

Signed-off-by: Richard Henderson 
---
 accel/tcg/translate-all.h |  3 ++-
 accel/tcg/cputlb.c|  6 +-
 accel/tcg/translate-all.c | 39 +++
 3 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/accel/tcg/translate-all.h b/accel/tcg/translate-all.h
index 31f2117188..135c1ea96a 100644
--- a/accel/tcg/translate-all.h
+++ b/accel/tcg/translate-all.h
@@ -27,7 +27,8 @@ struct page_collection *page_collection_lock(tb_page_addr_t 
start,
  tb_page_addr_t end);
 void page_collection_unlock(struct page_collection *set);
 void tb_invalidate_phys_page_fast(struct page_collection *pages,
-  tb_page_addr_t start, int len);
+  tb_page_addr_t start, int len,
+  uintptr_t retaddr);
 void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end);
 void tb_check_watchpoint(CPUState *cpu);
 
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 257c59c08c..eff129447d 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1093,11 +1093,7 @@ static void notdirty_write(CPUState *cpu, vaddr 
mem_vaddr, unsigned size,
 if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
 struct page_collection *pages
 = page_collection_lock(ram_addr, ram_addr + size);
-
-/* We require mem_io_pc in tb_invalidate_phys_page_range.  */
-cpu->mem_io_pc = retaddr;
-
-tb_invalidate_phys_page_fast(pages, ram_addr, size);
+tb_invalidate_phys_page_fast(pages, ram_addr, size, retaddr);
 page_collection_unlock(pages);
 }
 
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index de4b697163..db77fb221b 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1889,7 +1889,7 @@ static void
 tb_invalidate_phys_page_range__locked(struct page_collection *pages,
   PageDesc *p, tb_page_addr_t start,
   tb_page_addr_t end,
-  int is_cpu_write_access)
+  uintptr_t retaddr)
 {
 TranslationBlock *tb;
 tb_page_addr_t tb_start, tb_end;
@@ -1897,9 +1897,9 @@ tb_invalidate_phys_page_range__locked(struct 
page_collection *pages,
 #ifdef TARGET_HAS_PRECISE_SMC
 CPUState *cpu = current_cpu;
 CPUArchState *env = NULL;
-int current_tb_not_found = is_cpu_write_access;
+bool current_tb_not_found = retaddr != 0;
+bool current_tb_modified = false;
 TranslationBlock *current_tb = NULL;
-int current_tb_modified = 0;
 target_ulong current_pc = 0;
 target_ulong current_cs_base = 0;
 uint32_t current_flags = 0;
@@ -1931,24 +1931,21 @@ tb_invalidate_phys_page_range__locked(struct 
page_collection *pages,
 if (!(tb_end <= start || tb_start >= end)) {
 #ifdef TARGET_HAS_PRECISE_SMC
 if (current_tb_not_found) {
-current_tb_not_found = 0;
-current_tb = NULL;
-if (cpu->mem_io_pc) {
-/* now we have a real cpu fault */
-current_tb = tcg_tb_lookup(cpu->mem_io_pc);
-}
+current_tb_not_found = false;
+/* now we have a real cpu fault */
+current_tb = tcg_tb_lookup(retaddr);
 }
 if (current_tb == tb &&
 (tb_cflags(current_tb) & CF_COUNT_MASK) != 1) {
-/* If we are modifying the current TB, we must stop
-its execution. We could be more precise by checking
-that the modification is after the current PC, but it
-would require a specialized function to partially
-restore the CPU state */
-
-current_tb_modified = 1;
-cpu_restore_state_from_tb(cpu, current_tb,
-  cpu->mem_io_pc, true);
+/*
+ * If we are modifying the current TB, we must stop
+ * its execution. We could be more precise by checking
+ * that the modification is after the current PC, but it
+ * would require a specialized function to partially
+ * restore the CPU state.
+ */
+current_tb_modified = true;
+cpu_restore_state_from_tb(cpu, current_tb, retaddr, true);
 cpu_get_tb_cpu_state(env, _pc, _cs_base,
  _flags);
 

[PATCH v3 16/20] cputlb: Handle TLB_NOTDIRTY in probe_access

[Richard Henderson]

We can use notdirty_write for the write and
return a valid host pointer for this case.

Signed-off-by: Richard Henderson 
---
 accel/tcg/cputlb.c | 26 +-
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 6f685cb93a..6f4096bd0d 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1167,16 +1167,24 @@ void *probe_access(CPUArchState *env, target_ulong 
addr, int size,
 return NULL;
 }
 
-/* Handle watchpoints.  */
-if (tlb_addr & TLB_WATCHPOINT) {
-cpu_check_watchpoint(env_cpu(env), addr, size,
- env_tlb(env)->d[mmu_idx].iotlb[index].attrs,
- wp_access, retaddr);
-}
+if (unlikely(tlb_addr & TLB_FLAGS_MASK)) {
+CPUIOTLBEntry *iotlbentry = _tlb(env)->d[mmu_idx].iotlb[index];
 
-/* Reject I/O access, or other required slow-path.  */
-if (tlb_addr & (TLB_NOTDIRTY | TLB_MMIO | TLB_BSWAP | TLB_ROM)) {
-return NULL;
+/* Reject I/O access, or other required slow-path.  */
+if (tlb_addr & (TLB_MMIO | TLB_BSWAP | TLB_ROM)) {
+return NULL;
+}
+
+/* Handle watchpoints.  */
+if (tlb_addr & TLB_WATCHPOINT) {
+cpu_check_watchpoint(env_cpu(env), addr, size,
+ iotlbentry->attrs, wp_access, retaddr);
+}
+
+/* Handle clean RAM pages.  */
+if (tlb_addr & TLB_NOTDIRTY) {
+notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr);
+}
 }
 
 return (void *)((uintptr_t)addr + entry->addend);
-- 
2.17.1

[PATCH v3 17/20] cputlb: Remove cpu->mem_io_vaddr

[Richard Henderson]

With the merge of notdirty handling into store_helper,
the last user of cpu->mem_io_vaddr was removed.

Signed-off-by: Richard Henderson 
---
 include/hw/core/cpu.h | 2 --
 accel/tcg/cputlb.c| 2 --
 hw/core/cpu.c | 1 -
 3 files changed, 5 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index c7cda65c66..031f587e51 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -338,7 +338,6 @@ struct qemu_work_item;
  * @next_cpu: Next CPU sharing TB cache.
  * @opaque: User data.
  * @mem_io_pc: Host Program Counter at which the memory was accessed.
- * @mem_io_vaddr: Target virtual address at which the memory was accessed.
  * @kvm_fd: vCPU file descriptor for KVM.
  * @work_mutex: Lock to prevent multiple access to queued_work_*.
  * @queued_work_first: First asynchronous work pending.
@@ -413,7 +412,6 @@ struct CPUState {
  * we store some rarely used information in the CPU context.
  */
 uintptr_t mem_io_pc;
-vaddr mem_io_vaddr;
 /*
  * This is only needed for the legacy cpu_unassigned_access() hook;
  * when all targets using it have been converted to use
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 6f4096bd0d..257c59c08c 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -927,7 +927,6 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry 
*iotlbentry,
 cpu_io_recompile(cpu, retaddr);
 }
 
-cpu->mem_io_vaddr = addr;
 cpu->mem_io_access_type = access_type;
 
 if (mr->global_locking && !qemu_mutex_iothread_locked()) {
@@ -967,7 +966,6 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
*iotlbentry,
 if (!cpu->can_do_io) {
 cpu_io_recompile(cpu, retaddr);
 }
-cpu->mem_io_vaddr = addr;
 cpu->mem_io_pc = retaddr;
 
 if (mr->global_locking && !qemu_mutex_iothread_locked()) {
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 0035845511..73b1ee34d0 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -261,7 +261,6 @@ static void cpu_common_reset(CPUState *cpu)
 cpu->interrupt_request = 0;
 cpu->halted = 0;
 cpu->mem_io_pc = 0;
-cpu->mem_io_vaddr = 0;
 cpu->icount_extra = 0;
 atomic_set(>icount_decr_ptr->u32, 0);
 cpu->can_do_io = 1;
-- 
2.17.1

[PATCH v3 13/20] cputlb: Move NOTDIRTY handling from I/O path to TLB path

[Richard Henderson]

Pages that we want to track for NOTDIRTY are RAM.  We do not
really need to go through the I/O path to handle them.

Signed-off-by: Richard Henderson 
---
 include/exec/cpu-common.h |  2 --
 accel/tcg/cputlb.c| 26 +---
 exec.c| 50 ---
 memory.c  | 16 -
 4 files changed, 23 insertions(+), 71 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 1c0e03ddc2..81753bbb34 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -100,8 +100,6 @@ void qemu_flush_coalesced_mmio_buffer(void);
 
 void cpu_flush_icache_range(hwaddr start, hwaddr len);
 
-extern struct MemoryRegion io_mem_notdirty;
-
 typedef int (RAMBlockIterFunc)(RAMBlock *rb, void *opaque);
 
 int qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque);
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 7ab523d7ec..b7bd738115 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -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 != _mem_notdirty && !cpu->can_do_io) {
+if (!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 != _mem_notdirty && !cpu->can_do_io) {
+if (!cpu->can_do_io) {
 cpu_io_recompile(cpu, retaddr);
 }
 cpu->mem_io_vaddr = addr;
@@ -1606,7 +1606,7 @@ store_helper(CPUArchState *env, target_ulong addr, 
uint64_t val,
 }
 
 /* Handle I/O access.  */
-if (likely(tlb_addr & (TLB_MMIO | TLB_NOTDIRTY))) {
+if (tlb_addr & TLB_MMIO) {
 io_writex(env, iotlbentry, mmu_idx, val, addr, retaddr,
   op ^ (tlb_addr & TLB_BSWAP ? MO_BSWAP : 0));
 return;
@@ -1619,6 +1619,26 @@ store_helper(CPUArchState *env, target_ulong addr, 
uint64_t val,
 
 haddr = (void *)((uintptr_t)addr + entry->addend);
 
+/* Handle clean RAM pages.  */
+if (tlb_addr & TLB_NOTDIRTY) {
+NotDirtyInfo ndi;
+
+/* We require mem_io_pc in tb_invalidate_phys_page_range.  */
+env_cpu(env)->mem_io_pc = retaddr;
+
+memory_notdirty_write_prepare(, env_cpu(env), addr,
+  addr + iotlbentry->addr, size);
+
+if (unlikely(tlb_addr & TLB_BSWAP)) {
+direct_swap(haddr, val);
+} else {
+direct(haddr, val);
+}
+
+memory_notdirty_write_complete();
+return;
+}
+
 if (unlikely(tlb_addr & TLB_BSWAP)) {
 direct_swap(haddr, val);
 } else {
diff --git a/exec.c b/exec.c
index e21e068535..abf58b68a0 100644
--- a/exec.c
+++ b/exec.c
@@ -88,7 +88,6 @@ static MemoryRegion *system_io;
 AddressSpace address_space_io;
 AddressSpace address_space_memory;
 
-MemoryRegion io_mem_notdirty;
 static MemoryRegion io_mem_unassigned;
 #endif
 
@@ -157,7 +156,6 @@ typedef struct subpage_t {
 } subpage_t;
 
 #define PHYS_SECTION_UNASSIGNED 0
-#define PHYS_SECTION_NOTDIRTY 1
 
 static void io_mem_init(void);
 static void memory_map_init(void);
@@ -1438,9 +1436,6 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
 if (memory_region_is_ram(section->mr)) {
 /* Normal RAM.  */
 iotlb = memory_region_get_ram_addr(section->mr) + xlat;
-if (!section->readonly) {
-iotlb |= PHYS_SECTION_NOTDIRTY;
-}
 } else {
 AddressSpaceDispatch *d;
 
@@ -2749,42 +2744,6 @@ void memory_notdirty_write_complete(NotDirtyInfo *ndi)
 }
 }
 
-/* Called within RCU critical section.  */
-static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
-   uint64_t val, unsigned size)
-{
-NotDirtyInfo ndi;
-
-memory_notdirty_write_prepare(, current_cpu, current_cpu->mem_io_vaddr,
- ram_addr, size);
-
-stn_p(qemu_map_ram_ptr(NULL, ram_addr), size, val);
-memory_notdirty_write_complete();
-}
-
-static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
- unsigned size, bool is_write,
- MemTxAttrs attrs)
-{
-return is_write;
-}
-
-static const MemoryRegionOps notdirty_mem_ops = {
-.write = notdirty_mem_write,
-.valid.accepts = notdirty_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 = 

[PATCH v3 14/20] cputlb: Partially inline memory_region_section_get_iotlb

[Richard Henderson]

There is only one caller, tlb_set_page_with_attrs.  We cannot
inline the entire function because the AddressSpaceDispatch
structure is private to exec.c, and cannot easily be moved to
include/exec/memory-internal.h.

Compute is_ram and is_romd once within tlb_set_page_with_attrs.
Fold the number of tests against these predicates.  Compute
cpu_physical_memory_is_clean outside of the tlb lock region.

Signed-off-by: Richard Henderson 
---
 include/exec/exec-all.h |  6 +---
 accel/tcg/cputlb.c  | 68 ++---
 exec.c  | 22 ++---
 3 files changed, 47 insertions(+), 49 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 81b02eb2fe..49db07ba0b 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -509,11 +509,7 @@ address_space_translate_for_iotlb(CPUState *cpu, int 
asidx, hwaddr addr,
   hwaddr *xlat, hwaddr *plen,
   MemTxAttrs attrs, int *prot);
 hwaddr memory_region_section_get_iotlb(CPUState *cpu,
-   MemoryRegionSection *section,
-   target_ulong vaddr,
-   hwaddr paddr, hwaddr xlat,
-   int prot,
-   target_ulong *address);
+   MemoryRegionSection *section);
 #endif
 
 /* vl.c */
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index b7bd738115..1a839c0f82 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -704,13 +704,14 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong 
vaddr,
 MemoryRegionSection *section;
 unsigned int index;
 target_ulong address;
-target_ulong code_address;
+target_ulong write_address;
 uintptr_t addend;
 CPUTLBEntry *te, tn;
 hwaddr iotlb, xlat, sz, paddr_page;
 target_ulong vaddr_page;
 int asidx = cpu_asidx_from_attrs(cpu, attrs);
 int wp_flags;
+bool is_ram, is_romd;
 
 assert_cpu_is_self(cpu);
 
@@ -739,18 +740,46 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong 
vaddr,
 if (attrs.byte_swap) {
 address |= TLB_BSWAP;
 }
-if (!memory_region_is_ram(section->mr) &&
-!memory_region_is_romd(section->mr)) {
-/* IO memory case */
-address |= TLB_MMIO;
-addend = 0;
-} else {
+
+is_ram = memory_region_is_ram(section->mr);
+is_romd = memory_region_is_romd(section->mr);
+
+if (is_ram || is_romd) {
+/* RAM and ROMD both have associated host memory. */
 addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) + xlat;
+} else {
+/* I/O does not; force the host address to NULL. */
+addend = 0;
+}
+
+write_address = address;
+if (is_ram) {
+iotlb = memory_region_get_ram_addr(section->mr) + xlat;
+/*
+ * Computing is_clean is expensive; avoid all that unless
+ * the page is actually writable.
+ */
+if (prot & PAGE_WRITE) {
+if (section->readonly) {
+write_address |= TLB_ROM;
+} else if (cpu_physical_memory_is_clean(iotlb)) {
+write_address |= TLB_NOTDIRTY;
+}
+}
+} else {
+/* I/O or ROMD */
+iotlb = memory_region_section_get_iotlb(cpu, section) + xlat;
+/*
+ * Writes to romd devices must go through MMIO to enable write.
+ * Reads to romd devices go through the ram_ptr found above,
+ * but of course reads to I/O must go through MMIO.
+ */
+write_address |= TLB_MMIO;
+if (!is_romd) {
+address = write_address;
+}
 }
 
-code_address = address;
-iotlb = memory_region_section_get_iotlb(cpu, section, vaddr_page,
-paddr_page, xlat, prot, );
 wp_flags = cpu_watchpoint_address_matches(cpu, vaddr_page,
   TARGET_PAGE_SIZE);
 
@@ -790,8 +819,8 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong 
vaddr,
 /*
  * At this point iotlb contains a physical section number in the lower
  * TARGET_PAGE_BITS, and either
- *  + the ram_addr_t of the page base of the target RAM (if NOTDIRTY or 
ROM)
- *  + the offset within section->mr of the page base (otherwise)
+ *  + the ram_addr_t of the page base of the target RAM (RAM)
+ *  + the offset within section->mr of the page base (I/O, ROMD)
  * We subtract the vaddr_page (which is page aligned and thus won't
  * disturb the low bits) to give an offset which can be added to the
  * (non-page-aligned) vaddr of the eventual memory access to get
@@ -814,25 +843,14 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong 
vaddr,
 }
 
 if (prot & PAGE_EXEC) {
-tn.addr_code = code_address;
+tn.addr_code 

[PATCH v3 15/20] cputlb: Merge and move memory_notdirty_write_{prepare, complete}

[Richard Henderson]

Since 9458a9a1df1a, all readers of the dirty bitmaps wait
for the rcu lock, which means that they wait until the end
of any executing TranslationBlock.

As a consequence, there is no need for the actual access
to happen in between the _prepare and _complete.  Therefore,
we can improve things by merging the two functions into
notdirty_write and dropping the NotDirtyInfo structure.

In addition, the only users of notdirty_write are in cputlb.c,
so move the merged function there.  Pass in the CPUIOTLBEntry
from which the ram_addr_t may be computed.

Signed-off-by: Richard Henderson 
---
 include/exec/memory-internal.h | 65 -
 accel/tcg/cputlb.c | 76 +++---
 exec.c | 44 
 3 files changed, 42 insertions(+), 143 deletions(-)

diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index ef4fb92371..9fcc2af25c 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -49,70 +49,5 @@ void address_space_dispatch_free(AddressSpaceDispatch *d);
 
 void mtree_print_dispatch(struct AddressSpaceDispatch *d,
   MemoryRegion *root);
-
-struct page_collection;
-
-/* Opaque struct for passing info from memory_notdirty_write_prepare()
- * to memory_notdirty_write_complete(). Callers should treat all fields
- * as private, with the exception of @active.
- *
- * @active is a field which is not touched by either the prepare or
- * complete functions, but which the caller can use if it wishes to
- * track whether it has called prepare for this struct and so needs
- * to later call the complete function.
- */
-typedef struct {
-CPUState *cpu;
-struct page_collection *pages;
-ram_addr_t ram_addr;
-vaddr mem_vaddr;
-unsigned size;
-bool active;
-} NotDirtyInfo;
-
-/**
- * memory_notdirty_write_prepare: call before writing to non-dirty memory
- * @ndi: pointer to opaque NotDirtyInfo struct
- * @cpu: CPU doing the write
- * @mem_vaddr: virtual address of write
- * @ram_addr: the ram address of the write
- * @size: size of write in bytes
- *
- * Any code which writes to the host memory corresponding to
- * guest RAM which has been marked as NOTDIRTY must wrap those
- * writes in calls to memory_notdirty_write_prepare() and
- * memory_notdirty_write_complete():
- *
- *  NotDirtyInfo ndi;
- *  memory_notdirty_write_prepare(, );
- *  ... perform write here ...
- *  memory_notdirty_write_complete();
- *
- * These calls will ensure that we flush any TCG translated code for
- * the memory being written, update the dirty bits and (if possible)
- * remove the slowpath callback for writing to the memory.
- *
- * This must only be called if we are using TCG; it will assert otherwise.
- *
- * We may take locks in the prepare call, so callers must ensure that
- * they don't exit (via longjump or otherwise) without calling complete.
- *
- * This call must only be made inside an RCU critical section.
- * (Note that while we're executing a TCG TB we're always in an
- * RCU critical section, which is likely to be the case for callers
- * of these functions.)
- */
-void memory_notdirty_write_prepare(NotDirtyInfo *ndi,
-   CPUState *cpu,
-   vaddr mem_vaddr,
-   ram_addr_t ram_addr,
-   unsigned size);
-/**
- * memory_notdirty_write_complete: finish write to non-dirty memory
- * @ndi: pointer to the opaque NotDirtyInfo struct which was initialized
- * by memory_not_dirty_write_prepare().
- */
-void memory_notdirty_write_complete(NotDirtyInfo *ndi);
-
 #endif
 #endif
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 1a839c0f82..6f685cb93a 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -33,6 +33,7 @@
 #include "exec/helper-proto.h"
 #include "qemu/atomic.h"
 #include "qemu/atomic128.h"
+#include "translate-all.h"
 
 /* DEBUG defines, enable DEBUG_TLB_LOG to log to the CPU_LOG_MMU target */
 /* #define DEBUG_TLB */
@@ -1084,6 +1085,37 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, 
target_ulong addr)
 return qemu_ram_addr_from_host_nofail(p);
 }
 
+static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
+   CPUIOTLBEntry *iotlbentry, uintptr_t retaddr)
+{
+ram_addr_t ram_addr = mem_vaddr + iotlbentry->addr;
+
+trace_memory_notdirty_write_access(mem_vaddr, ram_addr, size);
+
+if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
+struct page_collection *pages
+= page_collection_lock(ram_addr, ram_addr + size);
+
+/* We require mem_io_pc in tb_invalidate_phys_page_range.  */
+cpu->mem_io_pc = retaddr;
+
+tb_invalidate_phys_page_fast(pages, ram_addr, size);
+page_collection_unlock(pages);
+}
+
+/*
+ * Set both VGA and migration bits for simplicity and to 

[PATCH v3 11/20] exec: Adjust notdirty tracing

[Richard Henderson]

The memory_region_tb_read tracepoint is unreachable, since notdirty
is supposed to apply only to writes.  The memory_region_tb_write
tracepoint is mis-named, because notdirty is not only used for TB
invalidation.  It is also used for e.g. VGA RAM updates and migration.

Replace memory_region_tb_write with memory_notdirty_write_access,
and place it in memory_notdirty_write_prepare where it can catch
all of the instances.  Add memory_notdirty_set_dirty to log when
we no longer intercept writes to a page.

Reviewed-by: David Hildenbrand 
Signed-off-by: Richard Henderson 
---
 exec.c   | 3 +++
 memory.c | 4 
 trace-events | 4 ++--
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index 33bd0e36c1..7ce0515635 100644
--- a/exec.c
+++ b/exec.c
@@ -2721,6 +2721,8 @@ void memory_notdirty_write_prepare(NotDirtyInfo *ndi,
 ndi->size = size;
 ndi->pages = NULL;
 
+trace_memory_notdirty_write_access(mem_vaddr, ram_addr, size);
+
 assert(tcg_enabled());
 if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
 ndi->pages = page_collection_lock(ram_addr, ram_addr + size);
@@ -2745,6 +2747,7 @@ void memory_notdirty_write_complete(NotDirtyInfo *ndi)
 /* we remove the notdirty callback only if the code has been
flushed */
 if (!cpu_physical_memory_is_clean(ndi->ram_addr)) {
+trace_memory_notdirty_set_dirty(ndi->mem_vaddr);
 tlb_set_dirty(ndi->cpu, ndi->mem_vaddr);
 }
 }
diff --git a/memory.c b/memory.c
index b9dd6b94ca..57c44c97db 100644
--- a/memory.c
+++ b/memory.c
@@ -438,7 +438,6 @@ static MemTxResult  
memory_region_read_accessor(MemoryRegion *mr,
 /* Accesses to code which has previously been translated into a TB show
  * up in the MMIO path, as accesses to the io_mem_notdirty
  * MemoryRegion. */
-trace_memory_region_tb_read(get_cpu_index(), addr, tmp, size);
 } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
 hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
 trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
@@ -465,7 +464,6 @@ static MemTxResult 
memory_region_read_with_attrs_accessor(MemoryRegion *mr,
 /* Accesses to code which has previously been translated into a TB show
  * up in the MMIO path, as accesses to the io_mem_notdirty
  * MemoryRegion. */
-trace_memory_region_tb_read(get_cpu_index(), addr, tmp, size);
 } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
 hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
 trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
@@ -490,7 +488,6 @@ static MemTxResult 
memory_region_write_accessor(MemoryRegion *mr,
 /* Accesses to code which has previously been translated into a TB show
  * up in the MMIO path, as accesses to the io_mem_notdirty
  * MemoryRegion. */
-trace_memory_region_tb_write(get_cpu_index(), addr, tmp, size);
 } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
 hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
 trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, 
size);
@@ -515,7 +512,6 @@ static MemTxResult 
memory_region_write_with_attrs_accessor(MemoryRegion *mr,
 /* Accesses to code which has previously been translated into a TB show
  * up in the MMIO path, as accesses to the io_mem_notdirty
  * MemoryRegion. */
-trace_memory_region_tb_write(get_cpu_index(), addr, tmp, size);
 } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
 hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
 trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, 
size);
diff --git a/trace-events b/trace-events
index 823a4ae64e..20821ba545 100644
--- a/trace-events
+++ b/trace-events
@@ -52,14 +52,14 @@ dma_map_wait(void *dbs) "dbs=%p"
 find_ram_offset(uint64_t size, uint64_t offset) "size: 0x%" PRIx64 " @ 0x%" 
PRIx64
 find_ram_offset_loop(uint64_t size, uint64_t candidate, uint64_t offset, 
uint64_t next, uint64_t mingap) "trying size: 0x%" PRIx64 " @ 0x%" PRIx64 ", 
offset: 0x%" PRIx64" next: 0x%" PRIx64 " mingap: 0x%" PRIx64
 ram_block_discard_range(const char *rbname, void *hva, size_t length, bool 
need_madvise, bool need_fallocate, int ret) "%s@%p + 0x%zx: madvise: %d 
fallocate: %d ret: %d"
+memory_notdirty_write_access(uint64_t vaddr, uint64_t ram_addr, unsigned size) 
"0x%" PRIx64 " ram_addr 0x%" PRIx64 " size %u"
+memory_notdirty_set_dirty(uint64_t vaddr) "0x%" PRIx64
 
 # memory.c
 memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, 
unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t 
value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_subpage_read(int cpu_index, void *mr, 

[PATCH v3 12/20] cputlb: Move ROM handling from I/O path to TLB path

[Richard Henderson]

It does not require going through the whole I/O path
in order to discard a write.

Signed-off-by: Richard Henderson 
---
 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))
 
 /* 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 != _mem_rom && mr != _mem_notdirty && !cpu->can_do_io) {
+if (mr != _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 != _mem_rom && mr != _mem_notdirty && !cpu->can_do_io) {
+if (mr != _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 @@ 

[PATCH v3 09/20] cputlb: Replace switches in load/store_helper with callback

[Richard Henderson]

Add a function parameter to perform the actual load/store to ram.
With optimization, this results in identical code.

Signed-off-by: Richard Henderson 
---
 accel/tcg/cputlb.c | 159 +++--
 1 file changed, 83 insertions(+), 76 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index b87764..b4a63d3928 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1280,11 +1280,38 @@ static void *atomic_mmu_lookup(CPUArchState *env, 
target_ulong addr,
 
 typedef uint64_t FullLoadHelper(CPUArchState *env, target_ulong addr,
 TCGMemOpIdx oi, uintptr_t retaddr);
+typedef uint64_t LoadHelper(const void *);
+
+/* Wrap the unaligned load helpers to that they have a common signature.  */
+static inline uint64_t wrap_ldub(const void *haddr)
+{
+return ldub_p(haddr);
+}
+
+static inline uint64_t wrap_lduw_be(const void *haddr)
+{
+return lduw_be_p(haddr);
+}
+
+static inline uint64_t wrap_lduw_le(const void *haddr)
+{
+return lduw_le_p(haddr);
+}
+
+static inline uint64_t wrap_ldul_be(const void *haddr)
+{
+return (uint32_t)ldl_be_p(haddr);
+}
+
+static inline uint64_t wrap_ldul_le(const void *haddr)
+{
+return (uint32_t)ldl_le_p(haddr);
+}
 
 static inline uint64_t QEMU_ALWAYS_INLINE
 load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi,
 uintptr_t retaddr, MemOp op, bool code_read,
-FullLoadHelper *full_load)
+FullLoadHelper *full_load, LoadHelper *direct)
 {
 uintptr_t mmu_idx = get_mmuidx(oi);
 uintptr_t index = tlb_index(env, mmu_idx, addr);
@@ -1373,33 +1400,7 @@ load_helper(CPUArchState *env, target_ulong addr, 
TCGMemOpIdx oi,
 
  do_aligned_access:
 haddr = (void *)((uintptr_t)addr + entry->addend);
-switch (op) {
-case MO_UB:
-res = ldub_p(haddr);
-break;
-case MO_BEUW:
-res = lduw_be_p(haddr);
-break;
-case MO_LEUW:
-res = lduw_le_p(haddr);
-break;
-case MO_BEUL:
-res = (uint32_t)ldl_be_p(haddr);
-break;
-case MO_LEUL:
-res = (uint32_t)ldl_le_p(haddr);
-break;
-case MO_BEQ:
-res = ldq_be_p(haddr);
-break;
-case MO_LEQ:
-res = ldq_le_p(haddr);
-break;
-default:
-g_assert_not_reached();
-}
-
-return res;
+return direct(haddr);
 }
 
 /*
@@ -1415,7 +1416,8 @@ load_helper(CPUArchState *env, target_ulong addr, 
TCGMemOpIdx oi,
 static uint64_t full_ldub_mmu(CPUArchState *env, target_ulong addr,
   TCGMemOpIdx oi, uintptr_t retaddr)
 {
-return load_helper(env, addr, oi, retaddr, MO_UB, false, full_ldub_mmu);
+return load_helper(env, addr, oi, retaddr, MO_UB, false,
+   full_ldub_mmu, wrap_ldub);
 }
 
 tcg_target_ulong helper_ret_ldub_mmu(CPUArchState *env, target_ulong addr,
@@ -1428,7 +1430,7 @@ static uint64_t full_le_lduw_mmu(CPUArchState *env, 
target_ulong addr,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
 return load_helper(env, addr, oi, retaddr, MO_LEUW, false,
-   full_le_lduw_mmu);
+   full_le_lduw_mmu, wrap_lduw_le);
 }
 
 tcg_target_ulong helper_le_lduw_mmu(CPUArchState *env, target_ulong addr,
@@ -1441,7 +1443,7 @@ static uint64_t full_be_lduw_mmu(CPUArchState *env, 
target_ulong addr,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
 return load_helper(env, addr, oi, retaddr, MO_BEUW, false,
-   full_be_lduw_mmu);
+   full_be_lduw_mmu, wrap_lduw_be);
 }
 
 tcg_target_ulong helper_be_lduw_mmu(CPUArchState *env, target_ulong addr,
@@ -1454,7 +1456,7 @@ static uint64_t full_le_ldul_mmu(CPUArchState *env, 
target_ulong addr,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
 return load_helper(env, addr, oi, retaddr, MO_LEUL, false,
-   full_le_ldul_mmu);
+   full_le_ldul_mmu, wrap_ldul_le);
 }
 
 tcg_target_ulong helper_le_ldul_mmu(CPUArchState *env, target_ulong addr,
@@ -1467,7 +1469,7 @@ static uint64_t full_be_ldul_mmu(CPUArchState *env, 
target_ulong addr,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
 return load_helper(env, addr, oi, retaddr, MO_BEUL, false,
-   full_be_ldul_mmu);
+   full_be_ldul_mmu, wrap_ldul_be);
 }
 
 tcg_target_ulong helper_be_ldul_mmu(CPUArchState *env, target_ulong addr,
@@ -1480,14 +1482,14 @@ uint64_t helper_le_ldq_mmu(CPUArchState *env, 
target_ulong addr,
TCGMemOpIdx oi, uintptr_t retaddr)
 {
 return load_helper(env, addr, oi, retaddr, MO_LEQ, false,
-   helper_le_ldq_mmu);
+   helper_le_ldq_mmu, ldq_le_p);
 }
 
 uint64_t helper_be_ldq_mmu(CPUArchState *env, target_ulong addr,
TCGMemOpIdx oi, uintptr_t retaddr)

[PATCH v3 05/20] exec: Promote TARGET_PAGE_MASK to target_long

[Richard Henderson]

There are some uint64_t uses that expect TARGET_PAGE_MASK to
extend for a 32-bit, so this must continue to be a signed type.
Define based on TARGET_PAGE_BITS not TARGET_PAGE_SIZE; this
will make a following patch more clear.

This should not have a functional effect so far.

Reviewed-by: Paolo Bonzini 
Signed-off-by: Richard Henderson 
---
 include/exec/cpu-all.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index b11ee1f711..34d36cebca 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -225,7 +225,7 @@ extern const TargetPageBits target_page;
 #endif
 
 #define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
-#define TARGET_PAGE_MASK ~(TARGET_PAGE_SIZE - 1)
+#define TARGET_PAGE_MASK ((target_long)-1 << TARGET_PAGE_BITS)
 #define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) & 
TARGET_PAGE_MASK)
 
 /* Using intptr_t ensures that qemu_*_page_mask is sign-extended even
-- 
2.17.1

[PATCH v3 08/20] cputlb: Disable __always_inline__ without optimization

[Richard Henderson]

This forced inlining can result in missing symbols,
which makes a debugging build harder to follow.

Reviewed-by: David Hildenbrand 
Reported-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 include/qemu/compiler.h | 11 +++
 accel/tcg/cputlb.c  |  4 ++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 09fc44cca4..d6d400c523 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -170,6 +170,17 @@
 # define QEMU_NONSTRING
 #endif
 
+/*
+ * Forced inlining may be desired to encourage constant propagation
+ * of function parameters.  However, it can also make debugging harder,
+ * so disable it for a non-optimizing build.
+ */
+#if defined(__OPTIMIZE__) && __has_attribute(always_inline)
+#define QEMU_ALWAYS_INLINE  __attribute__((always_inline))
+#else
+#define QEMU_ALWAYS_INLINE
+#endif
+
 /* Implement C11 _Generic via GCC builtins.  Example:
  *
  *QEMU_GENERIC(x, (float, sinf), (long double, sinl), sin) (x)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index abae79650c..b87764 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1281,7 +1281,7 @@ static void *atomic_mmu_lookup(CPUArchState *env, 
target_ulong addr,
 typedef uint64_t FullLoadHelper(CPUArchState *env, target_ulong addr,
 TCGMemOpIdx oi, uintptr_t retaddr);
 
-static inline uint64_t __attribute__((always_inline))
+static inline uint64_t QEMU_ALWAYS_INLINE
 load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi,
 uintptr_t retaddr, MemOp op, bool code_read,
 FullLoadHelper *full_load)
@@ -1530,7 +1530,7 @@ tcg_target_ulong helper_be_ldsl_mmu(CPUArchState *env, 
target_ulong addr,
  * Store Helpers
  */
 
-static inline void __attribute__((always_inline))
+static inline void QEMU_ALWAYS_INLINE
 store_helper(CPUArchState *env, target_ulong addr, uint64_t val,
  TCGMemOpIdx oi, uintptr_t retaddr, MemOp op)
 {
-- 
2.17.1

[PATCH v3 10/20] cputlb: Introduce TLB_BSWAP

[Richard Henderson]

Handle bswap on ram directly in load/store_helper.  This fixes a
bug with the previous implementation in that one cannot use the
I/O path for RAM.

Fixes: a26fc6f5152b47f1
Signed-off-by: Richard Henderson 
---
 include/exec/cpu-all.h |   4 +-
 accel/tcg/cputlb.c | 108 +
 2 files changed, 59 insertions(+), 53 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 2db73c7a27..1ebd1b59ab 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -346,12 +346,14 @@ CPUArchState *cpu_copy(CPUArchState *env);
 #define TLB_MMIO(1 << (TARGET_PAGE_BITS_MIN - 3))
 /* Set if TLB entry contains a watchpoint.  */
 #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))
 
 /* 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_INVALID_MASK | TLB_NOTDIRTY | TLB_MMIO | TLB_WATCHPOINT | TLB_BSWAP)
 
 /**
  * tlb_hit_page: return true if page aligned @addr is a hit against the
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index b4a63d3928..cb603917a2 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -737,8 +737,7 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong 
vaddr,
 address |= TLB_INVALID_MASK;
 }
 if (attrs.byte_swap) {
-/* Force the access through the I/O slow path.  */
-address |= TLB_MMIO;
+address |= TLB_BSWAP;
 }
 if (!memory_region_is_ram(section->mr) &&
 !memory_region_is_romd(section->mr)) {
@@ -901,10 +900,6 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry 
*iotlbentry,
 bool locked = false;
 MemTxResult r;
 
-if (iotlbentry->attrs.byte_swap) {
-op ^= MO_BSWAP;
-}
-
 section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs);
 mr = section->mr;
 mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr;
@@ -947,10 +942,6 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
*iotlbentry,
 bool locked = false;
 MemTxResult r;
 
-if (iotlbentry->attrs.byte_swap) {
-op ^= MO_BSWAP;
-}
-
 section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs);
 mr = section->mr;
 mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr;
@@ -1133,8 +1124,8 @@ void *probe_access(CPUArchState *env, target_ulong addr, 
int size,
  wp_access, retaddr);
 }
 
-if (tlb_addr & (TLB_NOTDIRTY | TLB_MMIO)) {
-/* I/O access */
+/* Reject I/O access, or other required slow-path.  */
+if (tlb_addr & (TLB_NOTDIRTY | TLB_MMIO | TLB_BSWAP)) {
 return NULL;
 }
 
@@ -1311,7 +1302,8 @@ static inline uint64_t wrap_ldul_le(const void *haddr)
 static inline uint64_t QEMU_ALWAYS_INLINE
 load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi,
 uintptr_t retaddr, MemOp op, bool code_read,
-FullLoadHelper *full_load, LoadHelper *direct)
+FullLoadHelper *full_load, LoadHelper *direct,
+LoadHelper *direct_swap)
 {
 uintptr_t mmu_idx = get_mmuidx(oi);
 uintptr_t index = tlb_index(env, mmu_idx, addr);
@@ -1361,17 +1353,21 @@ load_helper(CPUArchState *env, target_ulong addr, 
TCGMemOpIdx oi,
 /* On watchpoint hit, this will longjmp out.  */
 cpu_check_watchpoint(env_cpu(env), addr, size,
  iotlbentry->attrs, BP_MEM_READ, retaddr);
-
-/* The backing page may or may not require I/O.  */
-tlb_addr &= ~TLB_WATCHPOINT;
-if ((tlb_addr & ~TARGET_PAGE_MASK) == 0) {
-goto do_aligned_access;
-}
 }
 
 /* Handle I/O access.  */
-return io_readx(env, iotlbentry, mmu_idx, addr,
-retaddr, access_type, op);
+if (likely(tlb_addr & TLB_MMIO)) {
+return io_readx(env, iotlbentry, mmu_idx, addr,
+retaddr, access_type,
+op ^ (tlb_addr & TLB_BSWAP ? MO_BSWAP : 0));
+}
+
+haddr = (void *)((uintptr_t)addr + entry->addend);
+
+if (unlikely(tlb_addr & TLB_BSWAP)) {
+return direct_swap(haddr);
+}
+return direct(haddr);
 }
 
 /* Handle slow unaligned access (it spans two pages or IO).  */
@@ -1398,7 +1394,6 @@ load_helper(CPUArchState *env, target_ulong addr, 
TCGMemOpIdx oi,
 return res & MAKE_64BIT_MASK(0, size * 8);
 }
 
- do_aligned_access:
 haddr = (void *)((uintptr_t)addr + entry->addend);
 return direct(haddr);
 }
@@ -1417,7 +1412,7 @@ static uint64_t full_ldub_mmu(CPUArchState *env, 
target_ulong addr,
   TCGMemOpIdx oi, uintptr_t retaddr)
 {
 return load_helper(env, addr, oi, 

[PATCH v3 06/20] exec: Tidy TARGET_PAGE_ALIGN

[Richard Henderson]

Use TARGET_PAGE_MASK twice instead of TARGET_PAGE_SIZE once.
This is functionally identical, but will help a following patch.

Reviewed-by: Paolo Bonzini 
Signed-off-by: Richard Henderson 
---
 include/exec/cpu-all.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 34d36cebca..5246770271 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -226,7 +226,8 @@ extern const TargetPageBits target_page;
 
 #define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
 #define TARGET_PAGE_MASK ((target_long)-1 << TARGET_PAGE_BITS)
-#define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) & 
TARGET_PAGE_MASK)
+#define TARGET_PAGE_ALIGN(addr) \
+(((addr) + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK)
 
 /* Using intptr_t ensures that qemu_*_page_mask is sign-extended even
  * when intptr_t is 32-bit and we are aligning a long long.
-- 
2.17.1

[PATCH v3 07/20] exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY

[Richard Henderson]

This eliminates a set of runtime shifts.  It turns out that we
require TARGET_PAGE_MASK more often than TARGET_PAGE_SIZE, so
redefine TARGET_PAGE_SIZE based on TARGET_PAGE_MASK instead of
the other way around.

Reviewed-by: Paolo Bonzini 
Signed-off-by: Richard Henderson 
---
 include/exec/cpu-all.h | 8 ++--
 exec-vary.c| 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 5246770271..2db73c7a27 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -213,19 +213,23 @@ static inline void stl_phys_notdirty(AddressSpace *as, 
hwaddr addr, uint32_t val
 typedef struct {
 bool decided;
 int bits;
+target_long mask;
 } TargetPageBits;
 extern const TargetPageBits target_page;
 # ifdef CONFIG_DEBUG_TCG
 #  define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
+#  define TARGET_PAGE_MASK (assert(target_page.decided), target_page.mask)
 # else
 #  define TARGET_PAGE_BITS target_page.bits
+#  define TARGET_PAGE_MASK target_page.mask
 # endif
+# define TARGET_PAGE_SIZE  ((int)-TARGET_PAGE_MASK)
 #else
 #define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS
+#define TARGET_PAGE_SIZE   (1 << TARGET_PAGE_BITS)
+#define TARGET_PAGE_MASK   ((target_long)-1 << TARGET_PAGE_BITS)
 #endif
 
-#define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
-#define TARGET_PAGE_MASK ((target_long)-1 << TARGET_PAGE_BITS)
 #define TARGET_PAGE_ALIGN(addr) \
 (((addr) + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK)
 
diff --git a/exec-vary.c b/exec-vary.c
index 67cdf57a9c..26daf281f2 100644
--- a/exec-vary.c
+++ b/exec-vary.c
@@ -83,5 +83,6 @@ void finalize_target_page_bits(void)
 init_target_page.bits = TARGET_PAGE_BITS_MIN;
 }
 init_target_page.decided = true;
+init_target_page.mask = (target_long)-1 << init_target_page.bits;
 #endif
 }
-- 
2.17.1

[PATCH v3 03/20] exec: Use const alias for TARGET_PAGE_BITS_VARY

[Richard Henderson]

Using a variable that is declared "const" for this tells the
compiler that it may read the value once and assume that it
does not change across function calls.

For target_page_size, this means we have only one assert per
function, and one read of the variable.

This reduces the size of qemu-system-aarch64 by 8k.

Reviewed-by: Paolo Bonzini 
Signed-off-by: Richard Henderson 
---
 include/exec/cpu-all.h | 10 +
 exec-vary.c| 46 ++
 2 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index e0c8dc540c..a53b761b48 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -210,10 +210,12 @@ static inline void stl_phys_notdirty(AddressSpace *as, 
hwaddr addr, uint32_t val
 /* page related stuff */
 
 #ifdef TARGET_PAGE_BITS_VARY
-extern bool target_page_bits_decided;
-extern int target_page_bits;
-#define TARGET_PAGE_BITS ({ assert(target_page_bits_decided); \
-target_page_bits; })
+typedef struct {
+bool decided;
+int bits;
+} TargetPageBits;
+extern const TargetPageBits target_page;
+#define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
 #else
 #define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS
 #endif
diff --git a/exec-vary.c b/exec-vary.c
index 48c0ab306c..67cdf57a9c 100644
--- a/exec-vary.c
+++ b/exec-vary.c
@@ -22,8 +22,38 @@
 #include "exec/exec-all.h"
 
 #ifdef TARGET_PAGE_BITS_VARY
-int target_page_bits;
-bool target_page_bits_decided;
+/*
+ * We want to declare the "target_page" variable as const, which tells
+ * the compiler that it can cache any value that it reads across calls.
+ * This avoids multiple assertions and multiple reads within any one user.
+ *
+ * This works because we initialize the target_page data very early, in a
+ * location far removed from the functions that require the final results.
+ *
+ * This also requires that we have a non-constant symbol by which we can
+ * perform the actual initialization, and which forces the data to be
+ * allocated within writable memory.  Thus "init_target_page", and we use
+ * that symbol exclusively in the two functions that initialize this value.
+ *
+ * The "target_page" symbol is created as an alias of "init_target_page".
+ */
+static TargetPageBits init_target_page;
+
+/*
+ * Note that this is *not* a redundant decl, this is the definition of
+ * the "target_page" symbol.  The syntax for this definition requires
+ * the use of the extern keyword.  This seems to be a GCC bug in
+ * either the syntax for the alias attribute or in -Wredundant-decls.
+ *
+ * See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91765
+ */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wredundant-decls"
+
+extern const TargetPageBits target_page
+__attribute__((alias("init_target_page")));
+
+#pragma GCC diagnostic pop
 #endif
 
 bool set_preferred_target_page_bits(int bits)
@@ -36,11 +66,11 @@ bool set_preferred_target_page_bits(int bits)
  */
 #ifdef TARGET_PAGE_BITS_VARY
 assert(bits >= TARGET_PAGE_BITS_MIN);
-if (target_page_bits == 0 || target_page_bits > bits) {
-if (target_page_bits_decided) {
+if (init_target_page.bits == 0 || init_target_page.bits > bits) {
+if (init_target_page.decided) {
 return false;
 }
-target_page_bits = bits;
+init_target_page.bits = bits;
 }
 #endif
 return true;
@@ -49,9 +79,9 @@ bool set_preferred_target_page_bits(int bits)
 void finalize_target_page_bits(void)
 {
 #ifdef TARGET_PAGE_BITS_VARY
-if (target_page_bits == 0) {
-target_page_bits = TARGET_PAGE_BITS_MIN;
+if (init_target_page.bits == 0) {
+init_target_page.bits = TARGET_PAGE_BITS_MIN;
 }
-target_page_bits_decided = true;
+init_target_page.decided = true;
 #endif
 }
-- 
2.17.1

[PATCH v3 04/20] exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG

[Richard Henderson]

This reduces the size of a release build by about 10k.
Noticably, within the tlb miss helpers.

Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Paolo Bonzini 
Signed-off-by: Richard Henderson 
---
 include/exec/cpu-all.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index a53b761b48..b11ee1f711 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -215,7 +215,11 @@ typedef struct {
 int bits;
 } TargetPageBits;
 extern const TargetPageBits target_page;
-#define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
+# ifdef CONFIG_DEBUG_TCG
+#  define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
+# else
+#  define TARGET_PAGE_BITS target_page.bits
+# endif
 #else
 #define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS
 #endif
-- 
2.17.1

[PATCH v3 02/20] exec: Split out variable page size support to exec-vary.c

[Richard Henderson]

The next patch will play a trick with "const" that will
confuse the compiler about the uses of target_page_bits
within exec.c.  Moving everything to a new file prevents
this confusion.

No functional change so far.

Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Paolo Bonzini 
Signed-off-by: Richard Henderson 
---
 Makefile.target   |  2 +-
 include/qemu-common.h |  6 +
 exec-vary.c   | 57 +++
 exec.c| 34 --
 4 files changed, 64 insertions(+), 35 deletions(-)
 create mode 100644 exec-vary.c

diff --git a/Makefile.target b/Makefile.target
index 5e916230c4..ca3d14efe1 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -107,7 +107,7 @@ obj-y += trace/
 
 #
 # cpu emulator library
-obj-y += exec.o
+obj-y += exec.o exec-vary.o
 obj-y += accel/
 obj-$(CONFIG_TCG) += tcg/tcg.o tcg/tcg-op.o tcg/tcg-op-vec.o tcg/tcg-op-gvec.o
 obj-$(CONFIG_TCG) += tcg/tcg-common.o tcg/optimize.o
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 8d84db90b0..082da59e85 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -74,6 +74,12 @@ void cpu_exec_step_atomic(CPUState *cpu);
  */
 bool set_preferred_target_page_bits(int bits);
 
+/**
+ * finalize_target_page_bits:
+ * Commit the final value set by set_preferred_target_page_bits.
+ */
+void finalize_target_page_bits(void);
+
 /**
  * Sends a (part of) iovec down a socket, yielding when the socket is full, or
  * Receives data into a (part of) iovec from a socket,
diff --git a/exec-vary.c b/exec-vary.c
new file mode 100644
index 00..48c0ab306c
--- /dev/null
+++ b/exec-vary.c
@@ -0,0 +1,57 @@
+/*
+ * Variable page size handling
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "exec/exec-all.h"
+
+#ifdef TARGET_PAGE_BITS_VARY
+int target_page_bits;
+bool target_page_bits_decided;
+#endif
+
+bool set_preferred_target_page_bits(int bits)
+{
+/*
+ * The target page size is the lowest common denominator for all
+ * the CPUs in the system, so we can only make it smaller, never
+ * larger. And we can't make it smaller once we've committed to
+ * a particular size.
+ */
+#ifdef TARGET_PAGE_BITS_VARY
+assert(bits >= TARGET_PAGE_BITS_MIN);
+if (target_page_bits == 0 || target_page_bits > bits) {
+if (target_page_bits_decided) {
+return false;
+}
+target_page_bits = bits;
+}
+#endif
+return true;
+}
+
+void finalize_target_page_bits(void)
+{
+#ifdef TARGET_PAGE_BITS_VARY
+if (target_page_bits == 0) {
+target_page_bits = TARGET_PAGE_BITS_MIN;
+}
+target_page_bits_decided = true;
+#endif
+}
diff --git a/exec.c b/exec.c
index 8b998974f8..33bd0e36c1 100644
--- a/exec.c
+++ b/exec.c
@@ -92,11 +92,6 @@ MemoryRegion io_mem_rom, io_mem_notdirty;
 static MemoryRegion io_mem_unassigned;
 #endif
 
-#ifdef TARGET_PAGE_BITS_VARY
-int target_page_bits;
-bool target_page_bits_decided;
-#endif
-
 CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
 
 /* current CPU in the current thread. It is only valid inside
@@ -110,37 +105,8 @@ int use_icount;
 uintptr_t qemu_host_page_size;
 intptr_t qemu_host_page_mask;
 
-bool set_preferred_target_page_bits(int bits)
-{
-/* The target page size is the lowest common denominator for all
- * the CPUs in the system, so we can only make it smaller, never
- * larger. And we can't make it smaller once we've committed to
- * a particular size.
- */
-#ifdef TARGET_PAGE_BITS_VARY
-assert(bits >= TARGET_PAGE_BITS_MIN);
-if (target_page_bits == 0 || target_page_bits > bits) {
-if (target_page_bits_decided) {
-return false;
-}
-target_page_bits = bits;
-}
-#endif
-return true;
-}
-
 #if !defined(CONFIG_USER_ONLY)
 
-static void finalize_target_page_bits(void)
-{
-#ifdef TARGET_PAGE_BITS_VARY
-if (target_page_bits == 0) {
-target_page_bits = TARGET_PAGE_BITS_MIN;
-}
-target_page_bits_decided = true;
-#endif
-}
-
 typedef struct PhysPageEntry PhysPageEntry;
 
 struct PhysPageEntry {
-- 
2.17.1

[PATCH v3 01/20] exec: Use TARGET_PAGE_BITS_MIN for TLB flags

[Richard Henderson]

These bits do not need to vary with the actual page size
used by the guest.

Reviewed-by: Paolo Bonzini 
Signed-off-by: Richard Henderson 
---
 include/exec/cpu-all.h | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index d2d443c4f9..e0c8dc540c 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -317,20 +317,24 @@ CPUArchState *cpu_copy(CPUArchState *env);
 
 #if !defined(CONFIG_USER_ONLY)
 
-/* Flags stored in the low bits of the TLB virtual address.  These are
- * defined so that fast path ram access is all zeros.
+/*
+ * Flags stored in the low bits of the TLB virtual address.
+ * These are defined so that fast path ram access is all zeros.
  * The flags all must be between TARGET_PAGE_BITS and
  * maximum address alignment bit.
+ *
+ * Use TARGET_PAGE_BITS_MIN so that these bits are constant
+ * when TARGET_PAGE_BITS_VARY is in effect.
  */
 /* Zero if TLB entry is valid.  */
-#define TLB_INVALID_MASK(1 << (TARGET_PAGE_BITS - 1))
+#define TLB_INVALID_MASK(1 << (TARGET_PAGE_BITS_MIN - 1))
 /* Set if TLB entry references a clean RAM page.  The iotlb entry will
contain the page physical address.  */
-#define TLB_NOTDIRTY(1 << (TARGET_PAGE_BITS - 2))
+#define TLB_NOTDIRTY(1 << (TARGET_PAGE_BITS_MIN - 2))
 /* Set if TLB entry is an IO callback.  */
-#define TLB_MMIO(1 << (TARGET_PAGE_BITS - 3))
+#define TLB_MMIO(1 << (TARGET_PAGE_BITS_MIN - 3))
 /* Set if TLB entry contains a watchpoint.  */
-#define TLB_WATCHPOINT  (1 << (TARGET_PAGE_BITS - 4))
+#define TLB_WATCHPOINT  (1 << (TARGET_PAGE_BITS_MIN - 4))
 
 /* Use this mask to check interception with an alignment mask
  * in a TCG backend.
-- 
2.17.1

[PATCH v3 00/20] Move rom and notdirty handling to cputlb

[Richard Henderson]

Ok!  Third time is the charm, because this time it works.

New to v3:

  * Covert io_mem_rom with a new TLB_ROM bit.

  * This in turn means that there are no longer any special RAM
case along along the MMIO path -- they all have devices on
the other end.

  * This in turn means that we can fold the bulk of 
memory_region_section_get_iotlb into tlb_set_page_with_attrs,
a couple of redundant tests vs the MemoryRegion.
The result in patch 14 is, IMO, much more understandable.

  * Fold away uses of cpu->mem_io_pc in tb_invalidate_phys_page__locked,
the cause of the problems for my previous two patch sets.

BTW, I was correct with my guess in the v2 cover letter that the use
of memory_notdirty_write_{prepare,complete} within atomic_mmu_lookup
must have been broken, for not setting mem_io_pc.  :-P

  * Fix a missed use of cpu->mem_io_pc in tb_check_watchpoint,
which meant that the previous TLB_WATCHPOINT cleanup was a
titch broken.

The remaining two users of cpu->mem_io_pc are hw/misc/mips_itu.c and
target/i386/helper.c.  I haven't looked, but I assume that these are
legitimately on the MMIO path, and there probably isn't a decent way
to remove the uses.


r~


Richard Henderson (20):
  exec: Use TARGET_PAGE_BITS_MIN for TLB flags
  exec: Split out variable page size support to exec-vary.c
  exec: Use const alias for TARGET_PAGE_BITS_VARY
  exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG
  exec: Promote TARGET_PAGE_MASK to target_long
  exec: Tidy TARGET_PAGE_ALIGN
  exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY
  cputlb: Disable __always_inline__ without optimization
  cputlb: Replace switches in load/store_helper with callback
  cputlb: Introduce TLB_BSWAP
  exec: Adjust notdirty tracing
  cputlb: Move ROM handling from I/O path to TLB path
  cputlb: Move NOTDIRTY handling from I/O path to TLB path
  cputlb: Partially inline memory_region_section_get_iotlb
  cputlb: Merge and move memory_notdirty_write_{prepare,complete}
  cputlb: Handle TLB_NOTDIRTY in probe_access
  cputlb: Remove cpu->mem_io_vaddr
  cputlb: Remove tb_invalidate_phys_page_range is_cpu_write_access
  cputlb: Pass retaddr to tb_invalidate_phys_page_fast
  cputlb: Pass retaddr to tb_check_watchpoint

 Makefile.target|   2 +-
 accel/tcg/translate-all.h  |   8 +-
 include/exec/cpu-all.h |  48 ++--
 include/exec/cpu-common.h  |   3 -
 include/exec/exec-all.h|   6 +-
 include/exec/memory-internal.h |  65 --
 include/hw/core/cpu.h  |   2 -
 include/qemu-common.h  |   6 +
 include/qemu/compiler.h|  11 +
 accel/tcg/cputlb.c | 388 +++--
 accel/tcg/translate-all.c  |  51 ++---
 exec-vary.c|  88 
 exec.c | 192 +---
 hw/core/cpu.c  |   1 -
 memory.c   |  20 --
 trace-events   |   4 +-
 16 files changed, 403 insertions(+), 492 deletions(-)
 create mode 100644 exec-vary.c

-- 
2.17.1

Re: [PATCH v1 4/6] riscv/sifive_u: Add the start-in-flash property

[Bin Meng]

On Sat, Sep 21, 2019 at 6:12 AM Alistair Francis  wrote:
>
> On Thu, Sep 19, 2019 at 10:15 PM Bin Meng  wrote:
> >
> > On Fri, Sep 20, 2019 at 6:32 AM Alistair Francis
> >  wrote:
> > >
> > > Add a property that when set to true QEMU will jump from the ROM code to
> > > the start of flash memory instead of DRAM which is the default
> > > behaviour.
> > >
> > > Signed-off-by: Alistair Francis 
> > > ---
> > >  hw/riscv/sifive_u.c | 27 +++
> > >  include/hw/riscv/sifive_u.h |  2 ++
> > >  2 files changed, 29 insertions(+)
> > >
> > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > > index c3949fb316..b7cd3631cd 100644
> > > --- a/hw/riscv/sifive_u.c
> > > +++ b/hw/riscv/sifive_u.c
> > > @@ -373,6 +373,10 @@ static void riscv_sifive_u_init(MachineState 
> > > *machine)
> > > /* dtb: */
> > >  };
> > >
> > > +if (s->start_in_flash) {
> > > +reset_vec[6] = memmap[SIFIVE_U_FLASH0].base; /* start: .dword 
> > > FLASH0_BASE */
> > > +}
> > > +
> > >  /* copy in the reset vector in little_endian byte order */
> > >  for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
> > >  reset_vec[i] = cpu_to_le32(reset_vec[i]);
> > > @@ -544,8 +548,31 @@ static void riscv_sifive_u_soc_realize(DeviceState 
> > > *dev, Error **errp)
> > >  memmap[SIFIVE_U_GEM_MGMT].base, memmap[SIFIVE_U_GEM_MGMT].size);
> > >  }
> > >
> > > +static bool virt_get_start_in_flash(Object *obj, Error **errp)
> > > +{
> > > +SiFiveUState *s = RISCV_U_MACHINE(obj);
> > > +
> > > +return s->start_in_flash;
> > > +}
> > > +
> > > +static void virt_set_start_in_flash(Object *obj, bool value, Error 
> > > **errp)
> > > +{
> > > +SiFiveUState *s = RISCV_U_MACHINE(obj);
> > > +
> > > +s->start_in_flash = value;
> > > +}
> > > +
> > >  static void riscv_sifive_u_machine_instance_init(Object *obj)
> > >  {
> > > +SiFiveUState *s = RISCV_U_MACHINE(obj);
> > > +
> > > +s->start_in_flash = false;
> > > +object_property_add_bool(obj, "start-in-flash", 
> > > virt_get_start_in_flash,
> > > + virt_set_start_in_flash, NULL);
> > > +object_property_set_description(obj, "start-in-flash",
> > > +"Set on to tell QEMU's ROM to jump 
> > > to " \
> > > +"flash. Otherwise QEMU will jump to 
> > > DRAM",
> > > +NULL);
> > >
> > >  }
> > >
> > > diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> > > index a921079fbe..2656b43c58 100644
> > > --- a/include/hw/riscv/sifive_u.h
> > > +++ b/include/hw/riscv/sifive_u.h
> > > @@ -57,6 +57,8 @@ typedef struct SiFiveUState {
> > >
> > >  void *fdt;
> > >  int fdt_size;
> > > +
> > > +bool start_in_flash;
> > >  } SiFiveUState;
> > >
> > >  enum {
> >
> > This patch chose a different way from the one used in patch "[v1,6/6]
> > riscv/virt: Jump to pflash if specified":
> >
> > - this patch uses reset_vec[6] while patch [6/6] defines a variable 
> > start_addr
> > - this patch adds a "start-in-flash" property to the machine, while
> > patch [6/6] tests against drive IF_PFLASH
>
> Yes, we do it differently for the sifive_u board as the sifive_u board
> doesn't use pflash so there is no way to know if the user has loaded
> anything into the SPI memory.
>

OK.

> >
> > We should be consistent and I would prefer to use the patch [6/6] way.
> > On Unleashed an SPI flash is mounted so we cannot add a PFlash to
> > sifive_u machine like what was done on virt machine, so we should test
> > IF_MTD instead. Thoughts?
>
> How would we test that?
>
> Right now I am loading the binary in SPI with the -device loader
> option. The machine can't really know what is/isn't loaded there.
>
> It's not ideal, but I don't see a nicer way.

I think we need write a SiFive SPI model to support this in a clean
way. Ideally we should simulate the hardware boot workflow as
documented in the FU540 manual chapter 6 "Boot Process".

Regards,
Bin

Re: [PATCH v1 5/6] riscv/virt: Add the PFlash CFI01 device

[Bin Meng]

On Sat, Sep 21, 2019 at 6:16 AM Alistair Francis  wrote:
>
> On Thu, Sep 19, 2019 at 10:15 PM Bin Meng  wrote:
> >
> > On Fri, Sep 20, 2019 at 6:36 AM Alistair Francis
> >  wrote:
> > >
> > > Add the CFI01 PFlash to the RISC-V virt board. This is the same PFlash
> > > from the ARM Virt board and the implementation is based on the ARM Virt
> > > board. This allows users to specify flash files from the command line.
> > >
> > > Signed-off-by: Alistair Francis 
> > > ---
> > >  hw/riscv/Kconfig|  1 +
> > >  hw/riscv/virt.c | 81 +
> > >  include/hw/riscv/virt.h |  3 ++
> > >  3 files changed, 85 insertions(+)
> > >
> > > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> > > index fb19b2df3a..b12660b9f8 100644
> > > --- a/hw/riscv/Kconfig
> > > +++ b/hw/riscv/Kconfig
> > > @@ -36,4 +36,5 @@ config RISCV_VIRT
> > >  select SERIAL
> > >  select VIRTIO_MMIO
> > >  select PCI_EXPRESS_GENERIC_BRIDGE
> > > +select PFLASH_CFI01
> > >  select SIFIVE
> > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > > index d36f5625ec..ca002ecea7 100644
> > > --- a/hw/riscv/virt.c
> > > +++ b/hw/riscv/virt.c
> > > @@ -26,6 +26,7 @@
> > >  #include "hw/boards.h"
> > >  #include "hw/loader.h"
> > >  #include "hw/sysbus.h"
> > > +#include "hw/qdev-properties.h"
> > >  #include "hw/char/serial.h"
> > >  #include "target/riscv/cpu.h"
> > >  #include "hw/riscv/riscv_hart.h"
> > > @@ -61,12 +62,72 @@ static const struct MemmapEntry {
> > >  [VIRT_PLIC] ={  0xc00, 0x400 },
> > >  [VIRT_UART0] =   { 0x1000, 0x100 },
> > >  [VIRT_VIRTIO] =  { 0x10001000,0x1000 },
> > > +[VIRT_FLASH] =   { 0x2000, 0x200 },
> > >  [VIRT_DRAM] ={ 0x8000,   0x0 },
> > >  [VIRT_PCIE_MMIO] =   { 0x4000,0x4000 },
> > >  [VIRT_PCIE_PIO] ={ 0x0300,0x0001 },
> > >  [VIRT_PCIE_ECAM] =   { 0x3000,0x1000 },
> > >  };
> > >
> > > +#define VIRT_FLASH_SECTOR_SIZE (256 * KiB)
> > > +
> > > +static PFlashCFI01 *virt_flash_create1(RISCVVirtState *s,
> > > +   const char *name,
> > > +   const char *alias_prop_name)
> > > +{
> > > +/*
> > > + * Create a single flash device.  We use the same parameters as
> > > + * the flash devices on the ARM virt board.
> > > + */
> > > +DeviceState *dev = qdev_create(NULL, TYPE_PFLASH_CFI01);
> > > +
> > > +qdev_prop_set_uint64(dev, "sector-length", VIRT_FLASH_SECTOR_SIZE);
> > > +qdev_prop_set_uint8(dev, "width", 4);
> > > +qdev_prop_set_uint8(dev, "device-width", 2);
> > > +qdev_prop_set_bit(dev, "big-endian", false);
> > > +qdev_prop_set_uint16(dev, "id0", 0x89);
> > > +qdev_prop_set_uint16(dev, "id1", 0x18);
> > > +qdev_prop_set_uint16(dev, "id2", 0x00);
> > > +qdev_prop_set_uint16(dev, "id3", 0x00);
> > > +qdev_prop_set_string(dev, "name", name);
> >
> > alias_prop_name is unused? ARM virt has 2 more calls in the same function 
> > here.
>
> Yep, you are right. I have removed this.

Any reason of removing this?

>
> >
> > > +
> > > +return PFLASH_CFI01(dev);
> > > +}
> > > +
> > > +static void virt_flash_create(RISCVVirtState *s)
> > > +{
> > > +s->flash[0] = virt_flash_create1(s, "virt.flash0", "pflash0");
> > > +s->flash[1] = virt_flash_create1(s, "virt.flash1", "pflash1");
> >
> > I don't think we should mirror what is used on ARM virt board to
> > create 2 flash for sifive_u. For ARM virt, there are 2 flashes because
> > they need distinguish secure and non-secure. For sifive_u, only one is
> > enough.
>
> I went back and forward about 1 or 2. Two seems more usable as maybe
> someone wants to include two pflash files? The Xilinx machine also has
> two so I'm kind of used to 2, but I'm not really fussed.
>
> Unless anyone else wants two I will change it to 1.

Regards,
Bin

Re: [RFC v2 0/5] Move notdirty handling to cputlb

[Richard Henderson]

On 9/18/19 11:02 AM, Richard Henderson wrote:
> However this time the changes are extremely minimal, and now
> I really *really* don't understand why they don't work, because
> as far as I can tell the new locking is *identical* with the
> current i/o path.

The difference was failing to set cpu->mem_io_pc, so that
TARGET_HAS_PRECISE_SMC within tb_invalidate_phys_page_range__locked could look
up the current TB, and potentially restore state and exit to the main loop.

Version 3 will have this fixed.


r~

Re: [PATCH] hw/arm/boot: Use the IEC binary prefix definitions

[Richard Henderson]

On 9/21/19 3:34 AM, Philippe Mathieu-Daudé wrote:
> IEC binary prefixes ease code review: the unit is explicit.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/arm/boot.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [PATCH] hw/m68k/next-cube: Avoid static RTC variables and introduce control register

[Richard Henderson]

On 9/21/19 2:17 AM, Thomas Huth wrote:
> Coverity currently complains that the "if (0x00 & (0x80 >> (phase - 8))"
> in next-cube.c can never be true. Right it is. The "0x00" is meant as value
> of the control register of the RTC, which is currently not implemented yet.
> Thus, let's add a register variable for this now. However, the RTC
> registers are currently defined as static variables in nextscr2_write(),
> which is quite ugly. Thus let's also move the RTC variables to the main
> machine state instead. In the long run, we should likely even refactor
> the whole RTC code into a separate device in a separate file, but that's
> something for calm winter nights later... as a first step, cleaning up
> the static variables and shutting up the warning from Coverity should
> be sufficient.
> 
> Signed-off-by: Thomas Huth 

Reviewed-by: Richard Henderson 

>  /* for now 0x00 */
> -if (0x00 & (0x80 >> (phase - 8))) {
> +if (rtc->control & (0x80 >> (phase - 8))) {

You might kill the comment too.


r~

[PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c

[Thomas Huth]

We are going to make CONFIG_ARM_V7M optional, so the related cortex-m
CPUs should only be created if the switch is enabled. This can best
be done if the code resides in a separate file, thus move the related
functions to a new file v7m.c which only gets compiled if CONFIG_ARM_V7M
is enabled.

Signed-off-by: Thomas Huth 
---
 target/arm/Makefile.objs |   1 +
 target/arm/cpu.c | 146 -
 target/arm/v7m.c | 193 +++
 3 files changed, 194 insertions(+), 146 deletions(-)
 create mode 100644 target/arm/v7m.c

diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs
index cf26c16f5f..16b9417a8b 100644
--- a/target/arm/Makefile.objs
+++ b/target/arm/Makefile.objs
@@ -61,6 +61,7 @@ obj-y += translate.o op_helper.o
 obj-y += crypto_helper.o
 obj-y += iwmmxt_helper.o vec_helper.o neon_helper.o
 obj-y += m_helper.o
+obj-$(CONFIG_ARM_V7M) += v7m.o
 
 obj-$(CONFIG_SOFTMMU) += psci.o
 
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index f1f9eecdc8..d5f0d4af61 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -462,31 +462,6 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int 
interrupt_request)
 return ret;
 }
 
-#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
-static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
-{
-CPUClass *cc = CPU_GET_CLASS(cs);
-ARMCPU *cpu = ARM_CPU(cs);
-CPUARMState *env = >env;
-bool ret = false;
-
-/* ARMv7-M interrupt masking works differently than -A or -R.
- * There is no FIQ/IRQ distinction. Instead of I and F bits
- * masking FIQ and IRQ interrupts, an exception is taken only
- * if it is higher priority than the current execution priority
- * (which depends on state like BASEPRI, FAULTMASK and the
- * currently active exception).
- */
-if (interrupt_request & CPU_INTERRUPT_HARD
-&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
-cs->exception_index = EXCP_IRQ;
-cc->do_interrupt(cs);
-ret = true;
-}
-return ret;
-}
-#endif
-
 void arm_cpu_update_virq(ARMCPU *cpu)
 {
 /*
@@ -1881,119 +1856,6 @@ static void arm11mpcore_initfn(Object *obj)
 cpu->reset_auxcr = 1;
 }
 
-static void cortex_m0_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-set_feature(>env, ARM_FEATURE_V6);
-set_feature(>env, ARM_FEATURE_M);
-
-cpu->midr = 0x410cc200;
-}
-
-static void cortex_m3_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-set_feature(>env, ARM_FEATURE_V7);
-set_feature(>env, ARM_FEATURE_M);
-set_feature(>env, ARM_FEATURE_M_MAIN);
-cpu->midr = 0x410fc231;
-cpu->pmsav7_dregion = 8;
-cpu->id_pfr0 = 0x0030;
-cpu->id_pfr1 = 0x0200;
-cpu->id_dfr0 = 0x0010;
-cpu->id_afr0 = 0x;
-cpu->id_mmfr0 = 0x0030;
-cpu->id_mmfr1 = 0x;
-cpu->id_mmfr2 = 0x;
-cpu->id_mmfr3 = 0x;
-cpu->isar.id_isar0 = 0x01141110;
-cpu->isar.id_isar1 = 0x02111000;
-cpu->isar.id_isar2 = 0x21112231;
-cpu->isar.id_isar3 = 0x0110;
-cpu->isar.id_isar4 = 0x01310102;
-cpu->isar.id_isar5 = 0x;
-cpu->isar.id_isar6 = 0x;
-}
-
-static void cortex_m4_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-
-set_feature(>env, ARM_FEATURE_V7);
-set_feature(>env, ARM_FEATURE_M);
-set_feature(>env, ARM_FEATURE_M_MAIN);
-set_feature(>env, ARM_FEATURE_THUMB_DSP);
-set_feature(>env, ARM_FEATURE_VFP4);
-cpu->midr = 0x410fc240; /* r0p0 */
-cpu->pmsav7_dregion = 8;
-cpu->isar.mvfr0 = 0x10110021;
-cpu->isar.mvfr1 = 0x1111;
-cpu->isar.mvfr2 = 0x;
-cpu->id_pfr0 = 0x0030;
-cpu->id_pfr1 = 0x0200;
-cpu->id_dfr0 = 0x0010;
-cpu->id_afr0 = 0x;
-cpu->id_mmfr0 = 0x0030;
-cpu->id_mmfr1 = 0x;
-cpu->id_mmfr2 = 0x;
-cpu->id_mmfr3 = 0x;
-cpu->isar.id_isar0 = 0x01141110;
-cpu->isar.id_isar1 = 0x02111000;
-cpu->isar.id_isar2 = 0x21112231;
-cpu->isar.id_isar3 = 0x0110;
-cpu->isar.id_isar4 = 0x01310102;
-cpu->isar.id_isar5 = 0x;
-cpu->isar.id_isar6 = 0x;
-}
-
-static void cortex_m33_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-
-set_feature(>env, ARM_FEATURE_V8);
-set_feature(>env, ARM_FEATURE_M);
-set_feature(>env, ARM_FEATURE_M_MAIN);
-set_feature(>env, ARM_FEATURE_M_SECURITY);
-set_feature(>env, ARM_FEATURE_THUMB_DSP);
-set_feature(>env, ARM_FEATURE_VFP4);
-cpu->midr = 0x410fd213; /* r0p3 */
-cpu->pmsav7_dregion = 16;
-cpu->sau_sregion = 8;
-cpu->isar.mvfr0 = 0x10110021;
-cpu->isar.mvfr1 = 0x1111;
-cpu->isar.mvfr2 = 0x0040;
-cpu->id_pfr0 = 0x0030;
-cpu->id_pfr1 = 0x0210;
-cpu->id_dfr0 = 0x0020;
-cpu->id_afr0 = 0x;
-cpu->id_mmfr0 = 0x00101F40;
-cpu->id_mmfr1 = 0x;
-cpu->id_mmfr2 = 

[PATCH 3/4] hw/arm: Move armv7m_nvic.c to hw/arm/ and always enable it for arm builds

[Thomas Huth]

qemu-system-arm/-aarch64 currently can't be built without setting the
switch CONFIG_ARM_V7M=y - which we currently always do in the config file
default-configs/arm-softmmu.mak. This is because the code in target/arm/
calls many functions from this armv7m_nvic.c, and thus linking fails
without this file.

So armv7m_nvic.c should not be under the CONFIG_ARM_V7M switch, but always
compiled for arm builds. Since we can not simply do this in hw/intc/ (with
"obj-y += ..." it would get compiled for all other architectures, too),
let's move the file to hw/arm/ instead and always enable it there.

Signed-off-by: Thomas Huth 
---
 hw/arm/Makefile.objs   |  2 ++
 hw/{intc => arm}/armv7m_nvic.c |  0
 hw/arm/trace-events| 17 +
 hw/intc/Makefile.objs  |  1 -
 hw/intc/trace-events   | 17 -
 5 files changed, 19 insertions(+), 18 deletions(-)
 rename hw/{intc => arm}/armv7m_nvic.c (100%)

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 43ce8d5b19..3c94d383a0 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -27,6 +27,8 @@ obj-$(CONFIG_VEXPRESS) += vexpress.o
 obj-$(CONFIG_ZYNQ) += xilinx_zynq.o
 obj-$(CONFIG_SABRELITE) += sabrelite.o
 
+# Note: armv7m_nvic.o is currently always required for linking
+obj-y += armv7m_nvic.o
 obj-$(CONFIG_ARM_V7M) += armv7m.o
 obj-$(CONFIG_EXYNOS4) += exynos4210.o
 obj-$(CONFIG_PXA2XX) += pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
diff --git a/hw/intc/armv7m_nvic.c b/hw/arm/armv7m_nvic.c
similarity index 100%
rename from hw/intc/armv7m_nvic.c
rename to hw/arm/armv7m_nvic.c
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 0acedcedc6..3068202a4c 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -1,5 +1,22 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
+# armv7m_nvic.c
+nvic_recompute_state(int vectpending, int vectpending_prio, int 
exception_prio) "NVIC state recomputed: vectpending %d vectpending_prio %d 
exception_prio %d"
+nvic_recompute_state_secure(int vectpending, bool vectpending_is_s_banked, int 
vectpending_prio, int exception_prio) "NVIC state recomputed: vectpending %d 
is_s_banked %d vectpending_prio %d exception_prio %d"
+nvic_set_prio(int irq, bool secure, uint8_t prio) "NVIC set irq %d secure-bank 
%d priority %d"
+nvic_irq_update(int vectpending, int pendprio, int exception_prio, int level) 
"NVIC vectpending %d pending prio %d exception_prio %d: setting irq line to %d"
+nvic_escalate_prio(int irq, int irqprio, int runprio) "NVIC escalating irq %d 
to HardFault: insufficient priority %d >= %d"
+nvic_escalate_disabled(int irq) "NVIC escalating irq %d to HardFault: disabled"
+nvic_set_pending(int irq, bool secure, bool targets_secure, bool derived, int 
en, int prio) "NVIC set pending irq %d secure-bank %d targets_secure %d derived 
%d (enabled: %d priority %d)"
+nvic_clear_pending(int irq, bool secure, int en, int prio) "NVIC clear pending 
irq %d secure-bank %d (enabled: %d priority %d)"
+nvic_acknowledge_irq(int irq, int prio) "NVIC acknowledge IRQ: %d now active 
(prio %d)"
+nvic_get_pending_irq_info(int irq, bool secure) "NVIC next IRQ %d: 
targets_secure: %d"
+nvic_complete_irq(int irq, bool secure) "NVIC complete IRQ %d (secure %d)"
+nvic_set_irq_level(int irq, int level) "NVIC external irq %d level set to %d"
+nvic_set_nmi_level(int level) "NVIC external NMI level set to %d"
+nvic_sysreg_read(uint64_t addr, uint32_t value, unsigned size) "NVIC sysreg 
read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
+nvic_sysreg_write(uint64_t addr, uint32_t value, unsigned size) "NVIC sysreg 
write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
+
 # virt-acpi-build.c
 virt_acpi_setup(void) "No fw cfg or ACPI disabled. Bailing out."
 
diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index f726d87532..2d981abb4e 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -26,7 +26,6 @@ obj-$(CONFIG_APIC) += apic.o apic_common.o
 obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
 obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_kvm.o
 obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_its_kvm.o
-obj-$(CONFIG_ARM_V7M) += armv7m_nvic.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_gic.o exynos4210_combiner.o
 obj-$(CONFIG_GRLIB) += grlib_irqmp.o
 obj-$(CONFIG_IOAPIC) += ioapic.o
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 90c9d07c1a..09a7fedee8 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -179,23 +179,6 @@ gicv3_redist_badwrite(uint32_t cpu, uint64_t offset, 
uint64_t data, unsigned siz
 gicv3_redist_set_irq(uint32_t cpu, int irq, int level) "GICv3 redistributor 
0x%x interrupt %d level changed to %d"
 gicv3_redist_send_sgi(uint32_t cpu, int irq) "GICv3 redistributor 0x%x pending 
SGI %d"
 
-# armv7m_nvic.c
-nvic_recompute_state(int vectpending, int vectpending_prio, int 
exception_prio) "NVIC state recomputed: vectpending %d vectpending_prio %d 
exception_prio %d"

[PATCH 1/4] target/arm: Make cpu_register() and set_feature() available for other files

[Thomas Huth]

Move the common set_feature() and unset_feature() functions from cpu.c and
cpu64.c to internals.h, and make cpu_register() (renamed to arm_cpu_register())
available from there, too, so we can register CPUs also from other files
in the future.

Signed-off-by: Thomas Huth 
---
 target/arm/cpu.c   | 20 ++--
 target/arm/cpu64.c | 17 +
 target/arm/internals.h | 18 ++
 3 files changed, 21 insertions(+), 34 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 2399c14471..f1f9eecdc8 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -614,16 +614,6 @@ static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
 
 #endif
 
-static inline void set_feature(CPUARMState *env, int feature)
-{
-env->features |= 1ULL << feature;
-}
-
-static inline void unset_feature(CPUARMState *env, int feature)
-{
-env->features &= ~(1ULL << feature);
-}
-
 static int
 print_insn_thumb1(bfd_vma pc, disassemble_info *info)
 {
@@ -2515,12 +2505,6 @@ static void arm_max_initfn(Object *obj)
 
 #endif /* !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64) */
 
-struct ARMCPUInfo {
-const char *name;
-void (*initfn)(Object *obj);
-void (*class_init)(ObjectClass *oc, void *data);
-};
-
 static const ARMCPUInfo arm_cpus[] = {
 #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
 { .name = "arm926",  .initfn = arm926_initfn },
@@ -2681,7 +2665,7 @@ static void cpu_register_class_init(ObjectClass *oc, void 
*data)
 acc->info = data;
 }
 
-static void cpu_register(const ARMCPUInfo *info)
+void arm_cpu_register(const ARMCPUInfo *info)
 {
 TypeInfo type_info = {
 .parent = TYPE_ARM_CPU,
@@ -2722,7 +2706,7 @@ static void arm_cpu_register_types(void)
 type_register_static(_interface_type_info);
 
 while (info->name) {
-cpu_register(info);
+arm_cpu_register(info);
 info++;
 }
 
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index d7f5bf610a..5239ba5529 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "cpu.h"
+#include "internals.h"
 #include "qemu/module.h"
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/loader.h"
@@ -29,16 +30,6 @@
 #include "kvm_arm.h"
 #include "qapi/visitor.h"
 
-static inline void set_feature(CPUARMState *env, int feature)
-{
-env->features |= 1ULL << feature;
-}
-
-static inline void unset_feature(CPUARMState *env, int feature)
-{
-env->features &= ~(1ULL << feature);
-}
-
 #ifndef CONFIG_USER_ONLY
 static uint64_t a57_a53_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
@@ -396,12 +387,6 @@ static void aarch64_max_initfn(Object *obj)
 }
 }
 
-struct ARMCPUInfo {
-const char *name;
-void (*initfn)(Object *obj);
-void (*class_init)(ObjectClass *oc, void *data);
-};
-
 static const ARMCPUInfo aarch64_cpus[] = {
 { .name = "cortex-a57", .initfn = aarch64_a57_initfn },
 { .name = "cortex-a53", .initfn = aarch64_a53_initfn },
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 232d963875..e71196ed5f 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1046,4 +1046,22 @@ void arm_log_exception(int idx);
 
 #endif /* !CONFIG_USER_ONLY */
 
+static inline void set_feature(CPUARMState *env, int feature)
+{
+env->features |= 1ULL << feature;
+}
+
+static inline void unset_feature(CPUARMState *env, int feature)
+{
+env->features &= ~(1ULL << feature);
+}
+
+struct ARMCPUInfo {
+const char *name;
+void (*initfn)(Object *obj);
+void (*class_init)(ObjectClass *oc, void *data);
+};
+
+void arm_cpu_register(const ARMCPUInfo *info);
+
 #endif
-- 
2.18.1

[PATCH 4/4] default-configs: Do not enforce CONFIG_ARM_V7M anymore

[Thomas Huth]

The arm builds can now be done without CONFIG_ARM_V7M, so do not
enforce this config switch anymore, it's getting selected in
hw/arm/Kconfig automatically if needed.

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 1f2e0e7fde..64f5ac24bf 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -1,8 +1,5 @@
 # Default configuration for arm-softmmu
 
-# TODO: ARM_V7M is currently always required - make this more flexible!
-CONFIG_ARM_V7M=y
-
 # CONFIG_PCI_DEVICES=n
 # CONFIG_TEST_DEVICES=n
 
-- 
2.18.1

[PATCH 0/4] Make it possible to compile with CONFIG_ARM_V7M=n

[Thomas Huth]

We've got CONFIG_ARM_V7M, but it currently can't be disabled.
Here are some patches that should allow to disable the switch
(if the corresponding boards are disabled, too).

RFC -> v1:
 - Move the definitions into internals.h instead of cpu.h in the 1st patch
 - Instead of creating an ugly stubs file, simply make armv7m_nvic.c
   mandatory for linking.

Thomas Huth (4):
  target/arm: Make cpu_register() and set_feature() available for other
files
  target/arm: Move cortex-m related functions to new file v7m.c
  hw/arm: Move armv7m_nvic.c to hw/arm/ and always enable it for arm
builds
  default-configs: Do not enforce CONFIG_ARM_V7M anymore

 default-configs/arm-softmmu.mak |   3 -
 hw/arm/Makefile.objs|   2 +
 hw/{intc => arm}/armv7m_nvic.c  |   0
 hw/arm/trace-events |  17 +++
 hw/intc/Makefile.objs   |   1 -
 hw/intc/trace-events|  17 ---
 target/arm/Makefile.objs|   1 +
 target/arm/cpu.c| 166 +--
 target/arm/cpu64.c  |  17 +--
 target/arm/internals.h  |  18 +++
 target/arm/v7m.c| 193 
 11 files changed, 234 insertions(+), 201 deletions(-)
 rename hw/{intc => arm}/armv7m_nvic.c (100%)
 create mode 100644 target/arm/v7m.c

-- 
2.18.1

Re: Cannot login into emulated smartcard with OpenSC because it expects external PIN pad.

[Thomas Huth]

On 21/09/2019 10.54, Andrei Borzenkov wrote:
> USB card reader emulated by QEMU announces presence of PIN pad. OpenSC
> will not request PIN from user in this case and assumes PIN is being
> entered off-band on external device. Unfortunately QEMU does not seem to
> offer PIN entry and access to card always fails.
> 
> Changing device to not announce non-existing capability fixes it and
> allows to use OpenSC framework with emulated card.
> 
> --- a/hw/usb/dev-smartcard-reader.c
> +++ b/hw/usb/dev-smartcard-reader.c
> @@ -398,7 +398,7 @@ static const uint8_t qemu_ccid_descriptor[] = {
>   * u16 wLcdLayout; XXYY Number of lines (XX) and
> chars per
>   * line for LCD display used for PIN entry.  -
> no LCD
>   */
> -0x01,   /*
> +0x00,   /*
>   * u8  bPINSupport; 01h PIN Verification,
>   *  02h PIN Modification
>   */

 Hi,

when sending patches, please make sure to include a "Signed-off-by:"
line at the end of the patch description. For details see:

 https://wiki.qemu.org/Contribute/SubmitAPatch

Thanks,
 Thomas

RE: [Qemu-arm] [RFC PATCH 00/14] hw/arm: Add the Raspberry Pi 4B

[Stewart Hildebrand]

On Wednesday, September 4, 2019 1:13 PM, Philippe Mathieu-Daudé wrote:
>Esteban wrote me over the weekend asking about raspi4 progress.
>I cleaned up my patches/notes to pass him. Other help is also welcomed :)
>I got scared trying to understand how to use the GIC, and wire the various
>IRQs.
>
>Most important notes about testing are in patch #12:
>"Add the BCM2838 which uses a GICv2"
>
>Not much works yet, it only runs a bit until configuring the GIC.
>
>branch pushed at https://gitlab.com/philmd/qemu/commits/raspi4_wip
>
>Regards,
>
>Phil.

It seems upstream linux may be adopting the BCM2711 naming convention [1], 
though it doesn't look like the series [1] has been committed yet. The SoC name 
in documentation is BCM2711 [2]. I have no opinion on which naming convention 
the QEMU community adopts, I simply wanted to pass along this observation.

-Stew

[1] https://patchwork.kernel.org/cover/11092613/
[2] 
https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2711/README.md

Cannot login into emulated smartcard with OpenSC because it expects external PIN pad.

[Andrei Borzenkov]

USB card reader emulated by QEMU announces presence of PIN pad. OpenSC
will not request PIN from user in this case and assumes PIN is being
entered off-band on external device. Unfortunately QEMU does not seem to
offer PIN entry and access to card always fails.

Changing device to not announce non-existing capability fixes it and
allows to use OpenSC framework with emulated card.

--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -398,7 +398,7 @@ static const uint8_t qemu_ccid_descriptor[] = {
  * u16 wLcdLayout; XXYY Number of lines (XX) and
chars per
  * line for LCD display used for PIN entry.  -
no LCD
  */
-0x01,   /*
+0x00,   /*
  * u8  bPINSupport; 01h PIN Verification,
  *  02h PIN Modification
  */

Re: [Qemu-devel] [PATCH v3] virtio-mmio: implement modern (v2), personality (virtio-1)

[Vasyl Vavrychuk]

Hi, Sergio,

> For this reason, the v2 personality is disabled, keeping the legacy
> behavior as default. Machine types willing to use v2, can enable it
> using MachineClass's compat_props.
...
> +    DEFINE_PROP_BOOL("force-legacy", VirtIOMMIOProxy, legacy, true),

Currently, I am not enable to set "force-legacy" to false from command 
line for

virt machine.

I think, the "force-legacy" and compat_props should work the other way 
around.


The "force-legacy" should be set to false by default to select a new 
behaviour.

Instead of this hw_compat_4_1 should be modified to keep the old behaviour:

--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -29,6 +29,7 @@

 GlobalProperty hw_compat_4_1[] = {
 { "virtio-pci", "x-pcie-flr-init", "off" },
+    { "virtio-mmio", "force-legacy", "on" },
 };
 const size_t hw_compat_4_1_len = G_N_ELEMENTS(hw_compat_4_1);


This way, one can keep the old behaviour by doing "-M virt-4.1"

Best regards,
Vasyl

Re: [PATCH v2 1/2] riscv: hw: Drop "clock-frequency" property of cpu nodes

[Bin Meng]

Hi Philippe,

On Sat, Sep 21, 2019 at 4:51 PM Philippe Mathieu-Daudé
 wrote:
>
> Hi Bin,
>
> On 9/21/19 7:41 AM, Bin Meng wrote:
> > The "clock-frequency" property of cpu nodes isn't required. Drop it.
> >
> > This is to keep in sync with Linux kernel commit below:
> > https://patchwork.kernel.org/patch/11133031/
>
> What happens if you run a older kernel that doesn't contain the
> referenced patch?
>

This property has never been required by the kernel since the beginning.

Regards,
Bin

[Bug 1842787] Re: Writes permanently hang with very heavy I/O on virtio-scsi - worse on virtio-blk

[James Harvey]

Apologies, it looks like I ran into two separate bugs, one with XFS, and
one with BTRFS, that had the same symptom, initially making me to think
this must be a QEMU issue.

Using blktrace, I was able to see within the VM, that the virtio block
device wasn't getting the writes that were going into uninterruptible
sleep.

So, this should be able to be closed.  For some reason, virtio-blk
seemed to trigger the bugs more rapidly, but at this point, I can't say
there is anything at fault with it or virtio-scsi.


BTRFS issue was discussed and linked to here 
https://lore.kernel.org/linux-btrfs/CAL3q7H4peDv_bQa5vGJeOM=V--yq1a1=ahat5qcsxjbndos...@mail.gmail.com/
 and has been released.  I've been able to run it for several days without a 
lockup, so it seems to have fixed the issue for me.

I just emailed the XFS list about the separate problems with it.  No
idea if it's an issue in more recent kernels than 5.1.15-5.1.16, which
is what I was running at the time of the XFS errors.  (Like the original
report said, I was on 5.2.11 at that point.)  See
https://www.spinics.net/lists/linux-xfs/msg31927.html

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1842787

Title:
  Writes permanently hang with very heavy I/O on virtio-scsi - worse on
  virtio-blk

Status in QEMU:
  New

Bug description:
  Up to date Arch Linux on host and guest.  linux 5.2.11.  QEMU 4.1.0.
  Full command line at bottom.

  Host gives QEMU two thin LVM volumes.  The first is the root
  filesystem, and the second is for heavy I/O, on a Samsung 970 Evo 1TB.

  When maxing out the I/O on the second virtual block device using
  virtio-blk, I often get a "lockup" in about an hour or two.  From the
  advise of iggy in IRC, I switched over to virtio-scsi.  It ran
  perfectly for a few days, but then "locked up" in the same way.

  By "lockup", I mean writes to the second virtual block device
  permanently hang.  I can read files from it, but even "touch foo"
  never times out, cannot be "kill -9"'ed, and is stuck in
  uninterruptible sleep.

  When this happens, writes to the first virtual block device with the
  root filesystem are fine, so the O/S itself remains responsive.

  The second virtual block device uses BTRFS.  But, I have also tried
  XFS and reproduced the issue.

  In guest, when this starts, it starts logging "task X blocked for more
  than Y seconds".  Below is an example of one of these.  At this point,
  anything that is or does in the future write to this block device gets
  stuck in uninterruptible sleep.

  -

  INFO: task kcompactd:232 blocked for more than 860 seconds.
    Not tained 5.2.11-1 #1
  "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this messae.
  kcompactd0  D0   232  2 0x80004000
  Call Trace:
   ? __schedule+0x27f/0x6d0
   schedule+0x3d/0xc0
   io_schedule+0x12/0x40
   __lock_page+0x14a/0x250
   ? add_to_page_cache_lru+0xe0/0xe0
   migrate_pages+0x803/0xb70
   ? isolate_migratepages_block+0x9f0/0x9f0
   ? __reset_isolation_suitable+0x110/0x110
   compact_zone+0x6a2/0xd30
   kcompactd_do_work+0x134/0x260
   ? kvm_clock_read+0x14/0x30
   ? kvm_sched_clock_read+0x5/0x10
   kcompactd+0xd3/0x220
   ? wait_woken+0x80/0x80
   kthread+0xfd/0x130
   ? kcompactd_do_work+0x260/0x260
   ? kthread_park+0x80/0x80
   ret_from_fork+0x35/0x40

  -

  In guest, there are no other dmesg/journalctl entries other than
  "task...blocked".

  On host, there are no dmesg/journalctl entries whatsoever.  Everything
  else in host continues to work fine, including other QEMU VM's on the
  same underlying SSD (but obviously different lvm volumes.)

  I understand there might not be enough to go on here, and I also
  understand it's possible this isn't a QEMU bug.  Happy to run given
  commands or patches to help diagnose what's going on here.

  I'm now running a custom compiled QEMU 4.1.0, with debug symbols, so I
  can get a meaningful backtrace from the host point of view.

  I've only recently tried this level of I/O, so can't say if this is a
  new issue.

  When writes are hanging, on host, I can connect to the monitor.
  Running "info block" shows nothing unusual.

  -

  /usr/bin/qemu-system-x86_64
     -name arch,process=qemu:arch
     -no-user-config
     -nodefaults
     -nographic
     -uuid 0528162b-2371-41d5-b8da-233fe61b6458
     -pidfile /tmp/0528162b-2371-41d5-b8da-233fe61b6458.pid
     -machine q35,accel=kvm,vmport=off,dump-guest-core=off
     -cpu SandyBridge-IBRS
     -smp cpus=24,cores=12,threads=1,sockets=2
     -m 24G
     -drive if=pflash,format=raw,readonly,file=/usr/share/ovmf/x64/OVMF_CODE.fd
     -drive 
if=pflash,format=raw,readonly,file=/var/qemu/0528162b-2371-41d5-b8da-233fe61b6458.fd
     -monitor telnet:localhost:8000,server,nowait,nodelay
     -spice 
unix,addr=/tmp/0528162b-2371-41d5-b8da-233fe61b6458.sock,disable-ticketing
     -device ioh3420,id=pcie.1,bus=pcie.0,slot=0
 

[PATCH] hw/arm/boot: Use the IEC binary prefix definitions

[Philippe Mathieu-Daudé]

IEC binary prefixes ease code review: the unit is explicit.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/arm/boot.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index bf97ef3e33..59bb2fa0d3 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -1095,7 +1095,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
  * we might still make a bad choice here.
  */
 info->initrd_start = info->loader_start +
-MIN(info->ram_size / 2, 128 * 1024 * 1024);
+MIN(info->ram_size / 2, 128 * MiB);
 if (image_high_addr) {
 info->initrd_start = MAX(info->initrd_start, image_high_addr);
 }
@@ -1155,13 +1155,13 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
  *
  * Let's play safe and prealign it to 2MB to give us some 
space.
  */
-align = 2 * 1024 * 1024;
+align = 2 * MiB;
 } else {
 /*
  * Some 32bit kernels will trash anything in the 4K page the
  * initrd ends in, so make sure the DTB isn't caught up in 
that.
  */
-align = 4096;
+align = 4 * KiB;
 }
 
 /* Place the DTB after the initrd in memory with alignment. */
@@ -1178,7 +1178,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
 info->loader_start + KERNEL_ARGS_ADDR;
 fixupcontext[FIXUP_ARGPTR_HI] =
 (info->loader_start + KERNEL_ARGS_ADDR) >> 32;
-if (info->ram_size >= (1ULL << 32)) {
+if (info->ram_size >= 4 * GiB) {
 error_report("RAM size must be less than 4GB to boot"
  " Linux kernel using ATAGS (try passing a device 
tree"
  " using -dtb)");
-- 
2.20.1

[PATCH] hw/ptimer: Assert next_event is newer than last_event

[Philippe Mathieu-Daudé]

If the period is too big, the 'delta * period' product result
might overflow, resulting in a negative number, then the
next_event ends before the last_event. This is buggy, as there
is no forward progress. Assert this can not happen.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/core/ptimer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index d58e2dfdb0..88085d4c81 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -125,6 +125,9 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
 
 s->last_event = s->next_event;
 s->next_event = s->last_event + delta * period;
+/* Verify forward progress */
+g_assert(s->next_event > s->last_event);
+
 if (period_frac) {
 s->next_event += ((int64_t)period_frac * delta) >> 32;
 }
-- 
2.20.1

Re: [PATCH 3/3] docker: remove unused debian-sid and debian-ports

[Philippe Mathieu-Daudé]

Hi John,

On 9/20/19 2:14 AM, John Snow wrote:
> These are listed as "partial" images, but have no user.
> Remove them.
> 
> Signed-off-by: John Snow 
> ---
>  tests/docker/Makefile.include|  4 +--
>  tests/docker/dockerfiles/debian-ports.docker | 36 
>  tests/docker/dockerfiles/debian-sid.docker   | 35 ---
>  3 files changed, 2 insertions(+), 73 deletions(-)
>  delete mode 100644 tests/docker/dockerfiles/debian-ports.docker
>  delete mode 100644 tests/docker/dockerfiles/debian-sid.docker

Can you split this patch in 2, one per image? That will ease the reverts.

Once splitted feel free to add to each new patch:
Reviewed-by: Philippe Mathieu-Daudé 

Thanks,

Phil.

[PATCH] hw/m68k/next-cube: Avoid static RTC variables and introduce control register

[Thomas Huth]

Coverity currently complains that the "if (0x00 & (0x80 >> (phase - 8))"
in next-cube.c can never be true. Right it is. The "0x00" is meant as value
of the control register of the RTC, which is currently not implemented yet.
Thus, let's add a register variable for this now. However, the RTC
registers are currently defined as static variables in nextscr2_write(),
which is quite ugly. Thus let's also move the RTC variables to the main
machine state instead. In the long run, we should likely even refactor
the whole RTC code into a separate device in a separate file, but that's
something for calm winter nights later... as a first step, cleaning up
the static variables and shutting up the warning from Coverity should
be sufficient.

Signed-off-by: Thomas Huth 
---
 hw/m68k/next-cube.c | 72 +
 1 file changed, 40 insertions(+), 32 deletions(-)

diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
index 9a4a7328f9..2f305afa9f 100644
--- a/hw/m68k/next-cube.c
+++ b/hw/m68k/next-cube.c
@@ -60,6 +60,15 @@ typedef struct next_dma {
 uint32_t size;
 } next_dma;
 
+typedef struct NextRtc {
+uint8_t ram[32];
+uint8_t command;
+uint8_t value;
+uint8_t status;
+uint8_t control;
+uint8_t retval;
+} NextRtc;
+
 typedef struct {
 MachineState parent;
 
@@ -77,7 +86,7 @@ typedef struct {
 uint32_t scr1;
 uint32_t scr2;
 
-uint8_t rtc_ram[32];
+NextRtc rtc;
 } NeXTState;
 
 /* Thanks to NeXT forums for this */
@@ -105,11 +114,8 @@ static void nextscr2_write(NeXTState *s, uint32_t val, int 
size)
 static int led;
 static int phase;
 static uint8_t old_scr2;
-static uint8_t rtc_command;
-static uint8_t rtc_value;
-static uint8_t rtc_status = 0x90;
-static uint8_t rtc_return;
 uint8_t scr2_2;
+NextRtc *rtc = >rtc;
 
 if (size == 4) {
 scr2_2 = (val >> 8) & 0xFF;
@@ -135,52 +141,53 @@ static void nextscr2_write(NeXTState *s, uint32_t val, 
int size)
 if (((old_scr2 & SCR2_RTCLK) != (scr2_2 & SCR2_RTCLK)) &&
 ((scr2_2 & SCR2_RTCLK) == 0)) {
 if (phase < 8) {
-rtc_command = (rtc_command << 1) |
-  ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
+rtc->command = (rtc->command << 1) |
+   ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
 }
 if (phase >= 8 && phase < 16) {
-rtc_value = (rtc_value << 1) | ((scr2_2 & SCR2_RTDATA) ? 1 : 
0);
+rtc->value = (rtc->value << 1) |
+ ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
 
 /* if we read RAM register, output RT_DATA bit */
-if (rtc_command <= 0x1F) {
+if (rtc->command <= 0x1F) {
 scr2_2 = scr2_2 & (~SCR2_RTDATA);
-if (s->rtc_ram[rtc_command] & (0x80 >> (phase - 8))) {
+if (rtc->ram[rtc->command] & (0x80 >> (phase - 8))) {
 scr2_2 |= SCR2_RTDATA;
 }
 
-rtc_return = (rtc_return << 1) |
- ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
+rtc->retval = (rtc->retval << 1) |
+  ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
 }
 /* read the status 0x30 */
-if (rtc_command == 0x30) {
+if (rtc->command == 0x30) {
 scr2_2 = scr2_2 & (~SCR2_RTDATA);
 /* for now status = 0x98 (new rtc + FTU) */
-if (rtc_status & (0x80 >> (phase - 8))) {
+if (rtc->status & (0x80 >> (phase - 8))) {
 scr2_2 |= SCR2_RTDATA;
 }
 
-rtc_return = (rtc_return << 1) |
- ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
+rtc->retval = (rtc->retval << 1) |
+  ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
 }
 /* read the status 0x31 */
-if (rtc_command == 0x31) {
+if (rtc->command == 0x31) {
 scr2_2 = scr2_2 & (~SCR2_RTDATA);
 /* for now 0x00 */
-if (0x00 & (0x80 >> (phase - 8))) {
+if (rtc->control & (0x80 >> (phase - 8))) {
 scr2_2 |= SCR2_RTDATA;
 }
-rtc_return = (rtc_return << 1) |
- ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
+rtc->retval = (rtc->retval << 1) |
+  ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
 }
 
-if ((rtc_command >= 0x20) && (rtc_command <= 0x2F)) {
+if ((rtc->command >= 0x20) && (rtc->command <= 0x2F)) {
 scr2_2 = scr2_2 & (~SCR2_RTDATA);
 /* for now 0x00 */
   

Re: [PATCH 5/5] .travis.yml: Build and install EDK2 roms

[Philippe Mathieu-Daudé]

On 6/14/19 8:44 PM, Alex Bennée wrote:
> Laszlo Ersek  writes:
>> On 06/13/19 18:59, Philippe Mathieu-Daudé wrote:
>>> Hi Laszlo,
>>>
>>> On 3/12/19 5:29 PM, Laszlo Ersek wrote:
 On 03/11/19 01:30, Philippe Mathieu-Daudé wrote:
> Add a job to build and install the EDK2 platform firmware binaries.
>
> This job is only triggered if the last commit matches the EDK2
> name (case insensitive), or when tag are created (such releases
> or release candidates).
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  .travis.yml | 21 +
>  1 file changed, 21 insertions(+)
>
> diff --git a/.travis.yml b/.travis.yml
> index e942175dd3..628cc52c99 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -258,3 +258,24 @@ matrix:
>  - env:
>  - CONFIG="--disable-system"
>  - TEST_CMD="make -j3 check-tcg V=1"
> +
> +
> +# EDK2 roms builds
> +- if: tag IS present OR commit_message =~ /(edk2|EDK2)/
> +  env:
> +- CONFIG="--disable-system --disable-user --prefix=$PWD/dist"
> +- EDK2_BUILD_OPTIONS="--quiet --silent"
> +  script:
> +- git submodule update --init roms/edk2
> +- make -j3
> +- make -C roms efi -j2
> +- make install
> +  addons:
> +apt:
> +  packages:
> +- dos2unix
> +- gcc-aarch64-linux-gnu
> +- gcc-arm-linux-gnueabihf
> +- iasl
> +- nasm
> +- uuid-dev
>

 Regardless of what problem we're trying to address with "--quiet
 --silent", those options are wrong. You certainly want detailed build
 logs for the case a CI job fails (at build time or at runtime).
>>>
>>> On Travis we get:
>>>
>>> The job exceeded the maximum log length, and has been terminated.
>>> (https://travis-ci.org/philmd/qemu/jobs/545329905)
>>>
>>> So I moved to GitLab, but we still get:
>>>
>>> "Job's log exceeded limit of 4194304 bytes."
>>> (https://gitlab.com/philmd/qemu/-/jobs/230772314)
>>>
>>> Regarding the options to pass to edk2-build.sh,
>>>
>>> $ build --help
>>>   -j LOGFILE, --log=LOGFILE
>>>Put log in specified file as well as on console.
>>>   -s, --silent Make use of silent mode of (n)make.
>>>   -q, --quiet  Disable all messages except FATAL ERRORS.
>>>   -v, --verboseTurn on verbose output with informational messages
>>>printed, including library instances selected, final
>>>dependency expression, and warning messages, etc.
>>>
>>> '--log' duplicate the output, and I don't want to reduce the log
>>> details, so I understand I should use:
>>>
>>>   ./edk2-build.sh [...] --log=build.log >/dev/null || cat build.log
>>>
>>> Is that correct? But then I'd need to modify Makefile.edk2 to redirect
>>> stdout.
>>
>> Would it be possible to invoke the outermost make like this?
>>
>>   make -C roms -j2 efi >make.roms.efi.log 2>&1 \
>>   || ( tail -c 2M make.roms.efi.log; false )

The build gets killed:

  No output has been received in the last 10m0s, this potentially
  indicates a stalled build or something wrong with the build itself.
  Check the details on how to adjust your build configuration on:

https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received

  The build has been terminated

> Or something like:
> 
>  after_failure:
>- tail -c 2M make.roms.efi.log
> 
> as Cleber suggested for his acceptance tests. Generally we want to try
> and make the builds less noisy and only echo relevant details when it
> fails. However we've tended to turn up the noise so we can debug stuff
> and that is just painful to browser on the Travis website.

I'll try that.

 The reason why I only include DEBUG firmware builds in the edk2 bundling
 series is similar -- RELEASE builds lack DEBUG messages and ASSERT()s,
 and as such they are 100% unsupportable in my book. Bugs in software are
 the norm, not the exception, so we should allow (even force) the user
 (and remote systems) to provide as much information as they can.
>>>
>>> Sure, we have the same book here ;)
>>>
>>> Regards,
>>>
>>> Phil.
>>>
> --
> Alex Bennée
> 

Re: [Qemu-devel] [PATCH v1 1/2] RISC-V: Handle bus errors in the page table walker

[Philippe Mathieu-Daudé]

On 9/18/19 1:22 AM, Alistair Francis wrote:
> From: Palmer Dabbelt 
> 
> We directly access physical memory while walking the page tables on
> RISC-V, but while doing so we were using cpu_ld*() which does not report
> bus errors.  This patch converts the page table walker over to use
> address_space_ld*(), which allows bus errors to be detected.
> 
> Signed-off-by: Palmer Dabbelt 
> Signed-off-by: Alistair Francis 
> ---
>  target/riscv/cpu_helper.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 87dd6a6ece..c82e7ed52b 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -169,7 +169,8 @@ static int get_physical_address(CPURISCVState *env, 
> hwaddr *physical,
>  /* NOTE: the env->pc value visible here will not be
>   * correct, but the value visible to the exception handler
>   * (riscv_cpu_do_interrupt) is correct */
> -
> +MemTxResult res;
> +MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
>  int mode = mmu_idx;
>  
>  if (mode == PRV_M && access_type != MMU_INST_FETCH) {
> @@ -256,11 +257,16 @@ restart:
>  1 << MMU_DATA_LOAD, PRV_S)) {
>  return TRANSLATE_PMP_FAIL;
>  }
> +
>  #if defined(TARGET_RISCV32)
> -target_ulong pte = ldl_phys(cs->as, pte_addr);
> +target_ulong pte = address_space_ldl(cs->as, pte_addr, attrs, );
>  #elif defined(TARGET_RISCV64)
> -target_ulong pte = ldq_phys(cs->as, pte_addr);
> +target_ulong pte = address_space_ldq(cs->as, pte_addr, attrs, );
>  #endif
> +if (res != MEMTX_OK) {
> +return TRANSLATE_FAIL;
> +}
> +
>  hwaddr ppn = pte >> PTE_PPN_SHIFT;
>  
>  if (!(pte & PTE_V)) {
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH v1 2/2] RISC-V: Implement cpu_do_transaction_failed

[Philippe Mathieu-Daudé]

On 9/18/19 1:23 AM, Alistair Francis wrote:
> From: Palmer Dabbelt 
> 
> This converts our port over from cpu_do_unassigned_access to
> cpu_do_transaction_failed, as cpu_do_unassigned_access has been
> deprecated.
> 
> Signed-off-by: Palmer Dabbelt 
> Signed-off-by: Alistair Francis 
> ---
>  target/riscv/cpu.c|  2 +-
>  target/riscv/cpu.h|  7 +--
>  target/riscv/cpu_helper.c | 11 +++
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f13e298a36..3939963b71 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -484,7 +484,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void 
> *data)
>  cc->gdb_stop_before_watchpoint = true;
>  cc->disas_set_info = riscv_cpu_disas_set_info;
>  #ifndef CONFIG_USER_ONLY
> -cc->do_unassigned_access = riscv_cpu_unassigned_access;
> +cc->do_transaction_failed = riscv_cpu_do_transaction_failed;
>  cc->do_unaligned_access = riscv_cpu_do_unaligned_access;
>  cc->get_phys_page_debug = riscv_cpu_get_phys_page_debug;
>  #endif
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 124ed33ee4..8c64c68538 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -264,8 +264,11 @@ void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr 
> addr,
>  bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>  MMUAccessType access_type, int mmu_idx,
>  bool probe, uintptr_t retaddr);
> -void riscv_cpu_unassigned_access(CPUState *cpu, hwaddr addr, bool is_write,
> - bool is_exec, int unused, unsigned size);
> +void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> + vaddr addr, unsigned size,
> + MMUAccessType access_type,
> + int mmu_idx, MemTxAttrs attrs,
> + MemTxResult response, uintptr_t 
> retaddr);
>  char *riscv_isa_string(RISCVCPU *cpu);
>  void riscv_cpu_list(void);
>  
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index c82e7ed52b..917252f71b 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -408,20 +408,23 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, 
> vaddr addr)
>  return phys_addr;
>  }
>  
> -void riscv_cpu_unassigned_access(CPUState *cs, hwaddr addr, bool is_write,
> - bool is_exec, int unused, unsigned size)
> +void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> + vaddr addr, unsigned size,
> + MMUAccessType access_type,
> + int mmu_idx, MemTxAttrs attrs,
> + MemTxResult response, uintptr_t retaddr)
>  {
>  RISCVCPU *cpu = RISCV_CPU(cs);
>  CPURISCVState *env = >env;
>  
> -if (is_write) {
> +if (access_type == MMU_DATA_STORE) {
>  cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
>  } else {
>  cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
>  }
>  
>  env->badaddr = addr;
> -riscv_raise_exception(>env, cs->exception_index, GETPC());
> +riscv_raise_exception(>env, cs->exception_index, retaddr);
>  }
>  
>  void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 4/7] exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG

[Philippe Mathieu-Daudé]

On 9/20/19 1:29 AM, Richard Henderson wrote:
> This reduces the size of a release build by about 10k.
> Noticably, within the tlb miss helpers.
> 
> Signed-off-by: Richard Henderson 
> ---
>  include/exec/cpu-all.h | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index a53b761b48..b11ee1f711 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -215,7 +215,11 @@ typedef struct {
>  int bits;
>  } TargetPageBits;
>  extern const TargetPageBits target_page;
> -#define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
> +# ifdef CONFIG_DEBUG_TCG
> +#  define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
> +# else
> +#  define TARGET_PAGE_BITS target_page.bits
> +# endif
>  #else
>  #define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS
>  #endif
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 2/7] exec: Split out variable page size support to exec-vary.c

[Philippe Mathieu-Daudé]

On 9/20/19 1:29 AM, Richard Henderson wrote:
> The next patch will play a trick with "const" that will
> confuse the compiler about the uses of target_page_bits
> within exec.c.  Moving everything to a new file prevents
> this confusion.
> 
> No functional change so far.
> 
> Signed-off-by: Richard Henderson 
> ---
>  Makefile.target   |  2 +-
>  include/qemu-common.h |  6 +
>  exec-vary.c   | 57 +++
>  exec.c| 34 --
>  4 files changed, 64 insertions(+), 35 deletions(-)
>  create mode 100644 exec-vary.c
> 
> diff --git a/Makefile.target b/Makefile.target
> index 5e916230c4..ca3d14efe1 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -107,7 +107,7 @@ obj-y += trace/
>  
>  #
>  # cpu emulator library
> -obj-y += exec.o
> +obj-y += exec.o exec-vary.o
>  obj-y += accel/
>  obj-$(CONFIG_TCG) += tcg/tcg.o tcg/tcg-op.o tcg/tcg-op-vec.o 
> tcg/tcg-op-gvec.o
>  obj-$(CONFIG_TCG) += tcg/tcg-common.o tcg/optimize.o
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 0235cd3b91..3e800c2224 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -74,6 +74,12 @@ void cpu_exec_step_atomic(CPUState *cpu);
>   */
>  bool set_preferred_target_page_bits(int bits);
>  
> +/**
> + * finalize_target_page_bits:
> + * Commit the final value set by set_preferred_target_page_bits.
> + */
> +void finalize_target_page_bits(void);
> +
>  /**
>   * Sends a (part of) iovec down a socket, yielding when the socket is full, 
> or
>   * Receives data into a (part of) iovec from a socket,
> diff --git a/exec-vary.c b/exec-vary.c
> new file mode 100644
> index 00..48c0ab306c
> --- /dev/null
> +++ b/exec-vary.c
> @@ -0,0 +1,57 @@
> +/*
> + * Variable page size handling
> + *
> + *  Copyright (c) 2003 Fabrice Bellard
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "exec/exec-all.h"
> +
> +#ifdef TARGET_PAGE_BITS_VARY
> +int target_page_bits;
> +bool target_page_bits_decided;
> +#endif
> +
> +bool set_preferred_target_page_bits(int bits)
> +{
> +/*
> + * The target page size is the lowest common denominator for all
> + * the CPUs in the system, so we can only make it smaller, never
> + * larger. And we can't make it smaller once we've committed to
> + * a particular size.
> + */
> +#ifdef TARGET_PAGE_BITS_VARY
> +assert(bits >= TARGET_PAGE_BITS_MIN);
> +if (target_page_bits == 0 || target_page_bits > bits) {
> +if (target_page_bits_decided) {
> +return false;
> +}
> +target_page_bits = bits;
> +}
> +#endif
> +return true;
> +}
> +
> +void finalize_target_page_bits(void)
> +{
> +#ifdef TARGET_PAGE_BITS_VARY
> +if (target_page_bits == 0) {
> +target_page_bits = TARGET_PAGE_BITS_MIN;
> +}
> +target_page_bits_decided = true;
> +#endif
> +}
> diff --git a/exec.c b/exec.c
> index 8b998974f8..33bd0e36c1 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -92,11 +92,6 @@ MemoryRegion io_mem_rom, io_mem_notdirty;
>  static MemoryRegion io_mem_unassigned;
>  #endif
>  
> -#ifdef TARGET_PAGE_BITS_VARY
> -int target_page_bits;
> -bool target_page_bits_decided;
> -#endif
> -
>  CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
>  
>  /* current CPU in the current thread. It is only valid inside
> @@ -110,37 +105,8 @@ int use_icount;
>  uintptr_t qemu_host_page_size;
>  intptr_t qemu_host_page_mask;
>  
> -bool set_preferred_target_page_bits(int bits)
> -{
> -/* The target page size is the lowest common denominator for all
> - * the CPUs in the system, so we can only make it smaller, never
> - * larger. And we can't make it smaller once we've committed to
> - * a particular size.
> - */
> -#ifdef TARGET_PAGE_BITS_VARY
> -assert(bits >= TARGET_PAGE_BITS_MIN);
> -if (target_page_bits == 0 || target_page_bits > bits) {
> -if (target_page_bits_decided) {
> -return false;
> -}
> -target_page_bits = bits;
> -}
> -#endif
> -return true;
> -}
> -
>  #if !defined(CONFIG_USER_ONLY)
>  
> -static void finalize_target_page_bits(void)
> -{
> -#ifdef TARGET_PAGE_BITS_VARY
> -if 

Re: [PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s

[Philippe Mathieu-Daudé]

On 9/21/19 6:32 AM, Richard Henderson wrote:
> Remove a redundant masking of ignore.  Once that's gone it is
> obvious that the system-mode inner test is redundant with the
> outer test.  Move the fpcr_exc_enable masking up and tidy.
> 
> No functional change.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/alpha/fpu_helper.c | 15 ---
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/target/alpha/fpu_helper.c b/target/alpha/fpu_helper.c
> index 62a066d902..df8b58963b 100644
> --- a/target/alpha/fpu_helper.c
> +++ b/target/alpha/fpu_helper.c
> @@ -90,25 +90,18 @@ void helper_fp_exc_raise_s(CPUAlphaState *env, uint32_t 
> ignore, uint32_t regno)
>  uint32_t exc = env->error_code & ~ignore;
>  if (exc) {
>  env->fpcr |= exc;
> -exc &= ~ignore;
> -#ifdef CONFIG_USER_ONLY
> -/*
> - * In user mode, the kernel's software handler only
> - * delivers a signal if the exception is enabled.
> - */
> -if (!(exc & env->fpcr_exc_enable)) {
> -return;
> -}
> -#else
> +exc &= env->fpcr_exc_enable;
>  /*
>   * In system mode, the software handler gets invoked
>   * for any non-ignored exception.
> + * In user mode, the kernel's software handler only
> + * delivers a signal if the exception is enabled.
>   */
> +#ifdef CONFIG_USER_ONLY
>  if (!exc) {
>  return;
>  }
>  #endif
> -exc &= env->fpcr_exc_enable;
>  fp_exc_raise1(env, GETPC(), exc, regno, EXC_M_SWC);
>  }
>  }
> 

Re: [PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once

[Philippe Mathieu-Daudé]

On 9/21/19 6:32 AM, Richard Henderson wrote:
> Tidy the computation of the value; no functional change.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/alpha/helper.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/target/alpha/helper.c b/target/alpha/helper.c
> index 2f959c65ef..1b3479738b 100644
> --- a/target/alpha/helper.c
> +++ b/target/alpha/helper.c
> @@ -69,14 +69,13 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t 
> val)
>  env->fpcr_exc_enable = ~t & FPCR_STATUS_MASK;
>  
>  env->fpcr_dyn_round = rm_map[(fpcr & FPCR_DYN_MASK) >> FPCR_DYN_SHIFT];
> -
> -env->fpcr_flush_to_zero = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
>  env->fp_status.flush_inputs_to_zero = (fpcr & FPCR_DNZ) != 0;
> +
> +t = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
>  #ifdef CONFIG_USER_ONLY
> -if (env->swcr & SWCR_MAP_UMZ) {
> -env->fpcr_flush_to_zero = 1;
> -}
> +t |= (env->swcr & SWCR_MAP_UMZ) != 0;
>  #endif
> +env->fpcr_flush_to_zero = t;
>  }
>  
>  uint64_t helper_load_fpcr(CPUAlphaState *env)
> 

Re: [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion

[Philippe Mathieu-Daudé]

On 9/21/19 6:32 AM, Richard Henderson wrote:
> This is a bit more straight-forward than using a switch statement.
> No functional change.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/alpha/helper.c | 24 
>  1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/target/alpha/helper.c b/target/alpha/helper.c
> index 19cda0a2db..6c1703682e 100644
> --- a/target/alpha/helper.c
> +++ b/target/alpha/helper.c
> @@ -36,6 +36,13 @@ uint64_t cpu_alpha_load_fpcr(CPUAlphaState *env)
>  
>  void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
>  {
> +static const uint8_t rm_map[] = {
> +[FPCR_DYN_NORMAL >> FPCR_DYN_SHIFT] = float_round_nearest_even,
> +[FPCR_DYN_CHOPPED >> FPCR_DYN_SHIFT] = float_round_to_zero,
> +[FPCR_DYN_MINUS >> FPCR_DYN_SHIFT] = float_round_down,
> +[FPCR_DYN_PLUS >> FPCR_DYN_SHIFT] = float_round_up,
> +};
> +
>  uint32_t fpcr = val >> 32;
>  uint32_t t = 0;
>  
> @@ -48,22 +55,7 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
>  env->fpcr = fpcr;
>  env->fpcr_exc_enable = ~t & FPCR_STATUS_MASK;
>  
> -switch (fpcr & FPCR_DYN_MASK) {
> -case FPCR_DYN_NORMAL:
> -default:
> -t = float_round_nearest_even;
> -break;
> -case FPCR_DYN_CHOPPED:
> -t = float_round_to_zero;
> -break;
> -case FPCR_DYN_MINUS:
> -t = float_round_down;
> -break;
> -case FPCR_DYN_PLUS:
> -t = float_round_up;
> -break;
> -}
> -env->fpcr_dyn_round = t;
> +env->fpcr_dyn_round = rm_map[(fpcr & FPCR_DYN_MASK) >> FPCR_DYN_SHIFT];
>  
>  env->fpcr_flush_to_zero = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
>  env->fp_status.flush_inputs_to_zero = (fpcr & FPCR_DNZ) != 0;
> 

Re: [PATCH v2 2/2] riscv: sifive_u: Add ethernet0 to the aliases node

[Philippe Mathieu-Daudé]

On 9/21/19 7:41 AM, Bin Meng wrote:
> U-Boot expects this alias to be in place in order to fix up the mac
> address of the ethernet node.
> 
> This is to keep in sync with Linux kernel commit below:
> https://patchwork.kernel.org/patch/11133033/
> 
> Signed-off-by: Bin Meng 
> Reviewed-by: Alistair Francis 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
> 
> Changes in v2: None
> 
>  hw/riscv/sifive_u.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 02dd761..1ac51e3 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -270,6 +270,10 @@ static void create_fdt(SiFiveUState *s, const struct 
> MemmapEntry *memmap,
>  s->soc.gem.conf.macaddr.a, ETH_ALEN);
>  qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
>  qemu_fdt_setprop_cell(fdt, nodename, "#size-cells", 0);
> +
> +qemu_fdt_add_subnode(fdt, "/aliases");
> +qemu_fdt_setprop_string(fdt, "/aliases", "ethernet0", nodename);
> +
>  g_free(nodename);
>  
>  nodename = g_strdup_printf("/soc/ethernet@%lx/ethernet-phy@0",
> @@ -297,7 +301,6 @@ static void create_fdt(SiFiveUState *s, const struct 
> MemmapEntry *memmap,
>  qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
>  }
>  
> -qemu_fdt_add_subnode(fdt, "/aliases");
>  qemu_fdt_setprop_string(fdt, "/aliases", "serial0", nodename);
>  
>  g_free(nodename);
> 

Re: [PATCH v2 1/2] riscv: hw: Drop "clock-frequency" property of cpu nodes

[Philippe Mathieu-Daudé]

Hi Bin,

On 9/21/19 7:41 AM, Bin Meng wrote:
> The "clock-frequency" property of cpu nodes isn't required. Drop it.
> 
> This is to keep in sync with Linux kernel commit below:
> https://patchwork.kernel.org/patch/11133031/

What happens if you run a older kernel that doesn't contain the
referenced patch?

> Signed-off-by: Bin Meng 
> Reviewed-by: Alistair Francis 
> 
> ---
> 
> Changes in v2:
> - drop the one in spike and virt machines too
> 
>  hw/riscv/sifive_u.c | 2 --
>  hw/riscv/spike.c| 2 --
>  hw/riscv/virt.c | 2 --
>  include/hw/riscv/sifive_u.h | 1 -
>  include/hw/riscv/spike.h| 4 
>  include/hw/riscv/virt.h | 4 
>  6 files changed, 15 deletions(-)
> 
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 9f8e84b..02dd761 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -151,8 +151,6 @@ static void create_fdt(SiFiveUState *s, const struct 
> MemmapEntry *memmap,
>  char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-controller", 
> cpu);
>  char *isa;
>  qemu_fdt_add_subnode(fdt, nodename);
> -qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
> -  SIFIVE_U_CLOCK_FREQ);
>  /* cpu 0 is the management hart that does not have mmu */
>  if (cpu != 0) {
>  qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index d60415d..8bbffbc 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -102,8 +102,6 @@ static void create_fdt(SpikeState *s, const struct 
> MemmapEntry *memmap,
>  char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-controller", 
> cpu);
>  char *isa = riscv_isa_string(>soc.harts[cpu]);
>  qemu_fdt_add_subnode(fdt, nodename);
> -qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
> -  SPIKE_CLOCK_FREQ);
>  qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
>  qemu_fdt_setprop_string(fdt, nodename, "riscv,isa", isa);
>  qemu_fdt_setprop_string(fdt, nodename, "compatible", "riscv");
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d36f562..1303061 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -161,8 +161,6 @@ static void create_fdt(RISCVVirtState *s, const struct 
> MemmapEntry *memmap,
>  char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-controller", 
> cpu);
>  char *isa = riscv_isa_string(>soc.harts[cpu]);
>  qemu_fdt_add_subnode(fdt, nodename);
> -qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
> -  VIRT_CLOCK_FREQ);
>  qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
>  qemu_fdt_setprop_string(fdt, nodename, "riscv,isa", isa);
>  qemu_fdt_setprop_string(fdt, nodename, "compatible", "riscv");
> diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> index e4df298..4850805 100644
> --- a/include/hw/riscv/sifive_u.h
> +++ b/include/hw/riscv/sifive_u.h
> @@ -75,7 +75,6 @@ enum {
>  };
>  
>  enum {
> -SIFIVE_U_CLOCK_FREQ = 10,
>  SIFIVE_U_HFCLK_FREQ = ,
>  SIFIVE_U_RTCCLK_FREQ = 100
>  };
> diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
> index 03d8703..dc77042 100644
> --- a/include/hw/riscv/spike.h
> +++ b/include/hw/riscv/spike.h
> @@ -38,10 +38,6 @@ enum {
>  SPIKE_DRAM
>  };
>  
> -enum {
> -SPIKE_CLOCK_FREQ = 10
> -};
> -
>  #if defined(TARGET_RISCV32)
>  #define SPIKE_V1_09_1_CPU TYPE_RISCV_CPU_RV32GCSU_V1_09_1
>  #define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_RV32GCSU_V1_10_0
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 6e5fbe5..68978a1 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -55,10 +55,6 @@ enum {
>  VIRTIO_NDEV = 0x35 /* Arbitrary maximum number of interrupts */
>  };
>  
> -enum {
> -VIRT_CLOCK_FREQ = 10
> -};
> -
>  #define VIRT_PLIC_HART_CONFIG "MS"
>  #define VIRT_PLIC_NUM_SOURCES 127
>  #define VIRT_PLIC_NUM_PRIORITIES 7
> 

Re: [PATCH v2 1/5] docker: move tests from python2 to python3

[Philippe Mathieu-Daudé]

On 9/21/19 12:12 AM, John Snow wrote:
> As part of the push to drop python2 support, replace any explicit python2
> dependencies with python3 versions.
> 
> For centos, python2 still exists as an implicit dependency, but by adding
> python3 we will be able to build even if the configure script begins to
> require python 3.5+.
> 
> Tested with centos7, fedora, ubuntu, ubuntu1804, and debian 9 (amd64).
> Tested under a custom configure script that requires Python 3.5+.
> 
> the travis dockerfile is also moved to using python3, which was tested
> by running `make docker-test-build@travis`, which I hope is sufficient.
> 
> Signed-off-by: John Snow 

> Message-id: 20190920200049.27216-1-js...@redhat.com
  ^ remove

v1 has:
Reviewed-by: Eduardo Habkost 

> ---
>  tests/docker/dockerfiles/centos7.docker | 2 +-
>  tests/docker/dockerfiles/debian-ports.docker| 2 +-
>  tests/docker/dockerfiles/debian-sid.docker  | 2 +-
>  tests/docker/dockerfiles/debian-xtensa-cross.docker | 2 +-
>  tests/docker/dockerfiles/debian10.docker| 2 +-
>  tests/docker/dockerfiles/debian8.docker | 3 +--

If you queue this patch at the end of your series, no need to patch
debian8.docker.

Anyway,
Reviewed-by: Philippe Mathieu-Daudé 

>  tests/docker/dockerfiles/debian9.docker | 2 +-
>  tests/docker/dockerfiles/travis.docker  | 2 +-
>  tests/docker/dockerfiles/ubuntu.docker  | 2 +-
>  tests/docker/dockerfiles/ubuntu1804.docker  | 2 +-
>  10 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/tests/docker/dockerfiles/centos7.docker 
> b/tests/docker/dockerfiles/centos7.docker
> index e0b9d7dbe9..953637065c 100644
> --- a/tests/docker/dockerfiles/centos7.docker
> +++ b/tests/docker/dockerfiles/centos7.docker
> @@ -25,6 +25,7 @@ ENV PACKAGES \
>  nettle-devel \
>  perl-Test-Harness \
>  pixman-devel \
> +python3 \
>  SDL-devel \
>  spice-glib-devel \
>  spice-server-devel \
> @@ -34,4 +35,3 @@ ENV PACKAGES \
>  zlib-devel
>  RUN yum install -y $PACKAGES
>  RUN rpm -q $PACKAGES | sort > /packages.txt
> -
> diff --git a/tests/docker/dockerfiles/debian-ports.docker 
> b/tests/docker/dockerfiles/debian-ports.docker
> index 61bc3f2993..f1ea8d3bd1 100644
> --- a/tests/docker/dockerfiles/debian-ports.docker
> +++ b/tests/docker/dockerfiles/debian-ports.docker
> @@ -31,6 +31,6 @@ RUN apt-get update && \
>  git \
>  pkg-config \
>  psmisc \
> -python \
> +python3 \
>  texinfo \
>  $(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\  
> -f2)
> diff --git a/tests/docker/dockerfiles/debian-sid.docker 
> b/tests/docker/dockerfiles/debian-sid.docker
> index 2a1bcc33b2..dcb8e83715 100644
> --- a/tests/docker/dockerfiles/debian-sid.docker
> +++ b/tests/docker/dockerfiles/debian-sid.docker
> @@ -31,5 +31,5 @@ RUN apt update && \
>  git \
>  pkg-config \
>  psmisc \
> -python \
> +python3 \
>  texinfo || { echo "Failed to build - see debian-sid.docker notes"; 
> exit 1; }
> diff --git a/tests/docker/dockerfiles/debian-xtensa-cross.docker 
> b/tests/docker/dockerfiles/debian-xtensa-cross.docker
> index b9c2e2e531..e6f93f65ee 100644
> --- a/tests/docker/dockerfiles/debian-xtensa-cross.docker
> +++ b/tests/docker/dockerfiles/debian-xtensa-cross.docker
> @@ -18,7 +18,7 @@ RUN apt-get update && \
>  flex \
>  gettext \
>  git \
> -python-minimal
> +python3-minimal
>  
>  ENV CPU_LIST csp dc232b dc233c
>  ENV TOOLCHAIN_RELEASE 2018.02
> diff --git a/tests/docker/dockerfiles/debian10.docker 
> b/tests/docker/dockerfiles/debian10.docker
> index 30a78813f2..dad498b52e 100644
> --- a/tests/docker/dockerfiles/debian10.docker
> +++ b/tests/docker/dockerfiles/debian10.docker
> @@ -26,7 +26,7 @@ RUN apt update && \
>  git \
>  pkg-config \
>  psmisc \
> -python \
> +python3 \
>  python3-sphinx \
>  texinfo \
>  $(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\  
> -f2)
> diff --git a/tests/docker/dockerfiles/debian8.docker 
> b/tests/docker/dockerfiles/debian8.docker
> index 1212a85c35..be67023826 100644
> --- a/tests/docker/dockerfiles/debian8.docker
> +++ b/tests/docker/dockerfiles/debian8.docker
> @@ -30,5 +30,4 @@ RUN apt update && \
>  git \
>  gnupg \
>  pkg-config \
> -python-minimal
> -
> +python3-minimal
> diff --git a/tests/docker/dockerfiles/debian9.docker 
> b/tests/docker/dockerfiles/debian9.docker
> index b36f1d4ed8..8cbd742bb5 100644
> --- a/tests/docker/dockerfiles/debian9.docker
> +++ b/tests/docker/dockerfiles/debian9.docker
> @@ -26,7 +26,7 @@ RUN apt update && \
>  git \
>  pkg-config \
>  psmisc \
> -python \
> +python3 \
>  python3-sphinx \
>  texinfo \
>  $(apt-get -s 

Re: [PATCH v2 3/5] docker: remove debian8-mxe definitions

[Philippe Mathieu-Daudé]

On 9/21/19 12:12 AM, John Snow wrote:
> We don't have a debian8-mxe dockerfile anymore.
> 
> Signed-off-by: John Snow 

> Message-id: 20190920001413.22567-3-js...@redhat.com
  ^ remove?

Fixes: 67bd36beda1ae
Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  tests/docker/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 0d75260e46..0a7fc8bc79 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -6,7 +6,7 @@ DOCKER_SUFFIX := .docker
>  DOCKER_FILES_DIR := $(SRC_PATH)/tests/docker/dockerfiles
>  # we don't run tests on intermediate images (used as base by another image)
>  DOCKER_PARTIAL_IMAGES := debian8 debian9 debian10 debian-sid
> -DOCKER_PARTIAL_IMAGES += debian8-mxe debian9-mxe debian-ports 
> debian-bootstrap
> +DOCKER_PARTIAL_IMAGES += debian9-mxe debian-ports debian-bootstrap
>  DOCKER_IMAGES := $(sort $(notdir $(basename $(wildcard 
> $(DOCKER_FILES_DIR)/*.docker
>  DOCKER_TARGETS := $(patsubst %,docker-image-%,$(DOCKER_IMAGES))
>  # Use a global constant ccache directory to speed up repetitive builds
> 

Re: [PATCH v2 5/5] docker: removed unused debian8 partial image

[Philippe Mathieu-Daudé]

On 9/21/19 12:12 AM, John Snow wrote:
> debian8 partial base is also not consumed by any image, so remove it.

Reviewed-by: Philippe Mathieu-Daudé 

> 
> Signed-off-by: John Snow 
> ---
>  tests/docker/Makefile.include   |  3 +--
>  tests/docker/dockerfiles/debian8.docker | 33 -
>  2 files changed, 1 insertion(+), 35 deletions(-)
>  delete mode 100644 tests/docker/dockerfiles/debian8.docker
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index cb41652885..b9430822b8 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -5,8 +5,7 @@
>  DOCKER_SUFFIX := .docker
>  DOCKER_FILES_DIR := $(SRC_PATH)/tests/docker/dockerfiles
>  # we don't run tests on intermediate images (used as base by another image)
> -DOCKER_PARTIAL_IMAGES := debian8 debian9 debian10
> -DOCKER_PARTIAL_IMAGES += debian9-mxe debian-bootstrap
> +DOCKER_PARTIAL_IMAGES := debian9 debian9-mxe debian10 debian-bootstrap
>  DOCKER_IMAGES := $(sort $(notdir $(basename $(wildcard 
> $(DOCKER_FILES_DIR)/*.docker
>  DOCKER_TARGETS := $(patsubst %,docker-image-%,$(DOCKER_IMAGES))
>  # Use a global constant ccache directory to speed up repetitive builds
> diff --git a/tests/docker/dockerfiles/debian8.docker 
> b/tests/docker/dockerfiles/debian8.docker
> deleted file mode 100644
> index be67023826..00
> --- a/tests/docker/dockerfiles/debian8.docker
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -#
> -# Docker multiarch cross-compiler target
> -#
> -# This docker target is builds on Debian and Emdebian's cross compiler 
> targets
> -# to build distro with a selection of cross compilers for building test 
> binaries.
> -#
> -# On its own you can't build much but the docker-foo-cross targets
> -# build on top of the base debian image.
> -#
> -FROM debian:jessie-slim
> -
> -MAINTAINER Philippe Mathieu-Daudé 
> -
> -# Duplicate deb line as deb-src
> -RUN cat /etc/apt/sources.list | sed "s/^deb\ /deb-src /" >> 
> /etc/apt/sources.list
> -
> -# Setup some basic tools we need
> -RUN apt update && \
> -DEBIAN_FRONTEND=noninteractive apt-get install -yy eatmydata && \
> -DEBIAN_FRONTEND=noninteractive eatmydata \
> -apt-get install -y --no-install-recommends \
> -bison \
> -binutils-multiarch \
> -build-essential \
> -ca-certificates \
> -clang \
> -curl \
> -flex \
> -gettext \
> -git \
> -gnupg \
> -pkg-config \
> -python3-minimal
> 

Re: [PATCH v2 0/3] testing: Build WHPX enabled binaries

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190920113329.16787-1-phi...@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20190920113329.16787-1-phi...@redhat.com
Subject: [PATCH v2 0/3] testing: Build WHPX enabled binaries
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
87eaddb .shippable.yml: Build WHPX enabled binaries
e5d4b91 tests/docker: Add fedora-win10sdk-cross image
52ca140 target/i386: Fix broken build with WHPX enabled

=== OUTPUT BEGIN ===
1/3 Checking commit 52ca14097b01 (target/i386: Fix broken build with WHPX 
enabled)
2/3 Checking commit e5d4b91b58ca (tests/docker: Add fedora-win10sdk-cross image)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#38: 
new file mode 100644

ERROR: line over 90 characters
#83: FILE: tests/docker/dockerfiles/win10sdk-dl.sh:12:
+BASE_URL=$(curl --silent --include 
'http://go.microsoft.com/fwlink/?prd=11966=1.0=0x409=0x409=Windows10=SDK=10.0.18362.1'
 | sed -nE 's_Location: (.*)/\r_\1_p')/Installers

ERROR: line over 90 characters
#88: FILE: tests/docker/dockerfiles/win10sdk-dl.sh:17:
+CAB_NAME=$(msiextract Windows\ SDK\ Desktop\ Headers\ x86-x86_en-us.msi 
3>&1 2>&3 3>&-| sed -nE "s_.*Error opening file $PWD/(.*): No such file or 
directory_\1_p")

WARNING: line over 80 characters
#95: FILE: tests/docker/dockerfiles/win10sdk-dl.sh:24:
+for inc in "${WINDIR}/Program Files/Windows 
Kits/10/Include/10.0.18362.0/um"/WinHv*; do

total: 2 errors, 2 warnings, 58 lines checked

Patch 2/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/3 Checking commit 87eaddbdd398 (.shippable.yml: Build WHPX enabled binaries)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190920113329.16787-1-phi...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com