linux-kernel - Re: [PATCH] [RFC] apic: fix timer base macro definitions

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CACjP9X_A7aLmvypyOz1UrXM571gx_X5q7=w-1j+G+MSbCteiEw@mail.gmail.com>
Date:   Wed, 6 Apr 2022 16:54:39 +0200
From:   Daniel Vacek <neelx@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [RFC] apic: fix timer base macro definitions

On Wed, Apr 6, 2022 at 1:56 PM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> Daniel,
>
> On Wed, Feb 02 2022 at 15:02, Daniel Vacek wrote:
> > I was wondering if the aliasing of APIC_TIMER_BASE_TMBASE and
> > APIC_LVT_TIMER_TSCDEADLINE was intentional or we need to << 19?
>
> That's intentional. This is only used for the !lapic_is_integrated()
> case, which is the ancient i82489DX.
>
> Something like the below should make this more clear.

Nah. Makes sense. Thanks for clearing that up. Looks good to me now.

--nX

> Thanks,
>
>         tglx
> ---
> --- a/arch/x86/include/asm/apicdef.h
> +++ b/arch/x86/include/asm/apicdef.h
> @@ -95,12 +95,6 @@
>  #define        APIC_LVTTHMR    0x330
>  #define        APIC_LVTPC      0x340
>  #define        APIC_LVT0       0x350
> -#define                APIC_LVT_TIMER_BASE_MASK        (0x3 << 18)
> -#define                GET_APIC_TIMER_BASE(x)          (((x) >> 18) & 0x3)
> -#define                SET_APIC_TIMER_BASE(x)          (((x) << 18))
> -#define                APIC_TIMER_BASE_CLKIN           0x0
> -#define                APIC_TIMER_BASE_TMBASE          0x1
> -#define                APIC_TIMER_BASE_DIV             0x2
>  #define                APIC_LVT_TIMER_ONESHOT          (0 << 17)
>  #define                APIC_LVT_TIMER_PERIODIC         (1 << 17)
>  #define                APIC_LVT_TIMER_TSCDEADLINE      (2 << 17)
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -320,6 +320,9 @@ int lapic_get_maxlvt(void)
>  #define APIC_DIVISOR 16
>  #define TSC_DIVISOR  8
>
> +/* i82489DX specific */
> +#define                I82489DX_BASE_DIVIDER           (((0x2) << 18))
> +
>  /*
>   * This function sets up the local APIC timer, with a timeout of
>   * 'clocks' APIC bus clock. During calibration we actually call
> @@ -340,8 +343,14 @@ static void __setup_APIC_LVTT(unsigned i
>         else if (boot_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER))
>                 lvtt_value |= APIC_LVT_TIMER_TSCDEADLINE;
>
> +       /*
> +        * The i82489DX APIC uses bit 18 and 19 for the base divider.  This
> +        * overlaps with bit 18 on integrated APICs, but is not documented
> +        * in the SDM. No problem though. i82489DX equipped systems do not
> +        * have TSC deadline timer.
> +        */
>         if (!lapic_is_integrated())
> -               lvtt_value |= SET_APIC_TIMER_BASE(APIC_TIMER_BASE_DIV);
> +               lvtt_value |= I82489DX_BASE_DIVIDER;
>
>         if (!irqen)
>                 lvtt_value |= APIC_LVT_MASKED;
>