[Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Jan Kiszka]

If we enter __ipipe_handle_exception over a non-root domain and leave it
due to migration in the event handler over root, we must not restore the
root domain state so far saved on entry. This caused subtle pipeline
state corruptions. Instead, only save and restore them if we were
entering over root.

However, the x86-32 regs.flags fixup is required nevertheless to take
care of mismatches between the root domain state and the hardware flags
on entry. That may happen if we fault in the iret path. But also in this
case we must not restore an invalid root domain state. So if we entered
over non-root, pick up the input for __fixup_if from the root domain
after running the ipipe handler.

Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
---

Next try. But this time I think I finally understood what scenario
__fixup_if is actually fixing. Please correct me if I'm still missing
one.

 arch/x86/kernel/ipipe.c |   81 +++++++++++++++++++++++++++++-----------------
 1 files changed, 51 insertions(+), 30 deletions(-)

diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c
index 4442d96..b471355 100644
--- a/arch/x86/kernel/ipipe.c
+++ b/arch/x86/kernel/ipipe.c
@@ -702,19 +702,23 @@ static int __ipipe_xlate_signo[] = {
 
 int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
 {
-	unsigned long flags;
-
-	/* Pick up the root domain state of the interrupted context. */
-	local_save_flags(flags);
+	bool root_entry = false;
+	unsigned long flags = 0;
 
 	if (ipipe_root_domain_p) {
-		/*
-		 * Replicate hw interrupt state into the virtual mask before
-		 * calling the I-pipe event handler over the root domain. Also
-		 * required later when calling the Linux exception handler.
-		 */
-		if (irqs_disabled_hw())
+		root_entry = true;
+
+		local_save_flags(flags);
+
+		if (irqs_disabled_hw()) {
+			/*
+			 * Replicate hw interrupt state into the virtual mask
+			 * before calling the I-pipe event handler over the
+			 * root domain. Also required later when calling the
+			 * Linux exception handler.
+			 */
 			local_irq_disable();
+		}
 	}
 #ifdef CONFIG_KGDB
 	/* catch exception KGDB is interested in over non-root domains */
@@ -725,18 +729,22 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
 #endif /* CONFIG_KGDB */
 
 	if (unlikely(ipipe_trap_notify(vector, regs))) {
-		local_irq_restore_nosync(flags);
+		if (root_entry)
+			local_irq_restore_nosync(flags);
 		return 1;
 	}
 
-	/*
-	 * 32-bit: In case we migrated to root domain inside the event
-	 * handler, restore the original IF from exception entry as the
-	 * low-level return code will evaluate it.
-	 */
-	__fixup_if(raw_irqs_disabled_flags(flags), regs);
-
-	if (unlikely(!ipipe_root_domain_p)) {
+	if (likely(ipipe_root_domain_p)) {
+		/*
+		 * 32-bit: In case we faulted in the iret path, regs.flags do
+		 * not match the root domain state as the low-level return
+		 * code will evaluate it. Fix this up, either by the root
+		 * state sampled on entry or, if we migrated to root, with the
+		 * current state.
+		 */
+		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
+					raw_irqs_disabled(), regs);
+	} else {
 		/* Detect unhandled faults over non-root domains. */
 		struct ipipe_domain *ipd = ipipe_current_domain;
 
@@ -770,21 +778,29 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
 	 * Relevant for 64-bit: Restore root domain state as the low-level
 	 * return code will not align it to regs.flags.
 	 */
-	local_irq_restore_nosync(flags);
+	if (root_entry)
+		local_irq_restore_nosync(flags);
 
 	return 0;
 }
 
 int __ipipe_divert_exception(struct pt_regs *regs, int vector)
 {
-	unsigned long flags;
-
-	/* Same root state handling as in __ipipe_handle_exception. */
-	local_save_flags(flags);
+	bool root_entry = false;
+	unsigned long flags = 0;
 
 	if (ipipe_root_domain_p) {
-		if (irqs_disabled_hw())
+		root_entry = true;
+
+		local_save_flags(flags);
+
+		if (irqs_disabled_hw()) {
+			/*
+			 * Same root state handling as in
+			 * __ipipe_handle_exception.
+			 */
 			local_irq_disable();
+		}
 	}
 #ifdef CONFIG_KGDB
 	/* catch int1 and int3 over non-root domains */
@@ -804,16 +820,21 @@ int __ipipe_divert_exception(struct pt_regs *regs, int vector)
 #endif /* CONFIG_KGDB */
 
 	if (unlikely(ipipe_trap_notify(vector, regs))) {
-		local_irq_restore_nosync(flags);
+		if (root_entry)
+			local_irq_restore_nosync(flags);
 		return 1;
 	}
 
+	if (likely(ipipe_root_domain_p)) {
+		/* see __ipipe_handle_exception */
+		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
+					raw_irqs_disabled(), regs);
+	}
+
 	/*
-	 * 32-bit: Due to possible migration inside the event handler, we have
-	 * to restore IF so that low-level return code sets the root domain
-	 * state correctly.
+	 * No need to restore root state in the 64-bit case, the Linux handler
+	 * and the return code will take care of it.
 	 */
-	__fixup_if(raw_irqs_disabled_flags(flags), regs);
 
 	return 0;
 }
-- 
1.6.0.2

Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Wolfgang Mauerer]

Jan Kiszka wrote:
> If we enter __ipipe_handle_exception over a non-root domain and leave it
> due to migration in the event handler over root, we must not restore the
> root domain state so far saved on entry. This caused subtle pipeline
> state corruptions. Instead, only save and restore them if we were
> entering over root.
> 
> However, the x86-32 regs.flags fixup is required nevertheless to take
> care of mismatches between the root domain state and the hardware flags
> on entry. That may happen if we fault in the iret path. But also in this
> case we must not restore an invalid root domain state. So if we entered
> over non-root, pick up the input for __fixup_if from the root domain
> after running the ipipe handler.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
> ---
> 
> Next try. But this time I think I finally understood what scenario
> __fixup_if is actually fixing. Please correct me if I'm still missing
> one.

looks good - it works for my test cases and solves the problems with
the hw/pipeline state mismatch during early bootup. But do you happen
to have any scenario at hand with ipipe_domain_root_p && !root_entry?
Couldn't trigger this one yet so only the raw_irqs_disabled_flags
fixup is excercised, though I guess it can't do any harm to really
ensure that the explanation fits reality this time...

Wolfgang

> 
>  arch/x86/kernel/ipipe.c |   81 +++++++++++++++++++++++++++++-----------------
>  1 files changed, 51 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c
> index 4442d96..b471355 100644
> --- a/arch/x86/kernel/ipipe.c
> +++ b/arch/x86/kernel/ipipe.c
> @@ -702,19 +702,23 @@ static int __ipipe_xlate_signo[] = {
>  
>  int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
>  {
> -	unsigned long flags;
> -
> -	/* Pick up the root domain state of the interrupted context. */
> -	local_save_flags(flags);
> +	bool root_entry = false;
> +	unsigned long flags = 0;
>  
>  	if (ipipe_root_domain_p) {
> -		/*
> -		 * Replicate hw interrupt state into the virtual mask before
> -		 * calling the I-pipe event handler over the root domain. Also
> -		 * required later when calling the Linux exception handler.
> -		 */
> -		if (irqs_disabled_hw())
> +		root_entry = true;
> +
> +		local_save_flags(flags);
> +
> +		if (irqs_disabled_hw()) {
> +			/*
> +			 * Replicate hw interrupt state into the virtual mask
> +			 * before calling the I-pipe event handler over the
> +			 * root domain. Also required later when calling the
> +			 * Linux exception handler.
> +			 */
>  			local_irq_disable();
> +		}
>  	}
>  #ifdef CONFIG_KGDB
>  	/* catch exception KGDB is interested in over non-root domains */
> @@ -725,18 +729,22 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
>  #endif /* CONFIG_KGDB */
>  
>  	if (unlikely(ipipe_trap_notify(vector, regs))) {
> -		local_irq_restore_nosync(flags);
> +		if (root_entry)
> +			local_irq_restore_nosync(flags);
>  		return 1;
>  	}
>  
> -	/*
> -	 * 32-bit: In case we migrated to root domain inside the event
> -	 * handler, restore the original IF from exception entry as the
> -	 * low-level return code will evaluate it.
> -	 */
> -	__fixup_if(raw_irqs_disabled_flags(flags), regs);
> -
> -	if (unlikely(!ipipe_root_domain_p)) {
> +	if (likely(ipipe_root_domain_p)) {
> +		/*
> +		 * 32-bit: In case we faulted in the iret path, regs.flags do
> +		 * not match the root domain state as the low-level return
> +		 * code will evaluate it. Fix this up, either by the root
> +		 * state sampled on entry or, if we migrated to root, with the
> +		 * current state.
> +		 */
> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
> +					raw_irqs_disabled(), regs);
> +	} else {
>  		/* Detect unhandled faults over non-root domains. */
>  		struct ipipe_domain *ipd = ipipe_current_domain;
>  
> @@ -770,21 +778,29 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
>  	 * Relevant for 64-bit: Restore root domain state as the low-level
>  	 * return code will not align it to regs.flags.
>  	 */
> -	local_irq_restore_nosync(flags);
> +	if (root_entry)
> +		local_irq_restore_nosync(flags);
>  
>  	return 0;
>  }
>  
>  int __ipipe_divert_exception(struct pt_regs *regs, int vector)
>  {
> -	unsigned long flags;
> -
> -	/* Same root state handling as in __ipipe_handle_exception. */
> -	local_save_flags(flags);
> +	bool root_entry = false;
> +	unsigned long flags = 0;
>  
>  	if (ipipe_root_domain_p) {
> -		if (irqs_disabled_hw())
> +		root_entry = true;
> +
> +		local_save_flags(flags);
> +
> +		if (irqs_disabled_hw()) {
> +			/*
> +			 * Same root state handling as in
> +			 * __ipipe_handle_exception.
> +			 */
>  			local_irq_disable();
> +		}
>  	}
>  #ifdef CONFIG_KGDB
>  	/* catch int1 and int3 over non-root domains */
> @@ -804,16 +820,21 @@ int __ipipe_divert_exception(struct pt_regs *regs, int vector)
>  #endif /* CONFIG_KGDB */
>  
>  	if (unlikely(ipipe_trap_notify(vector, regs))) {
> -		local_irq_restore_nosync(flags);
> +		if (root_entry)
> +			local_irq_restore_nosync(flags);
>  		return 1;
>  	}
>  
> +	if (likely(ipipe_root_domain_p)) {
> +		/* see __ipipe_handle_exception */
> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
> +					raw_irqs_disabled(), regs);
> +	}
> +
>  	/*
> -	 * 32-bit: Due to possible migration inside the event handler, we have
> -	 * to restore IF so that low-level return code sets the root domain
> -	 * state correctly.
> +	 * No need to restore root state in the 64-bit case, the Linux handler
> +	 * and the return code will take care of it.
>  	 */
> -	__fixup_if(raw_irqs_disabled_flags(flags), regs);
>  
>  	return 0;
>  }

Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Jan Kiszka]

Wolfgang Mauerer wrote:
> Jan Kiszka wrote:
>> If we enter __ipipe_handle_exception over a non-root domain and leave it
>> due to migration in the event handler over root, we must not restore the
>> root domain state so far saved on entry. This caused subtle pipeline
>> state corruptions. Instead, only save and restore them if we were
>> entering over root.
>>
>> However, the x86-32 regs.flags fixup is required nevertheless to take
>> care of mismatches between the root domain state and the hardware flags
>> on entry. That may happen if we fault in the iret path. But also in this
>> case we must not restore an invalid root domain state. So if we entered
>> over non-root, pick up the input for __fixup_if from the root domain
>> after running the ipipe handler.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
>> ---
>>
>> Next try. But this time I think I finally understood what scenario
>> __fixup_if is actually fixing. Please correct me if I'm still missing
>> one.
> 
> looks good - it works for my test cases and solves the problems with
> the hw/pipeline state mismatch during early bootup. But do you happen
> to have any scenario at hand with ipipe_domain_root_p && !root_entry?
> Couldn't trigger this one yet so only the raw_irqs_disabled_flags
> fixup is excercised, though I guess it can't do any harm to really
> ensure that the explanation fits reality this time...

You mean non-root entry -> migration -> __fixup_if? In that case we pick
up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or
what do you mean?

> 
> Wolfgang
> 
>>  arch/x86/kernel/ipipe.c |   81 +++++++++++++++++++++++++++++-----------------
>>  1 files changed, 51 insertions(+), 30 deletions(-)
>>
>> diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c
>> index 4442d96..b471355 100644
>> --- a/arch/x86/kernel/ipipe.c
>> +++ b/arch/x86/kernel/ipipe.c
>> @@ -702,19 +702,23 @@ static int __ipipe_xlate_signo[] = {
>>  
>>  int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
>>  {
>> -	unsigned long flags;
>> -
>> -	/* Pick up the root domain state of the interrupted context. */
>> -	local_save_flags(flags);
>> +	bool root_entry = false;
>> +	unsigned long flags = 0;
>>  
>>  	if (ipipe_root_domain_p) {
>> -		/*
>> -		 * Replicate hw interrupt state into the virtual mask before
>> -		 * calling the I-pipe event handler over the root domain. Also
>> -		 * required later when calling the Linux exception handler.
>> -		 */
>> -		if (irqs_disabled_hw())
>> +		root_entry = true;
>> +
>> +		local_save_flags(flags);
>> +
>> +		if (irqs_disabled_hw()) {
>> +			/*
>> +			 * Replicate hw interrupt state into the virtual mask
>> +			 * before calling the I-pipe event handler over the
>> +			 * root domain. Also required later when calling the
>> +			 * Linux exception handler.
>> +			 */
>>  			local_irq_disable();
>> +		}
>>  	}
>>  #ifdef CONFIG_KGDB
>>  	/* catch exception KGDB is interested in over non-root domains */
>> @@ -725,18 +729,22 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
>>  #endif /* CONFIG_KGDB */
>>  
>>  	if (unlikely(ipipe_trap_notify(vector, regs))) {
>> -		local_irq_restore_nosync(flags);
>> +		if (root_entry)
>> +			local_irq_restore_nosync(flags);
>>  		return 1;
>>  	}
>>  
>> -	/*
>> -	 * 32-bit: In case we migrated to root domain inside the event
>> -	 * handler, restore the original IF from exception entry as the
>> -	 * low-level return code will evaluate it.
>> -	 */
>> -	__fixup_if(raw_irqs_disabled_flags(flags), regs);
>> -
>> -	if (unlikely(!ipipe_root_domain_p)) {
>> +	if (likely(ipipe_root_domain_p)) {
>> +		/*
>> +		 * 32-bit: In case we faulted in the iret path, regs.flags do
>> +		 * not match the root domain state as the low-level return
>> +		 * code will evaluate it. Fix this up, either by the root
>> +		 * state sampled on entry or, if we migrated to root, with the
>> +		 * current state.
>> +		 */
>> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>> +					raw_irqs_disabled(), regs);
>> +	} else {
>>  		/* Detect unhandled faults over non-root domains. */
>>  		struct ipipe_domain *ipd = ipipe_current_domain;
>>  
>> @@ -770,21 +778,29 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
>>  	 * Relevant for 64-bit: Restore root domain state as the low-level
>>  	 * return code will not align it to regs.flags.
>>  	 */
>> -	local_irq_restore_nosync(flags);
>> +	if (root_entry)
>> +		local_irq_restore_nosync(flags);
>>  
>>  	return 0;
>>  }
>>  
>>  int __ipipe_divert_exception(struct pt_regs *regs, int vector)
>>  {
>> -	unsigned long flags;
>> -
>> -	/* Same root state handling as in __ipipe_handle_exception. */
>> -	local_save_flags(flags);
>> +	bool root_entry = false;
>> +	unsigned long flags = 0;
>>  
>>  	if (ipipe_root_domain_p) {
>> -		if (irqs_disabled_hw())
>> +		root_entry = true;
>> +
>> +		local_save_flags(flags);
>> +
>> +		if (irqs_disabled_hw()) {
>> +			/*
>> +			 * Same root state handling as in
>> +			 * __ipipe_handle_exception.
>> +			 */
>>  			local_irq_disable();
>> +		}
>>  	}
>>  #ifdef CONFIG_KGDB
>>  	/* catch int1 and int3 over non-root domains */
>> @@ -804,16 +820,21 @@ int __ipipe_divert_exception(struct pt_regs *regs, int vector)
>>  #endif /* CONFIG_KGDB */
>>  
>>  	if (unlikely(ipipe_trap_notify(vector, regs))) {
>> -		local_irq_restore_nosync(flags);
>> +		if (root_entry)
>> +			local_irq_restore_nosync(flags);
>>  		return 1;
>>  	}
>>  
>> +	if (likely(ipipe_root_domain_p)) {
>> +		/* see __ipipe_handle_exception */
>> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>> +					raw_irqs_disabled(), regs);
>> +	}
>> +
>>  	/*
>> -	 * 32-bit: Due to possible migration inside the event handler, we have
>> -	 * to restore IF so that low-level return code sets the root domain
>> -	 * state correctly.
>> +	 * No need to restore root state in the 64-bit case, the Linux handler
>> +	 * and the return code will take care of it.
>>  	 */
>> -	__fixup_if(raw_irqs_disabled_flags(flags), regs);
>>  
>>  	return 0;
>>  }
> 

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Wolfgang Mauerer]

Jan Kiszka wrote:
> Wolfgang Mauerer wrote:
>> Jan Kiszka wrote:
>>> If we enter __ipipe_handle_exception over a non-root domain and leave it
>>> due to migration in the event handler over root, we must not restore the
>>> root domain state so far saved on entry. This caused subtle pipeline
>>> state corruptions. Instead, only save and restore them if we were
>>> entering over root.
>>>
>>> However, the x86-32 regs.flags fixup is required nevertheless to take
>>> care of mismatches between the root domain state and the hardware flags
>>> on entry. That may happen if we fault in the iret path. But also in this
>>> case we must not restore an invalid root domain state. So if we entered
>>> over non-root, pick up the input for __fixup_if from the root domain
>>> after running the ipipe handler.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
>>> ---
>>>
>>> Next try. But this time I think I finally understood what scenario
>>> __fixup_if is actually fixing. Please correct me if I'm still missing
>>> one.
>> looks good - it works for my test cases and solves the problems with
>> the hw/pipeline state mismatch during early bootup. But do you happen
>> to have any scenario at hand with ipipe_domain_root_p && !root_entry?
>> Couldn't trigger this one yet so only the raw_irqs_disabled_flags
>> fixup is excercised, though I guess it can't do any harm to really
>> ensure that the explanation fits reality this time...
> 
> You mean non-root entry -> migration -> __fixup_if? In that case we pick
> up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or
> what do you mean?

(...)
>>> -	if (unlikely(!ipipe_root_domain_p)) {
>>> +	if (likely(ipipe_root_domain_p)) {
>>> +		/*
>>> +		 * 32-bit: In case we faulted in the iret path, regs.flags do
>>> +		 * not match the root domain state as the low-level return
>>> +		 * code will evaluate it. Fix this up, either by the root
>>> +		 * state sampled on entry or, if we migrated to root, with the
>>> +		 * current state.
>>> +		 */
>>> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>>> +					raw_irqs_disabled(), regs);

I'm referring to the case that evaluates to
__fixup_if(raw_irqs_disabled(), regs); That is, something that
triggers

		if (!root_entry)
			do_something();

Could be that we're talking about to the same case, although I'm not
sure ;-)

Cheers, Wolfgang


>>> +	} else {
>>>  		/* Detect unhandled faults over non-root domains. */
>>>  		struct ipipe_domain *ipd = ipipe_current_domain;
>>>  
>>> @@ -770,21 +778,29 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
>>>  	 * Relevant for 64-bit: Restore root domain state as the low-level
>>>  	 * return code will not align it to regs.flags.
>>>  	 */
>>> -	local_irq_restore_nosync(flags);
>>> +	if (root_entry)
>>> +		local_irq_restore_nosync(flags);
>>>  
>>>  	return 0;
>>>  }
>>>  
>>>  int __ipipe_divert_exception(struct pt_regs *regs, int vector)
>>>  {
>>> -	unsigned long flags;
>>> -
>>> -	/* Same root state handling as in __ipipe_handle_exception. */
>>> -	local_save_flags(flags);
>>> +	bool root_entry = false;
>>> +	unsigned long flags = 0;
>>>  
>>>  	if (ipipe_root_domain_p) {
>>> -		if (irqs_disabled_hw())
>>> +		root_entry = true;
>>> +
>>> +		local_save_flags(flags);
>>> +
>>> +		if (irqs_disabled_hw()) {
>>> +			/*
>>> +			 * Same root state handling as in
>>> +			 * __ipipe_handle_exception.
>>> +			 */
>>>  			local_irq_disable();
>>> +		}
>>>  	}
>>>  #ifdef CONFIG_KGDB
>>>  	/* catch int1 and int3 over non-root domains */
>>> @@ -804,16 +820,21 @@ int __ipipe_divert_exception(struct pt_regs *regs, int vector)
>>>  #endif /* CONFIG_KGDB */
>>>  
>>>  	if (unlikely(ipipe_trap_notify(vector, regs))) {
>>> -		local_irq_restore_nosync(flags);
>>> +		if (root_entry)
>>> +			local_irq_restore_nosync(flags);
>>>  		return 1;
>>>  	}
>>>  
>>> +	if (likely(ipipe_root_domain_p)) {
>>> +		/* see __ipipe_handle_exception */
>>> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>>> +					raw_irqs_disabled(), regs);
>>> +	}
>>> +
>>>  	/*
>>> -	 * 32-bit: Due to possible migration inside the event handler, we have
>>> -	 * to restore IF so that low-level return code sets the root domain
>>> -	 * state correctly.
>>> +	 * No need to restore root state in the 64-bit case, the Linux handler
>>> +	 * and the return code will take care of it.
>>>  	 */
>>> -	__fixup_if(raw_irqs_disabled_flags(flags), regs);
>>>  
>>>  	return 0;
>>>  }
> 
> Jan
> 

Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Jan Kiszka]

Wolfgang Mauerer wrote:
> Jan Kiszka wrote:
>> Wolfgang Mauerer wrote:
>>> Jan Kiszka wrote:
>>>> If we enter __ipipe_handle_exception over a non-root domain and leave it
>>>> due to migration in the event handler over root, we must not restore the
>>>> root domain state so far saved on entry. This caused subtle pipeline
>>>> state corruptions. Instead, only save and restore them if we were
>>>> entering over root.
>>>>
>>>> However, the x86-32 regs.flags fixup is required nevertheless to take
>>>> care of mismatches between the root domain state and the hardware flags
>>>> on entry. That may happen if we fault in the iret path. But also in this
>>>> case we must not restore an invalid root domain state. So if we entered
>>>> over non-root, pick up the input for __fixup_if from the root domain
>>>> after running the ipipe handler.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
>>>> ---
>>>>
>>>> Next try. But this time I think I finally understood what scenario
>>>> __fixup_if is actually fixing. Please correct me if I'm still missing
>>>> one.
>>> looks good - it works for my test cases and solves the problems with
>>> the hw/pipeline state mismatch during early bootup. But do you happen
>>> to have any scenario at hand with ipipe_domain_root_p && !root_entry?
>>> Couldn't trigger this one yet so only the raw_irqs_disabled_flags
>>> fixup is excercised, though I guess it can't do any harm to really
>>> ensure that the explanation fits reality this time...
>> You mean non-root entry -> migration -> __fixup_if? In that case we pick
>> up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or
>> what do you mean?
> 
> (...)
>>>> -	if (unlikely(!ipipe_root_domain_p)) {
>>>> +	if (likely(ipipe_root_domain_p)) {
>>>> +		/*
>>>> +		 * 32-bit: In case we faulted in the iret path, regs.flags do
>>>> +		 * not match the root domain state as the low-level return
>>>> +		 * code will evaluate it. Fix this up, either by the root
>>>> +		 * state sampled on entry or, if we migrated to root, with the
>>>> +		 * current state.
>>>> +		 */
>>>> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>>>> +					raw_irqs_disabled(), regs);
> 
> I'm referring to the case that evaluates to
> __fixup_if(raw_irqs_disabled(), regs); That is, something that
> triggers
> 
> 		if (!root_entry)
> 			do_something();
> 
> Could be that we're talking about to the same case, although I'm not
> sure ;-)

Right, that's the case I described above. What problem do you precisely
see or what concerns to you have about the suggested behavior?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Wolfgang Mauerer]

Jan Kiszka wrote:
> Wolfgang Mauerer wrote:
>> Jan Kiszka wrote:
>>> Wolfgang Mauerer wrote:
>>>> Jan Kiszka wrote:
>>>>> If we enter __ipipe_handle_exception over a non-root domain and leave it
>>>>> due to migration in the event handler over root, we must not restore the
>>>>> root domain state so far saved on entry. This caused subtle pipeline
>>>>> state corruptions. Instead, only save and restore them if we were
>>>>> entering over root.
>>>>>
>>>>> However, the x86-32 regs.flags fixup is required nevertheless to take
>>>>> care of mismatches between the root domain state and the hardware flags
>>>>> on entry. That may happen if we fault in the iret path. But also in this
>>>>> case we must not restore an invalid root domain state. So if we entered
>>>>> over non-root, pick up the input for __fixup_if from the root domain
>>>>> after running the ipipe handler.
>>>>>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
>>>>> ---
>>>>>
>>>>> Next try. But this time I think I finally understood what scenario
>>>>> __fixup_if is actually fixing. Please correct me if I'm still missing
>>>>> one.
>>>> looks good - it works for my test cases and solves the problems with
>>>> the hw/pipeline state mismatch during early bootup. But do you happen
>>>> to have any scenario at hand with ipipe_domain_root_p && !root_entry?
>>>> Couldn't trigger this one yet so only the raw_irqs_disabled_flags
>>>> fixup is excercised, though I guess it can't do any harm to really
>>>> ensure that the explanation fits reality this time...
>>> You mean non-root entry -> migration -> __fixup_if? In that case we pick
>>> up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or
>>> what do you mean?
>> (...)
>>>>> -	if (unlikely(!ipipe_root_domain_p)) {
>>>>> +	if (likely(ipipe_root_domain_p)) {
>>>>> +		/*
>>>>> +		 * 32-bit: In case we faulted in the iret path, regs.flags do
>>>>> +		 * not match the root domain state as the low-level return
>>>>> +		 * code will evaluate it. Fix this up, either by the root
>>>>> +		 * state sampled on entry or, if we migrated to root, with the
>>>>> +		 * current state.
>>>>> +		 */
>>>>> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>>>>> +					raw_irqs_disabled(), regs);
>> I'm referring to the case that evaluates to
>> __fixup_if(raw_irqs_disabled(), regs); That is, something that
>> triggers
>>
>> 		if (!root_entry)
>> 			do_something();
>>
>> Could be that we're talking about to the same case, although I'm not
>> sure ;-)
> 
> Right, that's the case I described above. What problem do you precisely
> see or what concerns to you have about the suggested behavior?

None. I'd just like to be able to trigger it to avoid that
there are any unforseen problems we're still missing.
Since this corner of Ipipe seems to have proven tricky before
AFAIK, I thought it might perhaps be worth while to really excercise
every possible code path.

Wolfgang

Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Jan Kiszka]

Wolfgang Mauerer wrote:
> Jan Kiszka wrote:
>> Wolfgang Mauerer wrote:
>>> Jan Kiszka wrote:
>>>> Wolfgang Mauerer wrote:
>>>>> Jan Kiszka wrote:
>>>>>> If we enter __ipipe_handle_exception over a non-root domain and leave it
>>>>>> due to migration in the event handler over root, we must not restore the
>>>>>> root domain state so far saved on entry. This caused subtle pipeline
>>>>>> state corruptions. Instead, only save and restore them if we were
>>>>>> entering over root.
>>>>>>
>>>>>> However, the x86-32 regs.flags fixup is required nevertheless to take
>>>>>> care of mismatches between the root domain state and the hardware flags
>>>>>> on entry. That may happen if we fault in the iret path. But also in this
>>>>>> case we must not restore an invalid root domain state. So if we entered
>>>>>> over non-root, pick up the input for __fixup_if from the root domain
>>>>>> after running the ipipe handler.
>>>>>>
>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>> ---
>>>>>>
>>>>>> Next try. But this time I think I finally understood what scenario
>>>>>> __fixup_if is actually fixing. Please correct me if I'm still missing
>>>>>> one.
>>>>> looks good - it works for my test cases and solves the problems with
>>>>> the hw/pipeline state mismatch during early bootup. But do you happen
>>>>> to have any scenario at hand with ipipe_domain_root_p && !root_entry?
>>>>> Couldn't trigger this one yet so only the raw_irqs_disabled_flags
>>>>> fixup is excercised, though I guess it can't do any harm to really
>>>>> ensure that the explanation fits reality this time...
>>>> You mean non-root entry -> migration -> __fixup_if? In that case we pick
>>>> up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or
>>>> what do you mean?
>>> (...)
>>>>>> -	if (unlikely(!ipipe_root_domain_p)) {
>>>>>> +	if (likely(ipipe_root_domain_p)) {
>>>>>> +		/*
>>>>>> +		 * 32-bit: In case we faulted in the iret path, regs.flags do
>>>>>> +		 * not match the root domain state as the low-level return
>>>>>> +		 * code will evaluate it. Fix this up, either by the root
>>>>>> +		 * state sampled on entry or, if we migrated to root, with the
>>>>>> +		 * current state.
>>>>>> +		 */
>>>>>> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>>>>>> +					raw_irqs_disabled(), regs);
>>> I'm referring to the case that evaluates to
>>> __fixup_if(raw_irqs_disabled(), regs); That is, something that
>>> triggers
>>>
>>> 		if (!root_entry)
>>> 			do_something();
>>>
>>> Could be that we're talking about to the same case, although I'm not
>>> sure ;-)
>> Right, that's the case I described above. What problem do you precisely
>> see or what concerns to you have about the suggested behavior?
> 
> None. I'd just like to be able to trigger it to avoid that
> there are any unforseen problems we're still missing.
> Since this corner of Ipipe seems to have proven tricky before
> AFAIK, I thought it might perhaps be worth while to really excercise
> every possible code path.

To trigger it, you need to recreate the scenario that our colleagues
managed to generate on the MARS: Have Linux IRQs disabled (or enabled,
to have both variations), schedule in a Xenomai task, let it raise a
fault so that Xenomai migrates it back.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Wolfgang Mauerer]

Jan Kiszka wrote:
> Wolfgang Mauerer wrote:
>> Jan Kiszka wrote:
>>> Wolfgang Mauerer wrote:
>>>> Jan Kiszka wrote:
>>>>> Wolfgang Mauerer wrote:
>>>>>> Jan Kiszka wrote:
>>>>>>> If we enter __ipipe_handle_exception over a non-root domain and leave it
>>>>>>> due to migration in the event handler over root, we must not restore the
>>>>>>> root domain state so far saved on entry. This caused subtle pipeline
>>>>>>> state corruptions. Instead, only save and restore them if we were
>>>>>>> entering over root.
>>>>>>>
>>>>>>> However, the x86-32 regs.flags fixup is required nevertheless to take
>>>>>>> care of mismatches between the root domain state and the hardware flags
>>>>>>> on entry. That may happen if we fault in the iret path. But also in this
>>>>>>> case we must not restore an invalid root domain state. So if we entered
>>>>>>> over non-root, pick up the input for __fixup_if from the root domain
>>>>>>> after running the ipipe handler.
>>>>>>>
>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>> ---
>>>>>>>
>>>>>>> Next try. But this time I think I finally understood what scenario
>>>>>>> __fixup_if is actually fixing. Please correct me if I'm still missing
>>>>>>> one.
>>>>>> looks good - it works for my test cases and solves the problems with
>>>>>> the hw/pipeline state mismatch during early bootup. But do you happen
>>>>>> to have any scenario at hand with ipipe_domain_root_p && !root_entry?
>>>>>> Couldn't trigger this one yet so only the raw_irqs_disabled_flags
>>>>>> fixup is excercised, though I guess it can't do any harm to really
>>>>>> ensure that the explanation fits reality this time...
>>>>> You mean non-root entry -> migration -> __fixup_if? In that case we pick
>>>>> up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or
>>>>> what do you mean?
>>>> (...)
>>>>>>> -	if (unlikely(!ipipe_root_domain_p)) {
>>>>>>> +	if (likely(ipipe_root_domain_p)) {
>>>>>>> +		/*
>>>>>>> +		 * 32-bit: In case we faulted in the iret path, regs.flags do
>>>>>>> +		 * not match the root domain state as the low-level return
>>>>>>> +		 * code will evaluate it. Fix this up, either by the root
>>>>>>> +		 * state sampled on entry or, if we migrated to root, with the
>>>>>>> +		 * current state.
>>>>>>> +		 */
>>>>>>> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>>>>>>> +					raw_irqs_disabled(), regs);
>>>> I'm referring to the case that evaluates to
>>>> __fixup_if(raw_irqs_disabled(), regs); That is, something that
>>>> triggers
>>>>
>>>> 		if (!root_entry)
>>>> 			do_something();
>>>>
>>>> Could be that we're talking about to the same case, although I'm not
>>>> sure ;-)
>>> Right, that's the case I described above. What problem do you precisely
>>> see or what concerns to you have about the suggested behavior?
>> None. I'd just like to be able to trigger it to avoid that
>> there are any unforseen problems we're still missing.
>> Since this corner of Ipipe seems to have proven tricky before
>> AFAIK, I thought it might perhaps be worth while to really excercise
>> every possible code path.
> 
> To trigger it, you need to recreate the scenario that our colleagues
> managed to generate on the MARS: Have Linux IRQs disabled (or enabled,
> to have both variations), schedule in a Xenomai task, let it raise a
> fault so that Xenomai migrates it back.

Yep, but allocating a large amount of memory in non-rt, switching to
rt and then accessing this and switching back to non-rt
in the hope of generating many faults so that the scenario would be
synthesised without expensive and hard to access hardware was
unsuccessful, so my little innocent question was really just if you
happened to have a testcase.

But as I said in the very beginning, the patch nevertheless looks
good to me.

Cheers, Wolfgang

Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Jan Kiszka]

Wolfgang Mauerer wrote:
> Jan Kiszka wrote:
>> Wolfgang Mauerer wrote:
>>> Jan Kiszka wrote:
>>>> Wolfgang Mauerer wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Wolfgang Mauerer wrote:
>>>>>>> Jan Kiszka wrote:
>>>>>>>> If we enter __ipipe_handle_exception over a non-root domain and leave it
>>>>>>>> due to migration in the event handler over root, we must not restore the
>>>>>>>> root domain state so far saved on entry. This caused subtle pipeline
>>>>>>>> state corruptions. Instead, only save and restore them if we were
>>>>>>>> entering over root.
>>>>>>>>
>>>>>>>> However, the x86-32 regs.flags fixup is required nevertheless to take
>>>>>>>> care of mismatches between the root domain state and the hardware flags
>>>>>>>> on entry. That may happen if we fault in the iret path. But also in this
>>>>>>>> case we must not restore an invalid root domain state. So if we entered
>>>>>>>> over non-root, pick up the input for __fixup_if from the root domain
>>>>>>>> after running the ipipe handler.
>>>>>>>>
>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Next try. But this time I think I finally understood what scenario
>>>>>>>> __fixup_if is actually fixing. Please correct me if I'm still missing
>>>>>>>> one.
>>>>>>> looks good - it works for my test cases and solves the problems with
>>>>>>> the hw/pipeline state mismatch during early bootup. But do you happen
>>>>>>> to have any scenario at hand with ipipe_domain_root_p && !root_entry?
>>>>>>> Couldn't trigger this one yet so only the raw_irqs_disabled_flags
>>>>>>> fixup is excercised, though I guess it can't do any harm to really
>>>>>>> ensure that the explanation fits reality this time...
>>>>>> You mean non-root entry -> migration -> __fixup_if? In that case we pick
>>>>>> up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or
>>>>>> what do you mean?
>>>>> (...)
>>>>>>>> -	if (unlikely(!ipipe_root_domain_p)) {
>>>>>>>> +	if (likely(ipipe_root_domain_p)) {
>>>>>>>> +		/*
>>>>>>>> +		 * 32-bit: In case we faulted in the iret path, regs.flags do
>>>>>>>> +		 * not match the root domain state as the low-level return
>>>>>>>> +		 * code will evaluate it. Fix this up, either by the root
>>>>>>>> +		 * state sampled on entry or, if we migrated to root, with the
>>>>>>>> +		 * current state.
>>>>>>>> +		 */
>>>>>>>> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>>>>>>>> +					raw_irqs_disabled(), regs);
>>>>> I'm referring to the case that evaluates to
>>>>> __fixup_if(raw_irqs_disabled(), regs); That is, something that
>>>>> triggers
>>>>>
>>>>> 		if (!root_entry)
>>>>> 			do_something();
>>>>>
>>>>> Could be that we're talking about to the same case, although I'm not
>>>>> sure ;-)
>>>> Right, that's the case I described above. What problem do you precisely
>>>> see or what concerns to you have about the suggested behavior?
>>> None. I'd just like to be able to trigger it to avoid that
>>> there are any unforseen problems we're still missing.
>>> Since this corner of Ipipe seems to have proven tricky before
>>> AFAIK, I thought it might perhaps be worth while to really excercise
>>> every possible code path.
>> To trigger it, you need to recreate the scenario that our colleagues
>> managed to generate on the MARS: Have Linux IRQs disabled (or enabled,
>> to have both variations), schedule in a Xenomai task, let it raise a
>> fault so that Xenomai migrates it back.
> 
> Yep, but allocating a large amount of memory in non-rt, switching to
> rt and then accessing this and switching back to non-rt
> in the hope of generating many faults so that the scenario would be
> synthesised without expensive and hard to access hardware was
> unsuccessful, so my little innocent question was really just if you
> happened to have a testcase.

OK, then I totally misunderstood your question.

You do not need to generate a fixable fault, and unfixable one (*(int
*)NULL = 0;) should suffice.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Wolfgang Mauerer]

Jan Kiszka wrote:
> Wolfgang Mauerer wrote:
>> Jan Kiszka wrote:
>>> Wolfgang Mauerer wrote:
>>>> Jan Kiszka wrote:
>>>>> Wolfgang Mauerer wrote:
>>>>>> Jan Kiszka wrote:
>>>>>>> Wolfgang Mauerer wrote:
>>>>>>>> Jan Kiszka wrote:
>>>>>>>>> If we enter __ipipe_handle_exception over a non-root domain and leave it
>>>>>>>>> due to migration in the event handler over root, we must not restore the
>>>>>>>>> root domain state so far saved on entry. This caused subtle pipeline
>>>>>>>>> state corruptions. Instead, only save and restore them if we were
>>>>>>>>> entering over root.
>>>>>>>>>
>>>>>>>>> However, the x86-32 regs.flags fixup is required nevertheless to take
>>>>>>>>> care of mismatches between the root domain state and the hardware flags
>>>>>>>>> on entry. That may happen if we fault in the iret path. But also in this
>>>>>>>>> case we must not restore an invalid root domain state. So if we entered
>>>>>>>>> over non-root, pick up the input for __fixup_if from the root domain
>>>>>>>>> after running the ipipe handler.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Next try. But this time I think I finally understood what scenario
>>>>>>>>> __fixup_if is actually fixing. Please correct me if I'm still missing
>>>>>>>>> one.
>>>>>>>> looks good - it works for my test cases and solves the problems with
>>>>>>>> the hw/pipeline state mismatch during early bootup. But do you happen
>>>>>>>> to have any scenario at hand with ipipe_domain_root_p && !root_entry?
>>>>>>>> Couldn't trigger this one yet so only the raw_irqs_disabled_flags
>>>>>>>> fixup is excercised, though I guess it can't do any harm to really
>>>>>>>> ensure that the explanation fits reality this time...
>>>>>>> You mean non-root entry -> migration -> __fixup_if? In that case we pick
>>>>>>> up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or
>>>>>>> what do you mean?
>>>>>> (...)
>>>>>>>>> -	if (unlikely(!ipipe_root_domain_p)) {
>>>>>>>>> +	if (likely(ipipe_root_domain_p)) {
>>>>>>>>> +		/*
>>>>>>>>> +		 * 32-bit: In case we faulted in the iret path, regs.flags do
>>>>>>>>> +		 * not match the root domain state as the low-level return
>>>>>>>>> +		 * code will evaluate it. Fix this up, either by the root
>>>>>>>>> +		 * state sampled on entry or, if we migrated to root, with the
>>>>>>>>> +		 * current state.
>>>>>>>>> +		 */
>>>>>>>>> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>>>>>>>>> +					raw_irqs_disabled(), regs);
>>>>>> I'm referring to the case that evaluates to
>>>>>> __fixup_if(raw_irqs_disabled(), regs); That is, something that
>>>>>> triggers
>>>>>>
>>>>>> 		if (!root_entry)
>>>>>> 			do_something();
>>>>>>
>>>>>> Could be that we're talking about to the same case, although I'm not
>>>>>> sure ;-)
>>>>> Right, that's the case I described above. What problem do you precisely
>>>>> see or what concerns to you have about the suggested behavior?
>>>> None. I'd just like to be able to trigger it to avoid that
>>>> there are any unforseen problems we're still missing.
>>>> Since this corner of Ipipe seems to have proven tricky before
>>>> AFAIK, I thought it might perhaps be worth while to really excercise
>>>> every possible code path.
>>> To trigger it, you need to recreate the scenario that our colleagues
>>> managed to generate on the MARS: Have Linux IRQs disabled (or enabled,
>>> to have both variations), schedule in a Xenomai task, let it raise a
>>> fault so that Xenomai migrates it back.
>> Yep, but allocating a large amount of memory in non-rt, switching to
>> rt and then accessing this and switching back to non-rt
>> in the hope of generating many faults so that the scenario would be
>> synthesised without expensive and hard to access hardware was
>> unsuccessful, so my little innocent question was really just if you
>> happened to have a testcase.
> 
> OK, then I totally misunderstood your question.

Seems we have nevertheless finally managed to reach an agreement on
what we mean ;-)
> 
> You do not need to generate a fixable fault, and unfixable one (*(int
> *)NULL = 0;) should suffice.

yeah, although that would maybe be a bit ugly wrt. handling the SEGV
and continuing to allow continuous faults without
having to restart the program all the time.

As we just discussed offline, this would anyway not yet trigger the
"fault in iret"-scenario, and I don't think it's worth while spending
the large extra effort for this considering that the code will sooner
or later die anyway. Or does maybe anyone have an idea how to trigger
such a fault with simple means?

Cheers, Wolfgang

Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return

[Gilles Chanteperdrix]

Wolfgang Mauerer wrote:
> Jan Kiszka wrote:
>> Wolfgang Mauerer wrote:
>>> Jan Kiszka wrote:
>>>> Wolfgang Mauerer wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Wolfgang Mauerer wrote:
>>>>>>> Jan Kiszka wrote:
>>>>>>>> Wolfgang Mauerer wrote:
>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>> If we enter __ipipe_handle_exception over a non-root domain and leave it
>>>>>>>>>> due to migration in the event handler over root, we must not restore the
>>>>>>>>>> root domain state so far saved on entry. This caused subtle pipeline
>>>>>>>>>> state corruptions. Instead, only save and restore them if we were
>>>>>>>>>> entering over root.
>>>>>>>>>>
>>>>>>>>>> However, the x86-32 regs.flags fixup is required nevertheless to take
>>>>>>>>>> care of mismatches between the root domain state and the hardware flags
>>>>>>>>>> on entry. That may happen if we fault in the iret path. But also in this
>>>>>>>>>> case we must not restore an invalid root domain state. So if we entered
>>>>>>>>>> over non-root, pick up the input for __fixup_if from the root domain
>>>>>>>>>> after running the ipipe handler.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>> Next try. But this time I think I finally understood what scenario
>>>>>>>>>> __fixup_if is actually fixing. Please correct me if I'm still missing
>>>>>>>>>> one.
>>>>>>>>> looks good - it works for my test cases and solves the problems with
>>>>>>>>> the hw/pipeline state mismatch during early bootup. But do you happen
>>>>>>>>> to have any scenario at hand with ipipe_domain_root_p && !root_entry?
>>>>>>>>> Couldn't trigger this one yet so only the raw_irqs_disabled_flags
>>>>>>>>> fixup is excercised, though I guess it can't do any harm to really
>>>>>>>>> ensure that the explanation fits reality this time...
>>>>>>>> You mean non-root entry -> migration -> __fixup_if? In that case we pick
>>>>>>>> up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or
>>>>>>>> what do you mean?
>>>>>>> (...)
>>>>>>>>>> -	if (unlikely(!ipipe_root_domain_p)) {
>>>>>>>>>> +	if (likely(ipipe_root_domain_p)) {
>>>>>>>>>> +		/*
>>>>>>>>>> +		 * 32-bit: In case we faulted in the iret path, regs.flags do
>>>>>>>>>> +		 * not match the root domain state as the low-level return
>>>>>>>>>> +		 * code will evaluate it. Fix this up, either by the root
>>>>>>>>>> +		 * state sampled on entry or, if we migrated to root, with the
>>>>>>>>>> +		 * current state.
>>>>>>>>>> +		 */
>>>>>>>>>> +		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>>>>>>>>>> +					raw_irqs_disabled(), regs);
>>>>>>> I'm referring to the case that evaluates to
>>>>>>> __fixup_if(raw_irqs_disabled(), regs); That is, something that
>>>>>>> triggers
>>>>>>>
>>>>>>> 		if (!root_entry)
>>>>>>> 			do_something();
>>>>>>>
>>>>>>> Could be that we're talking about to the same case, although I'm not
>>>>>>> sure ;-)
>>>>>> Right, that's the case I described above. What problem do you precisely
>>>>>> see or what concerns to you have about the suggested behavior?
>>>>> None. I'd just like to be able to trigger it to avoid that
>>>>> there are any unforseen problems we're still missing.
>>>>> Since this corner of Ipipe seems to have proven tricky before
>>>>> AFAIK, I thought it might perhaps be worth while to really excercise
>>>>> every possible code path.
>>>> To trigger it, you need to recreate the scenario that our colleagues
>>>> managed to generate on the MARS: Have Linux IRQs disabled (or enabled,
>>>> to have both variations), schedule in a Xenomai task, let it raise a
>>>> fault so that Xenomai migrates it back.
>>> Yep, but allocating a large amount of memory in non-rt, switching to
>>> rt and then accessing this and switching back to non-rt
>>> in the hope of generating many faults so that the scenario would be
>>> synthesised without expensive and hard to access hardware was
>>> unsuccessful, so my little innocent question was really just if you
>>> happened to have a testcase.
>> OK, then I totally misunderstood your question.
> 
> Seems we have nevertheless finally managed to reach an agreement on
> what we mean ;-)
>> You do not need to generate a fixable fault, and unfixable one (*(int
>> *)NULL = 0;) should suffice.
> 
> yeah, although that would maybe be a bit ugly wrt. handling the SEGV
> and continuing to allow continuous faults without
> having to restart the program all the time.
> 
> As we just discussed offline, this would anyway not yet trigger the
> "fault in iret"-scenario, and I don't think it's worth while spending
> the large extra effort for this considering that the code will sooner
> or later die anyway. Or does maybe anyone have an idea how to trigger
> such a fault with simple means?

I think I will write a test case which tests the few cases we really
have, and check that __ipipe_handle_exception works as expected.
A small rtdm driver with some specialized ioctls should do.

> 
> Cheers, Wolfgang
> 


-- 
Gilles Chanteperdrix, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com