linux-kernel - RE: [PATCH 1/2] i8253: Disable PIT timer 0 when not in use

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <SN6PR02MB41576EAA0B95887BF9777645D4B32@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Fri, 2 Aug 2024 15:44:10 +0000
From: Michael Kelley <mhklinux@...look.com>
To: David Woodhouse <dwmw2@...radead.org>, "tglx@...uxtronix.de"
	<tglx@...uxtronix.de>, "x86@...nel.org" <x86@...nel.org>
CC: "lirongqing@...du.com" <lirongqing@...du.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "kys@...rosoft.com" <kys@...rosoft.com>,
	"haiyangz@...rosoft.com" <haiyangz@...rosoft.com>, "wei.liu@...nel.org"
	<wei.liu@...nel.org>, "decui@...rosoft.com" <decui@...rosoft.com>,
	"mingo@...hat.com" <mingo@...hat.com>, "bp@...en8.de" <bp@...en8.de>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Daniel Lezcano
	<daniel.lezcano@...aro.org>
Subject: RE: [PATCH 1/2] i8253: Disable PIT timer 0 when not in use

From: David Woodhouse <dwmw2@...radead.org> Sent: Friday, August 2, 2024 6:56 AM
> 
> Leaving the PIT interrupt running can cause noticeable steal time for
> virtual guests. The VMM generally has a timer which toggles the IRQ input
> to the PIC and I/O APIC, which takes CPU time away from the guest. Even
> on real hardware, running the counter may use power needlessly (albeit
> not much).
> 
> Make sure it's turned off if it isn't going to be used.
> 
> Signed-off-by: David Woodhouse <dwmw@...zon.co.uk>
> ---
>  arch/x86/kernel/i8253.c     | 11 +++++++++--
>  drivers/clocksource/i8253.c | 13 +++++++++----
>  include/linux/i8253.h       |  1 +
>  3 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/i8253.c b/arch/x86/kernel/i8253.c
> index 2b7999a1a50a..80e262bb627f 100644
> --- a/arch/x86/kernel/i8253.c
> +++ b/arch/x86/kernel/i8253.c
> @@ -8,6 +8,7 @@
>  #include <linux/timex.h>
>  #include <linux/i8253.h>
> 
> +#include <asm/hypervisor.h>
>  #include <asm/apic.h>
>  #include <asm/hpet.h>
>  #include <asm/time.h>
> @@ -39,9 +40,15 @@ static bool __init use_pit(void)
> 
>  bool __init pit_timer_init(void)
>  {
> -	if (!use_pit())
> +	if (!use_pit()) {
> +		/*
> +		 * Don't just ignore the PIT. Ensure it's stopped, because
> +		 * VMMs otherwise steal CPU time just to pointlessly waggle
> +		 * the (masked) IRQ.
> +		 */
> +		clockevent_i8253_disable();
>  		return false;
> -
> +	}
>  	clockevent_i8253_init(true);
>  	global_clock_event = &i8253_clockevent;
>  	return true;
> diff --git a/drivers/clocksource/i8253.c b/drivers/clocksource/i8253.c
> index d4350bb10b83..cb215e6f2e83 100644
> --- a/drivers/clocksource/i8253.c
> +++ b/drivers/clocksource/i8253.c
> @@ -108,11 +108,8 @@ int __init clocksource_i8253_init(void)
>  #endif
> 
>  #ifdef CONFIG_CLKEVT_I8253
> -static int pit_shutdown(struct clock_event_device *evt)
> +void clockevent_i8253_disable(void)
>  {
> -	if (!clockevent_state_oneshot(evt) && !clockevent_state_periodic(evt))
> -		return 0;
> -
>  	raw_spin_lock(&i8253_lock);
> 
>  	outb_p(0x30, PIT_MODE);
> @@ -123,6 +120,14 @@ static int pit_shutdown(struct clock_event_device *evt)
>  	}
> 
>  	raw_spin_unlock(&i8253_lock);
> +}
> +
> +static int pit_shutdown(struct clock_event_device *evt)
> +{
> +	if (!clockevent_state_oneshot(evt) && !clockevent_state_periodic(evt))
> +		return 0;
> +
> +	clockevent_i8253_disable();
>  	return 0;
>  }
> 
> diff --git a/include/linux/i8253.h b/include/linux/i8253.h
> index 8336b2f6f834..bf169cfef7f1 100644
> --- a/include/linux/i8253.h
> +++ b/include/linux/i8253.h
> @@ -24,6 +24,7 @@ extern raw_spinlock_t i8253_lock;
>  extern bool i8253_clear_counter_on_shutdown;
>  extern struct clock_event_device i8253_clockevent;
>  extern void clockevent_i8253_init(bool oneshot);
> +extern void clockevent_i8253_disable(void);
> 
>  extern void setup_pit_timer(void);
> 
> --
> 2.44.0

Did a basic smoke test of this two-patch series on a Hyper-V Gen 1
VM and on a Gen 2 VM. All looks good and behaves as expected.

On the Gen 1 VM, the PIT is used briefly at boot (takes ~35 interrupts)
before the Hyper-V synthetic timer takes over and the PIT is shutdown.
As expected, no further interrupts are received from the PIT.

On a Gen 2 VM, apic_needs_pit() returns true because
X86_FEATURE_ARAT isn't present. The PIT doesn't exist in a
Gen 2 VM, but the code paths handle this situation with no
problems, just as before the patch series.

For the two-patch series on Hyper-V,
Tested-by: Michael Kelley <mhkelley@...look.com>