On Mon, 5 Sept 2022 at 21:29, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> Now that we have collected all of the page data into
> CPUTLBEntryFull, provide an interface to record that
> all in one go, instead of using 4 arguments. This interface
> allows CPUTLBEntryFull to be extended without having to
> change the number of arguments.
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> include/exec/cpu-defs.h | 14 ++++++++++
> include/exec/exec-all.h | 22 +++++++++++++++
> accel/tcg/cputlb.c | 62 ++++++++++++++++++++++++++++-------------
> 3 files changed, 78 insertions(+), 20 deletions(-)
>
> diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
> index f70f54d850..5e12cc1854 100644
> --- a/include/exec/cpu-defs.h
> +++ b/include/exec/cpu-defs.h
> @@ -148,7 +148,21 @@ typedef struct CPUTLBEntryFull {
> * + the offset within the target MemoryRegion (otherwise)
> */
> hwaddr xlat_section;
> +
> + /*
> + * @phys_addr contains the physical address in the address space
> + * given by cpu_asidx_from_attrs(cpu, @attrs).
> + */
> + hwaddr phys_addr;
> +
> + /* @attrs contains the memory transaction attributes for the page. */
> MemTxAttrs attrs;
> +
> + /* @prot contains the complete protections for the page. */
> + uint8_t prot;
> +
> + /* @lg_page_size contains the log2 of the page size. */
> + uint8_t lg_page_size;
> } CPUTLBEntryFull;
>
> /*
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 758cf6bcc7..1a30c857f4 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -257,6 +257,28 @@ void tlb_flush_range_by_mmuidx_all_cpus_synced(CPUState
> *cpu,
> uint16_t idxmap,
> unsigned bits);
>
> +/**
> + * tlb_set_page_full:
> + * @cpu: CPU context
> + * @mmu_idx: mmu index of the tlb to modify
> + * @vaddr: virtual address of the entry to add
> + * @full: the details of the tlb entry
> + *
> + * Add an entry to @cpu tlb index @mmu_idx. All of the fields of
> + * @full must be filled, except for xlat_section, and constitute
> + * the complete description of the translated page.
> + *
> + * This is generally called by the target tlb_fill function after
> + * having performed a successful page table walk to find the physical
> + * address and attributes for the translation.
> + *
> + * At most one entry for a given virtual address is permitted. Only a
> + * single TARGET_PAGE_SIZE region is mapped; @full->ld_page_size is only
typo: lg_page_size
> + * used by tlb_flush_page.
> + */
> +void tlb_set_page_full(CPUState *cpu, int mmu_idx, target_ulong vaddr,
> + CPUTLBEntryFull *full);
> +
> /**
> * tlb_set_page_with_attrs:
> * @cpu: CPU to add this TLB entry for
> @@ -1272,15 +1275,34 @@ void tlb_set_page_with_attrs(CPUState *cpu,
> target_ulong vaddr,
> qemu_spin_unlock(&tlb->c.lock);
> }
>
> -/* Add a new TLB entry, but without specifying the memory
> - * transaction attributes to be used.
> - */
> +void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
> + hwaddr paddr, MemTxAttrs attrs, int prot,
> + int mmu_idx, target_ulong size)
> +{
> + CPUTLBEntryFull full = {
> + .phys_addr = paddr,
> + .attrs = attrs,
> + .prot = prot,
> + .lg_page_size = ctz64(size)
> + };
> +
> + assert(is_power_of_2(size));
> + tlb_set_page_full(cpu, mmu_idx, vaddr, &full);
> +}
> +
> void tlb_set_page(CPUState *cpu, target_ulong vaddr,
> hwaddr paddr, int prot,
> int mmu_idx, target_ulong size)
> {
> - tlb_set_page_with_attrs(cpu, vaddr, paddr, MEMTXATTRS_UNSPECIFIED,
> - prot, mmu_idx, size);
> + CPUTLBEntryFull full = {
> + .phys_addr = paddr,
> + .attrs = MEMTXATTRS_UNSPECIFIED,
> + .prot = prot,
> + .lg_page_size = ctz64(size)
> + };
> +
> + assert(is_power_of_2(size));
> + tlb_set_page_full(cpu, mmu_idx, vaddr, &full);
> }
Why not just leave tlb_set_page() the way it was? Writing
it out results in this code duplication...
Otherwise
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
