Re: [PATCH] setup vwfi correctly on cpu0

[Julien Grall <julien.grall@arm.com>]

Hi Stefano,

On 31/03/17 23:33, Stefano Stabellini wrote:
> On Fri, 31 Mar 2017, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 03/30/2017 11:35 PM, Stefano Stabellini wrote:
>>> parse_vwfi runs after init_traps on cpu0, potentially resulting in the
>>> wrong HCR_EL2 for it. Secondary cpus boot after parse_vwfi, so in their
>>> case init_traps will write the correct set of flags to HCR_EL2.
>>>
>>> For cpu0, fix the issue by changing HCR_EL2 setting directly in
>>> parse_vwfi.
>>>
>>> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>>>
>>> ---
>>> This patch should be apply to 4.8, 4.7, 4.6, not to unstable (it will be
>>> fixed differently there).
>>> ---
>>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>>> index 614501f..94d2e8a 100644
>>> --- a/xen/arch/arm/traps.c
>>> +++ b/xen/arch/arm/traps.c
>>> @@ -112,6 +112,16 @@ static void __init parse_vwfi(const char *s)
>>>  		vwfi = NATIVE;
>>>  	else
>>>  		vwfi = TRAP;
>>> +    /*
>>> +     * HCR_EL2 has already been set on cpu0, change the setting here, if
>>> +     * needed. Other cpus haven't booted yet, init_traps will setup
>>> +     * HCR_EL2 correctly.
>>> +     */
>>> +    if (vwfi == NATIVE) {
>>
>> Coding style:
>>
>> if ( ... )
>> {
>
> OK
>
>
>>> +        register_t hcr;
>>> +        hcr = READ_SYSREG(HCR_EL2);
>>> +        WRITE_SYSREG(hcr & ~(HCR_TWI|HCR_TWE), HCR_EL2);
>>
>> You are assuming the default value of vwfi and it makes very complicate for
>> someone to follow the code and modify it.
>
> Do you mean the default vwfi setting? If so, no, I am not: hcr &
> ~(HCR_TWI|HCR_TWE) works regardless of the default.

Sorry, I was not clear. You assume that vwfi == TRAP by default, 
although I guess it is fine because it is a backport.

Cheers,

-- 
Julien Grall

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