Re: [PATCH 08/18] xen/arm: Introduce a initcall to update cpu_hwcaps by serror_op

From: Stefano Stabellini <sstabellini@kernel.org>
To: Wei Chen <Wei.Chen@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Steve Capper <Steve.Capper@arm.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Kaly Xin <Kaly.Xin@arm.com>, Julien Grall <Julien.Grall@arm.com>,
	nd <nd@arm.com>
Subject: Re: [PATCH 08/18] xen/arm: Introduce a initcall to update cpu_hwcaps by serror_op
Date: Fri, 17 Mar 2017 10:21:45 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.10.1703171020230.15430@sstabellini-ThinkPad-X260> (raw)
In-Reply-To: <AM3PR08MB05783DB8BCF080D73911B9D99E390@AM3PR08MB0578.eurprd08.prod.outlook.com>

On Fri, 17 Mar 2017, Wei Chen wrote:
> Hi Stefano,
> On 2017/3/17 7:30, Stefano Stabellini wrote:
> > On Mon, 13 Mar 2017, Wei Chen wrote:
> >> In the later patches of this series, we want to use the alternative
> >> patching framework to avoid checking serror_op in every entries.
> >> So we define a new cpu feature "SKIP_CHECK_PENDING_VSERROR" for
> >> serror_op. When serror_op is not equal to SERROR_DIVERSE, this
> >> feature will be set to cpu_hwcaps.
> >>
> >> But we could not update cpu_hwcaps directly in the serror parameter
> >> parsing function. Because if the serror parameter is not placed in
> >> the command line, the parsing function would not be invoked.
> >
> > Wait, the only way to set serrors_op != SERRORS_DIVERSE is to pass the
> > "serror" command line parameter. The parsing function is always invoked
> > if serrors_op != SERRORS_DIVERSE.
> >
> 
> Yes, but that because we set serrors_op to SERRORS_DIVERSE by default.
> If we change the policy and set serrors_op to SERRORS_PANIC by default, 
> then the parsing function would never be invoked.

Ah, I see. It's not actually "to guarantee the cpu_hwcaps can be updated
no matter the serror parameter is placed in the command line or not",
because today it would always be the case.

It's to future-proof the code, in case one day we'll change the default
SERRORS behavior.

> >
> >> So, we introduce this initcall to guarantee the cpu_hwcaps can be
> >> updated no matter the serror parameter is placed in the command line
> >> or not.
> >>
> >> Signed-off-by: Wei Chen <Wei.Chen@arm.com>
> >> ---
> >>  xen/arch/arm/traps.c             | 9 +++++++++
> >>  xen/include/asm-arm/cpufeature.h | 3 ++-
> >>  2 files changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> >> index 5e31699..053b7fc 100644
> >> --- a/xen/arch/arm/traps.c
> >> +++ b/xen/arch/arm/traps.c
> >> @@ -134,6 +134,15 @@ static void __init parse_serrors_behavior(const char *str)
> >>  }
> >>  custom_param("serrors", parse_serrors_behavior);
> >>
> >> +static __init int update_serrors_cpu_caps(void)
> >
> > I think the coding style is "static int __init" ...
> >
> 
> Oh, thanks, I will fix it.
> 
> >
> >> +{
> >> +    if ( serrors_op != SERRORS_DIVERSE )
> >> +        cpus_set_cap(SKIP_CHECK_PENDING_VSERROR);
> >> +
> >> +    return 0;
> >> +}
> >> +__initcall(update_serrors_cpu_caps);
> >> +
> >>  register_t get_default_hcr_flags(void)
> >>  {
> >>      return  (HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
> >> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> >> index c0a25ae..ec3f9e5 100644
> >> --- a/xen/include/asm-arm/cpufeature.h
> >> +++ b/xen/include/asm-arm/cpufeature.h
> >> @@ -40,8 +40,9 @@
> >>  #define ARM32_WORKAROUND_766422 2
> >>  #define ARM64_WORKAROUND_834220 3
> >>  #define LIVEPATCH_FEATURE   4
> >> +#define SKIP_CHECK_PENDING_VSERROR 5
> >>
> >> -#define ARM_NCAPS           5
> >> +#define ARM_NCAPS           6
> >>
> >>  #ifndef __ASSEMBLY__
> >>
> >> --
> >> 2.7.4
> >>
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xen.org
> >> https://lists.xen.org/xen-devel
> >>
> >
> 
> 
> -- 
> Regards,
> Wei Chen
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
> 

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