Re: [PATCH v2 1/1] ArmPkg/TimerDxe: Add ISB for timer compare value reload

[Marc Zyngier <marc.zyngier@arm.com>]

On 15/03/18 07:30, Ard Biesheuvel wrote:
> On 15 March 2018 at 07:11, Guo Heyi <heyi.guo@linaro.org> wrote:
>> Hi Marc and Ard,
>>
>> I found the timer re-enable code was added by Ard for special reason:
>>
>> commit b1a633434ddc5fc28de817debd963f7845fb78c7
>> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Date:   Thu Sep 18 21:16:47 2014 +0000
>>
>>     ArmPkg/TimerDxe: add workaround for KVM timer interrupt handling
>>
>> So this line of code cannot be removed and I will add an ISB after enabling
>> timer.
>>
> 
> I'm not sure. IIUC, the KVM issue that required this has been fixed
> long ago, and I don't want to carry this forever. Marc?

This has been fixed quite a while ago:

commit f120cd6533d21075ab103ae6c225b1697853660d
Author: Marc Zyngier <marc.zyngier@arm.com>
Date:   Mon Jun 23 13:59:13 2014 +0100

    KVM: arm/arm64: timer: Allow the timer to control the active state

    In order to remove the crude hack where we sneak the masked bit
    into the timer's control register, make use of the phys_irq_map
    API control the active state of the interrupt.

    This causes some limited changes to allow for potential error
    propagation.

    Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
    Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

>> Another strange thing is that we got lots of "Spurious GIC interrupt" error
>> messages when timer enable code was removed, though at last it could boot to EFI
>> shell. Please let me know if you have any idea about the possible reason of this
>> issue.
>>
> 
> No idea, sorry.

Me neither. You'd have to dump the control register before and after,
and work out what is being changed exactly.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...