Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/tcg/tlb_helper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/arm/tcg/tlb_helper.c b/target/arm/tcg/tlb_helper.c
index 23c72a99f5c..df04ef351d1 100644
--- a/target/arm/tcg/tlb_helper.c
+++ b/target/arm/tcg/tlb_helper.c
@@ -349,7 +349,9 @@ bool arm_cpu_tlb_fill_align(CPUState *cs, CPUTLBEntryFull *out, vaddr address,
                               &res, fi)) {
         res.f.extra.arm.pte_attrs = res.cacheattrs.attrs;
         res.f.extra.arm.shareability = res.cacheattrs.shareability;
-        *out = res.f;
+        if (out) {
+            *out = res.f;
+        }
         return true;
     }
     if (probe) {
-- 
2.49.0

On 6/30/25 6:09 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/tcg/tlb_helper.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/tcg/tlb_helper.c b/target/arm/tcg/tlb_helper.c
> index 23c72a99f5c..df04ef351d1 100644
> --- a/target/arm/tcg/tlb_helper.c
> +++ b/target/arm/tcg/tlb_helper.c
> @@ -349,7 +349,9 @@ bool arm_cpu_tlb_fill_align(CPUState *cs, CPUTLBEntryFull *out, vaddr address,
>                                 &res, fi)) {
>           res.f.extra.arm.pte_attrs = res.cacheattrs.attrs;
>           res.f.extra.arm.shareability = res.cacheattrs.shareability;
> -        *out = res.f;
> +        if (out) {
> +            *out = res.f;
> +        }
>           return true;
>       }
>       if (probe) {

Would that be possible to provide more context about why it's needed?

The goal of tlb_fill_align is precisely to return a new CPUTLBEntryFull, 
while checking all the protection to access this page and generating a 
page fault if needed.

In case you just want to check if an address is valid, 
arm_cpu_get_phys_page_attrs_debug might be a better choice (checking for 
return value -1 in case of error).

Thanks,
Pierrick

On 6/30/25 07:09, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/tcg/tlb_helper.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/tcg/tlb_helper.c b/target/arm/tcg/tlb_helper.c
> index 23c72a99f5c..df04ef351d1 100644
> --- a/target/arm/tcg/tlb_helper.c
> +++ b/target/arm/tcg/tlb_helper.c
> @@ -349,7 +349,9 @@ bool arm_cpu_tlb_fill_align(CPUState *cs, CPUTLBEntryFull *out, vaddr address,
>                                 &res, fi)) {
>           res.f.extra.arm.pte_attrs = res.cacheattrs.attrs;
>           res.f.extra.arm.shareability = res.cacheattrs.shareability;
> -        *out = res.f;
> +        if (out) {
> +            *out = res.f;
> +        }
>           return true;
>       }
>       if (probe) {

Why?  There's no other way to get the phys addr result.
Are you only calling this for the raise-exception side effect?


r~

On 30/6/25 15:56, Richard Henderson wrote:
> On 6/30/25 07:09, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/arm/tcg/tlb_helper.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/arm/tcg/tlb_helper.c b/target/arm/tcg/tlb_helper.c
>> index 23c72a99f5c..df04ef351d1 100644
>> --- a/target/arm/tcg/tlb_helper.c
>> +++ b/target/arm/tcg/tlb_helper.c
>> @@ -349,7 +349,9 @@ bool arm_cpu_tlb_fill_align(CPUState *cs, 
>> CPUTLBEntryFull *out, vaddr address,
>>                                 &res, fi)) {
>>           res.f.extra.arm.pte_attrs = res.cacheattrs.attrs;
>>           res.f.extra.arm.shareability = res.cacheattrs.shareability;
>> -        *out = res.f;
>> +        if (out) {
>> +            *out = res.f;
>> +        }
>>           return true;
>>       }
>>       if (probe) {
> 
> Why?  There's no other way to get the phys addr result.
> Are you only calling this for the raise-exception side effect?

Yes:

-- >8 --
@@ -2121,6 +2121,25 @@ int hvf_vcpu_exec(CPUState *cpu)
              hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized(), 1);
          }
          break;
+    case EC_INSNABORT: {
+        uint32_t set = (syndrome >> 12) & 3;
+        bool fnv = (syndrome >> 10) & 1;
+        bool ea = (syndrome >> 9) & 1;
+        bool s1ptw = (syndrome >> 7) & 1;
+        uint32_t ifsc = (syndrome >> 0) & 0x3f;
+
+        trace_hvf_insn_abort(env->pc, set, fnv, ea, s1ptw, ifsc);
+
+        cpu_synchronize_state(cpu);
+        if (tcg_enabled()) {
+            ret = EXCP_EMULATE;
+            arm_cpu_tlb_fill_align(cpu, NULL, env->pc, MMU_INST_FETCH,
+                                   arm_env_mmu_index(env), MO_32, 4, 
false, -1);
+        }
+        break;
+    }
      default:
          cpu_synchronize_state(cpu);
          trace_hvf_exit(syndrome, ec, env->pc);
---

I see probe_access_full_mmu() uses discard_tlb, I can use a similar
stack variable if you rather.