| Message ID | 20220926133904.3297263-3-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gdbstub/next (MemTxAttrs and re-factoring) | expand |
On Mon, 26 Sept 2022 at 14:39, Alex Bennée <alex.bennee@linaro.org> wrote: > > Both arm_cpu_tlb_fill (for normal IO) and > arm_cpu_get_phys_page_attrs_debug (for debug access) come through > get_phys_addr which is setting the other memory attributes for the > transaction. As these are all by definition CPU accesses we can also > set the requested_type/index as appropriate. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > target/arm/ptw.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index 3261039d93..644d450662 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -2315,6 +2315,9 @@ bool get_phys_addr(CPUARMState *env, target_ulong address, > { > ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx); > > + attrs->requester_type = MEMTXATTRS_CPU; > + attrs->requester_id = env_cpu(env)->cpu_index; > + This only catches the case where the memory access is done via something that does a virtual-to-physical translation. It misses memory accesses done directly on physical addresses, such as those in arm_ldl_ptw() and arm_ldq_ptw(), plus various M-profile specific ones. thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Mon, 26 Sept 2022 at 14:39, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> Both arm_cpu_tlb_fill (for normal IO) and >> arm_cpu_get_phys_page_attrs_debug (for debug access) come through >> get_phys_addr which is setting the other memory attributes for the >> transaction. As these are all by definition CPU accesses we can also >> set the requested_type/index as appropriate. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> target/arm/ptw.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/target/arm/ptw.c b/target/arm/ptw.c >> index 3261039d93..644d450662 100644 >> --- a/target/arm/ptw.c >> +++ b/target/arm/ptw.c >> @@ -2315,6 +2315,9 @@ bool get_phys_addr(CPUARMState *env, target_ulong address, >> { >> ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx); >> >> + attrs->requester_type = MEMTXATTRS_CPU; >> + attrs->requester_id = env_cpu(env)->cpu_index; >> + > > This only catches the case where the memory access is > done via something that does a virtual-to-physical translation. > It misses memory accesses done directly on physical addresses, > such as those in arm_ldl_ptw() and arm_ldq_ptw(), plus various > M-profile specific ones. I thought they were just used for the page table walk. Can you place your page tables aliases with a piece of HW? > > thanks > -- PMM
On Mon, 26 Sept 2022 at 16:06, Alex Bennée <alex.bennee@linaro.org> wrote: > > > Peter Maydell <peter.maydell@linaro.org> writes: > > > On Mon, 26 Sept 2022 at 14:39, Alex Bennée <alex.bennee@linaro.org> wrote: > > This only catches the case where the memory access is > > done via something that does a virtual-to-physical translation. > > It misses memory accesses done directly on physical addresses, > > such as those in arm_ldl_ptw() and arm_ldq_ptw(), plus various > > M-profile specific ones. > > I thought they were just used for the page table walk. Can you place > your page tables aliases with a piece of HW? They are just used for the page table walk, but they are nonetheless still memory transactions initiated by the CPU, so if we're saying those should be marked up in the transaction-attributes struct then they should count. thanks -- PMM
diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 3261039d93..644d450662 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -2315,6 +2315,9 @@ bool get_phys_addr(CPUARMState *env, target_ulong address, { ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx); + attrs->requester_type = MEMTXATTRS_CPU; + attrs->requester_id = env_cpu(env)->cpu_index; + if (mmu_idx != s1_mmu_idx) { /* * Call ourselves recursively to do the stage 1 and then stage 2
Both arm_cpu_tlb_fill (for normal IO) and arm_cpu_get_phys_page_attrs_debug (for debug access) come through get_phys_addr which is setting the other memory attributes for the transaction. As these are all by definition CPU accesses we can also set the requested_type/index as appropriate. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- target/arm/ptw.c | 3 +++ 1 file changed, 3 insertions(+)