[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

Hi Julien,

On 31.01.18 00:06, Julien Grall wrote:
On 30 January 2018 at 19:35, Volodymyr Babchuk
<volodymyr_babchuk@xxxxxxxx> wrote:

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 Volodymyr,

On 26.01.18 20:15, Julien Grall wrote:


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,
-        return true;
-    }
           return fill_function_call_count(regs,

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

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?

Because a function returning a const value is pretty dumb.

I can't agree with you there. In my opinion - if it is needed for clarity - then it is fine.

Nevertheless, you have my opinion. It is up to you what to do with it.

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.

I thought about vpsci.h, but basically you will have only 2 functions in it and
the number of PSCI calls. That's it.

Is this really a problem? It is quite natural to find declarations for something.c in something.h. By moving declaration into different file, you are hiding it from anyone who does not carry sacred knowledge (or use grep/cscope, yes). And then, when people decide to extend something.c they continue to put declarations into inappropriate.h. Just look at processor.h as a good example. All functions it define are implemented either in traps.c or domain.c. But functions from processor.c are defined in procinfo.h.
I can tell for sure, that this confuses newbies.

So it is not going to help to update because the header will unlikely need to
change when adding new PSCI call.

Yes. But at least we can put comment above switch(fid):

/* Please don't forget to update call count in (v)psci.h */


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...

It is not about cannot conform but only implements partially SMCCC. I could name
ATF for that (yes).

So it makes me more confident the call count is not something people seem to

So, you propose to fix this only when something will trip over this?

Volodymyr Babchuk

Xen-devel mailing list



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