| Message ID | 20240428022906.373130-1-yuan.yao@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests,1/1] x86/apic: Fix test_apic_timer_one_shot() random failure | expand |
On 4/28/2024 10:29 AM, Yao Yuan wrote: > Explicitly clear TMICT to avoid test_apic_timer_one_shot() > negative failure. > > Clear TMICT to disable any enabled but masked local timer. > Otherwise timer interrupt may occur after lvtt_handler() is > set as handler and **before** TDCR or TIMCT is set to new > value, lead this test failure. Log comes from UEFI mode: The exact reason is that the new value written to APIC_LVTT unmasks the timer interrupt and if TMICT has non-zero and small value configured before (by firmware or whatever) it may get an additional one-shot timer interrupt before new TMICT being programmed. > PASS: PV IPIs testing > PASS: pending nmi > Got local timer intr before write to TDCR / TMICT > old tmict:0x989680 old lvtt:0x30020 tsc2 - tsc1 = 0xb68 > ^^^^^^^^ ^^^^^^^ It took me a while to figure out the above 2 two lines are added for debug yourself. > FAIL: APIC LVT timer one shot > > Fixes: 9f815b293961 ("x86: apic: add LVT timer test") > Signed-off-by: Yao Yuan <yuan.yao@intel.com> > --- > x86/apic.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/x86/apic.c b/x86/apic.c > index dd7e7834..2052e864 100644 > --- a/x86/apic.c > +++ b/x86/apic.c > @@ -480,6 +480,13 @@ static void test_apic_timer_one_shot(void) > uint64_t tsc1, tsc2; > static const uint32_t interval = 0x10000; > > + /* > + * clear TMICT to disable any enabled but masked local timer. > + * Otherwise timer interrupt may occur after lvtt_handler() is > + * set as handler and **before** TDCR or TIMCT is set to new value, > + * lead this test failure. > + */ > + apic_write(APIC_TMICT, 0); > #define APIC_LVT_TIMER_VECTOR (0xee) > > handle_irq(APIC_LVT_TIMER_VECTOR, lvtt_handler); > > base-commit: 9cab58249f98adc451933530fd7e618e1856eb94
On Mon, Apr 29, 2024 at 03:40:20PM +0800, Xiaoyao Li wrote: > On 4/28/2024 10:29 AM, Yao Yuan wrote: > > Explicitly clear TMICT to avoid test_apic_timer_one_shot() > > negative failure. > > > > Clear TMICT to disable any enabled but masked local timer. > > Otherwise timer interrupt may occur after lvtt_handler() is > > set as handler and **before** TDCR or TIMCT is set to new > > value, lead this test failure. Log comes from UEFI mode: > > The exact reason is that the new value written to APIC_LVTT unmasks the > timer interrupt and if TMICT has non-zero and small value configured before > (by firmware or whatever) it may get an additional one-shot timer interrupt > before new TMICT being programmed. How about: Clear TMICT to disable any enabled but masked local timer. Local timer interrupt is unmasked when configure APIC_LVTT for the testing, it may get an additional one-shot timer interrupt if TMICT has non-zero and small value configured before (by fimrware or whatever) new TMICT being programmed. > > > PASS: PV IPIs testing > > PASS: pending nmi > > > Got local timer intr before write to TDCR / TMICT > > old tmict:0x989680 old lvtt:0x30020 tsc2 - tsc1 = 0xb68 > > ^^^^^^^^ ^^^^^^^ > > It took me a while to figure out the above 2 two lines are added for debug > yourself. > > FAIL: APIC LVT timer one shot > > > > Fixes: 9f815b293961 ("x86: apic: add LVT timer test") > > Signed-off-by: Yao Yuan <yuan.yao@intel.com> > > --- > > x86/apic.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/x86/apic.c b/x86/apic.c > > index dd7e7834..2052e864 100644 > > --- a/x86/apic.c > > +++ b/x86/apic.c > > @@ -480,6 +480,13 @@ static void test_apic_timer_one_shot(void) > > uint64_t tsc1, tsc2; > > static const uint32_t interval = 0x10000; > > + /* > > + * clear TMICT to disable any enabled but masked local timer. > > + * Otherwise timer interrupt may occur after lvtt_handler() is > > + * set as handler and **before** TDCR or TIMCT is set to new value, > > + * lead this test failure. > > + */ > > + apic_write(APIC_TMICT, 0); > > #define APIC_LVT_TIMER_VECTOR (0xee) > > handle_irq(APIC_LVT_TIMER_VECTOR, lvtt_handler); > > > > base-commit: 9cab58249f98adc451933530fd7e618e1856eb94 >
diff --git a/x86/apic.c b/x86/apic.c index dd7e7834..2052e864 100644 --- a/x86/apic.c +++ b/x86/apic.c @@ -480,6 +480,13 @@ static void test_apic_timer_one_shot(void) uint64_t tsc1, tsc2; static const uint32_t interval = 0x10000; + /* + * clear TMICT to disable any enabled but masked local timer. + * Otherwise timer interrupt may occur after lvtt_handler() is + * set as handler and **before** TDCR or TIMCT is set to new value, + * lead this test failure. + */ + apic_write(APIC_TMICT, 0); #define APIC_LVT_TIMER_VECTOR (0xee) handle_irq(APIC_LVT_TIMER_VECTOR, lvtt_handler);
Explicitly clear TMICT to avoid test_apic_timer_one_shot() negative failure. Clear TMICT to disable any enabled but masked local timer. Otherwise timer interrupt may occur after lvtt_handler() is set as handler and **before** TDCR or TIMCT is set to new value, lead this test failure. Log comes from UEFI mode: PASS: PV IPIs testing PASS: pending nmi Got local timer intr before write to TDCR / TMICT old tmict:0x989680 old lvtt:0x30020 tsc2 - tsc1 = 0xb68 ^^^^^^^^ ^^^^^^^ FAIL: APIC LVT timer one shot Fixes: 9f815b293961 ("x86: apic: add LVT timer test") Signed-off-by: Yao Yuan <yuan.yao@intel.com> --- x86/apic.c | 7 +++++++ 1 file changed, 7 insertions(+) base-commit: 9cab58249f98adc451933530fd7e618e1856eb94