| Message ID | 20220703082419.770989-3-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Implement FEAT_HAFDBS | expand |
On Sun, 3 Jul 2022 at 09:25, Richard Henderson <richard.henderson@linaro.org> wrote: > > Copy attrs, sharability, and the NS bit into the TLB. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/cpu-param.h | 8 ++++++++ > target/arm/internals.h | 5 +++++ > target/arm/tlb_helper.c | 14 ++++++++++++-- > 3 files changed, 25 insertions(+), 2 deletions(-) > > diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h > index 68ffb12427..a14f167d11 100644 > --- a/target/arm/cpu-param.h > +++ b/target/arm/cpu-param.h > @@ -30,6 +30,14 @@ > */ > # define TARGET_PAGE_BITS_VARY > # define TARGET_PAGE_BITS_MIN 10 > +/* > + * Extra information stored in softmmu page tables. > + */ > +# define TARGET_PAGE_ENTRY_EXTRA > +struct PageEntryExtra { > + /* See PAGEENTRYEXTRA fields in cpu.h */ > + uint64_t x; > +}; > #endif > > #define NB_MMU_MODES 15 > diff --git a/target/arm/internals.h b/target/arm/internals.h > index c66f74a0db..2b38a83574 100644 > --- a/target/arm/internals.h > +++ b/target/arm/internals.h > @@ -74,6 +74,11 @@ FIELD(V7M_EXCRET, DCRS, 5, 1) > FIELD(V7M_EXCRET, S, 6, 1) > FIELD(V7M_EXCRET, RES1, 7, 25) /* including the must-be-1 prefix */ > > +/* Bit definitions for PageEntryExtra */ > +FIELD(PAGEENTRYEXTRA, ATTRS, 0, 8) > +FIELD(PAGEENTRYEXTRA, SHAREABILITY, 8, 2) > +FIELD(PAGEENTRYEXTRA, PA, 12, 52) So why do we want these things in particular? It would be helpful to describe the intended uses in the commit message to save the reader having to read the next 60 patches to find out :-) Is wanting to cache the physaddr an Arm-specific thing, or is it something we should consider having in the core softmmu code? > + > /* Minimum value which is a magic number for exception return */ > #define EXC_RETURN_MIN_MAGIC 0xff000000 > /* Minimum number which is a magic number for function or exception return > diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c > index 7d8a86b3c4..9de3099153 100644 > --- a/target/arm/tlb_helper.c > +++ b/target/arm/tlb_helper.c > @@ -226,21 +226,31 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > &phys_addr, &attrs, &prot, &page_size, > &fi, &cacheattrs); > if (likely(!ret)) { > + PageEntryExtra extra = {}; > + > /* > * Map a single [sub]page. Regions smaller than our declared > * target page size are handled specially, so for those we > - * pass in the exact addresses. > + * pass in the exact addresses. This only happens for M-profile, > + * which does not use or require PageEntryExtra. > */ Do we have to exclude M-profile here because the PageEntryExtra data is strictly-per-page, or because the way we've formatted our extra uint64_t requires the physaddr to be page-aligned, or both? > if (page_size >= TARGET_PAGE_SIZE) { > phys_addr &= TARGET_PAGE_MASK; > address &= TARGET_PAGE_MASK; > + > + /* Record some particulars for later lookup. */ > + extra.x = phys_addr; > + extra.x = FIELD_DP64(extra.x, PAGEENTRYEXTRA, ATTRS, > + cacheattrs.attrs); > + extra.x = FIELD_DP64(extra.x, PAGEENTRYEXTRA, SHAREABILITY, > + cacheattrs.shareability); > } > /* Notice and record tagged memory. */ > if (cpu_isar_feature(aa64_mte, cpu) && cacheattrs.attrs == 0xf0) { > arm_tlb_mte_tagged(&attrs) = true; > } > > - tlb_set_page_with_attrs(cs, address, phys_addr, attrs, > + tlb_set_page_with_extra(cs, address, phys_addr, attrs, extra, > prot, mmu_idx, page_size); > return true; > } else if (probe) { > -- > 2.34.1 thanks -- PMM
On 7/4/22 20:52, Peter Maydell wrote: > On Sun, 3 Jul 2022 at 09:25, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> Copy attrs, sharability, and the NS bit into the TLB. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> target/arm/cpu-param.h | 8 ++++++++ >> target/arm/internals.h | 5 +++++ >> target/arm/tlb_helper.c | 14 ++++++++++++-- >> 3 files changed, 25 insertions(+), 2 deletions(-) >> >> diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h >> index 68ffb12427..a14f167d11 100644 >> --- a/target/arm/cpu-param.h >> +++ b/target/arm/cpu-param.h >> @@ -30,6 +30,14 @@ >> */ >> # define TARGET_PAGE_BITS_VARY >> # define TARGET_PAGE_BITS_MIN 10 >> +/* >> + * Extra information stored in softmmu page tables. >> + */ >> +# define TARGET_PAGE_ENTRY_EXTRA >> +struct PageEntryExtra { >> + /* See PAGEENTRYEXTRA fields in cpu.h */ >> + uint64_t x; >> +}; >> #endif >> >> #define NB_MMU_MODES 15 >> diff --git a/target/arm/internals.h b/target/arm/internals.h >> index c66f74a0db..2b38a83574 100644 >> --- a/target/arm/internals.h >> +++ b/target/arm/internals.h >> @@ -74,6 +74,11 @@ FIELD(V7M_EXCRET, DCRS, 5, 1) >> FIELD(V7M_EXCRET, S, 6, 1) >> FIELD(V7M_EXCRET, RES1, 7, 25) /* including the must-be-1 prefix */ >> >> +/* Bit definitions for PageEntryExtra */ >> +FIELD(PAGEENTRYEXTRA, ATTRS, 0, 8) >> +FIELD(PAGEENTRYEXTRA, SHAREABILITY, 8, 2) >> +FIELD(PAGEENTRYEXTRA, PA, 12, 52) > > So why do we want these things in particular? It would be > helpful to describe the intended uses in the commit message > to save the reader having to read the next 60 patches to > find out :-) Heh, yes. Basically, it's what S1_ptw_translate requires (pa, attrs), so that we can report a stage1 ptw failure, and what do_ats_write requires (pa, sh, attrs) for filling in PAR_EL1. Although within these 62 patches I didn't came back to finish converting do_ats_write to use probe_access_flags_extra instead of using get_phys_addr directly, it was a goal. > Is wanting to cache the physaddr an Arm-specific thing, or is it > something we should consider having in the core softmmu code? I'm not sure what other targets require for their 2-stage page table walks. I guess I should have a look (riscv, i386, ?). It *is* possible to recover the phys addr from the iommutlb, because I was doing that in mte_helper.c (see code removed in patch 5), but it's certainly not simple. >> if (likely(!ret)) { >> + PageEntryExtra extra = {}; >> + >> /* >> * Map a single [sub]page. Regions smaller than our declared >> * target page size are handled specially, so for those we >> - * pass in the exact addresses. >> + * pass in the exact addresses. This only happens for M-profile, >> + * which does not use or require PageEntryExtra. >> */ > > Do we have to exclude M-profile here because the PageEntryExtra > data is strictly-per-page, or because the way we've formatted > our extra uint64_t requires the physaddr to be page-aligned, or both? Because our extra uint64_t requires page alignment, and reuses those bits. r~
diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h index 68ffb12427..a14f167d11 100644 --- a/target/arm/cpu-param.h +++ b/target/arm/cpu-param.h @@ -30,6 +30,14 @@ */ # define TARGET_PAGE_BITS_VARY # define TARGET_PAGE_BITS_MIN 10 +/* + * Extra information stored in softmmu page tables. + */ +# define TARGET_PAGE_ENTRY_EXTRA +struct PageEntryExtra { + /* See PAGEENTRYEXTRA fields in cpu.h */ + uint64_t x; +}; #endif #define NB_MMU_MODES 15 diff --git a/target/arm/internals.h b/target/arm/internals.h index c66f74a0db..2b38a83574 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -74,6 +74,11 @@ FIELD(V7M_EXCRET, DCRS, 5, 1) FIELD(V7M_EXCRET, S, 6, 1) FIELD(V7M_EXCRET, RES1, 7, 25) /* including the must-be-1 prefix */ +/* Bit definitions for PageEntryExtra */ +FIELD(PAGEENTRYEXTRA, ATTRS, 0, 8) +FIELD(PAGEENTRYEXTRA, SHAREABILITY, 8, 2) +FIELD(PAGEENTRYEXTRA, PA, 12, 52) + /* Minimum value which is a magic number for exception return */ #define EXC_RETURN_MIN_MAGIC 0xff000000 /* Minimum number which is a magic number for function or exception return diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c index 7d8a86b3c4..9de3099153 100644 --- a/target/arm/tlb_helper.c +++ b/target/arm/tlb_helper.c @@ -226,21 +226,31 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size, &phys_addr, &attrs, &prot, &page_size, &fi, &cacheattrs); if (likely(!ret)) { + PageEntryExtra extra = {}; + /* * Map a single [sub]page. Regions smaller than our declared * target page size are handled specially, so for those we - * pass in the exact addresses. + * pass in the exact addresses. This only happens for M-profile, + * which does not use or require PageEntryExtra. */ if (page_size >= TARGET_PAGE_SIZE) { phys_addr &= TARGET_PAGE_MASK; address &= TARGET_PAGE_MASK; + + /* Record some particulars for later lookup. */ + extra.x = phys_addr; + extra.x = FIELD_DP64(extra.x, PAGEENTRYEXTRA, ATTRS, + cacheattrs.attrs); + extra.x = FIELD_DP64(extra.x, PAGEENTRYEXTRA, SHAREABILITY, + cacheattrs.shareability); } /* Notice and record tagged memory. */ if (cpu_isar_feature(aa64_mte, cpu) && cacheattrs.attrs == 0xf0) { arm_tlb_mte_tagged(&attrs) = true; } - tlb_set_page_with_attrs(cs, address, phys_addr, attrs, + tlb_set_page_with_extra(cs, address, phys_addr, attrs, extra, prot, mmu_idx, page_size); return true; } else if (probe) {
Copy attrs, sharability, and the NS bit into the TLB. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/cpu-param.h | 8 ++++++++ target/arm/internals.h | 5 +++++ target/arm/tlb_helper.c | 14 ++++++++++++-- 3 files changed, 25 insertions(+), 2 deletions(-)