All of lore.kernel.org
 help / color / mirror / Atom feed
* [Adeos-main] [PATCH] x86: Virtualize cr2
@ 2010-04-13 17:40 Jan Kiszka
  2010-04-16  9:18 ` Jan Kiszka
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Kiszka @ 2010-04-13 17:40 UTC (permalink / raw)
  To: Philippe Gerum; +Cc: adeos-main

Ensure that we always call the Linux page fault handler with the correct
cr2 value set. So far this is not the case if a second fault happens
while we execute the domain fault handler (which may, e.g., perform a
sleepy migration before returning).

Instead of re-writing cr2 directly in hardware, make use of the paravirt
capabilities of Linux and simply overload read/write_cr2 for this
purpose.

Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
---
 arch/x86/include/asm/ipipe.h  |    2 ++
 arch/x86/include/asm/system.h |    5 +++++
 arch/x86/kernel/ipipe.c       |   10 ++++++++++
 3 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/ipipe.h b/arch/x86/include/asm/ipipe.h
index 32b2ece..baec016 100644
--- a/arch/x86/include/asm/ipipe.h
+++ b/arch/x86/include/asm/ipipe.h
@@ -33,6 +33,8 @@
 
 DECLARE_PER_CPU(struct pt_regs, __ipipe_tick_regs);
 
+DECLARE_PER_CPU(unsigned long, __ipipe_cr2);
+
 static inline unsigned __ipipe_get_irq_vector(int irq)
 {
 #ifdef CONFIG_X86_IO_APIC
diff --git a/arch/x86/include/asm/system.h b/arch/x86/include/asm/system.h
index 2760033..093687e 100644
--- a/arch/x86/include/asm/system.h
+++ b/arch/x86/include/asm/system.h
@@ -308,8 +308,13 @@ static inline void native_wbinvd(void)
 #else
 #define read_cr0()	(native_read_cr0())
 #define write_cr0(x)	(native_write_cr0(x))
+#ifdef CONFIG_IPIPE
+#define read_cr2()	__raw_get_cpu_var(__ipipe_cr2)
+#define write_cr2(x)	__raw_get_cpu_var(__ipipe_cr2) = (x)
+#else /* !CONFIG_IPIPE */
 #define read_cr2()	(native_read_cr2())
 #define write_cr2(x)	(native_write_cr2(x))
+#endif /* !CONFIG_IPIPE */
 #define read_cr3()	(native_read_cr3())
 #define write_cr3(x)	(native_write_cr3(x))
 #define read_cr4()	(native_read_cr4())
diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c
index b1b6912..9425fbd 100644
--- a/arch/x86/kernel/ipipe.c
+++ b/arch/x86/kernel/ipipe.c
@@ -53,6 +53,9 @@ int __ipipe_tick_irq = 0;	/* Legacy timer */
 
 DEFINE_PER_CPU(struct pt_regs, __ipipe_tick_regs);
 
+DEFINE_PER_CPU(unsigned long, __ipipe_cr2);
+EXPORT_PER_CPU_SYMBOL_GPL(__ipipe_cr2);
+
 #ifdef CONFIG_SMP
 
 static cpumask_t __ipipe_cpu_sync_map;
@@ -712,6 +715,7 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
 {
 	bool root_entry = false;
 	unsigned long flags = 0;
+	unsigned long cr2 = 0;
 
 	if (ipipe_root_domain_p) {
 		root_entry = true;
@@ -734,6 +738,9 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
 		return 1;
 #endif /* CONFIG_KGDB */
 
+	if (vector == ex_do_page_fault)
+		cr2 = native_read_cr2();
+
 	if (unlikely(ipipe_trap_notify(vector, regs))) {
 		if (root_entry)
 			local_irq_restore_nosync(flags);
@@ -779,6 +786,9 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
 		}
 	}
 
+	if (vector == ex_do_page_fault)
+		write_cr2(cr2);
+
 	__ipipe_std_extable[vector](regs, error_code);
 
 	/*



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Adeos-main] [PATCH] x86: Virtualize cr2
  2010-04-13 17:40 [Adeos-main] [PATCH] x86: Virtualize cr2 Jan Kiszka
@ 2010-04-16  9:18 ` Jan Kiszka
  2010-04-16  9:22   ` Philippe Gerum
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Kiszka @ 2010-04-16  9:18 UTC (permalink / raw)
  To: Philippe Gerum; +Cc: adeos-main

Jan Kiszka wrote:
> Ensure that we always call the Linux page fault handler with the correct
> cr2 value set. So far this is not the case if a second fault happens
> while we execute the domain fault handler (which may, e.g., perform a
> sleepy migration before returning).
> 
> Instead of re-writing cr2 directly in hardware, make use of the paravirt
> capabilities of Linux and simply overload read/write_cr2 for this
> purpose.

Already had a chance to look at it? Any concerns? We are about to ship
this patch as it proved to fix the bug we saw.

BTW: I haven't thought about all details yet, but this should finally
allow us to enter the Linux page fault handler with hardware interrupts
enabled, thus drop the related patches to enable them after reading cr2.

Jan

> 
> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
> ---
>  arch/x86/include/asm/ipipe.h  |    2 ++
>  arch/x86/include/asm/system.h |    5 +++++
>  arch/x86/kernel/ipipe.c       |   10 ++++++++++
>  3 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/include/asm/ipipe.h b/arch/x86/include/asm/ipipe.h
> index 32b2ece..baec016 100644
> --- a/arch/x86/include/asm/ipipe.h
> +++ b/arch/x86/include/asm/ipipe.h
> @@ -33,6 +33,8 @@
>  
>  DECLARE_PER_CPU(struct pt_regs, __ipipe_tick_regs);
>  
> +DECLARE_PER_CPU(unsigned long, __ipipe_cr2);
> +
>  static inline unsigned __ipipe_get_irq_vector(int irq)
>  {
>  #ifdef CONFIG_X86_IO_APIC
> diff --git a/arch/x86/include/asm/system.h b/arch/x86/include/asm/system.h
> index 2760033..093687e 100644
> --- a/arch/x86/include/asm/system.h
> +++ b/arch/x86/include/asm/system.h
> @@ -308,8 +308,13 @@ static inline void native_wbinvd(void)
>  #else
>  #define read_cr0()	(native_read_cr0())
>  #define write_cr0(x)	(native_write_cr0(x))
> +#ifdef CONFIG_IPIPE
> +#define read_cr2()	__raw_get_cpu_var(__ipipe_cr2)
> +#define write_cr2(x)	__raw_get_cpu_var(__ipipe_cr2) = (x)
> +#else /* !CONFIG_IPIPE */
>  #define read_cr2()	(native_read_cr2())
>  #define write_cr2(x)	(native_write_cr2(x))
> +#endif /* !CONFIG_IPIPE */
>  #define read_cr3()	(native_read_cr3())
>  #define write_cr3(x)	(native_write_cr3(x))
>  #define read_cr4()	(native_read_cr4())
> diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c
> index b1b6912..9425fbd 100644
> --- a/arch/x86/kernel/ipipe.c
> +++ b/arch/x86/kernel/ipipe.c
> @@ -53,6 +53,9 @@ int __ipipe_tick_irq = 0;	/* Legacy timer */
>  
>  DEFINE_PER_CPU(struct pt_regs, __ipipe_tick_regs);
>  
> +DEFINE_PER_CPU(unsigned long, __ipipe_cr2);
> +EXPORT_PER_CPU_SYMBOL_GPL(__ipipe_cr2);
> +
>  #ifdef CONFIG_SMP
>  
>  static cpumask_t __ipipe_cpu_sync_map;
> @@ -712,6 +715,7 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
>  {
>  	bool root_entry = false;
>  	unsigned long flags = 0;
> +	unsigned long cr2 = 0;
>  
>  	if (ipipe_root_domain_p) {
>  		root_entry = true;
> @@ -734,6 +738,9 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
>  		return 1;
>  #endif /* CONFIG_KGDB */
>  
> +	if (vector == ex_do_page_fault)
> +		cr2 = native_read_cr2();
> +
>  	if (unlikely(ipipe_trap_notify(vector, regs))) {
>  		if (root_entry)
>  			local_irq_restore_nosync(flags);
> @@ -779,6 +786,9 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
>  		}
>  	}
>  
> +	if (vector == ex_do_page_fault)
> +		write_cr2(cr2);
> +
>  	__ipipe_std_extable[vector](regs, error_code);
>  
>  	/*
> 
> 

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Adeos-main] [PATCH] x86: Virtualize cr2
  2010-04-16  9:18 ` Jan Kiszka
@ 2010-04-16  9:22   ` Philippe Gerum
  0 siblings, 0 replies; 3+ messages in thread
From: Philippe Gerum @ 2010-04-16  9:22 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: adeos-main

On Fri, 2010-04-16 at 11:18 +0200, Jan Kiszka wrote:
> Jan Kiszka wrote:
> > Ensure that we always call the Linux page fault handler with the correct
> > cr2 value set. So far this is not the case if a second fault happens
> > while we execute the domain fault handler (which may, e.g., perform a
> > sleepy migration before returning).
> > 
> > Instead of re-writing cr2 directly in hardware, make use of the paravirt
> > capabilities of Linux and simply overload read/write_cr2 for this
> > purpose.
> 
> Already had a chance to look at it? Any concerns? We are about to ship
> this patch as it proved to fix the bug we saw.

No concerns, I'll merge this. Thanks.

> 
> BTW: I haven't thought about all details yet, but this should finally
> allow us to enter the Linux page fault handler with hardware interrupts
> enabled, thus drop the related patches to enable them after reading cr2.
> 
> Jan
> 
> > 
> > Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
> > ---
> >  arch/x86/include/asm/ipipe.h  |    2 ++
> >  arch/x86/include/asm/system.h |    5 +++++
> >  arch/x86/kernel/ipipe.c       |   10 ++++++++++
> >  3 files changed, 17 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/ipipe.h b/arch/x86/include/asm/ipipe.h
> > index 32b2ece..baec016 100644
> > --- a/arch/x86/include/asm/ipipe.h
> > +++ b/arch/x86/include/asm/ipipe.h
> > @@ -33,6 +33,8 @@
> >  
> >  DECLARE_PER_CPU(struct pt_regs, __ipipe_tick_regs);
> >  
> > +DECLARE_PER_CPU(unsigned long, __ipipe_cr2);
> > +
> >  static inline unsigned __ipipe_get_irq_vector(int irq)
> >  {
> >  #ifdef CONFIG_X86_IO_APIC
> > diff --git a/arch/x86/include/asm/system.h b/arch/x86/include/asm/system.h
> > index 2760033..093687e 100644
> > --- a/arch/x86/include/asm/system.h
> > +++ b/arch/x86/include/asm/system.h
> > @@ -308,8 +308,13 @@ static inline void native_wbinvd(void)
> >  #else
> >  #define read_cr0()	(native_read_cr0())
> >  #define write_cr0(x)	(native_write_cr0(x))
> > +#ifdef CONFIG_IPIPE
> > +#define read_cr2()	__raw_get_cpu_var(__ipipe_cr2)
> > +#define write_cr2(x)	__raw_get_cpu_var(__ipipe_cr2) = (x)
> > +#else /* !CONFIG_IPIPE */
> >  #define read_cr2()	(native_read_cr2())
> >  #define write_cr2(x)	(native_write_cr2(x))
> > +#endif /* !CONFIG_IPIPE */
> >  #define read_cr3()	(native_read_cr3())
> >  #define write_cr3(x)	(native_write_cr3(x))
> >  #define read_cr4()	(native_read_cr4())
> > diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c
> > index b1b6912..9425fbd 100644
> > --- a/arch/x86/kernel/ipipe.c
> > +++ b/arch/x86/kernel/ipipe.c
> > @@ -53,6 +53,9 @@ int __ipipe_tick_irq = 0;	/* Legacy timer */
> >  
> >  DEFINE_PER_CPU(struct pt_regs, __ipipe_tick_regs);
> >  
> > +DEFINE_PER_CPU(unsigned long, __ipipe_cr2);
> > +EXPORT_PER_CPU_SYMBOL_GPL(__ipipe_cr2);
> > +
> >  #ifdef CONFIG_SMP
> >  
> >  static cpumask_t __ipipe_cpu_sync_map;
> > @@ -712,6 +715,7 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
> >  {
> >  	bool root_entry = false;
> >  	unsigned long flags = 0;
> > +	unsigned long cr2 = 0;
> >  
> >  	if (ipipe_root_domain_p) {
> >  		root_entry = true;
> > @@ -734,6 +738,9 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
> >  		return 1;
> >  #endif /* CONFIG_KGDB */
> >  
> > +	if (vector == ex_do_page_fault)
> > +		cr2 = native_read_cr2();
> > +
> >  	if (unlikely(ipipe_trap_notify(vector, regs))) {
> >  		if (root_entry)
> >  			local_irq_restore_nosync(flags);
> > @@ -779,6 +786,9 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
> >  		}
> >  	}
> >  
> > +	if (vector == ex_do_page_fault)
> > +		write_cr2(cr2);
> > +
> >  	__ipipe_std_extable[vector](regs, error_code);
> >  
> >  	/*
> > 
> > 
> 


-- 
Philippe.




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-04-16  9:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-13 17:40 [Adeos-main] [PATCH] x86: Virtualize cr2 Jan Kiszka
2010-04-16  9:18 ` Jan Kiszka
2010-04-16  9:22   ` Philippe Gerum

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.