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

[Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>]

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