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

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

> > +/*
> > + * 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.
[...]

> > diff --git a/xen/include/asm-arm/vpsci.h b/xen/include/asm-arm/vpsci.h
> > new file mode 100644
> > index 0000000000..d6a890f6a2
> > --- /dev/null
> > +++ b/xen/include/asm-arm/vpsci.h
> > @@ -0,0 +1,13 @@
> I'm not sure, should you add license information?
I think so.

> > +#ifndef __ASM_VPSCI_H__
> > +#define __ASM_VPSCI_H__
> > +
> > +#include <asm/psci.h>
> > +
> > +/* Number of function implemented by virtual PSCI (only 0.2 or later) */
> > +#define VPSCI_NR_FUNCS  11
> > +
> > +/* Functions handle PSCI calls from the guests */
> > +bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid);
> > +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid);
> > +
> > +#endif /* __ASM_VPSCI_H__ */
> And the same about Emacs tags.
Good catch. I will send a new version with the comments addressed.

Cheers,

--
Julien Grall

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