From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (NAM12-DM6-obe.outbound.protection.outlook.com [40.107.243.45]) by mx.groups.io with SMTP id smtpd.web11.5376.1634187260314171214 for ; Wed, 13 Oct 2021 21:54:20 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nvidia.com header.s=selector2 header.b=C/Jrwi6m; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: nvidia.com, ip: 40.107.243.45, mailfrom: ashishsingha@nvidia.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SmXs4t5aJPkC4gk8UUMF8AW4ilufXByN1Z99TwCuQZ60jhPJDQ80XGO2+LyeHVN5lmAdDEZojJPXhGY+swURYqWAR0wb8Sq8K3+c0yczhC9rBf14yj8huB/hwN1yWJN4OfWb9QQfyRYo0kzAfXCMTZ+ZdOuPaIBQz5mu9ebaABd87hZG3FC5zdF4RSpxRTM2uzBdzVXCpK5tVNG3uHxNPJfE+BxOowpRtvpv5DOfJerkjusW6NS5LAfs3hYLNTp1G2/ryXoyHO1IAuVvYTWlmXs2Lh/pNPcrc0ryFLvDr40k/WLC4Gr8mltlm1YOgDMrk5dK9/8ZWgJFYVVhB4VUrw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=oiaOYITe+ztHG7H7xm/7xFMooz0St8IQVsoGrJUQoPg=; b=cg+ZIbbJeXbu7ixh2LtkIc0TvQxh2lCg3w8aTPhE5TmL5VSqU9yTv+c5171mg5VEoS+Hz8ygx+0UggQBIu+VDgjKNvr9d4DJoc1qFOHwrgpTvfKNB08zUaSfNrB/NwVrPBiM74W5Ar0I48182uMgzcgI+BK7FqAx+FZdc+OWOqnFU0Q4dxoTJSHJ/NEDBamJ+Erp2gHhtutP+s2ppToGr/PegC31q0XYUgMYgmCigUFbV+cWoLV4Mjl45+VuKn2338/m7XE3lHEQ2TJakbwyRNnP5gg9g+uaaVPXkTNcfk8ZVqhPUcHq/cmTegZJOdYwA8+ZjaG4uoaF8F36V8C5MA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=oiaOYITe+ztHG7H7xm/7xFMooz0St8IQVsoGrJUQoPg=; b=C/Jrwi6m13Qv+4/w3keL6zIkcyVSgPL8/y9r11qvGemrGWEOvrmHraeN2YzsUDSbjo05hZUcDiz2zv1knurI6b0919vQDPJOweRV1tSt1TFb6AJDkbZLyMLh6LfjHl122kQ2wiCIUyCZNJ+zCGxqoGfitt3qWdwu8ScM53tKvpUfDYPbxRqzh7SBVm1rFmumZddSg/0MRn9D/ofnPdOm23wKIuMjAiiG7TixXlloYkmStUashxA5PhrA5C+rxL+VnfBvJzWqLVI7iJRd9/EP3Pp3nvtRmT4oilaUcJTgfdV6dwejRcU6v6gtJK/UG42mWxlx2lMcAto5WDKPF2g7Dw== Received: from CO6PR12MB5396.namprd12.prod.outlook.com (2603:10b6:303:139::8) by CO6PR12MB5442.namprd12.prod.outlook.com (2603:10b6:5:35b::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4608.16; Thu, 14 Oct 2021 04:54:18 +0000 Received: from CO6PR12MB5396.namprd12.prod.outlook.com ([fe80::3181:5873:ca0f:cf8b]) by CO6PR12MB5396.namprd12.prod.outlook.com ([fe80::3181:5873:ca0f:cf8b%7]) with mapi id 15.20.4587.030; Thu, 14 Oct 2021 04:54:18 +0000 From: "Ashish Singhal" To: "fanjianfeng@byosoft.com.cn" , "devel@edk2.groups.io" , Marc Zyngier , Ard Biesheuvel , Shanker Donthineni CC: Leif Lindholm Subject: Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal Thread-Topic: Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal Thread-Index: AQHXvuitkDcYT9ItPUy1xjbmHBblWKvOX7wAgACwtICAAGPiiYAAC/3kgAC3j32AAbinNw== Date: Thu, 14 Oct 2021 04:54:18 +0000 Message-ID: References: , , <87h7dmpqn2.wl-maz@kernel.org>, , <2021101310221882906110@byosoft.com.cn> In-Reply-To: <2021101310221882906110@byosoft.com.cn> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: suggested_attachment_session_id: 12312729-02b8-c049-c4c9-9c5e978551e3 authentication-results: byosoft.com.cn; dkim=none (message not signed) header.d=none;byosoft.com.cn; dmarc=none action=none header.from=nvidia.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 5d91876a-3845-4d1d-4e98-08d98eceae4a x-ms-traffictypediagnostic: CO6PR12MB5442: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:5797; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Pbr5NooOgK5ugU8BXI3rtI0UDWWYbeabOhFjnFwsyyGI9JISi6tlAzfn9kvWEwLnzf47k6q+BtjSfq+YpHB3k3hSqQK5WIay+uLDLc9wZ9nA6elp8FvJmFoOPgLhLJJO94EVQ9Lw/xR1+cf8iA9NXDqUOgTozEMCyvE1vopJHPEDhB+Ir/ySJim0oSJsYiuOO1jMjV2+86aypaLJy82CHoBCZaSmhJLLetF5SFfmlbsNLo2ue10kYzHN14cBt7NsxbjhaA+UsT9Mfcx8kqdmRf3t7eOhIhZ22BT7fVnNZTV1Wp3fkqaTKKa6WsmaX5lnSAHy5R5+lSlrCISjIBg/cXJ2WPzESxc1CtCwmaKkmrelziQWzbbYuohPV8/N5Pl3AvRGvtgH3Ju9omUzk3X54FzzKarpa4jOh2c5kEV8Os0KNooGA9hf46rp6VVaft0s4aOLIMDY4fRWXrOnErg7+c/B6OLs0p8eAPgV1xQSuuZK2mrUOsPN8uHDUytwUv2Xd1iYGVTVWtcTgGE+6599P4/wgvS9RRt1CisRB0wGjW4rziFb87hQpM/pP2uFJaoRuaKv2od8Go3CzZgdWN/JaUjb+S6/OLn0VTO3dCjkvUZxbhqQpD4KK7jBypKjJAFgwZocDL3/kCD1KSQVaq/UNl9vRkI6vgRzPlkIVHNEwBvYBlKUm5YyDiCwGYkcEDhA+J6RtL+uXYprYJtPkUitRxa89sQ3Lf7pPVxX7qAKY6YiUIaKrugT0nyfcU98n8LbLwSJpFH5U5Aqs6BGqnB3FLSeK6QehqOCaaY59Q7CkvtAfLfJPlwqf/+nwHmu5fpT98g/5ByqrgHVzOrGVvpZ3Q== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CO6PR12MB5396.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(366004)(38070700005)(76236003)(55016002)(966005)(122000001)(86362001)(186003)(6636002)(8676002)(5660300002)(19627405001)(4326008)(71200400001)(110136005)(26005)(33656002)(38100700002)(9686003)(2906002)(52536014)(316002)(53546011)(7696005)(66946007)(4001150100001)(45080400002)(166002)(83380400001)(6506007)(66556008)(8936002)(64756008)(76116006)(66476007)(508600001)(66446008)(91956017);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?VO6F3/lqdofKjq0yCAOJrSGGr7gtJIpez1PPx54/4YEvr/b+SLc84UFIiEhw?= =?us-ascii?Q?8P0xZ4kRSFFD3Oh4px/wgpLnaQFmke9TlrqziiFc3lh9xwymC8ZvWWUTD39l?= =?us-ascii?Q?VPUv3agAObiS4dOe7NuNWYms6STW80Z/07oXwXcG3PFnjQi0hr3q9nBt8d7V?= =?us-ascii?Q?jWTuArBrKz/xy6HjdaFUpTv8aQuc5cnvUY96uX5iZ6muwhpvRtbxCJe+fQeo?= =?us-ascii?Q?PEeMsGYlE2Of78ohzYe3cK7HN/ps+ZbmJr1y2NO9Nd59+z1ismim95ZRb7Qy?= =?us-ascii?Q?cnD8saIoIBgAxUX0JgZHxqcG8RMIllJ8EOsH6RxYIS9gQfuw6++QjHXZ1r11?= =?us-ascii?Q?h2lkB3R3edxRCuOYp/jjJ9eAIw8R4YUb23SuJnAi+kAE2qKAUm5KI9KjvJyg?= =?us-ascii?Q?aVPQE5Ghgaxm6e9SsNqjj+rgGMdCAqPzHuEyRSussVj540Y4uwhFXPMTDxs2?= =?us-ascii?Q?8ImF6tV2g/rO4LRGo1lK6ZV0NbuWeZoUD8sKWQGWxRj9J6XqcQhUa9ud5k2P?= =?us-ascii?Q?GQsq03gijKlw0RzKgPxY6bn94L2+r0tWlW0zbA5kDdR8BeWLl5etyKtyVzry?= =?us-ascii?Q?kf8TUC2YXYVzeXjpVWfqFdmW9NtGawGBk+LjviREyQ7RLUhHC/QyclIIXfIs?= =?us-ascii?Q?VJBWm5JETCG97VRlj2R8Uz9dU+bL70zv9fZ8wWAPO5kuaW7NSF78MrgD99G3?= =?us-ascii?Q?o1XFDxukRhW8IQt+ml72adohFzmbwngFuNcfrXgp+zxUAOKVYnV+GRMy4Hry?= =?us-ascii?Q?IF91rfcQFLunJrO+cKymW4d2Yz5g0S/WtOZd5ZpzB5e72C81WC15z6hnMbgI?= =?us-ascii?Q?ZZagGMK/oMTjWFvu3V1KQ18+yeagerXzvIdanSND1GYJD7xXWby5PdMQeXbk?= =?us-ascii?Q?AwElrq90aISB6FVwgtfmxTpgfOmvjBeOeD7jMYnHC+ra1wXqoKDAitAYfm90?= =?us-ascii?Q?CDf46y2YlUQY01RIonoqYJrgcbBhg7CGE/56mFOM9s/I4Vgw59SF+ysO6JFw?= =?us-ascii?Q?+xa2E/Vn7MhtoqcKuNSsHbkkjFmA9dqo4eKMKzr4lOnjmC7fZO2iV2XF23V+?= =?us-ascii?Q?rxv22EQib6uUcHO5CqwNpBAyKKAPaJ7FHSSLj80njJWFHvhKVVrcXTOY0tTn?= =?us-ascii?Q?7jaaR67gHNoB+oHvop6JO33cYEjXjjG/VkwCBMf843d+cwiwBowNuOrfoBzn?= =?us-ascii?Q?NqtM9VlRoKoc+JnVnOMtjZVAzm3sOBe5Tnfvid3F56Rb/RuBccHA4ma2sywF?= =?us-ascii?Q?6+K4z8DojhtDnOn4cekFMzQNHvopw4mUHXb81VQkqjFn4xeoXRDYzfNKeOZF?= =?us-ascii?Q?6rY=3D?= MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CO6PR12MB5396.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 5d91876a-3845-4d1d-4e98-08d98eceae4a X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Oct 2021 04:54:18.7501 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 9mTIMwb66Oc4HTSL9RKKjZGvS0UhdsbfuOaUXUBXzolnYg9ktl+8M1Q/ceiKzIJqimfNiZHiccE/pbX0zvk7KA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO6PR12MB5442 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CO6PR12MB53968AC454F3A592958BFBE6BAB89CO6PR12MB5396namp_" --_000_CO6PR12MB53968AC454F3A592958BFBE6BAB89CO6PR12MB5396namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hello Jeff, Thanks for the reference you provided of the change made by you. Leveraging= a similar change resolves the problem 90 percent for me as I do not get th= e ISR interrupted for the most part because of another timer interrupt. How= ever, even with your change, during the ISR there are few instructions duri= ng which IRQ is enabled and we may be interrupted by a timer interrupt duri= ng that time unless I am understanding it wrong. Thanks Ashish ________________________________ From: fanjianfeng@byosoft.com.cn Sent: Tuesday, October 12, 2021 8:32 PM To: devel@edk2.groups.io ; Ashish Singhal ; Marc Zyngier ; Ard Biesheuvel ; Shanker Donthineni Cc: devel@edk2.groups.io ; Leif Lindholm Subject: Re: Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End Of Inte= rrupt Signal External email: Use caution opening links or attachments OVMF did a similare change on Time Driver, please refer to https://github.c= om/tianocore/edk2/commit/239b50a863704f7960525799eda82de061c7c458 I do not think this will be apply for ArmPkg/TimerDxe. If one real issue happened on platform, it seems that interrupt was reenabl= ed by reigstered timer event functions. Jeff From: Ashish Singhal via groups.io Date: 2021-10-12 23:38 To: Marc Zyngier; Ard Biesheuvel; Shanker Donthineni CC: edk2-devel-groups-io; Leif Lindholm; Ard Biesheuvel Subject: Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrup= t Signal + Shaker Get Outlook for iOS ________________________________ From: Ashish Singhal Sent: Tuesday, October 12, 2021 8:56:58 AM To: Marc Zyngier ; Ard Biesheuvel Cc: edk2-devel-groups-io ; Leif Lindholm ; Ard Biesheuvel Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal Marc, In the document ARM062-1010708621-30 (AArch64 Programmer's Guides Generic T= imer), towards the end of section 3.4 it says: "When writing an interrupt h= andler for the timers, it is important that software clears the interrupt b= efore deactivating the interrupt in the GIC. Otherwise, the GIC will re-sig= nal the same interrupt again." My change was in accordance with this. We only clear the interrupt when we = update the compare value but were signaling EOI before that going against t= he guidance in the document. Thanks Ashish ________________________________ From: Marc Zyngier Sent: Tuesday, October 12, 2021 2:57 AM To: Ard Biesheuvel ; Ashish Singhal Cc: edk2-devel-groups-io ; Leif Lindholm ; Ard Biesheuvel Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal External email: Use caution opening links or attachments On Mon, 11 Oct 2021 23:24:38 +0100, Ard Biesheuvel wrote: > > (+ Marc) > > On Mon, 11 Oct 2021 at 23:40, Ashish Singhal wr= ote: > > > > In an interrupt handler for the timers, it is important that > > software clears the interrupt before deactivating the interrupt > > in the GIC. Otherwise the GIC will re-signal the same interrupt > > again. > > > > Signed-off-by: Ashish Singhal > > Please don't spam us with almost identical versions of the same patch > without even documenting what the difference is. > > > > --- > > ArmPkg/Drivers/TimerDxe/TimerDxe.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c b/ArmPkg/Drivers/TimerD= xe/TimerDxe.c > > index 0370620fae..46e5bbf287 100644 > > --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c > > +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c > > @@ -300,10 +300,6 @@ TimerInterruptHandler ( > > // > > OriginalTPL =3D gBS->RaiseTPL (TPL_HIGH_LEVEL); > > > > - // Signal end of interrupt early to help avoid losing subsequent tic= ks > > - // from long duration handlers > > - gInterrupt->EndOfInterrupt (gInterrupt, Source); > > - > > // Check if the timer interrupt is active > > if ((ArmGenericTimerGetTimerCtrlReg () ) & ARM_ARCH_TIMER_ISTATUS) { > > > > @@ -335,6 +331,11 @@ TimerInterruptHandler ( > > ArmInstructionSynchronizationBarrier (); > > } > > > > + // In an interrupt handler for the timers, it is important that soft= ware clears the interrupt > > + // before deactivating the interrupt in the GIC. Otherwise the GIC w= ill re-signal the same > > + // interrupt again. > > + gInterrupt->EndOfInterrupt (gInterrupt, Source); > > + > > gBS->RestoreTPL (OriginalTPL); > > } > > This isn't a requirement if you haven't re-enabled interrupts in PSTATE (and the TPL being raised seems to indicate that interrupts are actively masked here). The timer is a level interrupt, and lowering the level takes time. If your GIC implementation is good, it will notice that the lowering of the level quickly, before you can reach the point where you re-enable interrupts. If it is slow (or badly emulated if this is actually done in a hypervisor), you'll get a spurious interrupt. In any case, moving the EOI around doesn't make things any better. You are just moving the goal post, without additional guarantees that the level has been retired. As a consequence, I don't think this patch makes much sense. M. -- Without deviation from the norm, progress is not possible. --_000_CO6PR12MB53968AC454F3A592958BFBE6BAB89CO6PR12MB5396namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable
Hello Jeff,

Thanks for the reference you provided of the change made by you. Leveraging= a similar change resolves the problem 90 percent for me as I do not get th= e ISR interrupted for the most part because of another timer interrupt. How= ever, even with your change, during the ISR there are few instructions during which IRQ is enabled and we may = be interrupted by a timer interrupt during that time unless I am understand= ing it wrong.

Thanks
Ashish
From: fanjianfeng@byosoft.c= om.cn <fanjianfeng@byosoft.com.cn>
Sent: Tuesday, October 12, 2021 8:32 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Ashish Singha= l <ashishsingha@nvidia.com>; Marc Zyngier <maz@kernel.org>; Ard= Biesheuvel <ardb@kernel.org>; Shanker Donthineni <sdonthineni@nvi= dia.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; Leif Lindholm= <leif@nuviainc.com>
Subject: Re: Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End = Of Interrupt Signal
 
External email: Us= e caution opening links or attachments

OVMF did a similare change on Time Driver, please refer to https://gi= thub.com/tianocore/edk2/commit/239b50a863704f7960525799eda82de061c7c458=  

I do not think this will be apply for ArmPkg/TimerDxe. 

If one real issue happened on platform, it seems that interrupt was r= eenabled by reigstered timer event functions.

Jeff
 
Date: 2021-10-12 23:38
To: Marc Zyngier; Ard Biesheuvel; Shanker Donth= ineni
Subject: Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Dela= y End Of Interrupt Signal
+ Shaker

Get Outlook for iOS
From: Ashish Singhal <= ashishsingha@nvidia.com>
Sent: Tuesday, October 12, 2021 8:56:58 AM
To: Marc Zyngier <maz@kernel.org>; Ard Biesheuvel <ardb@ker= nel.org>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Leif Lindholm= <leif@nuviainc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>= ;
Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Sign= al
 
Marc,

In the document ARM062-1010708621-30 (AArch64 Programmer's Guides Gene= ric Timer), towards the end of section 3.4 it says: "When writing an i= nterrupt handler for the timers, it is important that software clears the i= nterrupt before deactivating the interrupt in the GIC. Otherwise, the GIC will re-signal the same interrupt again.&qu= ot;

My change was in accordance with this. We only clear the interrupt when we = update the compare value but were signaling EOI before that going against t= he guidance in the document.

Thanks
Ashish
From: Marc Zyngier <= maz@kernel.org>
Sent: Tuesday, October 12, 2021 2:57 AM
To: Ard Biesheuvel <ardb@kernel.org>; Ashish Singhal <ashis= hsingha@nvidia.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Leif Lindholm= <leif@nuviainc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>= ;
Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Sign= al
 
External email: Use caution opening links or a= ttachments


On Mon, 11 Oct 2021 23:24:38 +0100,
Ard Biesheuvel <ardb@kernel.org> wrote:
>
> (+ Marc)
>
> On Mon, 11 Oct 2021 at 23:40, Ashish Singhal <ashishsingha@nvidia.c= om> wrote:
> >
> > In an interrupt handler for the timers, it is important that
> > software clears the interrupt before deactivating the interrupt > > in the GIC. Otherwise the GIC will re-signal the same interrupt > > again.
> >
> > Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
>
> Please don't spam us with almost identical versions of the same patch<= br> > without even documenting what the difference is.
>
>
> > ---
> >  ArmPkg/Drivers/TimerDxe/TimerDxe.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c b/ArmPkg/Drivers/= TimerDxe/TimerDxe.c
> > index 0370620fae..46e5bbf287 100644
> > --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> > +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> > @@ -300,10 +300,6 @@ TimerInterruptHandler (
> >    //
> >    OriginalTPL =3D gBS->RaiseTPL (TPL_HIGH_LEVE= L);
> >
> > -  // Signal end of interrupt early to help avoid losing sub= sequent ticks
> > -  // from long duration handlers
> > -  gInterrupt->EndOfInterrupt (gInterrupt, Source);
> > -
> >    // Check if the timer interrupt is active
> >    if ((ArmGenericTimerGetTimerCtrlReg () ) & = ARM_ARCH_TIMER_ISTATUS) {
> >
> > @@ -335,6 +331,11 @@ TimerInterruptHandler (
> >      ArmInstructionSynchronizationBarrie= r ();
> >    }
> >
> > +  // In an interrupt handler for the timers, it is importan= t that software clears the interrupt
> > +  // before deactivating the interrupt in the GIC. Otherwis= e the GIC will re-signal the same
> > +  // interrupt again.
> > +  gInterrupt->EndOfInterrupt (gInterrupt, Source);
> > +
> >    gBS->RestoreTPL (OriginalTPL);
> >  }
> >

This isn't a requirement if you haven't re-enabled interrupts in
PSTATE (and the TPL being raised seems to indicate that interrupts are
actively masked here).

The timer is a level interrupt, and lowering the level takes time. If
your GIC implementation is good, it will notice that the lowering of
the level quickly, before you can reach the point where you re-enable
interrupts. If it is slow (or badly emulated if this is actually done
in a hypervisor), you'll get a spurious interrupt.

In any case, moving the EOI around doesn't make things any better. You
are just moving the goal post, without additional guarantees that the
level has been retired.

As a consequence, I don't think this patch makes much sense.

        M.

--
Without deviation from the norm, progress is not possible.
--_000_CO6PR12MB53968AC454F3A592958BFBE6BAB89CO6PR12MB5396namp_--