| Message ID | 1447682723-3977-10-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 11 January 2016 at 13:47, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 16/11/2015 15:05, Peter Maydell wrote: >> - hwaddr phys = cpu_get_phys_page_debug(cpu, pc); >> + MemTxAttrs attrs = {}; >> + hwaddr phys = cpu_get_phys_page_attrs_debug(cpu, pc, &attrs); >> + int asidx = cpu_asidx_from_attrs(cpu, attrs); >> if (phys != -1) { >> - tb_invalidate_phys_addr(cpu->as, >> + tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as, >> phys | (pc & ~TARGET_PAGE_MASK)); >> } > > The only question I have is whether it is right to have empty MemTxAttrs > here. Is there any way to use the MemTxAttrs for the "current state" of > the CPU, whatever that is (on x86 I have added cpu_get_mem_attrs to > target-i386/cpu.h)? That's what the call to cpu_get_phys_page_attrs_debug() does: it fills in the MemTxAttrs :secure and :user fields, and then cpu_asidx_from_attrs() uses the returned attributes to pick the right address space. (cpu_get_phys_page_attrs_debug() only fills in attribute fields it knows about, which is why we pass it an empty attrs struct to start with.) thanks -- PMM
On 11 January 2016 at 14:59, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 11/01/2016 15:18, Peter Maydell wrote: >> That's what the call to cpu_get_phys_page_attrs_debug() does: >> it fills in the MemTxAttrs :secure and :user fields, and then >> cpu_asidx_from_attrs() uses the returned attributes to pick >> the right address space. (cpu_get_phys_page_attrs_debug() >> only fills in attribute fields it knows about, which is why >> we pass it an empty attrs struct to start with.) > > Oops, that's not clear from the documentation in patch 4. But if that > was the case, shouldn't cpu_get_phys_page_attrs_debug leave *attrs > completely alone when using the fallback? > > I think it's clearer if you pass uninitialized MemTxAttrs to > cpu_get_phys_page_attrs_debug, assuming you can do it. I can see why > the current semantics make sense for helper.c's get_phys_addr, but I > think they are confusing for the topmost function call. Yes, I think you're right (and the doc comment I wrote for cpu_get_phys_page_attrs_debug agrees ;-)). thanks -- PMM
diff --git a/exec.c b/exec.c index 0d7af0c..de540e4 100644 --- a/exec.c +++ b/exec.c @@ -682,9 +682,11 @@ static void breakpoint_invalidate(CPUState *cpu, target_ulong pc) #else static void breakpoint_invalidate(CPUState *cpu, target_ulong pc) { - hwaddr phys = cpu_get_phys_page_debug(cpu, pc); + MemTxAttrs attrs = {}; + hwaddr phys = cpu_get_phys_page_attrs_debug(cpu, pc, &attrs); + int asidx = cpu_asidx_from_attrs(cpu, attrs); if (phys != -1) { - tb_invalidate_phys_addr(cpu->as, + tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as, phys | (pc & ~TARGET_PAGE_MASK)); } } @@ -3558,8 +3560,12 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, target_ulong page; while (len > 0) { + int asidx; + MemTxAttrs attrs = {}; + page = addr & TARGET_PAGE_MASK; - phys_addr = cpu_get_phys_page_debug(cpu, page); + phys_addr = cpu_get_phys_page_attrs_debug(cpu, page, &attrs); + asidx = cpu_asidx_from_attrs(cpu, attrs); /* if no physical page mapped, return an error */ if (phys_addr == -1) return -1; @@ -3568,9 +3574,11 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, l = len; phys_addr += (addr & ~TARGET_PAGE_MASK); if (is_write) { - cpu_physical_memory_write_rom(cpu->as, phys_addr, buf, l); + cpu_physical_memory_write_rom(cpu->cpu_ases[asidx].as, + phys_addr, buf, l); } else { - address_space_rw(cpu->as, phys_addr, MEMTXATTRS_UNSPECIFIED, + address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, + MEMTXATTRS_UNSPECIFIED, buf, l, 0); } len -= l;
Use cpu_get_phys_page_attrs_debug() when doing virtual-to-physical conversions in debug related code, so that we can obtain the right address space index and thus select the correct AddressSpace, rather than always using cpu->as. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- exec.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) -- 1.9.1