**** BEGIN LOGGING AT Thu Feb 11 02:59:57 2010 Feb 11 08:12:06 davidm Feb 11 08:32:10 saeed, i guess thats a bad time to catch him (2am at his place) Feb 11 08:41:45 hi ogra Feb 11 08:41:59 hey saeed Feb 11 08:42:28 hey saeed! Feb 11 08:42:31 I need some help with the power button on Dove DB Feb 11 08:42:36 Hi Michael Feb 11 08:42:41 ericm_, ^^^ Feb 11 08:42:50 saeed: whats the issue with the power button? Feb 11 08:43:09 saeed: long time no see Feb 11 08:43:11 I want to make it trigger user space suspend and power off Feb 11 08:43:32 ?? Feb 11 08:43:33 Hi bryan Feb 11 08:43:59 hey saeed, how u doing dude Feb 11 08:44:07 currenty that button implemented as input device Feb 11 08:44:27 and it reports EV_SUSPEND when pressed/unpressed Feb 11 08:47:14 hi eric Feb 11 08:51:06 Saeed, maybe you want your Xorg.log.0 pasted? Feb 11 08:52:33 here? Feb 11 08:52:40 paste.ubuntu.com Feb 11 08:53:51 http Feb 11 08:56:27 saeed: Try xev under Xorg; do you see any event? Feb 11 08:57:44 saeed: AFAIK, we expect all buttons to trigger Xorg key press events, then gnome-power-manager listens to them Feb 11 09:00:04 saeed: To quickly paste stuff to paste.ubuntu.com, you might find http://people.canonical.com/~cjwatson/ubuntu-paste useful Feb 11 09:00:13 Or install the pastebinit package Feb 11 09:01:34 check http://paste.ubuntu.com/373788/ Feb 11 09:02:21 You have evdev at least Feb 11 09:02:48 saeed: Could you try running xev in a terminal and checking whether the power button triggers a Xorg keypress event? Feb 11 09:02:57 xev doesn't show anything when pressing on the button Feb 11 09:03:04 (II) UnloadModule: "evdev" Feb 11 09:03:10 that's odd Feb 11 09:03:13 also lsof /dev/input/event0 is empty Feb 11 09:03:36 Ah the unload is for another device Feb 11 09:03:36 that it configures the kbd as a mouse looks wrong too Feb 11 09:04:13 saeed: Your xorg.log doesn't show any setup for event0 Feb 11 09:04:20 yes Feb 11 09:04:25 saeed: So probably Xorg doesn't think your event0 is an interesting device Feb 11 09:05:33 yes, I added some file to hal/fdi/policy Feb 11 09:06:01 I think we're trying not to use hal anymore, but straight udev Feb 11 09:06:39 In the xorg-server source package, patch debian/patches/12-Add-libudev-input-hotplug-backend.diff adds this stuff Feb 11 09:06:43 and xinput Feb 11 09:06:44 udev rules would look something like: Feb 11 09:06:44 SUBSYSTEM=="input", KERNEL=="event*", ENV{x11_driver}="evdev" Feb 11 09:06:44 SUBSYSTEM=="input", KERNEL=="event*", ENV{ID_CLASS}=="kbd", ENV{xkb.layout}="fr", ENV{xkb.options}="terminate:ctrl_alt_bksp,compose:lwin" Feb 11 09:07:00 i think we're supposed to use the xinput tool Feb 11 09:07:12 saeed: Check /lib/udev/rules.d/64-xorg-xkb.rules, does it match your device? Feb 11 09:07:17 but at least here i cant make it use any device Feb 11 09:07:28 (instead of xev) Feb 11 09:07:59 we should ask tseliot, he cares for the whole xinput stack Feb 11 09:08:10 That's just xkb actually Feb 11 09:08:35 saeed: I would add a ENV{x11_driver}="evdev" property to your device Feb 11 09:09:53 where should I add that? Feb 11 09:10:18 saeed: For now, add it manually to your local udev rules Feb 11 09:11:14 saeed: Something like /etc/udev/rules.d/90-power-button.rules Feb 11 09:11:18 at /etc/udev/rules.d/ ? Feb 11 09:11:39 ok Feb 11 09:11:49 with SUBSYSTEM=="input", KERNEL=="event0", ENV{x11_driver}="evdev" Feb 11 09:11:54 But I'm not an udev expert Feb 11 09:12:50 saeed: You can raise the verbosity of udev to debug this Feb 11 09:13:04 /etc/udev/udev.conf use info Feb 11 09:14:59 saeed: Ah I found some good examples Feb 11 09:15:01 saeed: /lib/udev/rules.d/66-xorg-synaptics.rules Feb 11 09:15:38 That sets ENV{x11_driver}="synaptics" and some additional options for some devices Feb 11 09:15:50 the generic rule is /lib/udev/rules.d/65-xorg-evdev.rules Feb 11 09:16:14 saeed: So your problem is that /lib/udev/rules.d/65-xorg-evdev.rules doesn't map your device to evdev Feb 11 09:16:38 saeed: It only maps devices of type keyboard, mouse, touchscreen or touchpad Feb 11 09:16:51 saeed: What intput type are you using? Feb 11 09:22:20 keyboard Feb 11 09:22:34 saeed: Odd, so the generic rule should have mapped it to the evdev xorg driver Feb 11 09:22:46 saeed: You probably want some udev debug to see if that gets properly assigned Feb 11 09:23:26 saeed: Basically make sure that x11_driver gets set to evdev, then it becomes a xorg-server issue if it didn't load an evdev xorg driver for this device Feb 11 09:25:17 (**) "Power Button": always reports core events Feb 11 09:25:17 (**) "Power Button": Device: "/dev/input/event0" Feb 11 09:25:17 (II) "Power Button": Found keys Feb 11 09:25:17 (II) "Power Button": Configuring as keyboard Feb 11 09:25:17 (II) XINPUT: Adding extended input device ""Power Button"" (type: KEYBOARD) Feb 11 09:25:23 thats what i have on my laptop Feb 11 09:25:35 so the keyboard driver should be fine for it Feb 11 09:26:35 s/keyboard driver/evdev driver in keyboard mode/ Feb 11 09:26:38 here is the lshal part for the button http://paste.ubuntu.com/373798/ Feb 11 09:28:26 hmm, you shouldnt be having hal installed if you run lucid Feb 11 09:28:51 * ogra wonders if that gets in the way somehow Feb 11 09:29:17 (it shouldnt though) Feb 11 09:29:34 saeed: udevadm info --query=all --path=/sys/class/input/event0 Feb 11 09:30:32 this is my power button http://paste.ubuntu.com/373800/ Feb 11 09:30:37 You can see x11_driver=evdev Feb 11 09:31:33 * lool had a great idea before sleeping in last night Feb 11 09:31:56 the info.capabilities is just input, maybe is should be input.keyboard? Feb 11 09:32:20 saeed: Yes, that's likely why the generic rule doesn't match Feb 11 09:32:38 saeed: On my power buttong I have E: ID_INPUT_KEY=1 Feb 11 09:32:44 saeed: Do you have it? Feb 11 09:36:27 no Feb 11 09:36:39 saeed: Ok, that's your issue then Feb 11 09:37:12 saeed: Actually there seem to be two types Feb 11 09:37:19 One for keyboard and one for key Feb 11 09:37:38 /lib/udev/rules.d/60-persistent-input.rules references ID_INPUT_KEYBOARD Feb 11 09:37:53 saeed: But you basically want to set your kernel driver evdev props to key or keyboard Feb 11 09:39:09 are we actually sure we want gpoi mapped to Xorg at all ? Feb 11 09:39:25 We're sure we want the power button mapped Feb 11 09:39:54 right, but that doesnt necessarily need to happen with evdev Feb 11 09:40:14 ogra: What do you propose? Feb 11 09:40:16 especially for gpio ... i.e. look at lirc which uses gpio buttons Feb 11 09:41:14 it doesnt have much useful stuff, but definately doesnt use evdev ... all we need is to capture the gpio even somehow and make g-p-m aware through devkit-power Feb 11 09:42:59 saeed: I checked the ACPI power button implementation, and it does: input->evbit[0] = BIT_MASK(EV_KEY); Feb 11 09:43:03 set_bit(KEY_POWER, input->keybit); Feb 11 09:43:04 though the devkit-power rules dont seem to have anything useful in them regarding buttons Feb 11 09:43:19 saeed: drivers/acpi/button.c:395 Feb 11 09:43:50 ok, I'll try that Feb 11 09:44:11 ogra: Can use your lirc device to power off? Feb 11 09:47:36 cooloney: WRT LP #516352, did you also cherry-pick the commit removing the file on package removal? Feb 11 09:47:37 Launchpad bug 516352 in linux-fsl-imx51 (Ubuntu) "fsl-imx51: need backport module.builtin to .31 kernel for lucid userspace compatibility (affects: 1)" [Medium,Fix released] https://launchpad.net/bugs/516352 Feb 11 09:48:14 cooloney: 69f2227cdca42c4bbaa6fc931e64385f7fdee38f Feb 11 09:48:20 grrr Feb 11 09:48:22 * ogra hates reconnects Feb 11 09:48:24 lool, /lib/udev/rules.d/95-keymap.rules has what we want Feb 11 09:48:26 for example ENV{DMI_VENDOR}=="Acer*", ATTR{[dmi/id]product_name}=="Extensa*", ATTR{[dmi/id]product_name}=="*5210*|*5220*|*5610*|*5620*|*5720*", RUN+="keymap $name 0xEE screenlock" Feb 11 09:48:29 (see the RUN part) Feb 11 09:48:31 saeed, do you see any events in dmesg if you press the button ? preferably with a keycode ? Feb 11 09:49:08 lool: oh, i just cherry picked 2 .32 patches to .31 kernel as we discussed before Feb 11 09:49:29 cooloney: You might want the above SHA too Feb 11 09:50:51 ogra: What you're pointing at is for extra keys on laptop keyboards Feb 11 09:51:00 lool, yes Feb 11 09:51:16 well, keymapping for buttons not used by X Feb 11 09:51:18 ogra: The mvl-dove driver doesn't need any specific rule IMO Feb 11 09:51:27 you want a low level power key event Feb 11 09:51:34 We just need to have the power key device using the proper class Feb 11 09:51:36 which isnt mapped atm Feb 11 09:51:49 I don't think that's needed, no Feb 11 09:52:00 well, as i understood it already emits an event Feb 11 09:52:12 its just not mapped to power Feb 11 09:52:26 ogra: As you can see in the Xorg log, the device is not seen by Xorg at all Feb 11 09:52:38 why should it be seen by Xorg at all ? Feb 11 09:52:40 While on your system and mine it is Feb 11 09:52:49 you want a power event from the kernel Feb 11 09:52:59 No Feb 11 09:53:09 You want a keypress event Feb 11 09:53:18 which you likely already get Feb 11 09:53:31 currenty that button implemented as input device Feb 11 09:53:31 and it reports EV_SUSPEND when pressed/unpressed Feb 11 09:54:11 if there is a keycode emitted too a rule will just map it properly Feb 11 09:54:32 ogra: Try it out Feb 11 09:54:39 lool: do you mean this "UBUNTU: [Config] armel -- cleanup to-be builtin modules", right? Feb 11 09:55:05 cooloney: No, I mean UBUNTU: Add modules.builtin.bin to prerm rm list Feb 11 09:55:57 cooloney: Try installing and removing a fsl-im51 kernel package, you will see that a file is left behind Feb 11 09:56:31 cooloney: (modules.builtin.bin) Feb 11 09:57:37 now xinput show the gpio device Feb 11 09:57:47 saeed: Wee Feb 11 09:57:50 xev shows nothing when pressing on the button Feb 11 09:57:55 thanks lool Feb 11 09:57:57 lool, * include modules.builtin in the binary debs ?? that one ? Feb 11 09:58:24 ogra: I don't understand what you're commenting on Feb 11 09:58:32 your discussion with cooloney Feb 11 09:58:40 brb Feb 11 09:58:42 ogra: Your comment doesn't make any sense in the discussion Feb 11 09:58:54 lool: ok, got you, i just recall that this patch is from you, right? Feb 11 09:58:57 saeed: You need to have the proper flags when sending the event too Feb 11 09:59:09 saeed: Specifically, it needs to be a key event Feb 11 09:59:26 cooloney: It is from me indeed, which is why I recall it Feb 11 09:59:44 cooloney: It should go together with the modules.builtin backport/cherry-pick you're doing Feb 11 10:00:27 yeah, i noticed that, thanks for reminding me Feb 11 10:00:37 but the SHA on my side is 48d7349bbc9e47a27c74cdb3e56bbcc92d9bab29 Feb 11 10:00:59 roc@roc-desktop:/opt/git/Ubuntu/lucid$ git show 48d7349bbc9e47a27c74cdb3e56bbcc92d9bab29 Feb 11 10:01:00 cooloney: You're right, it's 48d7349bbc9e47a27c74cdb3e56bbcc92d9bab29 Feb 11 10:01:02 commit 48d7349bbc9e47a27c74cdb3e56bbcc92d9bab29 Feb 11 10:01:05 Author: Loïc Minier Feb 11 10:01:07 Date: Wed Feb 3 05:32:45 2010 +0000 Feb 11 10:01:10 UBUNTU: Add modules.builtin.bin to prerm rm list Feb 11 10:01:12 BugLink: http://bugs.launchpad.net/bugs/516584 Feb 11 10:01:15 Launchpad bug 516584 in linux (Ubuntu) "modules.builtin.bin not removed on package removal (affects: 3) (dups: 1)" [Undecided,Fix released] Feb 11 10:01:29 cooloney: Must be a copy-paste error, sorry Feb 11 10:01:35 lool: ok, cool. I will cherry pick that to my branch Feb 11 10:01:43 which was included already Feb 11 10:01:44 lool: no problem, thanks a lot Feb 11 10:01:54 * ogra really wonders what you guys are discussing here Feb 11 10:02:11 ogra: oh, really? Feb 11 10:02:34 look at the imx changelog, apw usually includes the package fixes frim the linux package Feb 11 10:02:40 *from Feb 11 10:03:55 ogra: It's certainly not in the git log of the fsl-imx51 branch Feb 11 10:04:56 Besides, it needs to be adapted to apply on debian.fsl-imx51/ anyway Feb 11 10:05:07 cooloney: I checked the fsl-imx51 and it still needs fixing Feb 11 10:05:12 ogra: lool is right. Feb 11 10:05:13 lool, because there was never a modules.builtin.bin until recntly in imx51 Feb 11 10:05:13 cooloney: debian.fsl-imx51/control-scripts/prerm, @files_to_remove Feb 11 10:05:15 yep i missed that one when i pulled fsl-imx51 togther Feb 11 10:05:26 i need to cherry pick that patch with others Feb 11 10:05:26 ogra: And there is one now, because it was cherry-picked Feb 11 10:05:39 lool, right, since the very last upload Feb 11 10:05:50 which i pointed to before Feb 11 10:05:51 I wonder how many people it takes to convince ogra that I'm right Feb 11 10:06:01 oh, guys, no problem, Feb 11 10:06:07 i will fix that soon, Feb 11 10:06:09 its in mvl-dove as part of the rebase Feb 11 10:06:14 cooloney: thanks; not a big deal anyway Feb 11 10:06:26 Just a leftover file, but it causes a warning on package removal Feb 11 10:06:32 not in fsl-imx51 cause its a franken kernel Feb 11 10:06:33 apw: right, because mvl-dove is .32 based anyway Feb 11 10:06:48 apw: Do you have some script to adapt the debian.foo/ pathname when doing these rebases? Feb 11 10:06:58 yep, cooloney get it to me soon :) Feb 11 10:07:10 lool, nope, just use vi on the patch Feb 11 10:07:16 hehe ok Feb 11 10:07:49 apw: Would it be much work to turn on udebs for versatile? Feb 11 10:08:05 I'm sure that's something you'd be excited about :-) Feb 11 10:08:13 what the heck would we want those for Feb 11 10:08:28 i am sure the aa's would love a heap of more binary debs to wave though Feb 11 10:08:37 apw: I don't know for mobile team, but I'd love to point people at debian-installer images to install Ubuntu in qemu Feb 11 10:09:09 apw: That would allow providing just vmlinux on ports.ubuntu.com Feb 11 10:09:13 and a d-i initrd Feb 11 10:09:16 the work is not normally in eanbling them but in getting the result to pass the d-i pass afterwards Feb 11 10:09:38 if you don't have all modules built Feb 11 10:09:43 apw: something like http://ports.ubuntu.com/ubuntu-ports/dists/lucid/main/installer-armel/current/images/dove/ but for versatile Feb 11 10:10:20 apw: Ack; the work would probably involve adding some Provides to the binary package Feb 11 10:10:29 * cooloney is vi on the patch file as apw said. heh Feb 11 10:10:31 apw: Because a lot of modules are builtin in the versatile config Feb 11 10:11:08 loads of ?'s to be added and you have to build it to find out which ones Feb 11 10:11:17 apw: The d-i side of things is just commented out for versatile BTW; it's well supported in d-i Feb 11 10:11:27 apw: Right Feb 11 10:11:40 something like that Feb 11 10:12:40 apw: Currently, if we want to pull the official kernel for versatile from lucid, we have to download a .deb and unpack it to get to the vmlinuz file which allows running qemu Feb 11 10:13:06 i have no objection to udebs or not, they arn't a big bit of the build Feb 11 10:13:08 So having d-i images would allow people to use them for install, and would also make it easier for e.g. rootstock to pull a kernel Feb 11 10:13:19 apw: I'd love that Feb 11 10:13:44 apw: Do you want me to check formally whether there are objections/interest in that from the mobile team? Feb 11 10:13:49 sure Feb 11 10:14:08 Ok, mailing ubuntu-mobile@ Feb 11 10:19:52 apw: i found lool's patch can not apply to debian.fsl-imx51 directory, Feb 11 10:20:42 apw: FYI I just tested the in archive linux-image-2.6.32-13-versatile_2.6.32-13.18_armel.deb and it boots fine with lucid and karmic userspace Feb 11 10:20:48 apw: it does not have the whole line 'modules.alias.bin modules.dep.bin modules.symbols.bin' at all Feb 11 10:21:05 cooloney: You don't have .bin files with your fsl-imx51 kernel? Feb 11 10:21:40 apw: (I mailed ubuntu-mobile@) Feb 11 10:23:08 * cooloney wants to punch his head to the wall, messed up his board. Feb 11 10:23:25 i need to reinstall my lucid on my board now Feb 11 10:31:43 cooloney, then have a look at the debian.master version of that file Feb 11 10:31:54 there is another patch which pulls the other two in you need as well Feb 11 10:33:20 cooloney, ironically it was your first patch i think Feb 11 10:33:29 commit 8b453930465c15f490ea44c2987b0a1bfe71ed66 Feb 11 10:33:29 Author: Bryan Wu Feb 11 10:33:29 Date: Wed Mar 25 17:31:26 2009 +0800 Feb 11 10:33:29 UBUNTU: Add 3 missing files to prerm remove file list Feb 11 10:35:10 cooloney, so you need that commit copied over first, then lool's one Feb 11 10:41:16 apw: that is a good memory for me Feb 11 10:41:29 apw: will do it soon Feb 11 10:41:35 whenever man Feb 11 10:54:56 I changed the button to send EV_KEY instead on EV_PWR Feb 11 10:55:24 now I see message "Failed to suspend" (hibernate) Feb 11 10:55:46 I'll change the code to KEY_SLEEP instead of KEY_SUSPEND Feb 11 10:57:32 saeed: Great Feb 11 10:57:40 saeed: Where do you see the message? Feb 11 10:57:50 saeed: Are you running gnome-power-manager? Feb 11 10:58:11 yes, Feb 11 10:58:28 I think it failed to hibernate because the resume param was wrong Feb 11 10:58:52 saeed: drivers/acpi/button.c uses KEY_POWER for ACPI_BUTTON_TYPE_POWER, so that seems correct Feb 11 10:59:15 saeed: Ack Feb 11 11:00:01 saeed: And it uses KEY_SLEEP for ACPI_BUTTON_TYPE_SLEEP; it doesn't use KEY_SUSPEND Feb 11 11:00:54 oh, hibernation is disabled Feb 11 11:01:04 saeed: Yeah, you certainly don't want KEY_SUSPEND Feb 11 11:02:05 ok Feb 11 11:02:20 look, we are looking for the folowing behavior Feb 11 11:02:40 if button pressed for less that 3 seconds , the system enters standby mode Feb 11 11:03:10 othewise we want the system to shutdown Feb 11 11:03:16 saeed: Ok; so you would send KEY_SLEEP for < 3 secs and KEY_POWER otherwise Feb 11 11:03:38 that means driver change, right? Feb 11 11:03:53 saeed: Oh you would like to implement the 3 secs in userspace? Feb 11 11:04:15 yes Feb 11 11:05:06 btw, lucid keeps showing the "failed to suspend" again and again Feb 11 11:19:57 lool, bug 520028 ... what was the reason for dropping ubuntu-minimal ? Feb 11 11:19:58 Launchpad bug 520028 in project-rootstock "ubuntu-minimal doesn't build correctly for karmic (affects: 1)" [Undecided,New] https://launchpad.net/bugs/520028 Feb 11 11:20:08 iirc a patch from you dropped it Feb 11 11:20:51 (though if its really upstart being at fault, there should be a hard dep) Feb 11 11:42:49 guys, the suspend works fine now Feb 11 11:43:17 however I wish ubuntu show message that the system going to suspend to ram Feb 11 11:43:19 \o/ Feb 11 11:43:52 the rules file not needed of course Feb 11 11:45:07 saeed, /var/log/pm-suspend.log should have something Feb 11 11:51:18 ogra: here it is root@lucid-dove:~# cat /var/log/pm-suspend.log Initial commandline parameters: Tue Jan 11 21:19:32 CST 2011: Running hooks for suspend. /usr/lib/pm-utils/sleep.d/000kernel-change suspend suspend:success. /usr/lib/pm-utils/sleep.d/00logging suspend suspend:Linux lucid-dove 2.6.32.3-dove-5.0.0-rc3-00671-g43c704e-dirty #200 Thu Feb 11 13:34:59 IST 2010 armv7l GNx Module Size Used by Feb 11 11:51:22 sorry Feb 11 11:51:38 here it is http://paste.ubuntu.com/373887/ Feb 11 11:52:23 ue Jan 11 21:19:33 CST 2011: performing suspend Feb 11 11:52:29 thats the actual suspend Feb 11 11:52:41 /usr/lib/pm-utils/sleep.d/000kernel-change resume suspend:success. Feb 11 11:52:41 Tue Jan 11 21:19:48 CST 2011: Finished. Feb 11 11:52:46 is the final wakeup Feb 11 11:53:25 ogra: I was expecting gui message telling that the system going to suspend Feb 11 11:53:33 ah Feb 11 11:53:37 because it takce few seconds to do that Feb 11 11:53:58 usually the screen fades out immediately Feb 11 11:54:50 lool, eric, I see this kernel message when pressing on the button: Feb 11 11:54:51 keyboard.c: can't emulate rawmode for keycode 142 Feb 11 12:34:02 http://people.canonical.com/~ogra/babbage2-lucid-20100211-3.png Feb 11 12:34:21 netbook-launcher is on the screen after 35 sec :) Feb 11 12:34:47 Nice! Feb 11 12:35:06 That's getting close to boot speeds I find acceptable in my laptop. Feb 11 12:35:13 What's the suspend/resume delay? Feb 11 12:35:30 i think if jockey wouldnt run all the time in the background we'd get another 5 sec speedup Feb 11 12:35:49 suspend takes a while to process the scripts, resume is instant Feb 11 12:35:50 Don't we need jockey? Feb 11 12:36:01 we will need it once we have 3D drivers Feb 11 12:36:18 the prob is that its *scanning* permanently due to a bug Feb 11 12:36:28 That's extra nice. On my netwalker, I get about 2 seconds suspend and 3 seconds to resume (which Sharp advertises as a "3-second boot") Feb 11 12:36:55 heh Feb 11 12:37:21 i think suspend takes rather five here Feb 11 12:37:28 Well, I mean, really. How often to people actually boot. Even my laptop only gets booted for kernel updates. Feb 11 12:37:36 but resume is just up after pressing the button Feb 11 12:37:40 probably 1sec Feb 11 12:37:47 That's nice. Feb 11 12:38:48 saeed: I'm glad it works :) Feb 11 12:38:55 saeed: Did you try emulating the power key as well? Feb 11 12:39:00 saeed: Will you do this in kernel? Feb 11 12:39:36 saeed: Concerning the feedback on suspend: a) some devices blink the power led in the kernel, and b) the suspend is supposed to be so quick that you don't need feedback, right? :-) Feb 11 12:39:54 I didn't try the power key Feb 11 12:41:30 in karmic the system enters standby immediately Feb 11 12:41:38 using echo mem > .. Feb 11 12:42:54 regarding the power key, it's easier for me to do it kernel, but I'm not sure if this is the right way Feb 11 12:59:54 saeed: I would think that an userspace hack wouldn't get upstream either Feb 11 13:00:04 saeed: IMO it's a device design problem Feb 11 13:00:54 saeed: This is going to be in a physical device for which the user experience is going to be "this button can do multiple things"; the button is going to be labelled with or more icons depending on what it does Feb 11 13:01:00 In another design you would have two buttons Feb 11 13:01:41 So I don't know whether you're trying to do a generic device design or whether you're trying to demonstrate shutdown and suspend with a single button on a particular design Feb 11 13:01:56 It's hack either way, but it's really related to your hardware layout Feb 11 13:02:56 What's the goal again? Feb 11 13:03:47 The kernel can send events for either a power or a suspend button press Feb 11 13:04:06 saeed would like a shutdown to happen if pressed more than 3 seconds and a suspend otherwise Feb 11 13:04:11 But on what does that depend, in terms of user experience. Feb 11 13:04:20 Ah, yeah, that needs to be a kernel thing. Feb 11 13:04:42 I'd hope additionally for a hardware shutdown if the key is held longer. At least I use that on my Netwalker. Feb 11 13:04:50 (because sometimes software hangs) Feb 11 13:05:34 So HW differentiation for whether hold > 10s, and kernel differentiation for whether hold > 3s. Feb 11 13:06:12 But again, why bother shutting down with a button? Is it that frequent a use-case to turn a device off? Feb 11 13:07:54 saeed: Or alternatively, call your key "User button 1", send your own event and keycode and write a tool to handle it ;-) Feb 11 13:10:59 Um, in that case, just use the power button. Feb 11 13:11:22 We already have software that can take a power button event and perform various actions, which could be extended to meet the timing case. Feb 11 13:11:52 It might be simpler to have another software handle a timed key press and call into gpm rather than patching gpm Feb 11 13:11:57 (and easier to maintain/less intrusive) Feb 11 13:12:21 I guess. I just don't like to have too many special HW hacks. Feb 11 13:12:35 Which is why I'd prefer having it separate Feb 11 13:12:43 instead of running the hack in gpm Feb 11 13:12:49 I'd rather enable a use case for a broad spectrum of HW, and have the HW produce something that works with existing software, to preserve user choice. Feb 11 13:13:04 You can always remap the key in software if you want Feb 11 13:13:44 Right, that would be the special HW hack: "If you have this board, you need to remap your keys like this: ..." Feb 11 13:14:18 Well you're proposing to patch GPM to have a mode "If this hardware is detected and button is pressed more than n seconds, do something else" Feb 11 13:14:20 Adding a fork function for timing in g-p-m would enable this class of behaviour for arbitrary HW which didn't expose a sleep button (and there's lots of that) Feb 11 13:14:49 I'm proposing to have a separate software which doesn't have the "if you have this hardware" and if people want to replace the software to use gpm, they can either patch gpm or remap thier keys Feb 11 13:14:50 No. I'm proposing that g-p-m be patched to be able to have different actions happen for different timing delays in power button presses. Feb 11 13:14:54 No special HW detection. Feb 11 13:15:16 I don't think that'd fly Feb 11 13:15:42 Why not? Feb 11 13:15:54 I have hardware for three different architectures that doesn't have sleep keys. Feb 11 13:15:59 We'd have to run and maintain the patch for everybody instead of maintaining a single isolated trampoline Feb 11 13:16:13 Why wouldn't the patch go upstream? Feb 11 13:16:32 dmart, i remember you filed a bug once on building the kernel with thumb2 but i cant find it on LP, do you happen to remember the bug number ? Feb 11 13:16:34 Well that's my opinion that's it's too specific Feb 11 13:16:44 Upstream mught have a different opinion Feb 11 13:17:12 lool: How specific. Like I said, I have HW for three different architectures with no sleep button, and none of it is saeed's HW. Feb 11 13:17:41 persia: For instance, why do it for the power button and not the sleep button? Feb 11 13:18:12 lool: No reason at all. Generalisation is good. Feb 11 13:18:13 Then you also need some GUI, more GUI == more confusion Feb 11 13:18:28 ogra: can you remember what it was about? I don't remeber filing a bug on Thumb-2 kernels yet. Feb 11 13:18:37 Users with both power and sleep would be able to get to all of shutdown, hibernate, suspend, and screen-lock. Feb 11 13:19:06 dmart, well, that the kernel should be built with thumb2 (not the config option but the kernel binary being a thumb2 binary) Feb 11 13:19:31 i know we reviwed that in dallas and i thought it resulted in a bug you filed Feb 11 13:19:55 lool: My basic point being that the utility you describe would simply not work for any of my hardware without a sleep button. To me, this is HW-specific. Feb 11 13:20:31 ogra: I don't think I filed a bug. It's not required that we build the kernel in Thumb-2, but we can try it if you think we're ready. Feb 11 13:20:39 ogra: I think it is a config option actually Feb 11 13:20:53 dmart, well, apw seems to just research that Feb 11 13:21:29 yep ... 'we' have a sprint in a half hour on this stuff right? Feb 11 13:21:30 having smaller kernels surely helps speed up the boot Feb 11 13:21:41 apw, thats more about porting packages :) Feb 11 13:21:53 not sure kernel packages were meant there :) Feb 11 13:21:58 linux was listed, i noticed when i read the announcements Feb 11 13:22:04 ogra: Bear in mind that there may be platform-specific drivers with assembler code, so it might not all work out of the box. I would hope that there's not much of that however. Feb 11 13:22:06 ah Feb 11 13:22:37 Most of the assembler is in the general arch/arm code though, and that should be OK in the lucid kernel versions. Feb 11 13:25:35 persia: Technically you could make the utility I describe listen to power button instead and disable GPM Feb 11 13:28:44 lool: That would address all of my concerns :) Feb 11 13:59:59 I filed lp #520465 on the qemu/linux/eglibc/glib2.0 issue Feb 11 14:00:01 Launchpad bug 520465 in ubuntu "armel/versatile: glibc detected double-free or corruption (!prev) (affects: 1)" [Undecided,New] https://launchpad.net/bugs/520465 Feb 11 14:00:12 * asac waves Feb 11 14:00:19 It's not a priority for me, but it would be nice to fix it for lucid Feb 11 14:01:25 * ogra is here Feb 11 14:01:52 lool, its a priority for me to be able to use the archive kernels though Feb 11 14:01:58 * ogra subscribes Feb 11 14:02:07 * JamieBen1ett waves back Feb 11 14:02:26 ah, armel is subbed already, good Feb 11 14:02:30 dmart: dyfet: StevenK: persia: ping ;) Feb 11 14:02:40 Hi there Feb 11 14:02:44 pong Feb 11 14:02:51 * persia will be about 10 minutes late Feb 11 14:03:07 persia: ok. please read the backlog then ;) Feb 11 14:03:13 anyone else here for the porting sprint? Feb 11 14:03:14 Always :) Feb 11 14:03:16 dealing also with replacing a furnace this morning :( Feb 11 14:03:32 what is a furnace? Feb 11 14:03:36 * asac goes for dictionary Feb 11 14:04:01 big firey thing Feb 11 14:04:03 dyfet: so you cannot attend? Feb 11 14:04:07 http://en.wikipedia.org/wiki/Furnace :) Feb 11 14:04:13 oven :) Feb 11 14:04:15 No I can... Feb 11 14:04:22 good Feb 11 14:04:32 Just annoying distraction ;) Feb 11 14:04:42 ok Feb 11 14:04:44 and a bit cold this morning ;) Feb 11 14:04:59 While we're waiting for persia, people might want to pull up https://wiki.ubuntu.com/ARM/Thumb2PortingHowto Feb 11 14:05:01 that should be fine ... our excersizes will make you feel hot i am sure ;) Feb 11 14:05:19 NO reason to wait: I'm loosely about, just distracted for another 5 minutes or so. Feb 11 14:05:25 erm Feb 11 14:05:35 did anyone notify any MOTUs ? Feb 11 14:05:48 we announced it on mobile Feb 11 14:05:50 given the univers list is HUGE Feb 11 14:06:00 i doubt many MOTUs read mobile Feb 11 14:06:05 and on internal list Feb 11 14:06:05 ogra: may not be an issue for the first pass Feb 11 14:06:21 ogra: we can spread the news to wider forum for the next time Feb 11 14:06:23 Partly I want to know what else needs to be documented Feb 11 14:06:24 dmart: The first pass is not enough to fix all bugs? ;-) Feb 11 14:06:36 we have https://wiki.ubuntu.com/ARM/Thumb2PackageReviewList Feb 11 14:06:42 lool: well, obviously, if you're feeling energetic ;) Feb 11 14:06:46 asac, definately should be announced on ubuntu-motu@ next time Feb 11 14:06:47 which should give us a decent amount of work Feb 11 14:07:03 ogra: well. i usually dont hunt in random territory ... we could also have it announced on -devel Feb 11 14:07:21 which most motus are subscribed to too Feb 11 14:07:25 yeah Feb 11 14:07:31 What's the best way to do this? Feb 11 14:07:37 I suggest we pick one package from the list and all take a look, and discuss. Feb 11 14:07:41 right Feb 11 14:07:48 i suggest that dmart takes the lead ;) Feb 11 14:07:49 either way i dont expect that our litle crew manages all these universe issues Feb 11 14:07:58 so boost stuff is already done Feb 11 14:08:08 dmart: maybe we can go by topic? Feb 11 14:08:17 e.g. today focus on "mov" issues? Feb 11 14:08:27 That was going to be my suggestion Feb 11 14:08:37 any ideas about which one? Feb 11 14:08:55 ok lets pick a first mov one Feb 11 14:08:57 * asac checks list Feb 11 14:09:05 mono ! :) Feb 11 14:09:11 first in list is djvulibre Feb 11 14:09:18 maybe start with something simple? Feb 11 14:09:20 djvulibre looks like it was a simple one Feb 11 14:09:24 yeah Feb 11 14:09:32 * ogra wasnt serious with mono :) Feb 11 14:09:46 ok everony get the source ;) Feb 11 14:10:44 so how do we find the right places? Feb 11 14:10:56 grep ? Feb 11 14:10:59 mono comes later :) Feb 11 14:11:12 *shudder* Feb 11 14:11:27 djvulibre-3.5.22$ grep -r mov * | pastebinit Feb 11 14:11:27 http://pastebin.com/f55740dea Feb 11 14:11:28 and mono jit? :) Feb 11 14:11:33 guess that grep can be tweaked Feb 11 14:12:16 so is this a mov only problem or does that include mov's such as movq? Feb 11 14:12:44 grep -R " mov " * ? Feb 11 14:12:57 grep -R "mov " * Feb 11 14:13:02 yeah Feb 11 14:13:07 7 occurances Feb 11 14:13:12 So we're just looking at ZPCodec.cpp and MMX.cpp Feb 11 14:13:31 And GThreads.cpp? Feb 11 14:13:32 Can we ignore MMX.cpp, under the assumption that we're not going to take that codepath for HW that doesn't declare MMX? Feb 11 14:13:50 * persia doesn't see GThreads.cpp from ogra's grep. Feb 11 14:13:55 so no mov's that use the sp Feb 11 14:14:01 sorry pc Feb 11 14:14:02 grep -r mov.*pc * Feb 11 14:14:03 libdjvu/GThreads.cpp: "mov%?\t%|pc, %1" // branch to address %1 Feb 11 14:14:16 * dmart was distracted, back now Feb 11 14:15:54 so the GThreads.cpp seems to be right match? Feb 11 14:15:55 dmart: ? Feb 11 14:16:01 To find the occurrences, it's probably a good idea to look at the original grep list Feb 11 14:16:03 https://wiki.ubuntu.com/ARM/Thumb2PackageReviewList?action=AttachFile&do=get&target=search-arm-mov.filt.gz Feb 11 14:16:36 (linked from https://wiki.ubuntu.com/ARM/Thumb2PackageReviewList) Feb 11 14:17:00 GThreads.cpp is indeed the location of the hit we got. Feb 11 14:17:03 So GThreads is the only issue, and we can ignore ZPCodec.cpp as well? Feb 11 14:17:09 asac: it's a mov pc, someregister op...a candidate for bx? Feb 11 14:17:16 grep djvulibre * Feb 11 14:17:16 /var/cache/debmirror/pool/main/d/djvulibre/djvulibre_3.5.22-1ubuntu2.dsc Feb 11 14:17:16 ./djvulibre-3.5.22/libdjvu/GThreads.cpp: "mov%?\t%|pc, %1" // branch to address %1 Feb 11 14:17:35 persia: only stuff that messes with "pc" is relevant from what i understood Feb 11 14:17:37 dyfet: would you like to suggest a fix? ;) Feb 11 14:18:13 Well, it may not expand to a simple fix because the actual mov might be a movl, etc... Feb 11 14:18:27 movl? Feb 11 14:19:18 The closest relevant fix might be using a string constant, and maybe #ifdef (___ARM_ARCH_4T__) || defined (__ARM_ARCH_4__) Feb 11 14:19:41 #define ARM_MOVPC "mov%?\t%|pc, %1" Feb 11 14:19:43 #else Feb 11 14:20:40 #define ARM_MOVPC "bx%? , %1" ??? well, its hard to say how this expands exactly Feb 11 14:21:08 What did you mean by movl? This doesn't mean anything to me (at least for arm) Feb 11 14:21:58 Sorry, I meant the mov may not be for pc since it seems like some conditional substitution....thats why I am uncertain Feb 11 14:22:54 oooh, that's a new one on me. What does %? do in gcc asm? Feb 11 14:23:08 Yea that is what I was wondering... Feb 11 14:23:57 A good first step could be a stick a #error in there and build (since the code depends on -DCOTHREAD_UNTESTED, which might not be defined in our build?) Feb 11 14:24:13 http://www.ibiblio.org/gferg/ldp/GCC-Inline-Assembly-HOWTO.html Feb 11 14:24:33 AT&T Code ... heh Feb 11 14:24:42 i dont see anything about %? Feb 11 14:25:13 can't find it; maybe it adds the right AT&T operand size suffix. That wouldn't apply to ARM, but then %? is only present in the ARM version. Feb 11 14:25:57 weird Feb 11 14:26:09 yeah. lets ignore that Feb 11 14:26:56 Then I think the simplest case is we want effectively a "bx %1"... Feb 11 14:27:05 thats the obvious idea Feb 11 14:27:10 what does the |pc mean? Feb 11 14:27:10 yeah Feb 11 14:28:39 Dunno! I'll see if I can find out from anyone here. Feb 11 14:28:49 its probably %| Feb 11 14:29:06 asac: I read the original as effectively reducing to "mov pc, %1" but yea, I wondered about that too Feb 11 14:29:18 right Feb 11 14:29:29 It might be the case that GCC can substitute condition codes for %? Feb 11 14:29:37 but it's no documented if so Feb 11 14:29:38 But is it even a valid code path? Its tied to COTHREAD_UNTESTED being defined... Feb 11 14:29:46 indeed Feb 11 14:29:56 I think the best thing iintially is Feb 11 14:29:58 #ifdef __thumb__ Feb 11 14:30:14 #error "Needs porting to Thumb-2" Feb 11 14:30:15 #endif Feb 11 14:30:28 Yes, lets see if it even builds that path first :) Feb 11 14:30:34 If this doesn't ftbfs, then someone can worry about it later :) Feb 11 14:30:41 dmart: agreed Feb 11 14:30:58 dyfet: ok want to create the diff for the file ... and i just upload ;)? Feb 11 14:31:10 well, we have the other mov's to look at too :) Feb 11 14:31:12 we will see if it fails then in a bit Feb 11 14:31:15 OK Feb 11 14:31:19 dyfet: that was the only mov Feb 11 14:31:19 hold on Feb 11 14:31:24 sure Feb 11 14:31:33 * asac waits Feb 11 14:31:34 dyfet, only "mov, pc" Feb 11 14:31:43 mov.*pc Feb 11 14:32:38 true....but then if its not explored, there must be another reason for the ftbfs :) Feb 11 14:33:28 http://pastebin.ubuntu.com/373995/ Feb 11 14:33:30 dyfet: it currently ftbfs? Feb 11 14:33:42 hmmm, guess we should look at the log Feb 11 14:33:54 And since the end of it reaches a #error if none of the paths are explored, it has to be here...but yes, let's look at the log Feb 11 14:33:56 dyfet: I don't see it at http://qa.ubuntuwire.com/ftbfs/ Feb 11 14:34:02 dmart: right. i wanted dyfet to produce that ... but ok ;) ... let me upload Feb 11 14:34:49 Um, should we not try a local build first to make sure that fixes it? Feb 11 14:35:14 persia, queue is empty Feb 11 14:35:23 persia: its not a fix, its just a warning in case that code path is taken Feb 11 14:35:24 doing one, but it may take some minutes Feb 11 14:35:28 I am going to try a local build quickly... Feb 11 14:35:30 (or so I understand) Feb 11 14:35:32 we can play with the buildds unless they are busy Feb 11 14:35:57 JamieBennett: Right: it's that I don't tend to like to generate lots of archive churn when it might fail :) Feb 11 14:36:20 Our first priority is to get everything building and working in lucid. Fixing code paths which are not built is nice to have, but putting a #error bomb is a reasonable way of helping ensure it gets fixed at the appropriate time Feb 11 14:36:50 dmart: I think that code path has to be built, otherwise it would fall into a default #error after that section Feb 11 14:37:42 ...unless the package is built with -DTHREADMODEL=NOTHREADS Feb 11 14:37:53 ok can we use a pastebin != ubuntu? Feb 11 14:37:58 Hmm...we could also do that :) Feb 11 14:38:04 asac: Why? Feb 11 14:38:04 thats just annoying. you cant wget the plain text because of openid Feb 11 14:38:15 any suggestions? Feb 11 14:38:20 just pastebin.com Feb 11 14:38:21 * persia thought that was fixed, but tends to have trouble parsing other pastebins Feb 11 14:38:25 dmart: debian's work Feb 11 14:38:31 alias debian-paste='pastebinit -b http://paste.debian.net' Feb 11 14:38:37 every pastebin works ... just paste.ubuntu.com is broken Feb 11 14:38:44 \o/ :) Feb 11 14:38:49 i already kicked #is for that ... seems they didnt listen Feb 11 14:38:52 * asac does that again Feb 11 14:38:58 persia: It got fixed and borken a couple of days later sadly Feb 11 14:39:09 asac: It was fixed for a day or two Feb 11 14:39:40 I don't know the answer to thathttp://paste.debian.net/59542/ Feb 11 14:39:43 oops Feb 11 14:39:45 http://paste.debian.net/59542/ Feb 11 14:39:59 That's one I can parse cleanly :) Feb 11 14:40:05 dmart: thanks. i did it manually now Feb 11 14:40:07 uploaded Feb 11 14:41:05 /bin/bash ../libtool --mode=compile g++ -DHAVE_CONFIG_H -I.. -I. -Wall -g -O2 -pthread -DTHREADMODEL=POSIXTHREADS -c GThreads.cpp Feb 11 14:41:07 g++ -DHAVE_CONFIG_H -I.. -I. -Wall -g -O2 -pthread -DTHREADMODEL=POSIXTHREADS -c GThreads.cpp -fPIC -DPIC -o .libs/GThreads.o Feb 11 14:41:08 In file included from DjVuMessageLite.h:74, Feb 11 14:41:10 from GThreads.cpp:76: Feb 11 14:41:12 GString.h:745: note: the mangling of 'va_list' has changed in GCC 4.4 Feb 11 14:41:13 g++ -DHAVE_CONFIG_H -I.. -I. -Wall -g -O2 -pthread -DTHREADMODEL=POSIXTHREADS -c GThreads.cpp -o GThreads.o >/dev/null 2>&1 Feb 11 14:41:27 File seemed to build OK Feb 11 14:42:16 So we conclude that this code path is never exercised, and just leave the comment as a note? Feb 11 14:42:49 i uploaded the patch that would catch it for __thumb__ Feb 11 14:42:51 thats ok imo Feb 11 14:43:11 As it turns out, yes :) Feb 11 14:43:13 If it's considered OK to add #error (provided it's for a case we know definitely won't work) Feb 11 14:43:17 we should upstream that or suggest the right way ... e.g. bx Feb 11 14:43:39 yes, it might be a good idea to make the comment clearer Feb 11 14:43:40 dmart: i think for cases where we cant try the code path its ok to use #error ... otherwise we should obviously fix it Feb 11 14:44:26 next package ? Feb 11 14:44:35 I'm perfectly happy to use #error as a base of test builds, and if we don't hit the code path, replace with a useful comment. Feb 11 14:44:47 gmp seems small Feb 11 14:44:51 Probably worth discussing this one a bit further first Feb 11 14:45:13 persia: Why is a comment (which will go unnoticed) better than #error (which won't) Feb 11 14:45:15 well, what if we hit it and have no solution ? Feb 11 14:45:27 i think #error is fine. we should upstream that so they know Feb 11 14:45:28 hope for upstream ? Feb 11 14:45:28 #error potentially saves someone a lot of debugging Feb 11 14:45:44 ogra: we would have a solution. but we cant test it because the code is not used Feb 11 14:45:46 dmart: I want *both* the comment and the #error : the #error to force it not working if the codepath is hit, and the comment to tell someone why we added that patch. Feb 11 14:45:54 so should we add something untested? or just ad an error Feb 11 14:46:04 Oh, OK. Yes adding extra explanation in a comment is of course a very good idea :) Feb 11 14:46:08 persia: right. thats what we did Feb 11 14:46:16 we can improve the comment on next one though Feb 11 14:46:16 +#ifdef __thumb__ Feb 11 14:46:17 +#error "This code needs some porting to work correctly in Thumb." Feb 11 14:46:17 +#endif Feb 11 14:46:25 maybe point to the porting page Feb 11 14:46:34 good idea Feb 11 14:46:34 anyway. lets move on Feb 11 14:46:36 gmp ;) Feb 11 14:46:56 Since djvulibre is a simple example, I think it's worth discussing how we would fix it (if we needed to) Feb 11 14:47:13 Someone suggested bx Feb 11 14:47:24 dmart: yes :) Feb 11 14:47:29 dyfet, did Feb 11 14:47:50 #define ARM_MOVPC "bx%? , %1" Feb 11 14:47:51 ... so far so good, but it's worth taking a step back and looking at the context of this code so we are sure we're doing the right thing Feb 11 14:48:11 Well, I was not sure what the %? was about....but essentially something like that... Feb 11 14:48:40 "mov%?\t%|pc, %1" // branch to address %1 Feb 11 14:48:52 so it uses that to branch to %1 ... which is .... hmm Feb 11 14:48:56 I _think_ we could pretend the %? is not there. I can't think of anything vital that it would do. Feb 11 14:49:17 but best not to alter that aspect of the code in the fix in case it matters Feb 11 14:49:39 scrolling up a bit Feb 11 14:49:42 static void Feb 11 14:49:43 mach_start(mach_state *st1, void *pc, char *stacklo, char *stackhi) Feb 11 14:50:25 Scrolling up a bit more, mach_state contains a jmp_buf Feb 11 14:50:45 So this looks like a long jump / thread starting function if some sort. Feb 11 14:51:11 Yes, a funny kind of procedure call to start a thread.... Feb 11 14:51:24 %1 is the argument pc, which is just a pointer Feb 11 14:52:13 But it's not a function pointer which is slightly suspicious Feb 11 14:52:46 ...because it could be passed a pointer to some data (containing run-time generated code) Feb 11 14:52:54 dmart: its a func pointer Feb 11 14:53:01 just not declared explicitly Feb 11 14:53:10 mach_start(&st1, (void*)th2relay, stack, stack+sizeof(stack)); Feb 11 14:53:19 void th2relay() { Feb 11 14:53:28 certainly that's the likely case --- I was just going to suggest seeing how it's called, but you beat me to it :) Feb 11 14:53:31 and pc is the second argument Feb 11 14:53:59 ok lets grep for mor mach_start ;) Feb 11 14:54:17 the other is startone Feb 11 14:54:19 line 950 Feb 11 14:54:22 which is also a func Feb 11 14:54:43 persia: right. thats the function where the asm code is in Feb 11 14:54:54 we are looking for callers now to see if its always a func Feb 11 14:55:31 so ... based on that is bx %1 still the right thing? Feb 11 14:55:50 After a couple of examples, we can be reasonably confident, since passing a function is certainly how we would expect a thread start func to be used. Feb 11 14:56:11 right Feb 11 14:56:26 but I just wanted to highlight the issue Feb 11 14:56:41 Oh, btw: Feb 11 14:56:42 %? - condition code used only in conditional execution. Feb 11 14:56:44 %| - some kind of register prefix and legacy assemblers. Largely irrelevant today. Feb 11 14:56:57 it was a good educational example :) Feb 11 14:56:59 dmart: where did you find that? Feb 11 14:57:26 Um, our tools guys here ;) I'll try and get it merged in the docs if it's not upstream Feb 11 14:57:31 ok good. so are we confident enough to replace the error with bx %1? or want to keep the error as its not used? Feb 11 14:58:16 I think we should keep the error, but maybe add a comment suggesting bx %1 : we can't test the codepath, so we have no way of knowing if our solution works. Feb 11 14:58:18 before that, I was going to talk about what would happen if we were passed a data pointer here Feb 11 14:58:30 Given that it's already uploaded, I think we should leave it alone. Feb 11 14:58:30 Maybe someone wants to guess? Feb 11 14:58:52 ok Feb 11 14:59:01 dmart: bad things Feb 11 14:59:11 dmart: trying to call in data memory Feb 11 14:59:11 :) Feb 11 14:59:13 exploitable Feb 11 14:59:48 But what if the data is code? Maybe the caller created a trampoline sequence. (GCC does this internally for some things) Feb 11 15:00:02 then it just runs that code? Feb 11 15:00:09 or is there some protection stuff going on? Feb 11 15:00:43 well, you need to synchronise and data and instruction caches and flush the pipeline, so that the code executed is what you wrote. Feb 11 15:01:25 This is what the GCC builtin __clear_cache is for (look at the docs) Feb 11 15:01:47 ...however, this is not a Thumb-2 specific issue, so we would not expect to need to fix that if it already worked in ARM. Feb 11 15:01:49 oh cool. so i assume those steps are generated at each "real" function started by gcc? Feb 11 15:02:26 Well, the kernel always does this when paging in an executable page Feb 11 15:02:41 So for the text sections of programs and libraries, you never need to worry. Feb 11 15:03:07 Only self-modifying code or programs which generate code at runtime need to be concerned about this. Feb 11 15:03:19 ok good info. thanks Feb 11 15:03:34 ok move on to gmp? Feb 11 15:03:35 There certainly were some packages like that in the list... Feb 11 15:03:46 still wanted to make one more point: Feb 11 15:03:52 erlang was marked as "potentially creating code" Feb 11 15:03:56 sure go ahea Feb 11 15:03:57 d Feb 11 15:04:20 JITs, VMs, interpreters etc. should be treated with extra care for these reasons. Feb 11 15:04:21 ... Feb 11 15:04:41 anyway, a non-Thumb aware program which generates code an runtime probably generates ARM instruction encodings (not Thumb) Feb 11 15:05:56 right Feb 11 15:06:01 Such packages may need extra attention. Especially if the generated code may be called by other random (probably Thumb-2) code, or calls out to other functions (also probably Thumb-2) ... it's unlikely to work out of the box Feb 11 15:06:39 Anyway, enough on that for now, until we find a real instance ;) Feb 11 15:06:47 maybe mono ;) Feb 11 15:06:56 *shudder* Feb 11 15:07:03 ok so gmp? Feb 11 15:07:03 Going back to the suggestion of how to fix djvulibre Feb 11 15:07:04 ;) Feb 11 15:07:09 hehe Feb 11 15:07:13 endles story it seems Feb 11 15:07:38 (I know, nearly finished!) Feb 11 15:07:49 (but good to have all details first) Feb 11 15:07:55 ... we're just trying to jump to a passed function pointer Feb 11 15:07:56 * persia likes the long explanation, having not much prior understanding Feb 11 15:08:07 ack Feb 11 15:08:07 * ogra too Feb 11 15:08:48 I thought this would be a good idea --- the wiki page has lots of details, but may not be so easy to understand as an example Feb 11 15:09:25 Anyway, BX is definitely the right way to do jump to a function pointer where we don't expect to return. Feb 11 15:09:58 So adding a comment to this effect would be a good idea, something like Feb 11 15:10:48 /* instead of "mov pc, ", "bx " should be used on ARMv5 and above */ Feb 11 15:11:27 ok let me add that too and upload ... next time i will wait longer ;) Feb 11 15:11:36 sorry! Feb 11 15:11:49 dmart: No need to apologies. asac is just impatient :) Feb 11 15:11:54 I'm sure I can think of something else to add when you're done ;) Feb 11 15:12:05 We could add the wiki link for completeness Feb 11 15:12:11 #ifdef __thumb__ Feb 11 15:12:11 /* instead of "mov pc, ", "bx " should be used on ARMv5 and above */ Feb 11 15:12:14 #error "This code needs some porting to work correctly in Thumb: https://wiki.ubuntu.com/ARM/Thumb2PortingHowto" Feb 11 15:12:17 #endif Feb 11 15:12:20 thats what i have now ;) Feb 11 15:12:25 are we happy with that? Feb 11 15:12:26 cool, looks good Feb 11 15:12:34 Seems sufficiently complete. Feb 11 15:12:55 Of course, it might have been quicker just to fix it :P but it helps to educate more people this way. Feb 11 15:13:38 itsw really good ... especially since i know that at least persia is eager to work on these things ;) Feb 11 15:14:00 dmart: The risk with fixing it is that we can't test the code path, so we have no way to know if the "fix" worked. Feb 11 15:14:01 * dmart spies a volunteer Feb 11 15:14:21 agreed, it's better not to commit anything we can't test Feb 11 15:14:21 * persia doesn't really know assembler, and will be slow, but willing Feb 11 15:14:31 only joking Feb 11 15:14:32 dyfet is definitly a senior volunteer ... Feb 11 15:14:42 * asac goes with persia Feb 11 15:14:48 anyway Feb 11 15:15:04 so thanks. do we want to look at a second package now? Feb 11 15:15:10 Generally best to stay out of assembler as much as possible (it would certainly make this porting job easier) Feb 11 15:15:25 Yes, if people want to grab that I'll just fetch a quick cuppa Feb 11 15:15:29 well, in gmp, we have a function return in mpn/arm/copyi/d.asm, and in udiv.asm we find mov pc, lr also...at least to start with :) Feb 11 15:17:24 mov's in copyd, udiv and copyi Feb 11 15:17:30 * JamieBennett goes to get a coffee Feb 11 15:18:04 copyd.asm looks like a copy paste job from the wiki :) Feb 11 15:18:21 and copyi.asm... Feb 11 15:18:37 yeah, its the same Feb 11 15:19:09 all three actually Feb 11 15:19:38 so who wants to prepare a patch suggestion? Feb 11 15:19:46 feels like its exactly as on the wiki ;) Feb 11 15:20:00 L(return): Feb 11 15:20:00 #ifdef __thumb__ Feb 11 15:20:00 bx lr Feb 11 15:20:00 #else Feb 11 15:20:00 mov pc, lr Feb 11 15:20:01 #endif Feb 11 15:20:27 dyfet: on the wiki it uses something else for ifdef Feb 11 15:20:36 Hmmm Feb 11 15:20:40 #ifdef (___ARM_ARCH_4T__) || defined (__ARM_ARCH_4__) Feb 11 15:20:41 ... Feb 11 15:20:43 #else Feb 11 15:20:45 These are not C files Feb 11 15:20:47 "bx lr" Feb 11 15:20:53 oh right ;) Feb 11 15:21:09 Separate assembler files are a bit of a different animal Feb 11 15:21:17 how annoying Feb 11 15:21:19 Yes, that's right... Feb 11 15:21:43 yeah, no ifdef i guess ? Feb 11 15:21:52 whats the best way to do this if we dont want to do a full copy? Feb 11 15:22:07 well, unless you want to use cpp in front of it, ogra :) Feb 11 15:22:15 For assembler files which are preprocessed by the C preprocessor (.S) you can use ifdef in the normal way Feb 11 15:22:23 But it looks like these are processed by m4 Feb 11 15:22:52 So we probably need to get a configure argument passed in somehow Feb 11 15:23:58 nice Feb 11 15:24:01 lovely Feb 11 15:24:15 And then use a .ifdef, .else,.endif Feb 11 15:24:29 * dmart knows nothing about m4 Feb 11 15:24:55 * asac_ had to clone himself Feb 11 15:25:07 Assembler macros could work as you suggest Feb 11 15:25:33 L(return): Feb 11 15:25:33 .ifdef thumb2_config_from_config.m4... Feb 11 15:25:33 bx lr Feb 11 15:25:33 .else Feb 11 15:25:34 mov pc, lr Feb 11 15:25:34 .endif Feb 11 15:25:37 as a starting point.... Feb 11 15:25:38 But there are no magic predefined macros in the assembler to tell you what your build target it. Feb 11 15:26:01 yep, something like that Feb 11 15:26:10 Yes, so we have to feed it into one of the included files which are generated.... Feb 11 15:26:15 Or maybe you could have a common macro somewhere else Feb 11 15:26:23 .macro return register:req Feb 11 15:26:36 .ifdef Feb 11 15:26:44 bx \reg Feb 11 15:26:46 .else Feb 11 15:26:51 mov pc, \reg Feb 11 15:26:54 .fi Feb 11 15:26:56 .endm Feb 11 15:27:17 Anyway, that should work Feb 11 15:27:27 that too would be a good idea.... Feb 11 15:27:46 Exactly how to create the configure argument, I'm not sure Feb 11 15:28:24 You could preprocess some C with #ifdefs to check the architecture version and look at the output, e.g. Feb 11 15:28:36 we have to see how config.m4 is created since it includes it... Feb 11 15:28:42 #if defined(__ARM_ARCH_4__) || defined(__ARM_ARCH_4T__) Feb 11 15:28:48 use_mov Feb 11 15:28:50 #else Feb 11 15:28:52 use_bx Feb 11 15:28:54 #endif Feb 11 15:29:24 acinclude.m4 Feb 11 15:29:27 has GMP_INIT Feb 11 15:30:55 i think there are various options ... not sure we should discuss them here Feb 11 15:31:13 Agreed, I think everyone understands what's needed. Feb 11 15:31:24 Once we've done this once, we can reuse the solution if it's needed again. Feb 11 15:31:32 lets ask this, is there any other porting issue in gmp other than the mov instructions? Feb 11 15:31:58 good question --- we should always take a careful look at any assembler which pops up Feb 11 15:32:07 "uses mov; also "add" assumes arm" Feb 11 15:32:11 thats the comment we had from review Feb 11 15:32:18 yes....thats why I asked :) Feb 11 15:32:29 what does add assumes arm mean? dmart ? Feb 11 15:32:52 I meant "assumes it's running as ARM instructions and not as Thumb instructions" Feb 11 15:33:39 yeah Feb 11 15:33:42 There's a bit more info on this on the wiki page at PC and "." arithmetic and position-independent addressing Feb 11 15:34:27 But in brief, when you read pc in Thumb, the result is not that same as you'd get if you were executing in ARM. Feb 11 15:35:00 ok so we are looking for add.*pc here? Feb 11 15:35:17 mpn/arm/invert_limb.asm: add r2, pc, #invtab-.-8 Feb 11 15:35:30 That's the one. Feb 11 15:35:45 In ARM, when you read PC, you (usually) get the address of the current instruction + 8 Feb 11 15:35:59 So this trick gives you the address invtab in r2 Feb 11 15:36:12 right Feb 11 15:36:37 whats the right way to find a good offset? Feb 11 15:36:47 But in Thumb, pc is different. You get
+ 2 or 4 (depending on the alignment of the instruction in memory) Feb 11 15:37:07 It is possible to work out the correct offset, but ugly Feb 11 15:37:28 ...and you need #ifdefs or other special cases hacks for ARM versus Thumb Feb 11 15:37:35 ... Feb 11 15:37:42 Fortunately, the assmbler can do it for you Feb 11 15:38:23 There's a special pseudo-op to get the address of a nearby label in the text section: Feb 11 15:38:31 adr ,