arch/x86/include/asm/apicdef.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
I was wondering if the aliasing of APIC_TIMER_BASE_TMBASE and
APIC_LVT_TIMER_TSCDEADLINE was intentional or we need to << 19?
Also it seems the GET_APIC_TIMER_BASE, APIC_TIMER_BASE_CLKIN and
APIC_TIMER_BASE_TMBASE are not even being used. Perhaps, can we
just remove them?
Signed-off-by: Daniel Vacek <neelx@redhat.com>
---
arch/x86/include/asm/apicdef.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
index 5716f22f81ac..00b4ca49f3ea 100644
--- a/arch/x86/include/asm/apicdef.h
+++ b/arch/x86/include/asm/apicdef.h
@@ -95,9 +95,9 @@
#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_LVT_TIMER_BASE_MASK (0x3 << 19)
+#define GET_APIC_TIMER_BASE(x) (((x) >> 19) & 0x3)
+#define SET_APIC_TIMER_BASE(x) (((x) << 19))
#define APIC_TIMER_BASE_CLKIN 0x0
#define APIC_TIMER_BASE_TMBASE 0x1
#define APIC_TIMER_BASE_DIV 0x2
--
2.34.1
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.
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;
On Wed, Apr 6, 2022 at 1:56 PM Thomas Gleixner <tglx@linutronix.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; >
© 2016 - 2026 Red Hat, Inc.