[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 08/11] arm: PSCI: use definitions provided by asm/smccc.h

On 25/08/17 12:00, Volodymyr Babchuk wrote:

On 24.08.17 20:22, Julien Grall wrote:
Hi Volodymyr,

On 21/08/17 21:27, Volodymyr Babchuk wrote:
smccc.h provides definitions to construct SMC call function number according to SMCCC. We don't need multiple definitions for one thing, and definitions
in smccc.h are more generic than ones used in psci.h.

So psci.h will only provide function codes, while whole SMC function
identifier will be constructed using generic macros from smccc.h.

PSCI_0_2_FN_xxx was deliberately renamed to PSCI_0_2_FUNC_xxx, because this
is a new entity. It can lead to problems, if we'll just change value of
PSCI_0_2_FN_xxx without renaming it.

I don't think "new entity" is a good reason to rename them. And the previous naming was kind of nice to read. More that you still use PSCI_0_2_FN{32,64}. We should definitely stay consistent in naming.

So what is the exact problem? Is it because you are worry to miss some of them?
Actually yes. That helped me to find references to PSCI code in seattle.c

But now you have inconsistent naming which is not better. I would prefer much prefer to keep the same name as before.

diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index 956d4ef..46a2fde 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -102,7 +102,7 @@ static bool handle_psci_0_x(struct cpu_user_regs *regs)
 /* helper function for checking arm mode 32/64 bit */
 static inline int psci_mode_check(struct domain *d, register_t fid)
-    return !( is_64bit_domain(d)^( (fid & PSCI_0_2_64BIT) >> 30 ) );
+    return is_64bit_domain(d) || !ARM_SMCCC_IS_64(fid);

I don't see any reason to do the renaming in psci_mode_check given that you will remove this function in the next patch.
Yep, but in comments for the previous series you asked me to make this code
easier to read :)
PSCI_0_2_64BIT is dropped now, so I had to use long definitions from smccc.h

If you remove the code in this series, then there are no need to review it. You can just mention in the commit message why you didn't clean-up that function.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.