[PATCH v3 09/20] target/hppa: Fix priority of T, D, and B page faults

Richard Henderson posted 20 patches 1 month, 2 weeks ago
[PATCH v3 09/20] target/hppa: Fix priority of T, D, and B page faults
Posted by Richard Henderson 1 month, 2 weeks ago
Drop the 'else' so that ret is overridden with the
highest priority fault.

Fixes: d8bc1381250 ("target/hppa: Implement PSW_X")
Reviewed-by: Helge Deller <deller@gmx.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/mem_helper.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index f027c494e2..f71cedd7a9 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -288,7 +288,7 @@ int hppa_get_physical_address(CPUHPPAState *env, vaddr addr, int mmu_idx,
     }
 
     /*
-     * In priority order, check for conditions which raise faults.
+     * In reverse priority order, check for conditions which raise faults.
      * Remove PROT bits that cover the condition we want to check,
      * so that the resulting PROT will force a re-check of the
      * architectural TLB entry for the next access.
@@ -299,13 +299,15 @@ int hppa_get_physical_address(CPUHPPAState *env, vaddr addr, int mmu_idx,
             /* The T bit is set -- Page Reference Fault.  */
             ret = EXCP_PAGE_REF;
         }
-    } else if (!ent->d) {
+    }
+    if (unlikely(!ent->d)) {
         prot &= PAGE_READ | PAGE_EXEC;
         if (type & PAGE_WRITE) {
             /* The D bit is not set -- TLB Dirty Bit Fault.  */
             ret = EXCP_TLB_DIRTY;
         }
-    } else if (unlikely(ent->b)) {
+    }
+    if (unlikely(ent->b)) {
         prot &= PAGE_READ | PAGE_EXEC;
         if (type & PAGE_WRITE) {
             /*
-- 
2.43.0


Re: [PATCH v3 09/20] target/hppa: Fix priority of T, D, and B page faults
Posted by Michael Tokarev 1 month, 1 week ago
On 09.10.2024 03:04, Richard Henderson wrote:
> Drop the 'else' so that ret is overridden with the
> highest priority fault.
> 
> Fixes: d8bc1381250 ("target/hppa: Implement PSW_X")
> Reviewed-by: Helge Deller <deller@gmx.de>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Is this a qemu-stable material?  For now I assume yes, please
let me know if it is not.

Thanks,

/mjt

Re: [PATCH v3 09/20] target/hppa: Fix priority of T, D, and B page faults
Posted by Richard Henderson 1 month, 1 week ago
On 10/14/24 11:02, Michael Tokarev wrote:
> On 09.10.2024 03:04, Richard Henderson wrote:
>> Drop the 'else' so that ret is overridden with the
>> highest priority fault.
>>
>> Fixes: d8bc1381250 ("target/hppa: Implement PSW_X")
>> Reviewed-by: Helge Deller <deller@gmx.de>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
> Is this a qemu-stable material?  For now I assume yes, please
> let me know if it is not.

The kernel fault thing is pretty nasty.
Fixing that probably requires all of patches 2-11.

I don't think the arm fix is serious enough to backport.


r~

Re: [PATCH v3 09/20] target/hppa: Fix priority of T, D, and B page faults
Posted by Michael Tokarev 1 month, 1 week ago
On 14.10.2024 22:31, Richard Henderson wrote:
> On 10/14/24 11:02, Michael Tokarev wrote:
>> On 09.10.2024 03:04, Richard Henderson wrote:
>>> Drop the 'else' so that ret is overridden with the
>>> highest priority fault.
>>>
>>> Fixes: d8bc1381250 ("target/hppa: Implement PSW_X")
>>> Reviewed-by: Helge Deller <deller@gmx.de>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>
>> Is this a qemu-stable material?  For now I assume yes, please
>> let me know if it is not.
> 
> The kernel fault thing is pretty nasty.
> Fixing that probably requires all of patches 2-11.
> 
> I don't think the arm fix is serious enough to backport.

Count me confused.

So, am I right you suggest picking up changes 2-11 for 9.1?
It looks maybe a bit too much, no?

Thanks,

/mjt