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

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Stefano,

On 2017/3/18 1:21, Stefano Stabellini wrote:
> 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.
>

Yes, it's future-proof. I will update the commit message to be more
accurate.

>
>>>
>>>> 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@xxxxxxx>
>>>> ---
>>>>  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@xxxxxxxxxxxxx
>>>> https://lists.xen.org/xen-devel
>>>>
>>>
>>
>>
>> --
>> Regards,
>> Wei Chen
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxx
>> https://lists.xen.org/xen-devel
>>
>


-- 
Regards,
Wei Chen

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