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

Linux 2.6.20-rc7

[Linus Torvalds]

Yes, I know I said I would only do -rc6 and then the final 2.6.20, but the 
thing is, the known regressions list didn't get whittled down as quickly 
as I hoped, and as a result we now have a -rc7.

It's in good enough shape that I'd probably have been happy to just 
release it as 2.6.20, but since I want 2.6.20 to be a stability release, I 
didn't want to risk any stupid bugs while the regressions got fixed, so 
here's a final -rc7.

In other words, please do give it a good testing. We should have fixed the 
nasty stuff on Adrian's list (and here's another thanks to Adrian for 
keeping me on my toes!) and it's all good. But please give it a quick 
shake-down to make sure that nothing silly happened while fixing the bad 
stuff.

The shortlog really does say most of it - this is just various fixes for a 
number of mostly fairly inconsequential things, but the SG_IO timeout bug 
that hit any NeroLinux user would quite possibly impact other DVD/CD 
reader/writer programs too that used raw commands with timeouts.

The diffstat just looks like line-noise: 244 files changed with an average 
of less than 10 lines per file changed in 179 commits. In other words, 
really no big diffs: it's just a lot of really small stuff.

		Linus

---
Adam Litke (1):
      Don't allow the stack to grow into hugetlb reserved regions

Adrian Bunk (1):
      fs/lockd/clntlock.c: add missing newlines to dprintk's

Ahmed S. Darwish (1):
      [CPUFREQ] check sysfs_create_link return value

Al Viro (9):
      b44: src_desc->addr is little-endian
      missing exports of pm_power_off() on alpha and sparc32
      mtd/nand/cafe.c missing include of dma-mapping.h
      sym53c500_cs: remove bogus call fo free_dma()
      pata_platform: fallout from set_mode() change
      missing dma_sync_single_range_for{cpu,device} on alpha
      dma-mapping.h stubs fix
      b44: src_desc->addr is little-endian
      fix indentation-related breakage in Kconfig.i386

Alan Cox (5):
      ide/generic: Jmicron has its own drivers now
      libata cmd64x: whack into a shape that looks like the documentation
      libata hpt3xn: Hopefully sort out the DPLL logic versus the vendor code
      libata: set_mode, Fix the FIXME
      libata-sff: Don't call bmdma_stop on non DMA capable controllers

Alexey Dobriyan (2):
      Fix NULL ->nsproxy dereference in /proc/*/mounts
      core-dumping unreadable binaries via PT_INTERP

Andrew Morton (5):
      jmicron: fix warning
      pata_platform: set_mode fix
      82596 warning fixes
      m68k: uaccess.h needs sched.h
      ntfs: kmap_atomic() atomicity fix

Andrew Victor (6):
      [ARM] 4084/1: Remove CONFIG_DEBUG_WAITQ
      [ARM] 4085/1: AT91: Header fixes.
      [ARM] 4086/1: AT91: Whitespace cleanup
      [ARM] 4087/1: AT91: CPU reset for SAM9x processors
      [ARM] 4088/1: AT91: Unbalanced IRQ in serial driver suspend/resume
      [ARM] 4089/1: AT91: GPIO wake IRQ cleanup

Andy Gospodarek (1):
      bonding: ARP monitoring broken on x86_64

Atsushi Nemoto (1):
      SPI: alternative fix for spi_busnum_to_master

Auke Kok (1):
      e100: fix irq leak on suspend/resume

Avi Kivity (3):
      KVM: Emulate IA32_MISC_ENABLE msr
      KVM: MMU: Perform access checks in walk_addr()
      KVM: MMU: Report nx faults to the guest

Bartlomiej Zolnierkiewicz (3):
      ide: update MAINTAINERS entry
      ia64: add pci_get_legacy_ide_irq()
      ide: add missing __init tags to IDE PCI host drivers

Baruch Even (1):
      [TCP]: Fix sorting of SACK blocks.

Ben Dooks (4):
      [ARM] 4095/1: S3C24XX: Fix GPIO set for Bank A
      [ARM] 4096/1: S3C24XX: change return code form s3c2410_gpio_getcfg()
      S3C24XX: fix passing spi chipselect to select routine
      [ARM] 4117/1: S3C2412: Fix writel() usage in selection code

Benjamin Herrenschmidt (1):
      [POWERPC] Fix sys_pciconfig_iobase bus matching

Catalin Marinas (2):
      [ARM] 4112/1: Only ioremap to supersections if DOMAIN_IO is zero
      [ARM] 4111/1: Allow VFP to work with thread migration on SMP

Conke Hu (3):
      atiixp.c: remove unused code
      atiixp.c: sb600 ide only has one channel
      atiixp.c: add cable detection support for ATI IDE

Dan Williams (1):
      [ARM] 4100/1: iop3xx: fix cpu mask for iop333

Dave Jones (5):
      [AGPGART] Prevent (unlikely) memory leak in amd_create_gatt_pages()
      [AGPGART] Remove pointless typedef in ati-agp
      [AGPGART] Remove pointless assignment.
      [AGPGART] Add new IDs to VIA AGP.
      [CPUFREQ] Remove unneeded errata workaround from p4-clockmod.

David Barksdale (1):
      IPMI: fix timeout list handling

David Milburn (1):
      libata-scsi: ata_task_ioctl should return ATA registers from sense data

David S. Miller (4):
      [AF_PACKET]: Fix BPF handling.
      [AF_PACKET]: Check device down state before hard header callbacks.
      [TCP]: Restore SKB socket owner setting in tcp_transmit_skb().
      [SPARC64]: Set g4/g5 properly in sun4v dtlb-prot handling.

David Woodhouse (1):
      Fix Maple PATA IRQ assignment.

Dmitriy Monakhov (1):
      Broadcom 4400 resume small fix

Eric Van Hensbergen (5):
      9p: fix bogus return code checks during initialization
      9p: fix rename return code
      9p: update documentation regarding server applications
      9p: fix segfault caused by race condition in meta-data operations
      9p: null terminate error strings for debug print

Eric W. Biederman (3):
      [IPV4]: Fix the fib trie iterator to work with a single entry routing tables
      [DECNET]: Handle a failure in neigh_parms_alloc (take 2)
      i386: In assign_irq_vector look at all vectors before giving up

Evgeniy Dushistov (3):
      ufs: alloc metadata null page fix
      ufs: truncate negative to unsigned fix
      ufs: reallocation fix

Francois Romieu (1):
      netdev: add a MAINTAINERS entry for via-velocity and update my address

Geert Uytterhoeven (3):
      [POWERPC] PS3: Fix uniprocessor kernel build
      [POWERPC] ps3_free_io_irq: Fix inverted error check
      `make help' in build tree doesn't show headers_* targets

Geoff Levand (1):
      [POWERPC] PS3: add not complete comment to kconfig

H. Peter Anvin (1):
      Boot loader ID for Gujin

Haavard Skinnemoen (2):
      [AVR32] Export clear_page symbol
      [AVR32] Update ATSTK1000 defconfig: Enable macb by default

Hugh Dickins (1):
      mm: mremap correct rmap accounting

Ingo Molnar (1):
      ACPI: fix cpufreq regression

Jan Altenberg (1):
      Malta: Fix build if CONFIG_MTD is diabled.

Jan Engelhardt (1):
      cdev.h: forward declarations

Jean Delvare (1):
      Fix VIA quirks

Jeff Dike (2):
      Fix UML on non-standard VM split hosts
      uml: fix signal frame alignment

Jiri Kosina (3):
      HID: fix memleaking of collection
      USB HID: fix hid_blacklist clash for 0x08ca/0x0010
      HID: fix pb_fnmode and move it to generic HID

Joerg Roedel (1):
      KVM: SVM: Propagate cpu shutdown events to userspace

Johannes Stezenbach (1):
      uml: fix mknod

Josepch Chan (1):
      via82cxxx/pata_via: correct PCI_DEVICE_ID_VIA_SATA_EIDE ID and add support for CX700 and 8237S

Jun'ichi Nomura (1):
      dm-multipath: fix stall on noflush suspend/resume

Justin Clacherty (1):
      spi: fix error setting the spi mode in pxa2xx_spi.c

Lennert Buytenhek (1):
      ata_if_xfermask() word 51 fix

Leonard Norrgard (1):
      KVM: SVM: Fix SVM idt confusion

Linus Torvalds (6):
      Resurrect 'try_to_free_buffers()' VM hackery
      Write back inode data pages even when the inode itself is locked
      Fix balance_dirty_page() calculations with CONFIG_HIGHMEM
      Revert "[PATCH] namespaces: fix exit race by splitting exit"
      Revert "net: ifb error path loop fix"
      Linux 2.6.20-rc7

Linus Walleij (1):
      [ARM] 4102/1: Allow for PHYS_OFFSET on any valid 2MiB address

Mariusz Kozlowski (1):
      net: ifb error path loop fix

Mark Fasheh (1):
      ocfs2: fix thinko in ocfs2_backup_super_blkno()

Masami Hiramatsu (1):
      kprobes: replace magic numbers with enum

Matt Domsch (1):
      Fix race in efi variable delete code

Matt Reimer (1):
      [ARM] 4106/1: S3C2410: typo fixes in register definitions

Michael Chan (2):
      [BNX2]: Fix 2nd port's MAC address.
      b44: Fix frequent link changes

Mike Christie (1):
      Fix SG_IO timeout jiffy conversion

Mike Frysinger (4):
      remove __devinit markings from rtc_sysfs_add_device()
      use __u8/__u32 in userspace ioctl defines for I2O
      use __u8 rather than u8 in userspace SIZE defines in hdreg.h
      translate dashes in filenames for headers install

Miklos Szeredi (1):
      fuse: fix bug in control filesystem mount

Neil Brown (1):
      Remove warning: VFS is out of sync with lock manager

NeilBrown (12):
      knfsd: update email address and status for NFSD in MAINTAINERS
      knfsd: fix setting of ACL server versions
      knfsd: fix an NFSD bug with full sized, non-page-aligned reads
      knfsd: replace some warning ins nfsfh.h with BUG_ON or WARN_ON
      md: update email address and status for MD in MAINTAINERS
      md: make 'repair' actually work for raid1
      md: make sure the events count in an md array never returns to zero
      md: avoid reading past the end of a bitmap file
      knfsd: Fix type mismatch with filldir_t used by nfsd
      md: fix potential memalloc deadlock in md
      md: remove unnecessary printk when raid5 gets an unaligned read.
      knfsd: ratelimit some nfsd messages that are triggered by external events

Nick Piggin (1):
      Fix try_to_free_buffer() locking

Patrick McHardy (3):
      [NETFILTER]: nf_nat: fix ICMP translation with statically linked conntrack
      [NETFILTER]: nf_nat_pptp: fix expectation removal
      [NETFILTER]: nf_conntrack_pptp: fix NAT setup of expected GRE connections

Pavel Pisa (1):
      [ARM] 4092/1: i.MX/MX1 CPU Frequency scaling latency definition

Peter Staubach (1):
      knfsd: Don't mess with the 'mode' when storing a exclusive-create cookie

Ralf Baechle (1):
      [MIPS] Ocelot G: Fix a few misspellings of CONFIG_GALILEO_GT64240_ETH

Robert Hancock (1):
      libata: fix translation for START STOP UNIT

Robert Olsson (1):
      [IPV4]: Fix single-entry /proc/net/fib_trie output.

Robert P. J. Day (3):
      fix various kernel-doc in header files
      [MIPS] Fix typo of "CONFIG_MT_SMP".
      Fix "CONFIG_X86_64_" typo in drivers/kvm/svm.c

Roland McGrath (8):
      x86_64: fix put_user for 64-bit constant
      Fix CONFIG_COMPAT_VDSO
      Fix gate_vma.vm_flags
      Add VM_ALWAYSDUMP
      i386 vDSO: use VM_ALWAYSDUMP
      x86_64 ia32 vDSO: use VM_ALWAYSDUMP
      powerpc vDSO: use VM_ALWAYSDUMP
      x86_64 ia32 vDSO: define arch_vma_name

Russell King (3):
      [ARM] Fix show_mem() for discontigmem
      [ARM] Update mach-types
      [ARM] Fix AMBA serial drivers for non-first serial ports

Serge E. Hallyn (2):
      namespaces: fix exit race by splitting exit
      namespaces: fix task exit disaster

Sergei Shtylyov (1):
      pata_sil680: PIO1 taskfile transfers overclocking fix (repost)

Simon Bennett (1):
      HID: fix hid-input mapping for Firefly Mini Remote Control

Stephen Hemminger (1):
      sky2: revert IRQ dance on suspend/resume

Takashi Iwai (1):
      ALSA: Fix sysfs breakage

Tejun Heo (9):
      sata_via: don't diddle with ATA_NIEN in ->freeze
      ahci: improve and limit spurious interrupt messages, take#3
      libata: implement ATA_FLAG_IGN_SIMPLEX and use it in sata_uli
      ahci: fix endianness in spurious interrupt message
      sata_via: style clean up, no indirect method call in LLD
      ahci: use 0x80 as wait stat value instead of 0xff
      ahci: port_no should be used when clearing IRQ in ahci_thaw()
      libata: fix ata_eh_suspend() return value
      ide: unregister idepnp driver on unload

Thomas Klein (2):
      ehea: Fixed wrong jumbo frames status query
      ehea: Fixed missing tasklet_kill() call

Tilman Schmidt (1):
      Gigaset ISDN driver error handling fixes

Trond Myklebust (1):
      MM: Remove [PATCH] invalidate_inode_pages2_range() debug

Venkat Yekkirala (1):
      [SELINUX]: Fix 2.6.20-rc6 build when no xfrm

Vitaly Bordug (1):
      FS_ENET: OF-related fixup for FEC and SCC MAC's

Wang Zhenyu (1):
      [AGPGART] intel_agp: restore graphics device's pci space early in resume

ethanhsiao@jmicron.com (1):
      jmicron: 40/80pin primary detection

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

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.

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, 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

[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 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

[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

[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]

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: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]

> +#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 à 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

[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.

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

[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

[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

[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: 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

[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

[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