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;
+
     if (mmu_idx != s1_mmu_idx) {
         /*
          * Call ourselves recursively to do the stage 1 and then stage 2
-- 
2.34.1

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


-- 
Alex Bennée

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