* [PATCH 01/13] target/sparc: Use tlb_set_page_full
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-02-23 21:25 ` Philippe Mathieu-Daudé
2023-03-01 16:37 ` Mark Cave-Ayland
2023-02-23 20:43 ` [PATCH 02/13] accel/tcg: Retain prot flags from tlb_fill Richard Henderson
` (11 subsequent siblings)
12 siblings, 2 replies; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Mark Cave-Ayland, Artyom Tarasenko
Pass CPUTLBEntryFull to get_physical_address instead
of a collection of pointers.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Artyom Tarasenko <atar4qemu@gmail.com>
---
target/sparc/mmu_helper.c | 121 +++++++++++++++++---------------------
1 file changed, 54 insertions(+), 67 deletions(-)
diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
index 158ec2ae8f..a98dd0abd4 100644
--- a/target/sparc/mmu_helper.c
+++ b/target/sparc/mmu_helper.c
@@ -64,10 +64,9 @@ static const int perm_table[2][8] = {
}
};
-static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
- int *prot, int *access_index, MemTxAttrs *attrs,
- target_ulong address, int rw, int mmu_idx,
- target_ulong *page_size)
+static int get_physical_address(CPUSPARCState *env, CPUTLBEntryFull *full,
+ int *access_index, target_ulong address,
+ int rw, int mmu_idx)
{
int access_perms = 0;
hwaddr pde_ptr;
@@ -80,20 +79,20 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
is_user = mmu_idx == MMU_USER_IDX;
if (mmu_idx == MMU_PHYS_IDX) {
- *page_size = TARGET_PAGE_SIZE;
+ full->lg_page_size = TARGET_PAGE_BITS;
/* Boot mode: instruction fetches are taken from PROM */
if (rw == 2 && (env->mmuregs[0] & env->def.mmu_bm)) {
- *physical = env->prom_addr | (address & 0x7ffffULL);
- *prot = PAGE_READ | PAGE_EXEC;
+ full->phys_addr = env->prom_addr | (address & 0x7ffffULL);
+ full->prot = PAGE_READ | PAGE_EXEC;
return 0;
}
- *physical = address;
- *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+ full->phys_addr = address;
+ full->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
return 0;
}
*access_index = ((rw & 1) << 2) | (rw & 2) | (is_user ? 0 : 1);
- *physical = 0xffffffffffff0000ULL;
+ full->phys_addr = 0xffffffffffff0000ULL;
/* SPARC reference MMU table walk: Context table->L1->L2->PTE */
/* Context base + context number */
@@ -157,16 +156,17 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
case 2: /* L3 PTE */
page_offset = 0;
}
- *page_size = TARGET_PAGE_SIZE;
+ full->lg_page_size = TARGET_PAGE_BITS;
break;
case 2: /* L2 PTE */
page_offset = address & 0x3f000;
- *page_size = 0x40000;
+ full->lg_page_size = 18;
}
break;
case 2: /* L1 PTE */
page_offset = address & 0xfff000;
- *page_size = 0x1000000;
+ full->lg_page_size = 24;
+ break;
}
}
@@ -188,16 +188,16 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
}
/* the page can be put in the TLB */
- *prot = perm_table[is_user][access_perms];
+ full->prot = perm_table[is_user][access_perms];
if (!(pde & PG_MODIFIED_MASK)) {
/* only set write access if already dirty... otherwise wait
for dirty access */
- *prot &= ~PAGE_WRITE;
+ full->prot &= ~PAGE_WRITE;
}
/* Even if large ptes, we map only one 4KB page in the cache to
avoid filling it too fast */
- *physical = ((hwaddr)(pde & PTE_ADDR_MASK) << 4) + page_offset;
+ full->phys_addr = ((hwaddr)(pde & PTE_ADDR_MASK) << 4) + page_offset;
return error_code;
}
@@ -208,11 +208,9 @@ bool sparc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
{
SPARCCPU *cpu = SPARC_CPU(cs);
CPUSPARCState *env = &cpu->env;
- hwaddr paddr;
+ CPUTLBEntryFull full = {};
target_ulong vaddr;
- target_ulong page_size;
- int error_code = 0, prot, access_index;
- MemTxAttrs attrs = {};
+ int error_code = 0, access_index;
/*
* TODO: If we ever need tlb_vaddr_to_host for this target,
@@ -223,16 +221,15 @@ bool sparc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
assert(!probe);
address &= TARGET_PAGE_MASK;
- error_code = get_physical_address(env, &paddr, &prot, &access_index, &attrs,
- address, access_type,
- mmu_idx, &page_size);
+ error_code = get_physical_address(env, &full, &access_index,
+ address, access_type, mmu_idx);
vaddr = address;
if (likely(error_code == 0)) {
qemu_log_mask(CPU_LOG_MMU,
"Translate at %" VADDR_PRIx " -> "
HWADDR_FMT_plx ", vaddr " TARGET_FMT_lx "\n",
- address, paddr, vaddr);
- tlb_set_page(cs, vaddr, paddr, prot, mmu_idx, page_size);
+ address, full.phys_addr, vaddr);
+ tlb_set_page_full(cs, mmu_idx, vaddr, &full);
return true;
}
@@ -247,8 +244,8 @@ bool sparc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
permissions. If no mapping is available, redirect accesses to
neverland. Fake/overridden mappings will be flushed when
switching to normal mode. */
- prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
- tlb_set_page(cs, vaddr, paddr, prot, mmu_idx, TARGET_PAGE_SIZE);
+ full.prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+ tlb_set_page_full(cs, mmu_idx, vaddr, &full);
return true;
} else {
if (access_type == MMU_INST_FETCH) {
@@ -545,8 +542,7 @@ static uint64_t build_sfsr(CPUSPARCState *env, int mmu_idx, int rw)
return sfsr;
}
-static int get_physical_address_data(CPUSPARCState *env, hwaddr *physical,
- int *prot, MemTxAttrs *attrs,
+static int get_physical_address_data(CPUSPARCState *env, CPUTLBEntryFull *full,
target_ulong address, int rw, int mmu_idx)
{
CPUState *cs = env_cpu(env);
@@ -579,11 +575,12 @@ static int get_physical_address_data(CPUSPARCState *env, hwaddr *physical,
for (i = 0; i < 64; i++) {
/* ctx match, vaddr match, valid? */
- if (ultrasparc_tag_match(&env->dtlb[i], address, context, physical)) {
+ if (ultrasparc_tag_match(&env->dtlb[i], address, context,
+ &full->phys_addr)) {
int do_fault = 0;
if (TTE_IS_IE(env->dtlb[i].tte)) {
- attrs->byte_swap = true;
+ full->attrs.byte_swap = true;
}
/* access ok? */
@@ -616,9 +613,9 @@ static int get_physical_address_data(CPUSPARCState *env, hwaddr *physical,
}
if (!do_fault) {
- *prot = PAGE_READ;
+ full->prot = PAGE_READ;
if (TTE_IS_W_OK(env->dtlb[i].tte)) {
- *prot |= PAGE_WRITE;
+ full->prot |= PAGE_WRITE;
}
TTE_SET_USED(env->dtlb[i].tte);
@@ -645,8 +642,7 @@ static int get_physical_address_data(CPUSPARCState *env, hwaddr *physical,
return 1;
}
-static int get_physical_address_code(CPUSPARCState *env, hwaddr *physical,
- int *prot, MemTxAttrs *attrs,
+static int get_physical_address_code(CPUSPARCState *env, CPUTLBEntryFull *full,
target_ulong address, int mmu_idx)
{
CPUState *cs = env_cpu(env);
@@ -681,7 +677,7 @@ static int get_physical_address_code(CPUSPARCState *env, hwaddr *physical,
for (i = 0; i < 64; i++) {
/* ctx match, vaddr match, valid? */
if (ultrasparc_tag_match(&env->itlb[i],
- address, context, physical)) {
+ address, context, &full->phys_addr)) {
/* access ok? */
if (TTE_IS_PRIV(env->itlb[i].tte) && is_user) {
/* Fault status register */
@@ -708,7 +704,7 @@ static int get_physical_address_code(CPUSPARCState *env, hwaddr *physical,
return 1;
}
- *prot = PAGE_EXEC;
+ full->prot = PAGE_EXEC;
TTE_SET_USED(env->itlb[i].tte);
return 0;
}
@@ -722,14 +718,13 @@ static int get_physical_address_code(CPUSPARCState *env, hwaddr *physical,
return 1;
}
-static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
- int *prot, int *access_index, MemTxAttrs *attrs,
- target_ulong address, int rw, int mmu_idx,
- target_ulong *page_size)
+static int get_physical_address(CPUSPARCState *env, CPUTLBEntryFull *full,
+ int *access_index, target_ulong address,
+ int rw, int mmu_idx)
{
/* ??? We treat everything as a small page, then explicitly flush
everything when an entry is evicted. */
- *page_size = TARGET_PAGE_SIZE;
+ full->lg_page_size = TARGET_PAGE_BITS;
/* safety net to catch wrong softmmu index use from dynamic code */
if (env->tl > 0 && mmu_idx != MMU_NUCLEUS_IDX) {
@@ -747,17 +742,15 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
}
if (mmu_idx == MMU_PHYS_IDX) {
- *physical = ultrasparc_truncate_physical(address);
- *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+ full->phys_addr = ultrasparc_truncate_physical(address);
+ full->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
return 0;
}
if (rw == 2) {
- return get_physical_address_code(env, physical, prot, attrs, address,
- mmu_idx);
+ return get_physical_address_code(env, full, address, mmu_idx);
} else {
- return get_physical_address_data(env, physical, prot, attrs, address,
- rw, mmu_idx);
+ return get_physical_address_data(env, full, address, rw, mmu_idx);
}
}
@@ -768,25 +761,17 @@ bool sparc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
{
SPARCCPU *cpu = SPARC_CPU(cs);
CPUSPARCState *env = &cpu->env;
- target_ulong vaddr;
- hwaddr paddr;
- target_ulong page_size;
- MemTxAttrs attrs = {};
- int error_code = 0, prot, access_index;
+ CPUTLBEntryFull full = {};
+ int error_code = 0, access_index;
address &= TARGET_PAGE_MASK;
- error_code = get_physical_address(env, &paddr, &prot, &access_index, &attrs,
- address, access_type,
- mmu_idx, &page_size);
+ error_code = get_physical_address(env, &full, &access_index,
+ address, access_type, mmu_idx);
if (likely(error_code == 0)) {
- vaddr = address;
-
- trace_mmu_helper_mmu_fault(address, paddr, mmu_idx, env->tl,
+ trace_mmu_helper_mmu_fault(address, full.phys_addr, mmu_idx, env->tl,
env->dmmu.mmu_primary_context,
env->dmmu.mmu_secondary_context);
-
- tlb_set_page_with_attrs(cs, vaddr, paddr, attrs, prot, mmu_idx,
- page_size);
+ tlb_set_page_full(cs, mmu_idx, address, &full);
return true;
}
if (probe) {
@@ -888,12 +873,14 @@ void dump_mmu(CPUSPARCState *env)
static int cpu_sparc_get_phys_page(CPUSPARCState *env, hwaddr *phys,
target_ulong addr, int rw, int mmu_idx)
{
- target_ulong page_size;
- int prot, access_index;
- MemTxAttrs attrs = {};
+ CPUTLBEntryFull full = {};
+ int access_index, ret;
- return get_physical_address(env, phys, &prot, &access_index, &attrs, addr,
- rw, mmu_idx, &page_size);
+ ret = get_physical_address(env, &full, &access_index, addr, rw, mmu_idx);
+ if (ret == 0) {
+ *phys = full.phys_addr;
+ }
+ return ret;
}
#if defined(TARGET_SPARC64)
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 01/13] target/sparc: Use tlb_set_page_full
2023-02-23 20:43 ` [PATCH 01/13] target/sparc: Use tlb_set_page_full Richard Henderson
@ 2023-02-23 21:25 ` Philippe Mathieu-Daudé
2023-03-01 16:37 ` Mark Cave-Ayland
1 sibling, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-23 21:25 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: qemu-arm, Mark Cave-Ayland, Artyom Tarasenko
On 23/2/23 21:43, Richard Henderson wrote:
> Pass CPUTLBEntryFull to get_physical_address instead
> of a collection of pointers.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
> target/sparc/mmu_helper.c | 121 +++++++++++++++++---------------------
> 1 file changed, 54 insertions(+), 67 deletions(-)
Nice.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 01/13] target/sparc: Use tlb_set_page_full
2023-02-23 20:43 ` [PATCH 01/13] target/sparc: Use tlb_set_page_full Richard Henderson
2023-02-23 21:25 ` Philippe Mathieu-Daudé
@ 2023-03-01 16:37 ` Mark Cave-Ayland
1 sibling, 0 replies; 27+ messages in thread
From: Mark Cave-Ayland @ 2023-03-01 16:37 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm, Artyom Tarasenko
On 23/02/2023 20:43, Richard Henderson wrote:
> Pass CPUTLBEntryFull to get_physical_address instead
> of a collection of pointers.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
> target/sparc/mmu_helper.c | 121 +++++++++++++++++---------------------
> 1 file changed, 54 insertions(+), 67 deletions(-)
>
> diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
> index 158ec2ae8f..a98dd0abd4 100644
> --- a/target/sparc/mmu_helper.c
> +++ b/target/sparc/mmu_helper.c
> @@ -64,10 +64,9 @@ static const int perm_table[2][8] = {
> }
> };
>
> -static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
> - int *prot, int *access_index, MemTxAttrs *attrs,
> - target_ulong address, int rw, int mmu_idx,
> - target_ulong *page_size)
> +static int get_physical_address(CPUSPARCState *env, CPUTLBEntryFull *full,
> + int *access_index, target_ulong address,
> + int rw, int mmu_idx)
> {
> int access_perms = 0;
> hwaddr pde_ptr;
> @@ -80,20 +79,20 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
> is_user = mmu_idx == MMU_USER_IDX;
>
> if (mmu_idx == MMU_PHYS_IDX) {
> - *page_size = TARGET_PAGE_SIZE;
> + full->lg_page_size = TARGET_PAGE_BITS;
> /* Boot mode: instruction fetches are taken from PROM */
> if (rw == 2 && (env->mmuregs[0] & env->def.mmu_bm)) {
> - *physical = env->prom_addr | (address & 0x7ffffULL);
> - *prot = PAGE_READ | PAGE_EXEC;
> + full->phys_addr = env->prom_addr | (address & 0x7ffffULL);
> + full->prot = PAGE_READ | PAGE_EXEC;
> return 0;
> }
> - *physical = address;
> - *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> + full->phys_addr = address;
> + full->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> return 0;
> }
>
> *access_index = ((rw & 1) << 2) | (rw & 2) | (is_user ? 0 : 1);
> - *physical = 0xffffffffffff0000ULL;
> + full->phys_addr = 0xffffffffffff0000ULL;
>
> /* SPARC reference MMU table walk: Context table->L1->L2->PTE */
> /* Context base + context number */
> @@ -157,16 +156,17 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
> case 2: /* L3 PTE */
> page_offset = 0;
> }
> - *page_size = TARGET_PAGE_SIZE;
> + full->lg_page_size = TARGET_PAGE_BITS;
> break;
> case 2: /* L2 PTE */
> page_offset = address & 0x3f000;
> - *page_size = 0x40000;
> + full->lg_page_size = 18;
> }
> break;
> case 2: /* L1 PTE */
> page_offset = address & 0xfff000;
> - *page_size = 0x1000000;
> + full->lg_page_size = 24;
> + break;
> }
> }
>
> @@ -188,16 +188,16 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
> }
>
> /* the page can be put in the TLB */
> - *prot = perm_table[is_user][access_perms];
> + full->prot = perm_table[is_user][access_perms];
> if (!(pde & PG_MODIFIED_MASK)) {
> /* only set write access if already dirty... otherwise wait
> for dirty access */
> - *prot &= ~PAGE_WRITE;
> + full->prot &= ~PAGE_WRITE;
> }
>
> /* Even if large ptes, we map only one 4KB page in the cache to
> avoid filling it too fast */
> - *physical = ((hwaddr)(pde & PTE_ADDR_MASK) << 4) + page_offset;
> + full->phys_addr = ((hwaddr)(pde & PTE_ADDR_MASK) << 4) + page_offset;
> return error_code;
> }
>
> @@ -208,11 +208,9 @@ bool sparc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> {
> SPARCCPU *cpu = SPARC_CPU(cs);
> CPUSPARCState *env = &cpu->env;
> - hwaddr paddr;
> + CPUTLBEntryFull full = {};
> target_ulong vaddr;
> - target_ulong page_size;
> - int error_code = 0, prot, access_index;
> - MemTxAttrs attrs = {};
> + int error_code = 0, access_index;
>
> /*
> * TODO: If we ever need tlb_vaddr_to_host for this target,
> @@ -223,16 +221,15 @@ bool sparc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> assert(!probe);
>
> address &= TARGET_PAGE_MASK;
> - error_code = get_physical_address(env, &paddr, &prot, &access_index, &attrs,
> - address, access_type,
> - mmu_idx, &page_size);
> + error_code = get_physical_address(env, &full, &access_index,
> + address, access_type, mmu_idx);
> vaddr = address;
> if (likely(error_code == 0)) {
> qemu_log_mask(CPU_LOG_MMU,
> "Translate at %" VADDR_PRIx " -> "
> HWADDR_FMT_plx ", vaddr " TARGET_FMT_lx "\n",
> - address, paddr, vaddr);
> - tlb_set_page(cs, vaddr, paddr, prot, mmu_idx, page_size);
> + address, full.phys_addr, vaddr);
> + tlb_set_page_full(cs, mmu_idx, vaddr, &full);
> return true;
> }
>
> @@ -247,8 +244,8 @@ bool sparc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> permissions. If no mapping is available, redirect accesses to
> neverland. Fake/overridden mappings will be flushed when
> switching to normal mode. */
> - prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> - tlb_set_page(cs, vaddr, paddr, prot, mmu_idx, TARGET_PAGE_SIZE);
> + full.prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> + tlb_set_page_full(cs, mmu_idx, vaddr, &full);
> return true;
> } else {
> if (access_type == MMU_INST_FETCH) {
> @@ -545,8 +542,7 @@ static uint64_t build_sfsr(CPUSPARCState *env, int mmu_idx, int rw)
> return sfsr;
> }
>
> -static int get_physical_address_data(CPUSPARCState *env, hwaddr *physical,
> - int *prot, MemTxAttrs *attrs,
> +static int get_physical_address_data(CPUSPARCState *env, CPUTLBEntryFull *full,
> target_ulong address, int rw, int mmu_idx)
> {
> CPUState *cs = env_cpu(env);
> @@ -579,11 +575,12 @@ static int get_physical_address_data(CPUSPARCState *env, hwaddr *physical,
>
> for (i = 0; i < 64; i++) {
> /* ctx match, vaddr match, valid? */
> - if (ultrasparc_tag_match(&env->dtlb[i], address, context, physical)) {
> + if (ultrasparc_tag_match(&env->dtlb[i], address, context,
> + &full->phys_addr)) {
> int do_fault = 0;
>
> if (TTE_IS_IE(env->dtlb[i].tte)) {
> - attrs->byte_swap = true;
> + full->attrs.byte_swap = true;
> }
>
> /* access ok? */
> @@ -616,9 +613,9 @@ static int get_physical_address_data(CPUSPARCState *env, hwaddr *physical,
> }
>
> if (!do_fault) {
> - *prot = PAGE_READ;
> + full->prot = PAGE_READ;
> if (TTE_IS_W_OK(env->dtlb[i].tte)) {
> - *prot |= PAGE_WRITE;
> + full->prot |= PAGE_WRITE;
> }
>
> TTE_SET_USED(env->dtlb[i].tte);
> @@ -645,8 +642,7 @@ static int get_physical_address_data(CPUSPARCState *env, hwaddr *physical,
> return 1;
> }
>
> -static int get_physical_address_code(CPUSPARCState *env, hwaddr *physical,
> - int *prot, MemTxAttrs *attrs,
> +static int get_physical_address_code(CPUSPARCState *env, CPUTLBEntryFull *full,
> target_ulong address, int mmu_idx)
> {
> CPUState *cs = env_cpu(env);
> @@ -681,7 +677,7 @@ static int get_physical_address_code(CPUSPARCState *env, hwaddr *physical,
> for (i = 0; i < 64; i++) {
> /* ctx match, vaddr match, valid? */
> if (ultrasparc_tag_match(&env->itlb[i],
> - address, context, physical)) {
> + address, context, &full->phys_addr)) {
> /* access ok? */
> if (TTE_IS_PRIV(env->itlb[i].tte) && is_user) {
> /* Fault status register */
> @@ -708,7 +704,7 @@ static int get_physical_address_code(CPUSPARCState *env, hwaddr *physical,
>
> return 1;
> }
> - *prot = PAGE_EXEC;
> + full->prot = PAGE_EXEC;
> TTE_SET_USED(env->itlb[i].tte);
> return 0;
> }
> @@ -722,14 +718,13 @@ static int get_physical_address_code(CPUSPARCState *env, hwaddr *physical,
> return 1;
> }
>
> -static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
> - int *prot, int *access_index, MemTxAttrs *attrs,
> - target_ulong address, int rw, int mmu_idx,
> - target_ulong *page_size)
> +static int get_physical_address(CPUSPARCState *env, CPUTLBEntryFull *full,
> + int *access_index, target_ulong address,
> + int rw, int mmu_idx)
> {
> /* ??? We treat everything as a small page, then explicitly flush
> everything when an entry is evicted. */
> - *page_size = TARGET_PAGE_SIZE;
> + full->lg_page_size = TARGET_PAGE_BITS;
>
> /* safety net to catch wrong softmmu index use from dynamic code */
> if (env->tl > 0 && mmu_idx != MMU_NUCLEUS_IDX) {
> @@ -747,17 +742,15 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
> }
>
> if (mmu_idx == MMU_PHYS_IDX) {
> - *physical = ultrasparc_truncate_physical(address);
> - *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> + full->phys_addr = ultrasparc_truncate_physical(address);
> + full->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> return 0;
> }
>
> if (rw == 2) {
> - return get_physical_address_code(env, physical, prot, attrs, address,
> - mmu_idx);
> + return get_physical_address_code(env, full, address, mmu_idx);
> } else {
> - return get_physical_address_data(env, physical, prot, attrs, address,
> - rw, mmu_idx);
> + return get_physical_address_data(env, full, address, rw, mmu_idx);
> }
> }
>
> @@ -768,25 +761,17 @@ bool sparc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> {
> SPARCCPU *cpu = SPARC_CPU(cs);
> CPUSPARCState *env = &cpu->env;
> - target_ulong vaddr;
> - hwaddr paddr;
> - target_ulong page_size;
> - MemTxAttrs attrs = {};
> - int error_code = 0, prot, access_index;
> + CPUTLBEntryFull full = {};
> + int error_code = 0, access_index;
>
> address &= TARGET_PAGE_MASK;
> - error_code = get_physical_address(env, &paddr, &prot, &access_index, &attrs,
> - address, access_type,
> - mmu_idx, &page_size);
> + error_code = get_physical_address(env, &full, &access_index,
> + address, access_type, mmu_idx);
> if (likely(error_code == 0)) {
> - vaddr = address;
> -
> - trace_mmu_helper_mmu_fault(address, paddr, mmu_idx, env->tl,
> + trace_mmu_helper_mmu_fault(address, full.phys_addr, mmu_idx, env->tl,
> env->dmmu.mmu_primary_context,
> env->dmmu.mmu_secondary_context);
> -
> - tlb_set_page_with_attrs(cs, vaddr, paddr, attrs, prot, mmu_idx,
> - page_size);
> + tlb_set_page_full(cs, mmu_idx, address, &full);
> return true;
> }
> if (probe) {
> @@ -888,12 +873,14 @@ void dump_mmu(CPUSPARCState *env)
> static int cpu_sparc_get_phys_page(CPUSPARCState *env, hwaddr *phys,
> target_ulong addr, int rw, int mmu_idx)
> {
> - target_ulong page_size;
> - int prot, access_index;
> - MemTxAttrs attrs = {};
> + CPUTLBEntryFull full = {};
> + int access_index, ret;
>
> - return get_physical_address(env, phys, &prot, &access_index, &attrs, addr,
> - rw, mmu_idx, &page_size);
> + ret = get_physical_address(env, &full, &access_index, addr, rw, mmu_idx);
> + if (ret == 0) {
> + *phys = full.phys_addr;
> + }
> + return ret;
> }
>
> #if defined(TARGET_SPARC64)
Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 02/13] accel/tcg: Retain prot flags from tlb_fill
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
2023-02-23 20:43 ` [PATCH 01/13] target/sparc: Use tlb_set_page_full Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-03-03 16:29 ` Peter Maydell
2023-02-23 20:43 ` [PATCH 03/13] accel/tcg: Store some tlb flags in CPUTLBEntryFull Richard Henderson
` (10 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
While changes are made to prot within tlb_set_page_full, they are
an implementation detail of softmmu. Retain the original for any
target use of probe_access_full.
Fixes: 4047368938f6 ("accel/tcg: Introduce tlb_set_page_full")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/cputlb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index d7ca90e3b4..169adc0262 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1251,7 +1251,6 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
desc->fulltlb[index] = *full;
desc->fulltlb[index].xlat_section = iotlb - vaddr_page;
desc->fulltlb[index].phys_addr = paddr_page;
- desc->fulltlb[index].prot = prot;
/* Now calculate the new entry */
tn.addend = addend - vaddr_page;
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 02/13] accel/tcg: Retain prot flags from tlb_fill
2023-02-23 20:43 ` [PATCH 02/13] accel/tcg: Retain prot flags from tlb_fill Richard Henderson
@ 2023-03-03 16:29 ` Peter Maydell
0 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2023-03-03 16:29 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Thu, 23 Feb 2023 at 20:47, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> While changes are made to prot within tlb_set_page_full, they are
> an implementation detail of softmmu. Retain the original for any
> target use of probe_access_full.
>
> Fixes: 4047368938f6 ("accel/tcg: Introduce tlb_set_page_full")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> accel/tcg/cputlb.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index d7ca90e3b4..169adc0262 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1251,7 +1251,6 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
> desc->fulltlb[index] = *full;
> desc->fulltlb[index].xlat_section = iotlb - vaddr_page;
> desc->fulltlb[index].phys_addr = paddr_page;
> - desc->fulltlb[index].prot = prot;
>
> /* Now calculate the new entry */
> tn.addend = addend - vaddr_page;
> --
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 03/13] accel/tcg: Store some tlb flags in CPUTLBEntryFull
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
2023-02-23 20:43 ` [PATCH 01/13] target/sparc: Use tlb_set_page_full Richard Henderson
2023-02-23 20:43 ` [PATCH 02/13] accel/tcg: Retain prot flags from tlb_fill Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-03-03 16:45 ` Peter Maydell
2023-02-23 20:43 ` [PATCH 04/13] accel/tcg: Honor TLB_DISCARD_WRITE in atomic_mmu_lookup Richard Henderson
` (9 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
We have run out of bits we can use within the CPUTLBEntry comparators,
as TLB_FLAGS_MASK cannot overlap alignment.
Store slow_flags[] in CPUTLBEntryFull, and merge with the flags from
the comparator. A new TLB_FORCE_SLOW bit is set within the comparator
as an indication that the slow path must be used.
Move TLB_BSWAP to TLB_SLOW_FLAGS_MASK. Since we are out of bits,
we cannot create a new bit without moving an old one.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-all.h | 21 ++++++++--
include/exec/cpu-defs.h | 6 +++
accel/tcg/cputlb.c | 93 ++++++++++++++++++++++++-----------------
3 files changed, 77 insertions(+), 43 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 2eb1176538..080cb3112e 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -380,17 +380,30 @@ 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))
+/* Set if the slow path must be used; more flags in CPUTLBEntryFull. */
+#define TLB_FORCE_SLOW (1 << (TARGET_PAGE_BITS_MIN - 5))
/* Set if TLB entry writes ignored. */
#define TLB_DISCARD_WRITE (1 << (TARGET_PAGE_BITS_MIN - 6))
-/* Use this mask to check interception with an alignment mask
+/*
+ * 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_DISCARD_WRITE)
+ | TLB_WATCHPOINT | TLB_FORCE_SLOW | TLB_DISCARD_WRITE)
+
+/*
+ * Flags stored in CPUTLBEntryFull.slow_flags[x].
+ * TLB_FORCE_SLOW must be set in CPUTLBEntry.addr_idx[x].
+ */
+/* Set if TLB entry requires byte swap. */
+#define TLB_BSWAP (1 << 0)
+
+#define TLB_SLOW_FLAGS_MASK TLB_BSWAP
+
+/* The two sets of flags must not overlap. */
+QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & TLB_SLOW_FLAGS_MASK);
/**
* tlb_hit_page: return true if page aligned @addr is a hit against the
diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index 7ce3bcb06b..ef10c625d4 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -170,6 +170,12 @@ typedef struct CPUTLBEntryFull {
/* @lg_page_size contains the log2 of the page size. */
uint8_t lg_page_size;
+ /*
+ * Additional tlb flags for use by the slow path. If non-zero,
+ * the corresponding CPUTLBEntry comparator must have TLB_FORCE_SLOW.
+ */
+ uint8_t slow_flags[3];
+
/*
* Allow target-specific additions to this structure.
* This may be used to cache items from the guest cpu
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 169adc0262..e9848b3ab6 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1106,6 +1106,24 @@ static void tlb_add_large_page(CPUArchState *env, int mmu_idx,
env_tlb(env)->d[mmu_idx].large_page_mask = lp_mask;
}
+static inline void tlb_set_compare(CPUTLBEntryFull *full, CPUTLBEntry *ent,
+ target_ulong address, int flags,
+ MMUAccessType access_type, bool enable)
+{
+ if (enable) {
+ address |= flags & TLB_FLAGS_MASK;
+ flags &= TLB_SLOW_FLAGS_MASK;
+ if (flags) {
+ address |= TLB_FORCE_SLOW;
+ }
+ } else {
+ address = -1;
+ flags = 0;
+ }
+ ent->addr_idx[access_type] = address;
+ full->slow_flags[access_type] = flags;
+}
+
/*
* Add a new TLB entry. At most one entry for a given virtual address
* is permitted. Only a single TARGET_PAGE_SIZE region is mapped, the
@@ -1121,9 +1139,7 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
CPUTLB *tlb = env_tlb(env);
CPUTLBDesc *desc = &tlb->d[mmu_idx];
MemoryRegionSection *section;
- unsigned int index;
- target_ulong address;
- target_ulong write_address;
+ unsigned int index, read_flags, write_flags;
uintptr_t addend;
CPUTLBEntry *te, tn;
hwaddr iotlb, xlat, sz, paddr_page;
@@ -1152,13 +1168,13 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
" prot=%x idx=%d\n",
vaddr, full->phys_addr, prot, mmu_idx);
- address = vaddr_page;
+ read_flags = 0;
if (full->lg_page_size < TARGET_PAGE_BITS) {
/* Repeat the MMU check and TLB fill on every access. */
- address |= TLB_INVALID_MASK;
+ read_flags |= TLB_INVALID_MASK;
}
if (full->attrs.byte_swap) {
- address |= TLB_BSWAP;
+ read_flags |= TLB_BSWAP;
}
is_ram = memory_region_is_ram(section->mr);
@@ -1172,7 +1188,7 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
addend = 0;
}
- write_address = address;
+ write_flags = read_flags;
if (is_ram) {
iotlb = memory_region_get_ram_addr(section->mr) + xlat;
/*
@@ -1181,9 +1197,9 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
*/
if (prot & PAGE_WRITE) {
if (section->readonly) {
- write_address |= TLB_DISCARD_WRITE;
+ write_flags |= TLB_DISCARD_WRITE;
} else if (cpu_physical_memory_is_clean(iotlb)) {
- write_address |= TLB_NOTDIRTY;
+ write_flags |= TLB_NOTDIRTY;
}
}
} else {
@@ -1194,9 +1210,9 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
* 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;
+ write_flags |= TLB_MMIO;
if (!is_romd) {
- address = write_address;
+ read_flags = write_flags;
}
}
@@ -1249,36 +1265,27 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
* vaddr we add back in io_readx()/io_writex()/get_page_addr_code().
*/
desc->fulltlb[index] = *full;
- desc->fulltlb[index].xlat_section = iotlb - vaddr_page;
- desc->fulltlb[index].phys_addr = paddr_page;
+ full = &desc->fulltlb[index];
+ full->xlat_section = iotlb - vaddr_page;
+ full->phys_addr = paddr_page;
/* Now calculate the new entry */
tn.addend = addend - vaddr_page;
- if (prot & PAGE_READ) {
- tn.addr_read = address;
- if (wp_flags & BP_MEM_READ) {
- tn.addr_read |= TLB_WATCHPOINT;
- }
- } else {
- tn.addr_read = -1;
- }
- if (prot & PAGE_EXEC) {
- tn.addr_code = address;
- } else {
- tn.addr_code = -1;
- }
+ tlb_set_compare(full, &tn, vaddr_page, read_flags,
+ MMU_INST_FETCH, prot & PAGE_EXEC);
- tn.addr_write = -1;
- if (prot & PAGE_WRITE) {
- tn.addr_write = write_address;
- if (prot & PAGE_WRITE_INV) {
- tn.addr_write |= TLB_INVALID_MASK;
- }
- if (wp_flags & BP_MEM_WRITE) {
- tn.addr_write |= TLB_WATCHPOINT;
- }
+ if (wp_flags & BP_MEM_READ) {
+ read_flags |= TLB_WATCHPOINT;
}
+ tlb_set_compare(full, &tn, vaddr_page, read_flags,
+ MMU_DATA_LOAD, prot & PAGE_READ);
+
+ if (wp_flags & BP_MEM_WRITE) {
+ write_flags |= TLB_WATCHPOINT;
+ }
+ tlb_set_compare(full, &tn, vaddr_page, write_flags, MMU_DATA_STORE,
+ (prot & PAGE_WRITE) && !(prot & PAGE_WRITE_INV));
copy_tlb_helper_locked(te, &tn);
tlb_n_used_entries_inc(env, mmu_idx);
@@ -1508,7 +1515,8 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr,
CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
target_ulong tlb_addr = tlb_read_idx(entry, access_type);
target_ulong page_addr = addr & TARGET_PAGE_MASK;
- int flags = TLB_FLAGS_MASK;
+ int flags = TLB_FLAGS_MASK & ~TLB_FORCE_SLOW;
+ CPUTLBEntryFull *full;
if (!tlb_hit_page(tlb_addr, page_addr)) {
if (!victim_tlb_hit(env, mmu_idx, index, access_type, page_addr)) {
@@ -1537,7 +1545,8 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr,
}
flags &= tlb_addr;
- *pfull = &env_tlb(env)->d[mmu_idx].fulltlb[index];
+ *pfull = full = &env_tlb(env)->d[mmu_idx].fulltlb[index];
+ flags |= full->slow_flags[access_type];
/* Fold all "mmio-like" bits into TLB_MMIO. This is not RAM. */
if (unlikely(flags & ~(TLB_WATCHPOINT | TLB_NOTDIRTY))) {
@@ -1744,6 +1753,8 @@ static bool mmu_lookup1(CPUArchState *env, MMULookupPageData *data,
CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
target_ulong tlb_addr = tlb_read_idx(entry, access_type);
bool maybe_resized = false;
+ CPUTLBEntryFull *full;
+ int flags;
/* If the TLB entry is for a different page, reload and try again. */
if (!tlb_hit(tlb_addr, addr)) {
@@ -1757,8 +1768,12 @@ static bool mmu_lookup1(CPUArchState *env, MMULookupPageData *data,
tlb_addr = tlb_read_idx(entry, access_type) & ~TLB_INVALID_MASK;
}
- data->flags = tlb_addr & TLB_FLAGS_MASK;
- data->full = &env_tlb(env)->d[mmu_idx].fulltlb[index];
+ full = &env_tlb(env)->d[mmu_idx].fulltlb[index];
+ flags = tlb_addr & (TLB_FLAGS_MASK & ~TLB_FORCE_SLOW);
+ flags |= full->slow_flags[access_type];
+
+ data->full = full;
+ data->flags = flags;
/* Compute haddr speculatively; depending on flags it might be invalid. */
data->haddr = (void *)((uintptr_t)addr + entry->addend);
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 03/13] accel/tcg: Store some tlb flags in CPUTLBEntryFull
2023-02-23 20:43 ` [PATCH 03/13] accel/tcg: Store some tlb flags in CPUTLBEntryFull Richard Henderson
@ 2023-03-03 16:45 ` Peter Maydell
2023-03-05 18:20 ` Richard Henderson
0 siblings, 1 reply; 27+ messages in thread
From: Peter Maydell @ 2023-03-03 16:45 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Thu, 23 Feb 2023 at 20:46, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We have run out of bits we can use within the CPUTLBEntry comparators,
> as TLB_FLAGS_MASK cannot overlap alignment.
>
> Store slow_flags[] in CPUTLBEntryFull, and merge with the flags from
> the comparator. A new TLB_FORCE_SLOW bit is set within the comparator
> as an indication that the slow path must be used.
>
> Move TLB_BSWAP to TLB_SLOW_FLAGS_MASK. Since we are out of bits,
> we cannot create a new bit without moving an old one.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> @@ -1249,36 +1265,27 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
> * vaddr we add back in io_readx()/io_writex()/get_page_addr_code().
> */
> desc->fulltlb[index] = *full;
> - desc->fulltlb[index].xlat_section = iotlb - vaddr_page;
> - desc->fulltlb[index].phys_addr = paddr_page;
> + full = &desc->fulltlb[index];
> + full->xlat_section = iotlb - vaddr_page;
> + full->phys_addr = paddr_page;
>
> /* Now calculate the new entry */
> tn.addend = addend - vaddr_page;
> - if (prot & PAGE_READ) {
> - tn.addr_read = address;
> - if (wp_flags & BP_MEM_READ) {
> - tn.addr_read |= TLB_WATCHPOINT;
> - }
> - } else {
> - tn.addr_read = -1;
> - }
>
> - if (prot & PAGE_EXEC) {
> - tn.addr_code = address;
> - } else {
> - tn.addr_code = -1;
> - }
> + tlb_set_compare(full, &tn, vaddr_page, read_flags,
> + MMU_INST_FETCH, prot & PAGE_EXEC);
>
> - tn.addr_write = -1;
> - if (prot & PAGE_WRITE) {
> - tn.addr_write = write_address;
> - if (prot & PAGE_WRITE_INV) {
> - tn.addr_write |= TLB_INVALID_MASK;
> - }
> - if (wp_flags & BP_MEM_WRITE) {
> - tn.addr_write |= TLB_WATCHPOINT;
> - }
> + if (wp_flags & BP_MEM_READ) {
> + read_flags |= TLB_WATCHPOINT;
> }
> + tlb_set_compare(full, &tn, vaddr_page, read_flags,
> + MMU_DATA_LOAD, prot & PAGE_READ);
> +
> + if (wp_flags & BP_MEM_WRITE) {
> + write_flags |= TLB_WATCHPOINT;
> + }
> + tlb_set_compare(full, &tn, vaddr_page, write_flags, MMU_DATA_STORE,
> + (prot & PAGE_WRITE) && !(prot & PAGE_WRITE_INV));
So in the old code, if PAGE_WRITE_INV then we set up the
addr_write field as normal, it just also has the TLB_INVALID_MASK bit
set. In the new code we won't do that, we'll set addr_write to -1.
I'm not fully familiar with the cputlb.c code, but doesn't this
break the code in probe_access_internal(), which assumes that
it can call tlb_fill (which will come through here) and then
fish out the TLB entry, clear out the TLB_INVALID_MASK bit and
use the TLB entry as a one-off ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 03/13] accel/tcg: Store some tlb flags in CPUTLBEntryFull
2023-03-03 16:45 ` Peter Maydell
@ 2023-03-05 18:20 ` Richard Henderson
0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2023-03-05 18:20 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, qemu-arm
On 3/3/23 08:45, Peter Maydell wrote:
>> +
>> + if (wp_flags & BP_MEM_WRITE) {
>> + write_flags |= TLB_WATCHPOINT;
>> + }
>> + tlb_set_compare(full, &tn, vaddr_page, write_flags, MMU_DATA_STORE,
>> + (prot & PAGE_WRITE) && !(prot & PAGE_WRITE_INV));
>
> So in the old code, if PAGE_WRITE_INV then we set up the
> addr_write field as normal, it just also has the TLB_INVALID_MASK bit
> set. In the new code we won't do that, we'll set addr_write to -1.
Gah. I must have had some sort of rebase fumble, because I know I fixed this, and the
WRITE_INV test should be above, not in the predicate.
r~
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 04/13] accel/tcg: Honor TLB_DISCARD_WRITE in atomic_mmu_lookup
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
` (2 preceding siblings ...)
2023-02-23 20:43 ` [PATCH 03/13] accel/tcg: Store some tlb flags in CPUTLBEntryFull Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-03-03 16:46 ` Peter Maydell
2023-02-23 20:43 ` [PATCH 05/13] softmmu/physmem: Check watchpoints for read+write at once Richard Henderson
` (8 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Using an atomic write or read-write insn on ROM is basically
a happens-never case. Handle it via stop-the-world, which
will generate non-atomic serial code, where we can correctly
ignore the write while producing the correct read result.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/cputlb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index e9848b3ab6..74ad8e0876 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1974,7 +1974,7 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
}
/* Notice an IO access or a needs-MMU-lookup access */
- if (unlikely(tlb_addr & TLB_MMIO)) {
+ if (unlikely(tlb_addr & (TLB_MMIO | TLB_DISCARD_WRITE))) {
/* There's really nothing that can be done to
support this apart from stop-the-world. */
goto stop_the_world;
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 05/13] softmmu/physmem: Check watchpoints for read+write at once
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
` (3 preceding siblings ...)
2023-02-23 20:43 ` [PATCH 04/13] accel/tcg: Honor TLB_DISCARD_WRITE in atomic_mmu_lookup Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-02-23 21:27 ` Philippe Mathieu-Daudé
2023-02-23 20:43 ` [PATCH 06/13] accel/tcg: Trigger watchpoints from atomic_mmu_lookup Richard Henderson
` (7 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Atomic operations are read-modify-write, and we'd like to
be able to test both read and write with one call. This is
easy enough, with BP_MEM_READ | BP_MEM_WRITE.
Add BP_HIT_SHIFT to make it easy to set BP_WATCHPOINT_HIT_*.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/hw/core/cpu.h | 7 ++++---
softmmu/physmem.c | 19 ++++++++++---------
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 2417597236..2f85ba14b3 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -921,9 +921,10 @@ void cpu_single_step(CPUState *cpu, int enabled);
#define BP_GDB 0x10
#define BP_CPU 0x20
#define BP_ANY (BP_GDB | BP_CPU)
-#define BP_WATCHPOINT_HIT_READ 0x40
-#define BP_WATCHPOINT_HIT_WRITE 0x80
-#define BP_WATCHPOINT_HIT (BP_WATCHPOINT_HIT_READ | BP_WATCHPOINT_HIT_WRITE)
+#define BP_HIT_SHIFT 6
+#define BP_WATCHPOINT_HIT_READ (BP_MEM_READ << BP_HIT_SHIFT)
+#define BP_WATCHPOINT_HIT_WRITE (BP_MEM_WRITE << BP_HIT_SHIFT)
+#define BP_WATCHPOINT_HIT (BP_MEM_ACCESS << BP_HIT_SHIFT)
int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags,
CPUBreakpoint **breakpoint);
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index cb998cdf23..c4f62dee60 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -915,9 +915,12 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
/* this is currently used only by ARM BE32 */
addr = cc->tcg_ops->adjust_watchpoint_address(cpu, addr, len);
}
+
+ assert((flags & ~BP_MEM_ACCESS) == 0);
QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
- if (watchpoint_address_matches(wp, addr, len)
- && (wp->flags & flags)) {
+ int hit_flags = wp->flags & flags;
+
+ if (hit_flags && watchpoint_address_matches(wp, addr, len)) {
if (replay_running_debug()) {
/*
* replay_breakpoint reads icount.
@@ -936,16 +939,14 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
replay_breakpoint();
return;
}
- if (flags == BP_MEM_READ) {
- wp->flags |= BP_WATCHPOINT_HIT_READ;
- } else {
- wp->flags |= BP_WATCHPOINT_HIT_WRITE;
- }
+
+ wp->flags |= hit_flags << BP_HIT_SHIFT;
wp->hitaddr = MAX(addr, wp->vaddr);
wp->hitattrs = attrs;
- if (wp->flags & BP_CPU && cc->tcg_ops->debug_check_watchpoint &&
- !cc->tcg_ops->debug_check_watchpoint(cpu, wp)) {
+ if (wp->flags & BP_CPU
+ && cc->tcg_ops->debug_check_watchpoint
+ && !cc->tcg_ops->debug_check_watchpoint(cpu, wp)) {
wp->flags &= ~BP_WATCHPOINT_HIT;
continue;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 05/13] softmmu/physmem: Check watchpoints for read+write at once
2023-02-23 20:43 ` [PATCH 05/13] softmmu/physmem: Check watchpoints for read+write at once Richard Henderson
@ 2023-02-23 21:27 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-23 21:27 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm
On 23/2/23 21:43, Richard Henderson wrote:
> Atomic operations are read-modify-write, and we'd like to
> be able to test both read and write with one call. This is
> easy enough, with BP_MEM_READ | BP_MEM_WRITE.
>
> Add BP_HIT_SHIFT to make it easy to set BP_WATCHPOINT_HIT_*.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/hw/core/cpu.h | 7 ++++---
> softmmu/physmem.c | 19 ++++++++++---------
> 2 files changed, 14 insertions(+), 12 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 06/13] accel/tcg: Trigger watchpoints from atomic_mmu_lookup
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
` (4 preceding siblings ...)
2023-02-23 20:43 ` [PATCH 05/13] softmmu/physmem: Check watchpoints for read+write at once Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-03-03 16:49 ` Peter Maydell
2023-02-23 20:43 ` [PATCH 07/13] accel/tcg: Move TLB_WATCHPOINT to TLB_SLOW_FLAGS_MASK Richard Henderson
` (6 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Fixes a bug in that we weren't reporting these changes.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/cputlb.c | 40 +++++++++++++++++++++++++++++-----------
1 file changed, 29 insertions(+), 11 deletions(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 74ad8e0876..e0765c8c10 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1908,6 +1908,7 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
CPUTLBEntry *tlbe;
target_ulong tlb_addr;
void *hostaddr;
+ CPUTLBEntryFull *full;
tcg_debug_assert(mmu_idx < NB_MMU_MODES);
@@ -1947,17 +1948,26 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
tlb_addr = tlb_addr_write(tlbe) & ~TLB_INVALID_MASK;
}
- /* Let the guest notice RMW on a write-only page. */
- if ((prot & PAGE_READ) &&
- unlikely(tlbe->addr_read != (tlb_addr & ~TLB_NOTDIRTY))) {
- tlb_fill(env_cpu(env), addr, size,
- MMU_DATA_LOAD, mmu_idx, retaddr);
+ if (prot & PAGE_READ) {
/*
- * Since we don't support reads and writes to different addresses,
- * and we do have the proper page loaded for write, this shouldn't
- * ever return. But just in case, handle via stop-the-world.
+ * Let the guest notice RMW on a write-only page.
+ * We have just verified that the page is writable.
+ * Subpage lookups may have left TLB_INVALID_MASK set,
+ * but addr_read will only be -1 if PAGE_READ was unset.
*/
- goto stop_the_world;
+ if (unlikely(tlbe->addr_read == -1)) {
+ tlb_fill(env_cpu(env), addr, size,
+ MMU_DATA_LOAD, mmu_idx, retaddr);
+ /*
+ * Since we don't support reads and writes to different
+ * addresses, and we do have the proper page loaded for
+ * write, this shouldn't ever return. But just in case,
+ * handle via stop-the-world.
+ */
+ goto stop_the_world;
+ }
+ /* Collect TLB_WATCHPOINT for read. */
+ tlb_addr |= tlbe->addr_read;
}
} else /* if (prot & PAGE_READ) */ {
tlb_addr = tlbe->addr_read;
@@ -1981,10 +1991,18 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
}
hostaddr = (void *)((uintptr_t)addr + tlbe->addend);
+ full = &env_tlb(env)->d[mmu_idx].fulltlb[index];
if (unlikely(tlb_addr & TLB_NOTDIRTY)) {
- notdirty_write(env_cpu(env), addr, size,
- &env_tlb(env)->d[mmu_idx].fulltlb[index], retaddr);
+ notdirty_write(env_cpu(env), addr, size, full, retaddr);
+ }
+
+ if (unlikely(tlb_addr & TLB_WATCHPOINT)) {
+ QEMU_BUILD_BUG_ON(PAGE_READ != BP_MEM_READ);
+ QEMU_BUILD_BUG_ON(PAGE_WRITE != BP_MEM_WRITE);
+ /* therefore prot == watchpoint bits */
+ cpu_check_watchpoint(env_cpu(env), addr, size,
+ full->attrs, prot, retaddr);
}
return hostaddr;
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 07/13] accel/tcg: Move TLB_WATCHPOINT to TLB_SLOW_FLAGS_MASK
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
` (5 preceding siblings ...)
2023-02-23 20:43 ` [PATCH 06/13] accel/tcg: Trigger watchpoints from atomic_mmu_lookup Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-03-03 16:53 ` Peter Maydell
2023-02-23 20:43 ` [PATCH 08/13] target/arm: Support 32-byte alignment in pow2_align Richard Henderson
` (5 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
This frees up one bit of the primary tlb flags without
impacting the TLB_NOTDIRTY logic.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-all.h | 12 ++++++------
accel/tcg/cputlb.c | 23 ++++++++++++++++-------
2 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 080cb3112e..f3b2f4229c 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -378,12 +378,10 @@ CPUArchState *cpu_copy(CPUArchState *env);
#define TLB_NOTDIRTY (1 << (TARGET_PAGE_BITS_MIN - 2))
/* Set if TLB entry is an IO callback. */
#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 writes ignored. */
+#define TLB_DISCARD_WRITE (1 << (TARGET_PAGE_BITS_MIN - 4))
/* Set if the slow path must be used; more flags in CPUTLBEntryFull. */
#define TLB_FORCE_SLOW (1 << (TARGET_PAGE_BITS_MIN - 5))
-/* Set if TLB entry writes ignored. */
-#define TLB_DISCARD_WRITE (1 << (TARGET_PAGE_BITS_MIN - 6))
/*
* Use this mask to check interception with an alignment mask
@@ -391,7 +389,7 @@ CPUArchState *cpu_copy(CPUArchState *env);
*/
#define TLB_FLAGS_MASK \
(TLB_INVALID_MASK | TLB_NOTDIRTY | TLB_MMIO \
- | TLB_WATCHPOINT | TLB_FORCE_SLOW | TLB_DISCARD_WRITE)
+ | TLB_FORCE_SLOW | TLB_DISCARD_WRITE)
/*
* Flags stored in CPUTLBEntryFull.slow_flags[x].
@@ -399,8 +397,10 @@ CPUArchState *cpu_copy(CPUArchState *env);
*/
/* Set if TLB entry requires byte swap. */
#define TLB_BSWAP (1 << 0)
+/* Set if TLB entry contains a watchpoint. */
+#define TLB_WATCHPOINT (1 << 1)
-#define TLB_SLOW_FLAGS_MASK TLB_BSWAP
+#define TLB_SLOW_FLAGS_MASK (TLB_BSWAP | TLB_WATCHPOINT)
/* The two sets of flags must not overlap. */
QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & TLB_SLOW_FLAGS_MASK);
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index e0765c8c10..cc98df9517 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1966,7 +1966,7 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
*/
goto stop_the_world;
}
- /* Collect TLB_WATCHPOINT for read. */
+ /* Collect tlb flags for read. */
tlb_addr |= tlbe->addr_read;
}
} else /* if (prot & PAGE_READ) */ {
@@ -1997,12 +1997,21 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
notdirty_write(env_cpu(env), addr, size, full, retaddr);
}
- if (unlikely(tlb_addr & TLB_WATCHPOINT)) {
- QEMU_BUILD_BUG_ON(PAGE_READ != BP_MEM_READ);
- QEMU_BUILD_BUG_ON(PAGE_WRITE != BP_MEM_WRITE);
- /* therefore prot == watchpoint bits */
- cpu_check_watchpoint(env_cpu(env), addr, size,
- full->attrs, prot, retaddr);
+ if (unlikely(tlb_addr & TLB_FORCE_SLOW)) {
+ int wp_flags = 0;
+
+ if ((prot & PAGE_WRITE) &&
+ (full->slow_flags[MMU_DATA_STORE] & TLB_WATCHPOINT)) {
+ wp_flags |= BP_MEM_WRITE;
+ }
+ if ((prot & PAGE_READ) &&
+ (full->slow_flags[MMU_DATA_LOAD] & TLB_WATCHPOINT)) {
+ wp_flags |= BP_MEM_READ;
+ }
+ if (wp_flags) {
+ cpu_check_watchpoint(env_cpu(env), addr, size,
+ full->attrs, wp_flags, retaddr);
+ }
}
return hostaddr;
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 08/13] target/arm: Support 32-byte alignment in pow2_align
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
` (6 preceding siblings ...)
2023-02-23 20:43 ` [PATCH 07/13] accel/tcg: Move TLB_WATCHPOINT to TLB_SLOW_FLAGS_MASK Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-03-03 16:54 ` Peter Maydell
2023-02-23 20:43 ` [PATCH 09/13] exec/memattrs: Remove target_tlb_bit* Richard Henderson
` (4 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Now that we have removed TARGET_PAGE_BITS_MIN-6 from
TLB_FLAGS_MASK, we can test for 32-byte alignment.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index c23a3462bf..412fc4aca8 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -940,13 +940,7 @@ static inline void store_reg_from_load(DisasContext *s, int reg, TCGv_i32 var)
MemOp pow2_align(unsigned i)
{
static const MemOp mop_align[] = {
- 0, MO_ALIGN_2, MO_ALIGN_4, MO_ALIGN_8, MO_ALIGN_16,
- /*
- * FIXME: TARGET_PAGE_BITS_MIN affects TLB_FLAGS_MASK such
- * that 256-bit alignment (MO_ALIGN_32) cannot be supported:
- * see get_alignment_bits(). Enforce only 128-bit alignment for now.
- */
- MO_ALIGN_16
+ 0, MO_ALIGN_2, MO_ALIGN_4, MO_ALIGN_8, MO_ALIGN_16, MO_ALIGN_32
};
g_assert(i < ARRAY_SIZE(mop_align));
return mop_align[i];
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 09/13] exec/memattrs: Remove target_tlb_bit*
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
` (7 preceding siblings ...)
2023-02-23 20:43 ` [PATCH 08/13] target/arm: Support 32-byte alignment in pow2_align Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-02-23 21:30 ` Philippe Mathieu-Daudé
2023-02-23 20:43 ` [PATCH 10/13] accel/tcg: Add tlb_fill_flags to CPUTLBEntryFull Richard Henderson
` (3 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
These fields are no longer used. Target specific extensions
to the page tables should be done with TARGET_PAGE_ENTRY_EXTRA.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/memattrs.h | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index 9fb98bc1ef..1bd7b6c5ca 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -47,16 +47,6 @@ typedef struct MemTxAttrs {
unsigned int requester_id:16;
/* Invert endianness for this page */
unsigned int byte_swap:1;
- /*
- * The following are target-specific page-table bits. These are not
- * related to actual memory transactions at all. However, this structure
- * is part of the tlb_fill interface, cached in the cputlb structure,
- * and has unused bits. These fields will be read by target-specific
- * helpers using env->iotlb[mmu_idx][tlb_index()].attrs.target_tlb_bitN.
- */
- unsigned int target_tlb_bit0 : 1;
- unsigned int target_tlb_bit1 : 1;
- unsigned int target_tlb_bit2 : 1;
} MemTxAttrs;
/* Bus masters which don't specify any attributes will get this,
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 10/13] accel/tcg: Add tlb_fill_flags to CPUTLBEntryFull
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
` (8 preceding siblings ...)
2023-02-23 20:43 ` [PATCH 09/13] exec/memattrs: Remove target_tlb_bit* Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-02-23 21:32 ` Philippe Mathieu-Daudé
2023-02-23 20:43 ` [PATCH 11/13] accel/tcg: Add TLB_CHECK_ALIGNED Richard Henderson
` (2 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Allow the target to set tlb flags to apply to all of the
comparators. Remove MemTxAttrs.byte_swap, as the bit is
not relevant to memory transactions, only the page mapping.
Adjust target/sparc to set TLB_BSWAP directly.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-defs.h | 3 +++
include/exec/memattrs.h | 2 --
accel/tcg/cputlb.c | 5 +----
target/sparc/mmu_helper.c | 2 +-
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index ef10c625d4..53743ff3f2 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -170,6 +170,9 @@ typedef struct CPUTLBEntryFull {
/* @lg_page_size contains the log2 of the page size. */
uint8_t lg_page_size;
+ /* Additional tlb flags requested by tlb_fill. */
+ uint8_t tlb_fill_flags;
+
/*
* Additional tlb flags for use by the slow path. If non-zero,
* the corresponding CPUTLBEntry comparator must have TLB_FORCE_SLOW.
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index 1bd7b6c5ca..5300649c8c 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -45,8 +45,6 @@ typedef struct MemTxAttrs {
unsigned int memory:1;
/* Requester ID (for MSI for example) */
unsigned int requester_id:16;
- /* Invert endianness for this page */
- unsigned int byte_swap:1;
} MemTxAttrs;
/* Bus masters which don't specify any attributes will get this,
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index cc98df9517..a90688ac30 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1168,14 +1168,11 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
" prot=%x idx=%d\n",
vaddr, full->phys_addr, prot, mmu_idx);
- read_flags = 0;
+ read_flags = full->tlb_fill_flags;
if (full->lg_page_size < TARGET_PAGE_BITS) {
/* Repeat the MMU check and TLB fill on every access. */
read_flags |= TLB_INVALID_MASK;
}
- if (full->attrs.byte_swap) {
- read_flags |= TLB_BSWAP;
- }
is_ram = memory_region_is_ram(section->mr);
is_romd = memory_region_is_romd(section->mr);
diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
index a98dd0abd4..fa58b4dc03 100644
--- a/target/sparc/mmu_helper.c
+++ b/target/sparc/mmu_helper.c
@@ -580,7 +580,7 @@ static int get_physical_address_data(CPUSPARCState *env, CPUTLBEntryFull *full,
int do_fault = 0;
if (TTE_IS_IE(env->dtlb[i].tte)) {
- full->attrs.byte_swap = true;
+ full->tlb_fill_flags |= TLB_BSWAP;
}
/* access ok? */
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 10/13] accel/tcg: Add tlb_fill_flags to CPUTLBEntryFull
2023-02-23 20:43 ` [PATCH 10/13] accel/tcg: Add tlb_fill_flags to CPUTLBEntryFull Richard Henderson
@ 2023-02-23 21:32 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-23 21:32 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm
On 23/2/23 21:43, Richard Henderson wrote:
> Allow the target to set tlb flags to apply to all of the
> comparators. Remove MemTxAttrs.byte_swap, as the bit is
> not relevant to memory transactions, only the page mapping.
> Adjust target/sparc to set TLB_BSWAP directly.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/cpu-defs.h | 3 +++
> include/exec/memattrs.h | 2 --
> accel/tcg/cputlb.c | 5 +----
> target/sparc/mmu_helper.c | 2 +-
> 4 files changed, 5 insertions(+), 7 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 11/13] accel/tcg: Add TLB_CHECK_ALIGNED
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
` (9 preceding siblings ...)
2023-02-23 20:43 ` [PATCH 10/13] accel/tcg: Add tlb_fill_flags to CPUTLBEntryFull Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-02-23 20:43 ` [PATCH 12/13] target/arm: Do memory type alignment check when translation disabled Richard Henderson
2023-02-23 20:43 ` [PATCH 13/13] target/arm: Do memory type alignment check when translation enabled Richard Henderson
12 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
This creates a per-page method for checking of alignment.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-all.h | 4 +++-
accel/tcg/cputlb.c | 25 ++++++++++++++++++++++++-
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f3b2f4229c..5bb04782ba 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -399,8 +399,10 @@ CPUArchState *cpu_copy(CPUArchState *env);
#define TLB_BSWAP (1 << 0)
/* Set if TLB entry contains a watchpoint. */
#define TLB_WATCHPOINT (1 << 1)
+/* Set if TLB entry requires aligned accesses. */
+#define TLB_CHECK_ALIGNED (1 << 2)
-#define TLB_SLOW_FLAGS_MASK (TLB_BSWAP | TLB_WATCHPOINT)
+#define TLB_SLOW_FLAGS_MASK (TLB_BSWAP | TLB_WATCHPOINT | TLB_CHECK_ALIGNED)
/* The two sets of flags must not overlap. */
QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & TLB_SLOW_FLAGS_MASK);
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index a90688ac30..c692e71766 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1546,7 +1546,7 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr,
flags |= full->slow_flags[access_type];
/* Fold all "mmio-like" bits into TLB_MMIO. This is not RAM. */
- if (unlikely(flags & ~(TLB_WATCHPOINT | TLB_NOTDIRTY))) {
+ if (flags & ~(TLB_WATCHPOINT | TLB_NOTDIRTY | TLB_CHECK_ALIGNED)) {
*phost = NULL;
return TLB_MMIO;
}
@@ -1885,6 +1885,29 @@ static bool mmu_lookup(CPUArchState *env, target_ulong addr, MemOpIdx oi,
tcg_debug_assert((flags & TLB_BSWAP) == 0);
}
+ /*
+ * This alignment check differs from the one above, in that this is
+ * based on the atomicity of the operation. The intended use case is
+ * the ARM memory type field of each PTE, where access to pages with
+ * Device memory type require alignment.
+ */
+ if (unlikely(flags & TLB_CHECK_ALIGNED)) {
+ MemOp atmax = l->memop & MO_ATMAX_MASK;
+ MemOp atom = l->memop & MO_ATOM_MASK;
+ MemOp size = l->memop & MO_SIZE;
+
+ if (size != MO_8 && atom != MO_ATOM_NONE) {
+ if (atmax == MO_ATMAX_SIZE) {
+ a_bits = size;
+ } else {
+ a_bits = atmax >> MO_ATMAX_SHIFT;
+ }
+ if (addr & ((1 << a_bits) - 1)) {
+ cpu_unaligned_access(env_cpu(env), addr, type, l->mmu_idx, ra);
+ }
+ }
+ }
+
return crosspage;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 12/13] target/arm: Do memory type alignment check when translation disabled
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
` (10 preceding siblings ...)
2023-02-23 20:43 ` [PATCH 11/13] accel/tcg: Add TLB_CHECK_ALIGNED Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
2023-02-23 21:41 ` Philippe Mathieu-Daudé
2023-02-23 20:43 ` [PATCH 13/13] target/arm: Do memory type alignment check when translation enabled Richard Henderson
12 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Idan Horowitz
If translation is disabled, the default memory type is Device,
which requires alignment checking. This is more optimially done
early via the MemOp given to the TCG memory operation.
Reported-by: Idan Horowitz <idan.horowitz@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1204
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/helper.c | 36 ++++++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 07d4100365..b1b664e0ad 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11867,6 +11867,37 @@ static inline bool fgt_svc(CPUARMState *env, int el)
FIELD_EX64(env->cp15.fgt_exec[FGTREG_HFGITR], HFGITR_EL2, SVC_EL1);
}
+/*
+ * Return true if memory alignment should be enforced.
+ */
+static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t sctlr)
+{
+#ifdef CONFIG_USER_ONLY
+ return false;
+#else
+ /* Check the alignment enable bit. */
+ if (sctlr & SCTLR_A) {
+ return true;
+ }
+
+ /*
+ * If translation is disabled, then the default memory type is
+ * Device(-nGnRnE) instead of Normal, which requires that alignment
+ * be enforced. Since this affects all ram, it is most efficient
+ * to handle this during translation.
+ */
+ if (sctlr & SCTLR_M) {
+ /* Translation enabled: memory type in PTE via MAIR_ELx. */
+ return false;
+ }
+ if (el < 2 && (arm_hcr_el2_eff(env) & (HCR_DC | HCR_VM))) {
+ /* Stage 2 translation enabled: memory type in PTE. */
+ return false;
+ }
+ return true;
+#endif
+}
+
static CPUARMTBFlags rebuild_hflags_common(CPUARMState *env, int fp_el,
ARMMMUIdx mmu_idx,
CPUARMTBFlags flags)
@@ -11936,8 +11967,9 @@ static CPUARMTBFlags rebuild_hflags_a32(CPUARMState *env, int fp_el,
{
CPUARMTBFlags flags = {};
int el = arm_current_el(env);
+ uint64_t sctlr = arm_sctlr(env, el);
- if (arm_sctlr(env, el) & SCTLR_A) {
+ if (aprofile_require_alignment(env, el, sctlr)) {
DP_TBFLAG_ANY(flags, ALIGN_MEM, 1);
}
@@ -12037,7 +12069,7 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
sctlr = regime_sctlr(env, stage1);
- if (sctlr & SCTLR_A) {
+ if (aprofile_require_alignment(env, el, sctlr)) {
DP_TBFLAG_ANY(flags, ALIGN_MEM, 1);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 12/13] target/arm: Do memory type alignment check when translation disabled
2023-02-23 20:43 ` [PATCH 12/13] target/arm: Do memory type alignment check when translation disabled Richard Henderson
@ 2023-02-23 21:41 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-23 21:41 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm, Idan Horowitz
On 23/2/23 21:43, Richard Henderson wrote:
> If translation is disabled, the default memory type is Device,
> which requires alignment checking. This is more optimially done
"optimally"?
> early via the MemOp given to the TCG memory operation.
>
> Reported-by: Idan Horowitz <idan.horowitz@gmail.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1204
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/helper.c | 36 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 07d4100365..b1b664e0ad 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -11867,6 +11867,37 @@ static inline bool fgt_svc(CPUARMState *env, int el)
> FIELD_EX64(env->cp15.fgt_exec[FGTREG_HFGITR], HFGITR_EL2, SVC_EL1);
> }
>
> +/*
> + * Return true if memory alignment should be enforced.
> + */
> +static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t sctlr)
> +{
> +#ifdef CONFIG_USER_ONLY
> + return false;
> +#else
> + /* Check the alignment enable bit. */
> + if (sctlr & SCTLR_A) {
> + return true;
> + }
> +
> + /*
> + * If translation is disabled, then the default memory type is
> + * Device(-nGnRnE) instead of Normal, which requires that alignment
> + * be enforced. Since this affects all ram, it is most efficient
> + * to handle this during translation.
> + */
> + if (sctlr & SCTLR_M) {
> + /* Translation enabled: memory type in PTE via MAIR_ELx. */
> + return false;
> + }
I haven't checked <...
> + if (el < 2 && (arm_hcr_el2_eff(env) & (HCR_DC | HCR_VM))) {
> + /* Stage 2 translation enabled: memory type in PTE. */
> + return false;
> + }
...> this part, but for the rest:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> + return true;
> +#endif
> +}
> +
> static CPUARMTBFlags rebuild_hflags_common(CPUARMState *env, int fp_el,
> ARMMMUIdx mmu_idx,
> CPUARMTBFlags flags)
> @@ -11936,8 +11967,9 @@ static CPUARMTBFlags rebuild_hflags_a32(CPUARMState *env, int fp_el,
> {
> CPUARMTBFlags flags = {};
> int el = arm_current_el(env);
> + uint64_t sctlr = arm_sctlr(env, el);
>
> - if (arm_sctlr(env, el) & SCTLR_A) {
> + if (aprofile_require_alignment(env, el, sctlr)) {
> DP_TBFLAG_ANY(flags, ALIGN_MEM, 1);
> }
>
> @@ -12037,7 +12069,7 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
>
> sctlr = regime_sctlr(env, stage1);
>
> - if (sctlr & SCTLR_A) {
> + if (aprofile_require_alignment(env, el, sctlr)) {
> DP_TBFLAG_ANY(flags, ALIGN_MEM, 1);
> }
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 13/13] target/arm: Do memory type alignment check when translation enabled
2023-02-23 20:43 [PATCH 00/13] {tcg,aarch64}: Add TLB_CHECK_ALIGNED Richard Henderson
` (11 preceding siblings ...)
2023-02-23 20:43 ` [PATCH 12/13] target/arm: Do memory type alignment check when translation disabled Richard Henderson
@ 2023-02-23 20:43 ` Richard Henderson
12 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2023-02-23 20:43 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
If translation is enabled, and the PTE memory type is Device,
enable checking alignment via TLB_CHECK_ALIGNMENT. While the
check is done later than it should be per the ARM, it's better
than not performing the check at all.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 2b125fff44..19afeb9135 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -194,6 +194,16 @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx,
return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0;
}
+static bool S1_attrs_are_device(uint8_t attrs)
+{
+ /*
+ * This slightly under-decodes the MAIR_ELx field:
+ * 0b0000dd01 is Device with FEAT_XS, otherwise UNPREDICTABLE;
+ * 0b0000dd1x is UNPREDICTABLE.
+ */
+ return (attrs & 0xf0) == 0;
+}
+
static bool S2_attrs_are_device(uint64_t hcr, uint8_t attrs)
{
/*
@@ -1188,6 +1198,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
bool aarch64 = arm_el_is_aa64(env, el);
uint64_t descriptor, new_descriptor;
bool nstable;
+ bool device;
/* TODO: This code does not support shareability levels. */
if (aarch64) {
@@ -1568,6 +1579,8 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
if (regime_is_stage2(mmu_idx)) {
result->cacheattrs.is_s2_format = true;
result->cacheattrs.attrs = extract32(attrs, 2, 4);
+ device = S2_attrs_are_device(arm_hcr_el2_eff_secstate(env, is_secure),
+ result->cacheattrs.attrs);
} else {
/* Index into MAIR registers for cache attributes */
uint8_t attrindx = extract32(attrs, 2, 3);
@@ -1575,6 +1588,21 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
assert(attrindx <= 7);
result->cacheattrs.is_s2_format = false;
result->cacheattrs.attrs = extract64(mair, attrindx * 8, 8);
+ device = S1_attrs_are_device(result->cacheattrs.attrs);
+ }
+
+ /*
+ * Enable alignment checks on Device memory.
+ *
+ * Per R_XCHFJ, this check is mis-ordered, in that this alignment check
+ * should have priority 30, while the permission check should be next at
+ * priority 31 and stage2 translation faults come after that.
+ * Due to the way the TCG softmmu TLB operates, we will have implicitly
+ * done the permission check and the stage2 lookup in finding the TLB
+ * entry, so the alignment check cannot be done sooner.
+ */
+ if (device) {
+ result->f.tlb_fill_flags |= TLB_CHECK_ALIGNED;
}
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread