Richard Henderson <[email protected]> writes:
> On 10/3/22 05:47, Alex Bennée wrote: >> Richard Henderson <[email protected]> writes: >> >>> Let tb->page_addr[0] contain the offset within the page of the >>> start of the translation block. We need to recover this value >>> anyway at various points, and it is easier to discard the page >>> offset when it's not needed, which happens naturally via the >>> existing find_page shift. >>> >>> Signed-off-by: Richard Henderson <[email protected]> >>> --- >>> accel/tcg/cpu-exec.c | 16 ++++++++-------- >>> accel/tcg/cputlb.c | 3 ++- >>> accel/tcg/translate-all.c | 9 +++++---- >>> 3 files changed, 15 insertions(+), 13 deletions(-) >>> >>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c >>> index 5f43b9769a..dd58a144a8 100644 >>> --- a/accel/tcg/cpu-exec.c >>> +++ b/accel/tcg/cpu-exec.c >>> @@ -174,7 +174,7 @@ struct tb_desc { >>> target_ulong pc; >>> target_ulong cs_base; >>> CPUArchState *env; >>> - tb_page_addr_t phys_page1; >>> + tb_page_addr_t page_addr0; >> We don't actually document that this is an offset here (or indeed in >> TranslationBlock) and the definition of tb_page_addr_t: >> /* Page tracking code uses ram addresses in system mode, and >> virtual >> addresses in userspace mode. Define tb_page_addr_t to be an >> appropriate >> type. */ >> #if defined(CONFIG_USER_ONLY) >> typedef abi_ulong tb_page_addr_t; >> #define TB_PAGE_ADDR_FMT TARGET_ABI_FMT_lx >> #else >> typedef ram_addr_t tb_page_addr_t; >> #define TB_PAGE_ADDR_FMT RAM_ADDR_FMT >> #endif >> implies these are full size pointers into the guests address space. > > And that's what I've got. What we we were storing in phys_page1 > before was a full size pointer that was page aligned. I'm now > dropping the page alignment and having a full size pointer to the > exact first byte of the translated code. OK then I'm confused by the commit message which says: Let tb->page_addr[0] contain the offset within the page of the start of the translation block > Is that clearer? How would you improve the wording? > > > r~ > >> Either we need a new type (tb_page_offset_t) or to properly comment the >> structures with what they mean. >> Otherwise: >> Reviewed-by: Alex Bennée <[email protected]> >> -- Alex Bennée
