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

Re: [Xen-devel] [PATCH 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c





On 30.01.18 20:44, Julien Grall wrote:


On 30/01/18 18:28, Volodymyr Babchuk wrote:
Hi Julien,

On 30.01.18 20:01, Julien Grall wrote:


On 26/01/18 18:27, Volodymyr Babchuk wrote:
Hi,

Hi Volodymyr,

On 26.01.18 20:15, Julien Grall wrote:
Hi,

On 26/01/18 18:09, Volodymyr Babchuk wrote:
On 24.01.18 20:34, Julien Grall wrote:
-    case PSCI_0_2_FN32(AFFINITY_INFO):
-    case PSCI_0_2_FN64(AFFINITY_INFO):
+    switch ( fid )
      {
-        register_t taff = PSCI_ARG(regs, 1);
-        uint32_t laff = PSCI_ARG32(regs, 2);
-
-        perfc_incr(vpsci_cpu_affinity_info);
-        PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
-        return true;
-    }
-
      case ARM_SMCCC_FUNC_CALL_COUNT(STANDARD):
          return fill_function_call_count(regs, SSSC_SMCCC_FUNCTION_COUNT);
Now definition SSSC_SMCCC_FUNCTION_COUNT depends on code in vscpi.c.
Maybe it is time to introduce function get_psci_0_2_fn_count() and use it there, what do you think?

Definitely not a function. It is a static number. But I can think of separate the call count.
Yep, separate call count for vPSCI and for SSSC itself would be a good thing.

Looking a bit more into it, this will not make a real improvement. This will be equally difficult to remember to update the call count.
Nevertheless, I think that it is right thing to hold call count in the
same file, where calls are implemented. This increases chances that call count will be held in sync.

So you are suggesting to implement a function? If so, that's a no-go from my side.
I'm not insist on function if you can propose alternative.
But why you are against getter function in the first place?

I wanted to propose another approach: define this call count in
vpsci.h, but there is no vpsci.h, instead you use psci.h, which is confusing in itself.


Also, based on the recent SMCCC update (2.1.5 [1]):
"The General Service Queries for SMCCC call ranges are described in SMCCC section 6.2. These functions are not always well suited to firmware that is integrated with multiple sub-services being combined into one service range. For example, PSCI and SDEI in the Standard Service range. In particular, the ‘call count’ and ‘revision’ functions do not provide useful information to the caller when multiple functions are provided. As a result, these are not widely used to identify firmware services."

So I would not worry that much if the function count is not sync in the future. BTW, it is already wrong in Xen 4.10. For standard service, we don't implement 13 but 52.
2.1.5 deprecates general service queries in SMCCC 1.1. So, either we upgradeSMCCC in Xen to 1.1 or we should be conform with SMCCC 1.0 and should return right number of functions, including changes in Xen 4.10 that you mentioned.

BTW, personally I'm not happy with call UID and call version deprecation. This makes impossible dynamic discovery of TEE, which was used in my TEE mediator patch series.

2.1.5 only deprecates for Architecture service which is not implemented for Xen at moment. Nothing is been said for the rest of them OP-TEE. The documentation only acknowledge the fact it is difficult for some service (such as SSSC) to infer information from that.
Ah, sorry. I missed that point. But, anyways, if it is deprecated only for Architecture service, then all other services still should provide right responses to standard queries.


I looked at some implementation of SMCCC and those calls are either not handled or the number are not correct.
I agree that *some* implementations can not conform to SMCCC.But, I thought you want Xen to follow standards as close as possible...

Anyway, I have a SMCCC 1.1 patch series coming up soon.
That would be great.

--
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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