Re: [Xen-devel] [PATCH v2 4/4] vsmc: psci: remove 64 bit mode check

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 07/03/2017 06:29 PM, Stefano Stabellini wrote:
> On Sun, 2 Jul 2017, Julien Grall wrote:
> > Hi,
> >
> > On 06/30/2017 10:19 PM, Stefano Stabellini wrote:
> > > On Thu, 22 Jun 2017, Volodymyr Babchuk wrote:
> > > > PSCI handling code had helper routine that checked calling convention.
> > > > It does not needed anymore, because:
> > > >
> > > >    - Generic handler checks that 64 bit calls can be made only by
> > > >      64 bit guests.
> > > >
> > > >    - SMCCC requires that 64-bit handler should support both 32 and 64 bit
> > > >      calls even if they originate from 64 bit caller.
> > > >
> > > > This patch removes that extra check.
> > > >
> > > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> > > > ---
> > > >    xen/arch/arm/vsmc.c | 13 +------------
> > > >    1 file changed, 1 insertion(+), 12 deletions(-)
> > > >
> > > > diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> > > > index 5f10fd1..1983e0e 100644
> > > > --- a/xen/arch/arm/vsmc.c
> > > > +++ b/xen/arch/arm/vsmc.c
> > > > @@ -98,12 +98,6 @@ static bool handle_arch(struct cpu_user_regs *regs)
> > > >        return false;
> > > >    }
> > > >    -/* helper function for checking arm mode 32/64 bit */
> > > > -static inline int psci_mode_check(struct domain *d, register_t fid)
> > > > -{
> > > > -        return !( is_64bit_domain(d)^( (fid & PSCI_0_2_64BIT) >> 30 ) );
> > > > -}
> > > > -
> > > >    /* PSCI 2.0 interface */
> > > >    static bool handle_ssc(struct cpu_user_regs *regs)
> > > >    {
> > > > @@ -125,8 +119,7 @@ static bool handle_ssc(struct cpu_user_regs *regs)
> > > >            return true;
> > > >        case ARM_SMCCC_FUNC_NUM(PSCI_0_2_FN_MIGRATE_INFO_UP_CPU):
> > > >            perfc_incr(vpsci_migrate_info_up_cpu);
> > > > -        if ( psci_mode_check(current->domain, fid) )
> > > > -            set_user_reg(regs, 0, do_psci_0_2_migrate_info_up_cpu());
> > > > +        set_user_reg(regs, 0, do_psci_0_2_migrate_info_up_cpu());
> > > >            return true;
> > > >        case ARM_SMCCC_FUNC_NUM(PSCI_0_2_FN_SYSTEM_OFF):
> > > >            perfc_incr(vpsci_system_off);
> > > > @@ -140,7 +133,6 @@ static bool handle_ssc(struct cpu_user_regs *regs)
> > > >            return true;
> > > >        case ARM_SMCCC_FUNC_NUM(PSCI_0_2_FN_CPU_ON):
> > > >            perfc_incr(vpsci_cpu_on);
> > > > -        if ( psci_mode_check(current->domain, fid) )
> > > I would prefer if the `return true' was within the { } block. But anyway
> > > it's just a code style issue, so:
> > Well, I think we should keep the coding style consistent within arch/arm. If
> > we have the return true within {} in other place. Then this should be done
> > here.
> >
> > In general, { } should only be used to en-globe everything in a case or for
> > if/else/while/for with more than a line. All the other kind of { } should be
> > avoided. I particularly dislike any code doing
> >
> > code
> >
> > {
> >     variable definition;
> >
> >     code
> > }
> >
> > code
> >
> > Unless you have a strong reason to do it (avoiding reworking the code is not
> > one), I will nack any code resulting to that.
> Right, care to submit a patch for CODING_STYLE? I noticed there are no
> entries on this topic.
I can write down a patch assuming the other hypervisor maintainers are 
happy with it.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel