[PATCH] xen/arm: Rename psr_mode_is_32bit to regs_mode_is_32bit

[PATCH] xen/arm: Rename psr_mode_is_32bit to regs_mode_is_32bit

[Michal Orzel]

Following a discussion [1] it seems like that renaming work has
been forgotten. Perform renaming of psr_mode_is_32bit to
regs_mode_is_32bit as the function no longer takes psr parameter.

[1] https://marc.info/?l=xen-devel&m=156457538423787&w=2

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
 xen/arch/arm/include/asm/regs.h |  2 +-
 xen/arch/arm/traps.c            | 30 +++++++++++++++---------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/include/asm/regs.h b/xen/arch/arm/include/asm/regs.h
index ec091a28a2..04e821138a 100644
--- a/xen/arch/arm/include/asm/regs.h
+++ b/xen/arch/arm/include/asm/regs.h
@@ -13,7 +13,7 @@
 
 #define psr_mode(psr,m) (((psr) & PSR_MODE_MASK) == m)
 
-static inline bool psr_mode_is_32bit(const struct cpu_user_regs *regs)
+static inline bool regs_mode_is_32bit(const struct cpu_user_regs *regs)
 {
 #ifdef CONFIG_ARM_32
     return true;
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 9339d12f58..0db8e42d65 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -896,7 +896,7 @@ static void _show_registers(const struct cpu_user_regs *regs,
 
     if ( guest_mode )
     {
-        if ( psr_mode_is_32bit(regs) )
+        if ( regs_mode_is_32bit(regs) )
             show_registers_32(regs, ctxt, guest_mode, v);
 #ifdef CONFIG_ARM_64
         else
@@ -1631,7 +1631,7 @@ int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr)
     {
         unsigned long it;
 
-        BUG_ON( !psr_mode_is_32bit(regs) || !(cpsr & PSR_THUMB) );
+        BUG_ON( !regs_mode_is_32bit(regs) || !(cpsr & PSR_THUMB) );
 
         it = ( (cpsr >> (10-2)) & 0xfc) | ((cpsr >> 25) & 0x3 );
 
@@ -1656,7 +1656,7 @@ int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr)
 void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
 {
     register_t itbits, cond, cpsr = regs->cpsr;
-    bool is_thumb = psr_mode_is_32bit(regs) && (cpsr & PSR_THUMB);
+    bool is_thumb = regs_mode_is_32bit(regs) && (cpsr & PSR_THUMB);
 
     if ( is_thumb && (cpsr & PSR_IT_MASK) )
     {
@@ -2098,37 +2098,37 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
         advance_pc(regs, hsr);
         break;
     case HSR_EC_CP15_32:
-        GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(!regs_mode_is_32bit(regs));
         perfc_incr(trap_cp15_32);
         do_cp15_32(regs, hsr);
         break;
     case HSR_EC_CP15_64:
-        GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(!regs_mode_is_32bit(regs));
         perfc_incr(trap_cp15_64);
         do_cp15_64(regs, hsr);
         break;
     case HSR_EC_CP14_32:
-        GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(!regs_mode_is_32bit(regs));
         perfc_incr(trap_cp14_32);
         do_cp14_32(regs, hsr);
         break;
     case HSR_EC_CP14_64:
-        GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(!regs_mode_is_32bit(regs));
         perfc_incr(trap_cp14_64);
         do_cp14_64(regs, hsr);
         break;
     case HSR_EC_CP14_DBG:
-        GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(!regs_mode_is_32bit(regs));
         perfc_incr(trap_cp14_dbg);
         do_cp14_dbg(regs, hsr);
         break;
     case HSR_EC_CP10:
-        GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(!regs_mode_is_32bit(regs));
         perfc_incr(trap_cp10);
         do_cp10(regs, hsr);
         break;
     case HSR_EC_CP:
-        GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(!regs_mode_is_32bit(regs));
         perfc_incr(trap_cp);
         do_cp(regs, hsr);
         break;
@@ -2139,7 +2139,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
          * ARMv7 (DDI 0406C.b): B1.14.8
          * ARMv8 (DDI 0487A.d): D1-1501 Table D1-44
          */
-        GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(!regs_mode_is_32bit(regs));
         perfc_incr(trap_smc32);
         do_trap_smc(regs, hsr);
         break;
@@ -2147,7 +2147,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
     {
         register_t nr;
 
-        GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(!regs_mode_is_32bit(regs));
         perfc_incr(trap_hvc32);
 #ifndef NDEBUG
         if ( (hsr.iss & 0xff00) == 0xff00 )
@@ -2162,7 +2162,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
     }
 #ifdef CONFIG_ARM_64
     case HSR_EC_HVC64:
-        GUEST_BUG_ON(psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(regs_mode_is_32bit(regs));
         perfc_incr(trap_hvc64);
 #ifndef NDEBUG
         if ( (hsr.iss & 0xff00) == 0xff00 )
@@ -2178,12 +2178,12 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
          *
          * ARMv8 (DDI 0487A.d): D1-1501 Table D1-44
          */
-        GUEST_BUG_ON(psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(regs_mode_is_32bit(regs));
         perfc_incr(trap_smc64);
         do_trap_smc(regs, hsr);
         break;
     case HSR_EC_SYSREG:
-        GUEST_BUG_ON(psr_mode_is_32bit(regs));
+        GUEST_BUG_ON(regs_mode_is_32bit(regs));
         perfc_incr(trap_sysreg);
         do_sysreg(regs, hsr);
         break;
-- 
2.29.0

Re: [PATCH] xen/arm: Rename psr_mode_is_32bit to regs_mode_is_32bit

[Julien Grall]

Hi Michal,

On 21/02/2022 10:59, Michal Orzel wrote:
> Following a discussion [1] it seems like that renaming work has
> been forgotten. 

This is in my todo list of clean-up I need to do for Xen. But I haven't 
yet had a chance to look at it. Thank you for taking a look!

> Perform renaming of psr_mode_is_32bit to
> regs_mode_is_32bit as the function no longer takes psr parameter.

If we modify psr_mode_is_32bit(), then we should also modify 
psr_mode_is_user() because they have the same prototype and we should 
keep the naming consistent.

> 
> [1] https://marc.info/?l=xen-devel&m=156457538423787&w=2

NIT: The first sentence and this link adds value for the review on the 
mailing list (we know where the request came from) but doesn't add any 
after the commit message (there are no extra information in them).

So I would move this information after ---. This will get dropped on commit.

Cheers,

-- 
Julien Grall

Re: [PATCH] xen/arm: Rename psr_mode_is_32bit to regs_mode_is_32bit

[Michal Orzel]

Hi Julien,

On 21.02.2022 16:58, Julien Grall wrote:
> Hi Michal,
> 
> On 21/02/2022 10:59, Michal Orzel wrote:
>> Following a discussion [1] it seems like that renaming work has
>> been forgotten. 
> 
> This is in my todo list of clean-up I need to do for Xen. But I haven't yet had a chance to look at it. Thank you for taking a look!
> 
>> Perform renaming of psr_mode_is_32bit to
>> regs_mode_is_32bit as the function no longer takes psr parameter.
> 
> If we modify psr_mode_is_32bit(), then we should also modify psr_mode_is_user() because they have the same prototype and we should keep the naming consistent.
> 
Ok, I agree. Do you think this should be done in a separate patch?
FWICS, psr_mode_is_user is used in traps.c, vcpreg.c ,vtimer.c and vsysreg.c whereas psr_mode_is_32bit - only in traps.c.

>>
>> [1] https://marc.info/?l=xen-devel&m=156457538423787&w=2
> 
> NIT: The first sentence and this link adds value for the review on the mailing list (we know where the request came from) but doesn't add any after the commit message (there are no extra information in them).
> 
> So I would move this information after ---. This will get dropped on commit.
> 
Ok.
> Cheers,
> 

Cheers,
Michal

Re: [PATCH] xen/arm: Rename psr_mode_is_32bit to regs_mode_is_32bit

[Bertrand Marquis]

Hi Michal,

> On 22 Feb 2022, at 07:07, Michal Orzel <michal.orzel@arm.com> wrote:
> 
> Hi Julien,
> 
> On 21.02.2022 16:58, Julien Grall wrote:
>> Hi Michal,
>> 
>> On 21/02/2022 10:59, Michal Orzel wrote:
>>> Following a discussion [1] it seems like that renaming work has
>>> been forgotten. 
>> 
>> This is in my todo list of clean-up I need to do for Xen. But I haven't yet had a chance to look at it. Thank you for taking a look!
>> 
>>> Perform renaming of psr_mode_is_32bit to
>>> regs_mode_is_32bit as the function no longer takes psr parameter.
>> 
>> If we modify psr_mode_is_32bit(), then we should also modify psr_mode_is_user() because they have the same prototype and we should keep the naming consistent.
>> 
> Ok, I agree. Do you think this should be done in a separate patch?
> FWICS, psr_mode_is_user is used in traps.c, vcpreg.c ,vtimer.c and vsysreg.c whereas psr_mode_is_32bit - only in traps.c.

I think it can be done in a separate patch.

Cheers
Bertrand

> 
>>> 
>>> [1] https://marc.info/?l=xen-devel&m=156457538423787&w=2
>> 
>> NIT: The first sentence and this link adds value for the review on the mailing list (we know where the request came from) but doesn't add any after the commit message (there are no extra information in them).
>> 
>> So I would move this information after ---. This will get dropped on commit.
>> 
> Ok.
>> Cheers,
>> 
> 
> Cheers,
> Michal
> 

Re: [PATCH] xen/arm: Rename psr_mode_is_32bit to regs_mode_is_32bit

[Michal Orzel]

Hi Bertrand,

On 22.02.2022 09:48, Bertrand Marquis wrote:
> Hi Michal,
> 
>> On 22 Feb 2022, at 07:07, Michal Orzel <michal.orzel@arm.com> wrote:
>>
>> Hi Julien,
>>
>> On 21.02.2022 16:58, Julien Grall wrote:
>>> Hi Michal,
>>>
>>> On 21/02/2022 10:59, Michal Orzel wrote:
>>>> Following a discussion [1] it seems like that renaming work has
>>>> been forgotten.
>>>
>>> This is in my todo list of clean-up I need to do for Xen. But I haven't yet had a chance to look at it. Thank you for taking a look!
>>>
>>>> Perform renaming of psr_mode_is_32bit to
>>>> regs_mode_is_32bit as the function no longer takes psr parameter.
>>>
>>> If we modify psr_mode_is_32bit(), then we should also modify psr_mode_is_user() because they have the same prototype and we should keep the naming consistent.
>>>
>> Ok, I agree. Do you think this should be done in a separate patch?
>> FWICS, psr_mode_is_user is used in traps.c, vcpreg.c ,vtimer.c and vsysreg.c whereas psr_mode_is_32bit - only in traps.c.
> 
> I think it can be done in a separate patch.
> 
Ok, so I will push a patch series consisting of two patches.

> Cheers
> Bertrand
> 
>>
>>>>
>>>> [1] https://marc.info/?l=xen-devel&m=156457538423787&w=2
>>>
>>> NIT: The first sentence and this link adds value for the review on the mailing list (we know where the request came from) but doesn't add any after the commit message (there are no extra information in them).
>>>
>>> So I would move this information after ---. This will get dropped on commit.
>>>
>> Ok.
>>> Cheers,
>>>
>>
>> Cheers,
>> Michal
>>
> 

Cheers,
Michal

Re: [PATCH] xen/arm: Rename psr_mode_is_32bit to regs_mode_is_32bit

[Julien Grall]

On 22/02/2022 07:07, Michal Orzel wrote:
> Hi Julien,

Hi Michal,

> On 21.02.2022 16:58, Julien Grall wrote:
>> Hi Michal,
>>
>> On 21/02/2022 10:59, Michal Orzel wrote:
>>> Following a discussion [1] it seems like that renaming work has
>>> been forgotten.
>>
>> This is in my todo list of clean-up I need to do for Xen. But I haven't yet had a chance to look at it. Thank you for taking a look!
>>
>>> Perform renaming of psr_mode_is_32bit to
>>> regs_mode_is_32bit as the function no longer takes psr parameter.
>>
>> If we modify psr_mode_is_32bit(), then we should also modify psr_mode_is_user() because they have the same prototype and we should keep the naming consistent.
>>
> Ok, I agree. Do you think this should be done in a separate patch?

I am fine either way so long the two changes are committed together.

Cheers,

-- 
Julien Grall