Re: 2.6.20-rc7: known regressions

Re: 2.6.20-rc7: known regressions

[Fabio Erculiani]

Adrian, 

and this one? 
http://bugzilla.kernel.org/show_bug.cgi?id=7589 

--- 
Fabio Erculiani 
http://www.sabayonlinux.org 
Sabayon Linux Founder 

Re: 2.6.20-rc7: known regressions

[Adrian Bunk]

On Fri, Feb 02, 2007 at 08:45:42AM +0000, Fabio Erculiani wrote:

> Adrian, 

Hi Fabio,

> and this one? 
> http://bugzilla.kernel.org/show_bug.cgi?id=7589 

Not a regression in 2.6.20-rc compared to 2.6.19 - it's already in 
broken in 2.6.19.

This bug should be fixed, but it's outside the scope of my list.

> Fabio Erculiani 

cu
Adrian

BTW: Please don't strip me from the Cc when asking me a question.

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: 2.6.20-rc7: known regressions

[Eric W. Biederman]

Andi Kleen <ak@suse.de> writes:

> On Sunday 04 February 2007 18:34, Linus Torvalds wrote:
>> 
>> On Sun, 4 Feb 2007, Frédéric Riss wrote:
>> > 
>> > New patch:
>> 
>> I didn't get how this would fix the ia64 issues? I thought ia64 needed 
>> the standard calling convention?
>
> asmlinkage is standard enough on ia64 as far as I can see.
>
> It defines to an undocumented attribute that seems to only affect the
> generated code for functions (basically it forces the compiler
> to not reuse input arguments, similar to prevent_tail_call() on i386), 
> not for pointers.

Regardless.  If we are serious about supporting EFI instead of just
nursing it along for the rare person stuck with it, we are going
to need to remove the brain-dead relocate exactly once to virtual addresses
call, so we can use kexec.

Therefore we will need a trampoline to switch to physical mode.

Of course that presumes the Open (but no one is allowed to read or implement
the standard, or join the committee ) EFI is sufficiently interesting at
some point to bother supporting properly.

Eric

Re: 2.6.20-rc7: known regressions

[Andi Kleen]

On Sunday 04 February 2007 18:34, Linus Torvalds wrote:
> 
> On Sun, 4 Feb 2007, Frédéric Riss wrote:
> > 
> > New patch:
> 
> I didn't get how this would fix the ia64 issues? I thought ia64 needed 
> the standard calling convention?

asmlinkage is standard enough on ia64 as far as I can see.

It defines to an undocumented attribute that seems to only affect the
generated code for functions (basically it forces the compiler
to not reuse input arguments, similar to prevent_tail_call() on i386), 
not for pointers.

-Andi

Re: 2.6.20-rc7: known regressions

[Linus Torvalds]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 564 bytes --]



On Sun, 4 Feb 2007, Frédéric Riss wrote:
> 
> I think Andi said that adding asmlinkage on the function pointers
> shouldn't harm ia64. If you prefer wrapper functions, one of the patches
> I sent ( http://lkml.org/lkml/2007/1/30/309 ) did that, but the casting
> it uses looks clumsy.

I'm more comfortable with that one, at least for now. It's guaranteed to 
not break ia64, at least. Also, it does what I think is right: do the 
calling convention conversion at the call-site rather than at a C compiler 
level.

Will apply a whitespace-fixed version,

		Linus

Re: 2.6.20-rc7: known regressions

[Frédéric Riss]

Le dimanche 04 février 2007 à 09:34 -0800, Linus Torvalds a écrit :
> 
> On Sun, 4 Feb 2007, Frédéric Riss wrote:
> > 
> > New patch:
> 
> I didn't get how this would fix the ia64 issues? I thought ia64 needed 
> the standard calling convention?

I think Andi said that adding asmlinkage on the function pointers
shouldn't harm ia64. If you prefer wrapper functions, one of the patches
I sent ( http://lkml.org/lkml/2007/1/30/309 ) did that, but the casting
it uses looks clumsy.

Fred.

Re: 2.6.20-rc7: known regressions

[Linus Torvalds]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 962 bytes --]



On Sun, 4 Feb 2007, Frédéric Riss wrote:
> 
> New patch:

I didn't get how this would fix the ia64 issues? I thought ia64 needed 
the standard calling convention?

My gut feel is that EFI should be handled exactly the same way that we 
used to handle APM: never even make it look like it's callable from C, but 
make architecture-specific wrapper functions that have bog-standard 
calling conventions, and then possibly even use inline asm to actually do 
the real call (but even if you don't, at that point it would be inside one 
particular arch-specific EFI source file - nobody outside of that would 
ever call into the firmware directly).

As it is, I don't think I dare apply this right now, which means that it 
will miss 2.6.20, and we'll have to backport it to the stable tree when 
everybody agrees and has acked it. I don't like having suspend broken on 
EFI macs, but on the other hand, I would hate to have an ia64 regression 
even more..

		Linus

Re: 2.6.20-rc7: known regressions

[Andi Kleen]

> 
> When calling into the EFI firmware, the parameters need to be passed on
> the stack. The recent change to use -mregparm=3 breaks x86 EFI support.
> This patch is needed to allow the new Intel-based Macs to suspend to ram
> (efi.get_time is called during the suspend phase).

Thanks looks good.

-Andi

Re: 2.6.20-rc7: known regressions

[Frédéric Riss]

Le samedi 03 février 2007 à 11:51 +0100, Andi Kleen a écrit :
> > +
> > +typedef efilinkage efi_status_t efi_get_time_t (efi_time_t *tm,
> > +						efi_time_cap_t *tc);
> 
> I assume you have double checked it actually works? (i vaguely recall some
> issues with applying attributes to typedefs). If not you would need
> to put them to the declarations.

OK, I tried the following patch (simply adding asmlinkage to the
typedefs) with gcc 3.3, 3.4, 4.0 and 4.1. I couldn't easily try older
compilers.

New patch:

When calling into the EFI firmware, the parameters need to be passed on
the stack. The recent change to use -mregparm=3 breaks x86 EFI support.
This patch is needed to allow the new Intel-based Macs to suspend to ram
(efi.get_time is called during the suspend phase).

Signed-off-by: Frederic Riss <frederic.riss@gmail.com>
---
diff --git a/include/linux/efi.h b/include/linux/efi.h
index f8ebd7c..578bc2a 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -157,22 +157,33 @@ typedef struct {
 	unsigned long reset_system;
 } efi_runtime_services_t;
 
-typedef efi_status_t efi_get_time_t (efi_time_t *tm, efi_time_cap_t *tc);
-typedef efi_status_t efi_set_time_t (efi_time_t *tm);
-typedef efi_status_t efi_get_wakeup_time_t (efi_bool_t *enabled, efi_bool_t *pending,
-					    efi_time_t *tm);
-typedef efi_status_t efi_set_wakeup_time_t (efi_bool_t enabled, efi_time_t *tm);
-typedef efi_status_t efi_get_variable_t (efi_char16_t *name, efi_guid_t *vendor, u32 *attr,
-					 unsigned long *data_size, void *data);
-typedef efi_status_t efi_get_next_variable_t (unsigned long *name_size, efi_char16_t *name,
-					      efi_guid_t *vendor);
-typedef efi_status_t efi_set_variable_t (efi_char16_t *name, efi_guid_t *vendor, 
-					 unsigned long attr, unsigned long data_size, 
-					 void *data);
-typedef efi_status_t efi_get_next_high_mono_count_t (u32 *count);
-typedef void efi_reset_system_t (int reset_type, efi_status_t status,
-				 unsigned long data_size, efi_char16_t *data);
-typedef efi_status_t efi_set_virtual_address_map_t (unsigned long memory_map_size,
+typedef asmlinkage efi_status_t efi_get_time_t (efi_time_t *tm,
+						efi_time_cap_t *tc);
+typedef asmlinkage efi_status_t efi_set_time_t (efi_time_t *tm);
+typedef asmlinkage efi_status_t efi_get_wakeup_time_t (efi_bool_t *enabled, 
+						       efi_bool_t *pending,
+						       efi_time_t *tm);
+typedef asmlinkage efi_status_t efi_set_wakeup_time_t (efi_bool_t enabled,
+						       efi_time_t *tm);
+typedef asmlinkage efi_status_t efi_get_variable_t (efi_char16_t *name,
+						    efi_guid_t *vendor,
+						    u32 *attr,
+						    unsigned long *data_size,
+						    void *data);
+typedef asmlinkage efi_status_t efi_get_next_variable_t (unsigned long *name_sz,
+							 efi_char16_t *name,
+							 efi_guid_t *vendor);
+typedef asmlinkage efi_status_t efi_set_variable_t (efi_char16_t *name,
+						    efi_guid_t *vendor,
+						    unsigned long attr,
+						    unsigned long data_size,
+						    void *data);
+typedef asmlinkage efi_status_t efi_get_next_high_mono_count_t (u32 *count);
+typedef asmlinkage void efi_reset_system_t (int reset_type, 
+					    efi_status_t status,
+					    unsigned long data_size,
+					    efi_char16_t *data);
+typedef asmlinkage efi_status_t efi_set_virtual_address_map_t (unsigned long memory_map_size,
 						unsigned long descriptor_size,
 						u32 descriptor_version,
 						efi_memory_desc_t *virtual_map);

Re: 2.6.20-rc7: known regressions

[Frédéric RISS]

Le samedi 03 février 2007 à 11:57 +0100, Frédéric Riss a écrit :
> Le samedi 03 février 2007 à 11:51 +0100, Andi Kleen a écrit :
> > > +
> > > +typedef efilinkage efi_status_t efi_get_time_t (efi_time_t *tm,
> > > +						efi_time_cap_t *tc);
> > 
> > I assume you have double checked it actually works? (i vaguely recall some
> > issues with applying attributes to typedefs). If not you would need
> > to put them to the declarations.
> 
> Of course, I tested 10 suspend/resume cycles. This is with gcc 4.1.2, I
> guess other compilers could mishandle it. Would you prefer the version
> putting asmlinkage inside the struct definition?

But then it then throws 'assignment from incompatible pointer type'
warnings in efi.c. This would need fixing in arch/ia64 and arch/i386. Or
should I put it in the typdefs _and_ in the struct to be safe ?

Fred

Re: 2.6.20-rc7: known regressions

[Frédéric Riss]

Le samedi 03 février 2007 à 11:51 +0100, Andi Kleen a écrit :
> > +
> > +typedef efilinkage efi_status_t efi_get_time_t (efi_time_t *tm,
> > +						efi_time_cap_t *tc);
> 
> I assume you have double checked it actually works? (i vaguely recall some
> issues with applying attributes to typedefs). If not you would need
> to put them to the declarations.

Of course, I tested 10 suspend/resume cycles. This is with gcc 4.1.2, I
guess other compilers could mishandle it. Would you prefer the version
putting asmlinkage inside the struct definition?

Fred.

Re: 2.6.20-rc7: known regressions

[Andi Kleen]

> +#ifdef CONFIG_X86_32
> +#define efilinkage asmlinkage
> +#else
> +#define efilinkage
> +#endif

No ifdefs, this should be somewhere in the headers for the EFI supporting
architectures

But I suspect you could actually get away with just using asmlinkage
(after reviewing the ia64 asmlinkage I think it's ok)

x86-64 when it is ever implemented will always need asm stubs though
because it uses completely incompatible calling conventions.

> +
> +typedef efilinkage efi_status_t efi_get_time_t (efi_time_t *tm,
> +						efi_time_cap_t *tc);

I assume you have double checked it actually works? (i vaguely recall some
issues with applying attributes to typedefs). If not you would need
to put them to the declarations.

-Andi

Re: 2.6.20-rc7: known regressions

[Frédéric Riss]

Le samedi 03 février 2007 à 10:58 +0100, Andi Kleen a écrit :
> Define a efilinkage macro then that expands to nothing on ia64
> 
> Probably asmlinkage would work already, syscall_linkage as used on ia64 doesn't
> seem to affect function pointers.

And here it goes:

When calling into the EFI firmware, the parameters need to be passed on
the stack. The recent change to use -mregparm=3 breaks x86 EFI support.
This patch is needed to allow the new Intel-based Macs to suspend to ram
(efi.get_time is called during the suspend phase).

Signed-off-by: Frederic Riss <frederic.riss@gmail.com>
---

diff --git a/include/linux/efi.h b/include/linux/efi.h
index f8ebd7c..596e806 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -157,22 +157,39 @@ typedef struct {
 	unsigned long reset_system;
 } efi_runtime_services_t;
 
-typedef efi_status_t efi_get_time_t (efi_time_t *tm, efi_time_cap_t *tc);
-typedef efi_status_t efi_set_time_t (efi_time_t *tm);
-typedef efi_status_t efi_get_wakeup_time_t (efi_bool_t *enabled, efi_bool_t *pending,
-					    efi_time_t *tm);
-typedef efi_status_t efi_set_wakeup_time_t (efi_bool_t enabled, efi_time_t *tm);
-typedef efi_status_t efi_get_variable_t (efi_char16_t *name, efi_guid_t *vendor, u32 *attr,
-					 unsigned long *data_size, void *data);
-typedef efi_status_t efi_get_next_variable_t (unsigned long *name_size, efi_char16_t *name,
-					      efi_guid_t *vendor);
-typedef efi_status_t efi_set_variable_t (efi_char16_t *name, efi_guid_t *vendor, 
-					 unsigned long attr, unsigned long data_size, 
-					 void *data);
-typedef efi_status_t efi_get_next_high_mono_count_t (u32 *count);
-typedef void efi_reset_system_t (int reset_type, efi_status_t status,
-				 unsigned long data_size, efi_char16_t *data);
-typedef efi_status_t efi_set_virtual_address_map_t (unsigned long memory_map_size,
+#ifdef CONFIG_X86_32
+#define efilinkage asmlinkage
+#else
+#define efilinkage
+#endif
+
+typedef efilinkage efi_status_t efi_get_time_t (efi_time_t *tm,
+						efi_time_cap_t *tc);
+typedef efilinkage efi_status_t efi_set_time_t (efi_time_t *tm);
+typedef efilinkage efi_status_t efi_get_wakeup_time_t (efi_bool_t *enabled, 
+						       efi_bool_t *pending,
+						       efi_time_t *tm);
+typedef efilinkage efi_status_t efi_set_wakeup_time_t (efi_bool_t enabled,
+						       efi_time_t *tm);
+typedef efilinkage efi_status_t efi_get_variable_t (efi_char16_t *name,
+						    efi_guid_t *vendor,
+						    u32 *attr,
+						    unsigned long *data_size,
+						    void *data);
+typedef efilinkage efi_status_t efi_get_next_variable_t (unsigned long *name_sz,
+							 efi_char16_t *name,
+							 efi_guid_t *vendor);
+typedef efilinkage efi_status_t efi_set_variable_t (efi_char16_t *name,
+						    efi_guid_t *vendor,
+						    unsigned long attr,
+						    unsigned long data_size,
+						    void *data);
+typedef efilinkage efi_status_t efi_get_next_high_mono_count_t (u32 *count);
+typedef efilinkage void efi_reset_system_t (int reset_type, 
+					    efi_status_t status,
+					    unsigned long data_size,
+					    efi_char16_t *data);
+typedef efilinkage efi_status_t efi_set_virtual_address_map_t (unsigned long memory_map_size,
 						unsigned long descriptor_size,
 						u32 descriptor_version,
 						efi_memory_desc_t *virtual_map);

Re: 2.6.20-rc7: known regressions

[Andi Kleen]

On Saturday 03 February 2007 10:49, Frédéric Riss wrote:

> Was what I did in the initial patch: 
>      http://lkml.org/lkml/2007/1/30/258 
> 
> The issue is that the structure definition is used on multiple
> architectures (for now ia64 and i386) which might used different calling
> conventions. As Bjorn Helgaas pointed out, ia64 already has such wrapper
> functions. I agree that the casting isn't the nicest thing, but I prefer
> that to writing asm stubs.

Define a efilinkage macro then that expands to nothing on ia64

Probably asmlinkage would work already, syscall_linkage as used on ia64 doesn't
seem to affect function pointers.

-Andi

Re: 2.6.20-rc7: known regressions

[Frédéric Riss]

Le samedi 03 février 2007 à 10:33 +0100, Andi Kleen a écrit :
> >  /*
> > + * Wrap all the virtual calls in a way that forces the parameters on the stack.
> > + */
> > +
> > +#define efi_call_virt(f, args...) \
> > +     ((efi_##f##_t __attribute__((regparm(0)))*)efi.systab->runtime->f)(args)
> > +
> > +static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
> > +{
> > +	return efi_call_virt(get_time, tm, tc);
> 
> Wouldn't it be better to just declare the pointers in efi.systab with 
> the correct attribute?  Then you wouldn't need all that ugly casting.

Was what I did in the initial patch: 
     http://lkml.org/lkml/2007/1/30/258 

The issue is that the structure definition is used on multiple
architectures (for now ia64 and i386) which might used different calling
conventions. As Bjorn Helgaas pointed out, ia64 already has such wrapper
functions. I agree that the casting isn't the nicest thing, but I prefer
that to writing asm stubs.

Fred

Re: 2.6.20-rc7: known regressions

[Andi Kleen]

>  /*
> + * Wrap all the virtual calls in a way that forces the parameters on the stack.
> + */
> +
> +#define efi_call_virt(f, args...) \
> +     ((efi_##f##_t __attribute__((regparm(0)))*)efi.systab->runtime->f)(args)
> +
> +static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
> +{
> +	return efi_call_virt(get_time, tm, tc);

Wouldn't it be better to just declare the pointers in efi.systab with 
the correct attribute?  Then you wouldn't need all that ugly casting.

-Andi

Re: 2.6.20-rc7: known regressions

[Andrew Morton]

On Sat, 03 Feb 2007 10:19:51 +0100 Frédéric Riss <frederic.riss@gmail.com> wrote:

> Le vendredi 02 février 2007 à 17:55 -0800, Andrew Morton a écrit :
> > - I have efi-x86-pass-firmware-call-parameters-on-the-stack.patch, but
> >   I'm not sure it's right and unless something really rapid happens, we'll
> >   ship with that bug unfixed.
> 
> Things I can say:
>  - Works for me :-)
>  - When you look at the code, it's obvious that switching to -mregparm=3
> changed the way we call into EFI runtime services. If you consider that
> that old code was correct, then the patch is needed to keep the good
> calling convention.
>  - It touches only arch/i386/kernel/efi.c which is compiled only with
> CONFIG_EFI && X86
>  - It changes code that is called only when booted in EFI mode.
> 
> Last 2 points mean the user base is pretty limited, which can be taken
> both as an argument to push it for the release or not to. I'd obviously
> prefer that someone knowledgeable about EFI looks at it and ACKs before
> it goes in.
> 

OK, well here it is again for everyone's reviwing pleasure:


From: Frederic Riss <frederic.riss@gmail.com>

When calling into the EFI firmware, the parameters need to be passed on
the stack. The recent change to use -mregparm=3 breaks x86 EFI support.
This patch is needed to allow the new Intel-based Macs to suspend to ram
(efi.get_time is called during the suspend phase).

Signed-off-by: Frederic Riss <frederic.riss@gmail.com>
Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
Cc: <artiom.myaskouvskey@intel.com>
Cc: Matt Domsch <Matt_Domsch@dell.com>
Cc: Andi Kleen <ak@suse.de>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 arch/i386/kernel/efi.c |   89 +++++++++++++++++++++++++++++++--------
 1 files changed, 73 insertions(+), 16 deletions(-)

diff -puN arch/i386/kernel/efi.c~efi-x86-pass-firmware-call-parameters-on-the-stack arch/i386/kernel/efi.c
--- a/arch/i386/kernel/efi.c~efi-x86-pass-firmware-call-parameters-on-the-stack
+++ a/arch/i386/kernel/efi.c
@@ -473,6 +473,70 @@ static inline void __init check_range_fo
 }
 
 /*
+ * Wrap all the virtual calls in a way that forces the parameters on the stack.
+ */
+
+#define efi_call_virt(f, args...) \
+     ((efi_##f##_t __attribute__((regparm(0)))*)efi.systab->runtime->f)(args)
+
+static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
+{
+	return efi_call_virt(get_time, tm, tc);
+}
+
+static efi_status_t virt_efi_set_time (efi_time_t *tm)
+{
+	return efi_call_virt(set_time, tm);
+}
+
+static efi_status_t virt_efi_get_wakeup_time (efi_bool_t *enabled,
+					      efi_bool_t *pending,
+					      efi_time_t *tm)
+{
+	return efi_call_virt(get_wakeup_time, enabled, pending, tm);
+}
+
+static efi_status_t virt_efi_set_wakeup_time (efi_bool_t enabled,
+					      efi_time_t *tm)
+{
+	return efi_call_virt(set_wakeup_time, enabled, tm);
+}
+
+static efi_status_t virt_efi_get_variable (efi_char16_t *name,
+					   efi_guid_t *vendor, u32 *attr,
+					   unsigned long *data_size, void *data)
+{
+	return efi_call_virt(get_variable, name, vendor, attr, data_size, data);
+}
+
+static efi_status_t virt_efi_get_next_variable (unsigned long *name_size,
+						efi_char16_t *name,
+						efi_guid_t *vendor)
+{
+	return efi_call_virt(get_next_variable, name_size, name, vendor);
+}
+
+static efi_status_t virt_efi_set_variable (efi_char16_t *name,
+					   efi_guid_t *vendor,
+					   unsigned long attr,
+					   unsigned long data_size, void *data)
+{
+	return efi_call_virt(set_variable, name, vendor, attr, data_size, data);
+}
+
+static efi_status_t virt_efi_get_next_high_mono_count (u32 *count)
+{
+	return efi_call_virt(get_next_high_mono_count, count);
+}
+
+static void virt_efi_reset_system (int reset_type, efi_status_t status,
+				   unsigned long data_size,
+				   efi_char16_t *data)
+{
+	efi_call_virt(reset_system, reset_type, status, data_size, data);
+}
+
+/*
  * This function will switch the EFI runtime services to virtual mode.
  * Essentially, look through the EFI memmap and map every region that
  * has the runtime attribute bit set in its memory descriptor and update
@@ -525,22 +589,15 @@ void __init efi_enter_virtual_mode(void)
 	 * pointers in the runtime service table to the new virtual addresses.
 	 */
 
-	efi.get_time = (efi_get_time_t *) efi.systab->runtime->get_time;
-	efi.set_time = (efi_set_time_t *) efi.systab->runtime->set_time;
-	efi.get_wakeup_time = (efi_get_wakeup_time_t *)
-					efi.systab->runtime->get_wakeup_time;
-	efi.set_wakeup_time = (efi_set_wakeup_time_t *)
-					efi.systab->runtime->set_wakeup_time;
-	efi.get_variable = (efi_get_variable_t *)
-					efi.systab->runtime->get_variable;
-	efi.get_next_variable = (efi_get_next_variable_t *)
-					efi.systab->runtime->get_next_variable;
-	efi.set_variable = (efi_set_variable_t *)
-					efi.systab->runtime->set_variable;
-	efi.get_next_high_mono_count = (efi_get_next_high_mono_count_t *)
-					efi.systab->runtime->get_next_high_mono_count;
-	efi.reset_system = (efi_reset_system_t *)
-					efi.systab->runtime->reset_system;
+	efi.get_time = virt_efi_get_time;
+	efi.set_time = virt_efi_set_time;
+	efi.get_wakeup_time = virt_efi_get_wakeup_time;
+	efi.set_wakeup_time = virt_efi_set_wakeup_time;
+	efi.get_variable = virt_efi_get_variable;
+	efi.get_next_variable = virt_efi_get_next_variable;
+	efi.set_variable = virt_efi_set_variable;
+	efi.get_next_high_mono_count = virt_efi_get_next_high_mono_count;
+	efi.reset_system = virt_efi_reset_system;
 }
 
 void __init
_

Re: 2.6.20-rc7: known regressions

[Frédéric Riss]

Le vendredi 02 février 2007 à 17:55 -0800, Andrew Morton a écrit :
> - I have efi-x86-pass-firmware-call-parameters-on-the-stack.patch, but
>   I'm not sure it's right and unless something really rapid happens, we'll
>   ship with that bug unfixed.

Things I can say:
 - Works for me :-)
 - When you look at the code, it's obvious that switching to -mregparm=3
changed the way we call into EFI runtime services. If you consider that
that old code was correct, then the patch is needed to keep the good
calling convention.
 - It touches only arch/i386/kernel/efi.c which is compiled only with
CONFIG_EFI && X86
 - It changes code that is called only when booted in EFI mode.

Last 2 points mean the user base is pretty limited, which can be taken
both as an argument to push it for the release or not to. I'd obviously
prefer that someone knowledgeable about EFI looks at it and ACKs before
it goes in.

Fred.

Re: 2.6.20-rc7: known regressions

[Andrew Morton]

On Fri, 02 Feb 2007 21:03:48 -0500
Jeff Garzik <jeff@garzik.org> wrote:

> Andrew Morton wrote:
> > On Fri, 2 Feb 2007 06:49:16 +0100
> > Adrian Bunk <bunk@stusta.de> wrote:
> > 
> >> This email lists some known regressions in 2.6.20-rc7 compared to 2.6.19
> >> that are not yet fixed in Linus' tree.
> > 
> > There are still a few things hanging around.
> > 
> > I have these queued:
> > 
> > aio-fix-buggy-put_ioctx-call-in-aio_complete-v2.patch
> > kexec-avoid-migration-of-already-disabled-irqs-ia64.patch
> > net-smc911x-match-up-spin-lock-unlock.patch
> > rtc-pcf8563-detect-polarity-of-century-bit-automatically.patch
> > alpha-fix-epoll-syscall-enumerations.patch
> > revert-blockdev-direct-io-back-to-2619-version.patch
> > scsi-sd-udev-accessing-an-uninitialized-scsi_disk-results-in-a-crash.patch
> > altix-more-acpi-prt-support.patch
> 
> Would you forward the x86-64 dma_noncoherent API build fix I posted? 
> Anything that uses that API won't build on x86-64 without my [simple and 
> obvious] patch.

Yup.  That's this:

--- a/include/asm-x86_64/dma-mapping.h~x86-64-define-dma-noncoherent-api-functions
+++ a/include/asm-x86_64/dma-mapping.h
@@ -63,6 +63,9 @@ static inline int dma_mapping_error(dma_
 	return (dma_addr == bad_dma_address);
 }
 
+#define dma_alloc_noncoherent(d, s, h, f) dma_alloc_coherent(d, s, h, f)
+#define dma_free_noncoherent(d, s, v, h) dma_free_coherent(d, s, v, h)
+
 extern void *dma_alloc_coherent(struct device *dev, size_t size,
 				dma_addr_t *dma_handle, gfp_t gfp);
 extern void dma_free_coherent(struct device *dev, size_t size, void *vaddr,
_

> 
> > - I have r8169-fix-a-race-between-pci-probe-and-dev_open.patch floating
> >   about, but I forget its status.  
> 
> I posted a preferred patch (which someone then noted need to use 
> setup_timer), and am waiting for an "it works" response of some sort

OK, thanks, I'll drop it.

Re: 2.6.20-rc7: known regressions

[Jeff Garzik]

Andrew Morton wrote:
> On Fri, 2 Feb 2007 06:49:16 +0100
> Adrian Bunk <bunk@stusta.de> wrote:
> 
>> This email lists some known regressions in 2.6.20-rc7 compared to 2.6.19
>> that are not yet fixed in Linus' tree.
> 
> There are still a few things hanging around.
> 
> I have these queued:
> 
> aio-fix-buggy-put_ioctx-call-in-aio_complete-v2.patch
> kexec-avoid-migration-of-already-disabled-irqs-ia64.patch
> net-smc911x-match-up-spin-lock-unlock.patch
> rtc-pcf8563-detect-polarity-of-century-bit-automatically.patch
> alpha-fix-epoll-syscall-enumerations.patch
> revert-blockdev-direct-io-back-to-2619-version.patch
> scsi-sd-udev-accessing-an-uninitialized-scsi_disk-results-in-a-crash.patch
> altix-more-acpi-prt-support.patch

Would you forward the x86-64 dma_noncoherent API build fix I posted? 
Anything that uses that API won't build on x86-64 without my [simple and 
obvious] patch.


> - I have r8169-fix-a-race-between-pci-probe-and-dev_open.patch floating
>   about, but I forget its status.  

I posted a preferred patch (which someone then noted need to use 
setup_timer), and am waiting for an "it works" response of some sort

	Jeff

Re: 2.6.20-rc7: known regressions

[Andrew Morton]

On Fri, 2 Feb 2007 06:49:16 +0100
Adrian Bunk <bunk@stusta.de> wrote:

> This email lists some known regressions in 2.6.20-rc7 compared to 2.6.19
> that are not yet fixed in Linus' tree.

There are still a few things hanging around.

I have these queued:

aio-fix-buggy-put_ioctx-call-in-aio_complete-v2.patch
kexec-avoid-migration-of-already-disabled-irqs-ia64.patch
net-smc911x-match-up-spin-lock-unlock.patch
rtc-pcf8563-detect-polarity-of-century-bit-automatically.patch
alpha-fix-epoll-syscall-enumerations.patch
revert-blockdev-direct-io-back-to-2619-version.patch
scsi-sd-udev-accessing-an-uninitialized-scsi_disk-results-in-a-crash.patch
altix-more-acpi-prt-support.patch

which I'll get through to Linus later today.  Plus:


- x86_64-irq-simplfy-__assign_irq_vector.patch and
  x86_64-irq-handle-irqs-pending-in-irr-during-irq-migration.patch which
  are big and scary.  Am awaiting feedback from Andi and Eric on what to do
  with these.

- A fix from Trond for http://bugzilla.kernel.org/show_bug.cgi?id=7923. 
  Am awaiting acks to merge that.

- sky2-flow-control-off.patch from shemminger which I assume Linus will
  be merging anyway.

- v9fs_vfs_mkdir-fix-a-double-free.patch which I guess I'll merge unless
  Eric suddenly nacks it.

- I have r8169-fix-a-race-between-pci-probe-and-dev_open.patch floating
  about, but I forget its status.  

- I have efi-x86-pass-firmware-call-parameters-on-the-stack.patch, but
  I'm not sure it's right and unless something really rapid happens, we'll
  ship with that bug unfixed.

- enable-mouse-button-23-emulation-for-x86-macs.patch looks simple
  enough, but I'm waiting for Ben to wake up.

- x86-fix-vdso-mapping-for-aout-executables.patch probably works OK, but
  Andi points out that it'd be better to implement this with
  attribute-weak.  So I guess 2.6.20 will ship with non-functional a.out on
  i386, like 2.6.29.

2.6.20-rc7: known regressions

[Adrian Bunk]

This email lists some known regressions in 2.6.20-rc7 compared to 2.6.19
that are not yet fixed in Linus' tree.

If you find your name in the Cc header, you are either submitter of one
of the bugs, maintainer of an affectected subsystem or driver, a patch
of you caused a breakage or I'm considering you in any other way possibly
involved with one or more of these issues.

Due to the huge amount of recipients, please trim the Cc when answering.


Subject    : NULL pointer dereference at as_move_to_dispatch()
References : http://lkml.org/lkml/2007/1/22/141
Submitter  : Andrew Vasquez <andrew.vasquez@qlogic.com>
Status     : unknown


Subject    : pktcdvd doesn't work with libata pata drivers
References : http://bugzilla.kernel.org/show_bug.cgi?id=7810
             http://lkml.org/lkml/2007/1/25/128
             http://bugzilla.kernel.org/show_bug.cgi?id=7910
             http://lkml.org/lkml/2007/1/30/289
Submitter  : Gerhard Dirschl <gd@spherenet.de>
             Luca Tettamanti <kronos.it@gmail.com>
Caused-By  : Christoph Hellwig <hch@lst.de>
             commit 3b00315799d78f76531b71435fbc2643cd71ae4c
             commit 406c9b605cbc45151c03ac9a3f95e9acf050808c
Status     : unknown


Subject    : powerpc64: performance monitor exception
References : http://ozlabs.org/pipermail/linuxppc-dev/2007-January/030045.html
Submitter  : Livio Soares <livio@eecg.toronto.edu>
Caused-By  : Paul Mackerras <paulus@samba.org>
             commit d04c56f73c30a5e593202ecfcf25ed43d42363a2
Status     : unknown


Subject    : reboot instead of powerdown  (CONFIG_USB_SUSPEND)
References : http://lkml.org/lkml/2006/12/25/40
             http://bugzilla.kernel.org/show_bug.cgi?id=7828
Submitter  : Berthold Cogel <cogel@rrz.uni-koeln.de>
             François Valenduc <francois.valenduc@skynet.be>
Handled-By : Alan Stern <stern@rowland.harvard.edu>
Status     : problem is being debugged


Subject    : usb somehow broken  (CONFIG_USB_SUSPEND)
References : http://lkml.org/lkml/2007/1/11/146
Submitter  : Prakash Punnoor <prakash@punnoor.de>
Handled-By : Oliver Neukum <oliver@neukum.org>
             Alan Stern <stern@rowland.harvard.edu>
Status     : problem is being debugged


Subject    : BUG: at fs/inotify.c:172 set_dentry_child_flags()
References : http://bugzilla.kernel.org/show_bug.cgi?id=7785
Submitter  : Cijoml Cijomlovic Cijomlov <cijoml@volny.cz>
Handled-By : Nick Piggin <nickpiggin@yahoo.com.au>
Status     : problem is being debugged


Subject    : fix geode_configure()
References : http://lkml.org/lkml/2007/1/9/216
Submitter  : Lennart Sorensen <lsorense@csclub.uwaterloo.ca>
Caused-By  : takada <takada@mbf.nifty.com>
             commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba
Handled-By : takada <takada@mbf.nifty.com>
             Lennart Sorensen <lsorense@csclub.uwaterloo.ca>
Status     : patches are being discussed