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

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 02/02/2018 03:40 PM, Volodymyr Babchuk wrote:
> Hi
Hi,


> On 02.02.18 16:31, Julien Grall wrote:
> > Hi,
> >
> > On 02/02/18 14:23, Volodymyr Babchuk wrote:
> > > On 02.02.18 13:41, Julien Grall wrote:
> > > > At the moment PSCI function dispatching is done in vsmc.c and the
> > > > function implementation in vpsci.c. Some bits of the implementation is
> > > > even done in vsmc.c (see PSCI_SYSTEM_RESET).
> > > >
> > > > This means that it is difficult to follow the implementation and also
> > > > requires to export functions for each PSCI functions.
> > > >
> > > > Therefore move PSCI dispatching in two new functions do_vpsci_0_1_call
> > > > and do_vpsci_0_2_call. The former will handle PSCI 0.1 call while 
> > > > the latter
> > > > 0.2 or later call.
> > > >
> > > > At the same time, a new header vpsci.h was created to contain all
> > > > definitions for virtual PSCI and avoid confusion with the host PSCI.
> > > >
> > > > Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> > > I'm okay with this generally, you can have my r-b:
> > > `Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>`
> > I would not have gave a reviewed-by with the comments you gave below ;).
> Point taken :)
>
> > > > +/*
> > > > + * PSCI 0.2 or later calls. It will return false if the function ID is
> > > > + * not handled.
> > > > + */
> > > > +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
> > > > +{
> > > > +    /*
> > > > +     * /!\ VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when
> > > > +     * adding/removing a function
> > > > +     */
> > > Should we also update revision of SSSC interface as well? SMCCC 
> > > requires this.
> > Meh, you can't rely on the SSSC revision most of the time as the 
> > version for PSCI is done through PSCI_get_version. I can add a comment 
> > if you want.
> Yes, I agree with you. But section 5.4 of SMCCC 1.0 applies to SSSC as 
> well. So you can write something like "ARM SMCCC requires that SSSC 
> revision and function call count should be updated every time you add or 
> remove a function"
Usually we update versioning number only once per release. I would 
follow the same approach for this one. So I would suggest
"VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when adding 
removing a function. SSCCC_SMCCC_*_REVISION should be updated once per 
Xen release".
Cheers,

--
Julien Grall

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