From patchwork Mon Jun 11 12:56:33 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 138235 Delivered-To: patches@linaro.org Received: by 2002:a2e:970d:0:0:0:0:0 with SMTP id r13-v6csp4023480lji; Mon, 11 Jun 2018 05:56:41 -0700 (PDT) X-Google-Smtp-Source: ADUXVKIz9pVh8cxxPj7sZq76+3lZkq7jF4PtTRezuI11DrnqqnKV31o+QKwEBwdy94B2rwKW9MEe X-Received: by 2002:a62:8b0a:: with SMTP id j10-v6mr17445421pfe.28.1528721801282; Mon, 11 Jun 2018 05:56:41 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1528721801; cv=none; d=google.com; s=arc-20160816; b=JAbJ4tMHZiEW8ve97g/KneqfshLIV3PEMgYqO6uwZ5FpCc187O5zj6gxxCtJSu/FV8 5dXT/L2PCt6vz9I9qslXJBq8PaIQXffSICmAomyUq8SfHJPg7ApfMzSYA775tNFPDvU+ XcaBhszczMfWE4zTmr3LgNYmGyf01eeIvAoLaSbDyJ4v/+bD+07CFBohN5RFk583b8Xn xfyOdr2EhtRn2oL6h+brXiWVEDglcUdVFitxnI7aW+tucbfjSPLmFjnSOi2bDSHHF/fN QGqRdtOS1dnf4JI2gSPzHG+yFTBEVT5EgH3Tn18tnYe9qk3K94dOQsuTn54GMKL773Kr Uysg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=references:in-reply-to:message-id:date:subject:cc:to:from :arc-authentication-results; bh=gNBjvSgIaLShf3W9CfJGQoGdHPLs8wIBBRe9/F/ZYTg=; b=f5Fi9owMnREOv2uWCNl9LpB+1HetiN4LFs60KnYLcwz1WTqEc/AZcsdjrO/NF4RhOK jEXCVXkvyuobbaP/cG9HlfMBmddEwmls+pA5KSgRyw6N22G/qZxaPhezOxBQXfht3fjV ACvzyK8zTYarSFYQfK3vCtMsuFnpivLcuUfhfWrMCEaab9TwndyWKdycxMoovsOU0zHl Y5yE6fd2dj78aKZ2jtRCwkV8Br1naOL3Crx2LbGYpXycfpNX49N2EOlls2fF251lEFfc 2lz+QcR++ywR750UHeCsQrJEz62FKFKAdJUW0Y0H5GaWuq0QCJ91niYfLgf4REVUZsCF SySw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of pm215@archaic.org.uk designates 2001:8b0:1d0::2 as permitted sender) smtp.mailfrom=pm215@archaic.org.uk; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from orth.archaic.org.uk (orth.archaic.org.uk. [2001:8b0:1d0::2]) by mx.google.com with ESMTPS id g61-v6si42438956plb.169.2018.06.11.05.56.39 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 11 Jun 2018 05:56:40 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of pm215@archaic.org.uk designates 2001:8b0:1d0::2 as permitted sender) client-ip=2001:8b0:1d0::2; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of pm215@archaic.org.uk designates 2001:8b0:1d0::2 as permitted sender) smtp.mailfrom=pm215@archaic.org.uk; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from pm215 by orth.archaic.org.uk with local (Exim 4.89) (envelope-from ) id 1fSMNA-00074G-2a; Mon, 11 Jun 2018 13:56:36 +0100 From: Peter Maydell To: qemu-devel@nongnu.org Cc: patches@linaro.org, Richard Henderson , Paolo Bonzini Subject: [PATCH 2/2] cputlb: Pass cpu_transaction_failed() the correct physaddr Date: Mon, 11 Jun 2018 13:56:33 +0100 Message-Id: <20180611125633.32755-3-peter.maydell@linaro.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180611125633.32755-1-peter.maydell@linaro.org> References: <20180611125633.32755-1-peter.maydell@linaro.org> The API for cpu_transaction_failed() says that it takes the physical address for the failed transaction. However we were actually passing it the offset within the target MemoryRegion. We don't currently have any target CPU implementations of this hook that require the physical address; fix this bug so we don't get confused if we ever do add one. Suggested-by: Paolo Bonzini Signed-off-by: Peter Maydell --- include/exec/exec-all.h | 13 ++++++++++-- accel/tcg/cputlb.c | 44 +++++++++++++++++++++++++++++------------ exec.c | 5 +++-- 3 files changed, 45 insertions(+), 17 deletions(-) -- 2.17.1 Reviewed-by: Richard Henderson diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 4d09eaba72d..aed55aaaa7d 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -437,8 +437,17 @@ void tb_lock_reset(void); #if !defined(CONFIG_USER_ONLY) -struct MemoryRegion *iotlb_to_region(CPUState *cpu, - hwaddr index, MemTxAttrs attrs); +/** + * iotlb_to_section: + * @cpu: CPU performing the access + * @index: TCG CPU IOTLB entry + * + * Given a TCG CPU IOTLB entry, return the MemoryRegionSection that + * it refers to. @index will have been initially created and returned + * by memory_region_section_get_iotlb(). + */ +struct MemoryRegionSection *iotlb_to_section(CPUState *cpu, + hwaddr index, MemTxAttrs attrs); void tlb_fill(CPUState *cpu, target_ulong addr, int size, MMUAccessType access_type, int mmu_idx, uintptr_t retaddr); diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index 355ded27024..77aa982cfd8 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -777,13 +777,16 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry, target_ulong addr, uintptr_t retaddr, int size) { CPUState *cpu = ENV_GET_CPU(env); - hwaddr physaddr = iotlbentry->addr; - MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs); + hwaddr mr_offset; + MemoryRegionSection *section; + MemoryRegion *mr; uint64_t val; bool locked = false; MemTxResult r; - physaddr = (physaddr & TARGET_PAGE_MASK) + addr; + section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs); + mr = section->mr; + mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr; cpu->mem_io_pc = retaddr; if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) { cpu_io_recompile(cpu, retaddr); @@ -795,9 +798,13 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry, qemu_mutex_lock_iothread(); locked = true; } - r = memory_region_dispatch_read(mr, physaddr, + r = memory_region_dispatch_read(mr, mr_offset, &val, size, iotlbentry->attrs); if (r != MEMTX_OK) { + hwaddr physaddr = mr_offset + + section->offset_within_address_space - + section->offset_within_region; + cpu_transaction_failed(cpu, physaddr, addr, size, MMU_DATA_LOAD, mmu_idx, iotlbentry->attrs, r, retaddr); } @@ -814,12 +821,15 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry, uintptr_t retaddr, int size) { CPUState *cpu = ENV_GET_CPU(env); - hwaddr physaddr = iotlbentry->addr; - MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs); + hwaddr mr_offset; + MemoryRegionSection *section; + MemoryRegion *mr; bool locked = false; MemTxResult r; - physaddr = (physaddr & TARGET_PAGE_MASK) + addr; + section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs); + mr = section->mr; + mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr; if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) { cpu_io_recompile(cpu, retaddr); } @@ -830,9 +840,13 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry, qemu_mutex_lock_iothread(); locked = true; } - r = memory_region_dispatch_write(mr, physaddr, + r = memory_region_dispatch_write(mr, mr_offset, val, size, iotlbentry->attrs); if (r != MEMTX_OK) { + hwaddr physaddr = mr_offset + + section->offset_within_address_space - + section->offset_within_region; + cpu_transaction_failed(cpu, physaddr, addr, size, MMU_DATA_STORE, mmu_idx, iotlbentry->attrs, r, retaddr); } @@ -880,12 +894,13 @@ static bool victim_tlb_hit(CPUArchState *env, size_t mmu_idx, size_t index, */ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr) { - int mmu_idx, index, pd; + int mmu_idx, index; void *p; MemoryRegion *mr; + MemoryRegionSection *section; CPUState *cpu = ENV_GET_CPU(env); CPUIOTLBEntry *iotlbentry; - hwaddr physaddr; + hwaddr physaddr, mr_offset; index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); mmu_idx = cpu_mmu_index(env, true); @@ -896,8 +911,8 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr) } } iotlbentry = &env->iotlb[mmu_idx][index]; - pd = iotlbentry->addr & ~TARGET_PAGE_MASK; - mr = iotlb_to_region(cpu, pd, iotlbentry->attrs); + section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs); + mr = section->mr; if (memory_region_is_unassigned(mr)) { qemu_mutex_lock_iothread(); if (memory_region_request_mmio_ptr(mr, addr)) { @@ -918,7 +933,10 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr) * and use the MemTXResult it produced). However it is the * simplest place we have currently available for the check. */ - physaddr = (iotlbentry->addr & TARGET_PAGE_MASK) + addr; + mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr; + physaddr = mr_offset + + section->offset_within_address_space - + section->offset_within_region; cpu_transaction_failed(cpu, physaddr, addr, 0, MMU_INST_FETCH, mmu_idx, iotlbentry->attrs, MEMTX_DECODE_ERROR, 0); diff --git a/exec.c b/exec.c index f6645ede0c2..9cbba6adcd3 100644 --- a/exec.c +++ b/exec.c @@ -2897,14 +2897,15 @@ static const MemoryRegionOps readonly_mem_ops = { }, }; -MemoryRegion *iotlb_to_region(CPUState *cpu, hwaddr index, MemTxAttrs attrs) +MemoryRegionSection *iotlb_to_section(CPUState *cpu, + hwaddr index, MemTxAttrs attrs) { int asidx = cpu_asidx_from_attrs(cpu, attrs); CPUAddressSpace *cpuas = &cpu->cpu_ases[asidx]; AddressSpaceDispatch *d = atomic_rcu_read(&cpuas->memory_dispatch); MemoryRegionSection *sections = d->map.sections; - return sections[index & ~TARGET_PAGE_MASK].mr; + return §ions[index & ~TARGET_PAGE_MASK]; } static void io_mem_init(void)