[Adeos-main] [PATCH 4/5] remove legacy code

[Adeos-main] [PATCH 4/5] remove legacy code

[Jan Kiszka]

[-- Attachment #1.1: Type: text/plain, Size: 469 bytes --]

This patch gets rid of code that is no longer needed or valid:

 - x86_64 APIC frequency is now delivered via ipipe_request_tickdev
 - __ipipe_tick_irq is maintained via ipipe_update_tick_evtdev also on
   x86_64
 - global_clock_event is no longer needed by ipipe_32.c
 - ipipe_tune_timer was dead code in x86_64 (there was no prototype in
   whatever header), and this arch does not really have a legacy code
   base to maintain, so kill this function

Jan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: remove-legacy.patch --]
[-- Type: text/x-patch; name="remove-legacy.patch", Size: 4889 bytes --]

---
 arch/x86/kernel/apic_64.c       |    6 ------
 arch/x86/kernel/ipipe_32.c      |    2 --
 arch/x86/kernel/ipipe_64.c      |   10 +---------
 include/asm-x86/ipipe_64.h      |   21 +++++++++++----------
 include/asm-x86/ipipe_base_64.h |    4 ----
 5 files changed, 12 insertions(+), 31 deletions(-)

Index: linux-2.6.24-rc6-xeno/arch/x86/kernel/apic_64.c
===================================================================
--- linux-2.6.24-rc6-xeno.orig/arch/x86/kernel/apic_64.c
+++ linux-2.6.24-rc6-xeno/arch/x86/kernel/apic_64.c
@@ -939,9 +939,6 @@ static void __init calibrate_APIC_clock(
 
 	printk(KERN_INFO "Detected %d.%03d MHz APIC timer.\n",
 		result / 1000 / 1000, result / 1000 % 1000);
-#ifdef CONFIG_IPIPE
-	__ipipe_apic_timer_freq = result;
-#endif
 
 	/* Calculate the scaled math multiplication factor */
 	lapic_clockevent.mult = div_sc(result, NSEC_PER_SEC, 32);
@@ -970,9 +967,6 @@ void __init setup_boot_APIC_clock (void)
 	}
 
 	printk(KERN_INFO "Using local APIC timer interrupts.\n");
-#ifdef CONFIG_IPIPE
-	__ipipe_tick_irq = ipipe_apic_vector_irq(LOCAL_TIMER_VECTOR);
-#endif
 	calibrate_APIC_clock();
 
 	/*
Index: linux-2.6.24-rc6-xeno/arch/x86/kernel/ipipe_64.c
===================================================================
--- linux-2.6.24-rc6-xeno.orig/arch/x86/kernel/ipipe_64.c
+++ linux-2.6.24-rc6-xeno/arch/x86/kernel/ipipe_64.c
@@ -46,8 +46,7 @@
 
 asmlinkage void preempt_schedule_irq(void);
 
-int __ipipe_tick_irq;
-unsigned long __ipipe_apic_timer_freq;
+int __ipipe_tick_irq = TIMER_IRQ;
 
 DEFINE_PER_CPU(struct pt_regs, __ipipe_tick_regs);
 
@@ -102,12 +101,6 @@ int ipipe_get_sysinfo(struct ipipe_sysin
 	return 0;
 }
 
-int ipipe_tune_timer(unsigned long ns, int flags)
-{
-	printk(KERN_WARNING "I-pipe: %s is deprecated - no action taken\n", __FUNCTION__);
-	return -ENOSYS;
-}
-
 asmlinkage unsigned int do_IRQ(struct pt_regs *regs);
 asmlinkage void smp_apic_timer_interrupt(struct pt_regs *regs);
 asmlinkage void smp_spurious_interrupt(struct pt_regs *regs);
@@ -792,7 +785,6 @@ EXPORT_SYMBOL(ipipe_critical_enter);
 EXPORT_SYMBOL(ipipe_critical_exit);
 EXPORT_SYMBOL(ipipe_trigger_irq);
 EXPORT_SYMBOL(ipipe_get_sysinfo);
-EXPORT_SYMBOL(ipipe_tune_timer);
 
 EXPORT_SYMBOL_GPL(irq_desc);
 struct task_struct *__switch_to(struct task_struct *prev_p,
Index: linux-2.6.24-rc6-xeno/include/asm-x86/ipipe_64.h
===================================================================
--- linux-2.6.24-rc6-xeno.orig/include/asm-x86/ipipe_64.h
+++ linux-2.6.24-rc6-xeno/include/asm-x86/ipipe_64.h
@@ -79,13 +79,6 @@ extern unsigned cpu_khz;
 #define ipipe_tsc2ns(t)	(((t) * 1000UL) / (ipipe_cpu_freq() / 1000000UL))
 #define ipipe_tsc2us(t)	((t) / (ipipe_cpu_freq() / 1000000UL))
 
-/*
- * The following interface will be deprecated once generic clockevents
- * are supported by this architecture, at which point
- * ipipe_request_tickdev() should be used instead.
- */
-extern unsigned long __ipipe_apic_timer_freq;
-
 /* Private interface -- Internal use only */
 
 #define __ipipe_check_platform()	do { } while(0)
@@ -125,6 +118,15 @@ static inline unsigned long __ipipe_ffnz
       return ul;
 }
 
+#define ipipe_update_tick_evtdev(evtdev)				\
+	do {								\
+		if (strcmp((evtdev)->name, "lapic") == 0)		\
+			__ipipe_tick_irq =				\
+				ipipe_apic_vector_irq(LOCAL_TIMER_VECTOR); \
+		else							\
+			__ipipe_tick_irq = TIMER_IRQ;			\
+	} while (0)
+
 struct irq_desc;
 
 void __ipipe_ack_edge_irq(unsigned irq, struct irq_desc *desc);
@@ -236,10 +238,9 @@ int __ipipe_check_tickdev(const char *de
 
 #else /* !CONFIG_IPIPE */
 
-#define task_hijacked(p)	0
+#define ipipe_update_tick_evtdev(evtdev)	do { } while (0)
+#define task_hijacked(p)			0
 
 #endif /* CONFIG_IPIPE */
 
-#define ipipe_update_tick_evtdev(evtdev)	do { } while (0)
-
 #endif	/* !__X86_IPIPE_64_H */
Index: linux-2.6.24-rc6-xeno/include/asm-x86/ipipe_base_64.h
===================================================================
--- linux-2.6.24-rc6-xeno.orig/include/asm-x86/ipipe_base_64.h
+++ linux-2.6.24-rc6-xeno/include/asm-x86/ipipe_base_64.h
@@ -182,8 +182,4 @@ static inline unsigned long __ipipe_test
 
 #endif /* !__ASSEMBLY__ */
 
-/* Arch-dependent features */
-
-#define __IPIPE_FEATURE_APIC_TIMER_FREQ  1
-
 #endif	/* !__X86_IPIPE_BASE_64_H */
Index: linux-2.6.24-rc6-xeno/arch/x86/kernel/ipipe_32.c
===================================================================
--- linux-2.6.24-rc6-xeno.orig/arch/x86/kernel/ipipe_32.c
+++ linux-2.6.24-rc6-xeno/arch/x86/kernel/ipipe_32.c
@@ -48,8 +48,6 @@
 #include <mach_ipi.h>
 #endif	/* CONFIG_X86_LOCAL_APIC */
 
-extern struct clock_event_device *global_clock_event;
-
 int __ipipe_tick_irq = TIMER_IRQ;
 
 DEFINE_PER_CPU(struct pt_regs, __ipipe_tick_regs);

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

Re: [Adeos-main] [PATCH 4/5] remove legacy code

[Philippe Gerum]

Jan Kiszka wrote:
> This patch gets rid of code that is no longer needed or valid:
> 
>  - x86_64 APIC frequency is now delivered via ipipe_request_tickdev

Ack.

>  - __ipipe_tick_irq is maintained via ipipe_update_tick_evtdev also on
>    x86_64

Client RTOS may/do assume that timing is always delivered through the
local APIC timer on x86_64. We could merge that and allow the 8253 to be
used as the timer device, but this would not work in practice.

>  - global_clock_event is no longer needed by ipipe_32.c

Ack.

>  - ipipe_tune_timer was dead code in x86_64 (there was no prototype in
>    whatever header), and this arch does not really have a legacy code
>    base to maintain, so kill this function
> 

Ack.

-- 
Philippe.

Re: [Adeos-main] [PATCH 4/5] remove legacy code

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

Philippe Gerum wrote:
> Jan Kiszka wrote:
>> This patch gets rid of code that is no longer needed or valid:
>>
>>  - x86_64 APIC frequency is now delivered via ipipe_request_tickdev
> 
> Ack.
> 
>>  - __ipipe_tick_irq is maintained via ipipe_update_tick_evtdev also on
>>    x86_64
> 
> Client RTOS may/do assume that timing is always delivered through the
> local APIC timer on x86_64. We could merge that and allow the 8253 to be
> used as the timer device, but this would not work in practice.

For forget HPET. There is no wiring of the timer IRQ to the APIC
anymore. The result is that there is no difference between x86_32 and
x86_64 anymore, and that's what the code reflects.

If the client RTOS is not yet fully prepared for this, this has to be
fixed. And only if there is no valid Linux configuration (e.g. HPET off)
that results in the same timer assignment as with 2.6.23, we need a
workaround at I-pipe level IMHO.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Adeos-main] [PATCH 4/5] remove legacy code

[Philippe Gerum]

Jan Kiszka wrote:
> Philippe Gerum wrote:
>> Jan Kiszka wrote:
>>> This patch gets rid of code that is no longer needed or valid:
>>>
>>>  - x86_64 APIC frequency is now delivered via ipipe_request_tickdev
>> Ack.
>>
>>>  - __ipipe_tick_irq is maintained via ipipe_update_tick_evtdev also on
>>>    x86_64
>> Client RTOS may/do assume that timing is always delivered through the
>> local APIC timer on x86_64. We could merge that and allow the 8253 to be
>> used as the timer device, but this would not work in practice.
> 
> For forget HPET. There is no wiring of the timer IRQ to the APIC
> anymore.

My point is that whatever timer IRQs the kernel routes to INT0, be them
from the HPET in legacy route replacement mode or from the i8253 with
some former routing, we currently rely on the local APIC timer vector.
No matter what, HPET it's just unsupported by Xenomai right now, so we
can't see any IRQ0 registered as our tick device interrupt.

If your point is about allowing HPET to be used, then your patch
currently expects the HPET to be used in legacy replacement route mode,
which is no more the default setup. In short, TIMER_IRQ may not be the
right -constant- value in the common case.

 The result is that there is no difference between x86_32 and
> x86_64 anymore, and that's what the code reflects.
> 
> If the client RTOS is not yet fully prepared for this, this has to be
> fixed. And only if there is no valid Linux configuration (e.g. HPET off)
> that results in the same timer assignment as with 2.6.23, we need a
> workaround at I-pipe level IMHO.
> 

Again, I see no basic issue in merging that patch, but the message is:
Xenomai still has to support more timer devices than the local APIC one
on x86_64 (e.g. HPET), before this patch has any real upside for it. In
short, it's ok with me, but I feel that we are still half-way from the
complete support.

-- 
Philippe.

Re: [Adeos-main] [PATCH 4/5] remove legacy code

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2513 bytes --]

Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Philippe Gerum wrote:
>>> Jan Kiszka wrote:
>>>> This patch gets rid of code that is no longer needed or valid:
>>>>
>>>>  - x86_64 APIC frequency is now delivered via ipipe_request_tickdev
>>> Ack.
>>>
>>>>  - __ipipe_tick_irq is maintained via ipipe_update_tick_evtdev also on
>>>>    x86_64
>>> Client RTOS may/do assume that timing is always delivered through the
>>> local APIC timer on x86_64. We could merge that and allow the 8253 to be
>>> used as the timer device, but this would not work in practice.
>> For forget HPET. There is no wiring of the timer IRQ to the APIC
>> anymore.
> 
> My point is that whatever timer IRQs the kernel routes to INT0, be them
> from the HPET in legacy route replacement mode or from the i8253 with
> some former routing, we currently rely on the local APIC timer vector.
> No matter what, HPET it's just unsupported by Xenomai right now, so we
> can't see any IRQ0 registered as our tick device interrupt.

__ipipe_tick_irq, as it is used now, is reflecting Linux' tick IRQ, not
necessarily the one that Xenomai prefers.

> 
> If your point is about allowing HPET to be used, then your patch
> currently expects the HPET to be used in legacy replacement route mode,
> which is no more the default setup. In short, TIMER_IRQ may not be the

static struct clock_event_device hpet_clockevent = {
...
        .irq            = 0,

which is TIMER_IRQ.

> right -constant- value in the common case.
> 
>  The result is that there is no difference between x86_32 and
>> x86_64 anymore, and that's what the code reflects.
>>
>> If the client RTOS is not yet fully prepared for this, this has to be
>> fixed. And only if there is no valid Linux configuration (e.g. HPET off)
>> that results in the same timer assignment as with 2.6.23, we need a
>> workaround at I-pipe level IMHO.
>>
> 
> Again, I see no basic issue in merging that patch, but the message is:
> Xenomai still has to support more timer devices than the local APIC one
> on x86_64 (e.g. HPET), before this patch has any real upside for it. In
> short, it's ok with me, but I feel that we are still half-way from the
> complete support.

As long as the APIC is available to Xenomai, we should no longer worry
about Linux using some other clockevent device. At least in theory. But
I will carefully check again during this week if there are any
regressions or .config limitation related to these hunks.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

Re: [Adeos-main] [PATCH 4/5] remove legacy code

[Philippe Gerum]

Jan Kiszka wrote:
> Philippe Gerum wrote:
>> Jan Kiszka wrote:
>>> Philippe Gerum wrote:
>>>> Jan Kiszka wrote:
>>>>> This patch gets rid of code that is no longer needed or valid:
>>>>>
>>>>>  - x86_64 APIC frequency is now delivered via ipipe_request_tickdev
>>>> Ack.
>>>>
>>>>>  - __ipipe_tick_irq is maintained via ipipe_update_tick_evtdev also on
>>>>>    x86_64
>>>> Client RTOS may/do assume that timing is always delivered through the
>>>> local APIC timer on x86_64. We could merge that and allow the 8253 to be
>>>> used as the timer device, but this would not work in practice.
>>> For forget HPET. There is no wiring of the timer IRQ to the APIC
>>> anymore.
>> My point is that whatever timer IRQs the kernel routes to INT0, be them
>> from the HPET in legacy route replacement mode or from the i8253 with
>> some former routing, we currently rely on the local APIC timer vector.
>> No matter what, HPET it's just unsupported by Xenomai right now, so we
>> can't see any IRQ0 registered as our tick device interrupt.
> 
> __ipipe_tick_irq, as it is used now, is reflecting Linux' tick IRQ, not
> necessarily the one that Xenomai prefers.
>

However, as a matter of fact, they are currently logically coupled. Let
me detail my point, considering it from the Xenomai viewpoint, depending
on the combinations of the two options we have been referring to:

** !CONFIG_X86_LOCAL_APIC && CONFIG_HPET_TIMER

=> Xenomai interposes on IRQ0 as normally generated by the HPET in LRR
mode, but unfortunately makes the false assumption that the PIT is the
active timer device and attempts to program it while managing timers
in the real-time space, so no IRQ0 is ever generated. This leads to a
complete lossage of clock event interrupts for both Linux and Xenomai.
This is why we currently prevent such configuration from being set, at
Kconfig-level.

** CONFIG_X86_LOCAL_APIC && CONFIG_HPET_TIMER

=> Xenomai always requests the LAPIC for timing because
CONFIG_X86_LOCAL_APIC is defined, and always emulates host ticks by
propagating forged interrupts to the LOCAL_TIMER_VECTOR (since v2.4.x,
at least). Then, we have to consider how Linux's evtdev is set:
o if evtdev == HPET (and of course LAPIC is not dummy), then we would
have two Linux timer ticks per period (one emulated by Xenomai to the
local APIC timer vector, one real from the HPET to IRQ0) which would
obviously be wrong,
o if evtdev == LAPIC, then things would work, Xenomai would both handle
the host and the real-time ticking by managing the timer device for both
domains.

** Therefore, a valid setup is either:

CONFIG_X86_LOCAL_APIC && CONFIG_HPET_TIMER, if evtdev == LAPIC
  or
CONFIG_X86_LOCAL_APIC && !CONFIG_HPET_TIMER, implying evtdev == LAPIC

=> In which cases, Linux's clock evtdev must be LAPIC. If so, testing
about the current evtdev name like your patch does, as if it could be
different from "lapic", and in consequence as if we would allow a
different interrupt source for the Linux tick than the LAPIC when
CONFIG_X86_LOCAL_APIC is set, is _currently_ useless because not yet
implemented at Xenomai level, even if the idea underlying the patch is
generally right.

For this reason, I earlier merged your patch into the x86_32 section
since we may have !CONFIG_X86_LOCAL_APIC, in which case the PIT may be
used and we have to consider __ipipe_tick_irq as a true variable.
However, I did not merge this change on purpose yet into the x86_64
section, mainly as a reminder that we currently assume that
CONFIG_X86_LOCAL_APIC is always enabled on this arch, which raises the
issues above.

Fortunately for us, the LAPIC has a better rating as a clock event
device than the HPET, provided the Linux calibrator is happy with its
stability. Therefore, we should have evtdev == LAPIC most of the time,
at least with "decent x86 hardware" (which depending one one's mood,
sometimes may look like a nice oxymoron).

This is why I basically agree with your patch because indeed, Xenomai
will have to be made HPET-compliant at some point in time, but at the
same time, I have to raise the concern that this patch won't be enough
for proper timing in the _current_ Xenomai situation.

>> If your point is about allowing HPET to be used, then your patch
>> currently expects the HPET to be used in legacy replacement route mode,
>> which is no more the default setup. In short, TIMER_IRQ may not be the
> 
> static struct clock_event_device hpet_clockevent = {
> ...
>         .irq            = 0,
> 
> which is TIMER_IRQ.
>

Yes, sorry for the noise, I was wrong. Using the HPET in LRR mode is
still clearly wired into the 2.6.24 code.

 >
> As long as the APIC is available to Xenomai, we should no longer worry
> about Linux using some other clockevent device.

Provided we don't relay the host tick when Xenomai grabs the LAPIC
whilst Linux is also notified from incoming timer ticks by interrupts
from another device.

 At least in theory. But
> I will carefully check again during this week if there are any
> regressions or .config limitation related to these hunks.
>

Great, thanks.

-- 
Philippe.

Re: [Adeos-main] [PATCH 4/5] remove legacy code

[Philippe Gerum]

Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Philippe Gerum wrote:
>>> Jan Kiszka wrote:
>>>> Philippe Gerum wrote:
>>>>> Jan Kiszka wrote:
>>>>>> This patch gets rid of code that is no longer needed or valid:
>>>>>>
>>>>>>  - x86_64 APIC frequency is now delivered via ipipe_request_tickdev
>>>>> Ack.
>>>>>
>>>>>>  - __ipipe_tick_irq is maintained via ipipe_update_tick_evtdev also on
>>>>>>    x86_64
>>>>> Client RTOS may/do assume that timing is always delivered through the
>>>>> local APIC timer on x86_64. We could merge that and allow the 8253 to be
>>>>> used as the timer device, but this would not work in practice.
>>>> For forget HPET. There is no wiring of the timer IRQ to the APIC
>>>> anymore.
>>> My point is that whatever timer IRQs the kernel routes to INT0, be them
>>> from the HPET in legacy route replacement mode or from the i8253 with
>>> some former routing, we currently rely on the local APIC timer vector.
>>> No matter what, HPET it's just unsupported by Xenomai right now, so we
>>> can't see any IRQ0 registered as our tick device interrupt.
>> __ipipe_tick_irq, as it is used now, is reflecting Linux' tick IRQ, not
>> necessarily the one that Xenomai prefers.
>>
> 
> However, as a matter of fact, they are currently logically coupled. Let
> me detail my point, considering it from the Xenomai viewpoint, depending
> on the combinations of the two options we have been referring to:
> 
> ** !CONFIG_X86_LOCAL_APIC && CONFIG_HPET_TIMER
> 
> => Xenomai interposes on IRQ0 as normally generated by the HPET in LRR
> mode, but unfortunately makes the false assumption that the PIT is the
> active timer device and attempts to program it while managing timers
> in the real-time space, so no IRQ0 is ever generated. This leads to a
> complete lossage of clock event interrupts for both Linux and Xenomai.
> This is why we currently prevent such configuration from being set, at
> Kconfig-level.
> 
> ** CONFIG_X86_LOCAL_APIC && CONFIG_HPET_TIMER
> 
> => Xenomai always requests the LAPIC for timing because
> CONFIG_X86_LOCAL_APIC is defined, and always emulates host ticks by
> propagating forged interrupts to the LOCAL_TIMER_VECTOR (since v2.4.x,
> at least). Then, we have to consider how Linux's evtdev is set:
> o if evtdev == HPET (and of course LAPIC is not dummy), then we would
> have two Linux timer ticks per period (one emulated by Xenomai to the
> local APIC timer vector, one real from the HPET to IRQ0)

Eek, ok, my brain is definitely in low power mode. if we don't use the
same clock device than Linux, then we should neither emulate the
periodic tick, nor receive any host tick programming requests anyway
from Linux. Ok, forget about this. Let's be happy, and merge this.

We should only have to take care of the remaining issue #1.

-- 
Philippe.