* grub2 code @ 2014-02-02 19:25 Robert Parnell 2014-02-03 0:41 ` SevenBits 2014-02-03 11:57 ` Colin Watson 0 siblings, 2 replies; 8+ messages in thread From: Robert Parnell @ 2014-02-02 19:25 UTC (permalink / raw) To: grub-devel Hi I was just looking at the grub2 source code and saw that https://bazaar.launchpad.net/~ubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/TODO <https://bazaar.launchpad.net/%7Eubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/TODO> says to contact with questions. I was looking at the source because I encountered a bug on my machine that has been previously reported (https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1027694) and thought I'd have a look to see if I could find the problem. So I have 2 questions: 1. Is the code on launchpad the up to date code? 2. Line 177 of https://bazaar.launchpad.net/~ubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/grub-core/boot/i386/pc/startup_raw.S <https://bazaar.launchpad.net/%7Eubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/grub-core/boot/i386/pc/startup_raw.S> is a function that flushes the keyboard buffer, could this be what is causing the bug? Sorry if 2 seems like a stupid question- I've never looked at the grub source code before. Thanks Rob ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: grub2 code 2014-02-02 19:25 grub2 code Robert Parnell @ 2014-02-03 0:41 ` SevenBits 2014-02-03 11:57 ` Colin Watson 1 sibling, 0 replies; 8+ messages in thread From: SevenBits @ 2014-02-03 0:41 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 1555 bytes --] On Sunday, February 2, 2014, Robert Parnell <wwolfe9.rp@gmail.com> wrote: > Hi > > I was just looking at the grub2 source code and saw that > > https://bazaar.launchpad.net/~ubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/TODO > < > https://bazaar.launchpad.net/%7Eubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/TODO > > > says to contact with questions. > I was looking at the source because I encountered a bug on my machine > that has been previously reported > (https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1027694) and > thought I'd have a look to see if I could find the problem. > So I have 2 questions: > 1. Is the code on launchpad the up to date code? That is probably maintained by the Ubuntu people, so I'd consult them. This list is generally for general and not distribution specific code since every distro can potentially use their own modified copies of GRUB. > 2. Line 177 of > > https://bazaar.launchpad.net/~ubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/grub-core/boot/i386/pc/startup_raw.S > < > https://bazaar.launchpad.net/%7Eubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/grub-core/boot/i386/pc/startup_raw.S > > > is a function that flushes the keyboard buffer, could this be what is > causing the bug? > > Sorry if 2 seems like a stupid question- I've never looked at the grub > source code before. > > Thanks > > Rob > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org <javascript:;> > https://lists.gnu.org/mailman/listinfo/grub-devel > [-- Attachment #2: Type: text/html, Size: 2771 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: grub2 code 2014-02-02 19:25 grub2 code Robert Parnell 2014-02-03 0:41 ` SevenBits @ 2014-02-03 11:57 ` Colin Watson 2014-02-03 12:42 ` Ben Guthro 2014-02-03 16:51 ` Vladimir 'φ-coder/phcoder' Serbinenko 1 sibling, 2 replies; 8+ messages in thread From: Colin Watson @ 2014-02-03 11:57 UTC (permalink / raw) To: grub-devel; +Cc: Ben Guthro On Sun, Feb 02, 2014 at 07:25:51PM +0000, Robert Parnell wrote: > I was just looking at the grub2 source code and saw that > https://bazaar.launchpad.net/~ubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/TODO > <https://bazaar.launchpad.net/%7Eubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/TODO> > says to contact with questions. > I was looking at the source because I encountered a bug on my machine > that has been previously reported > (https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1027694) and > thought I'd have a look to see if I could find the problem. > So I have 2 questions: > 1. Is the code on launchpad the up to date code? It's an automatically-imported version of the Ubuntu packaging. It's up-to-date with what we're shipping, yes, but we do our work in git these days. The upstream code is here: http://git.savannah.gnu.org/gitweb/?p=grub.git ... and the packaging used by both Debian and Ubuntu is here (the "experimental" branch is most current): http://anonscm.debian.org/gitweb/?p=pkg-grub/grub.git > 2. Line 177 of > https://bazaar.launchpad.net/~ubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/grub-core/boot/i386/pc/startup_raw.S > <https://bazaar.launchpad.net/%7Eubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/grub-core/boot/i386/pc/startup_raw.S> > is a function that flushes the keyboard buffer, could this be what is > causing the bug? That seems unrelated. Ben's patch referenced in https://savannah.gnu.org/bugs/?34547 touches code that I wrote and that I maintain as a patch, so this would presumably be my problem to fix. But I don't understand why Ben's patch fixes anything related to NumLock. The code being commented out is essentially: movw $(GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR + 0x17), %bx andb $3, (%bx) ... followed by something that tests the condition flags. But all that does is test whether either Shift key is held down, not actually set the NumLock state (see http://www.bioscentral.com/misc/bda.htm). That is, it reads from the BIOS Data Area, but it doesn't write to it. Ben (CCed), could you shed some light on why your patch would fix this? Thanks, -- Colin Watson [cjwatson@ubuntu.com] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: grub2 code 2014-02-03 11:57 ` Colin Watson @ 2014-02-03 12:42 ` Ben Guthro 2014-02-03 15:44 ` Roger Cruz 2014-02-03 16:51 ` Vladimir 'φ-coder/phcoder' Serbinenko 1 sibling, 1 reply; 8+ messages in thread From: Ben Guthro @ 2014-02-03 12:42 UTC (permalink / raw) To: Colin Watson; +Cc: grub-devel, Roger Cruz On Mon, Feb 3, 2014 at 6:57 AM, Colin Watson <cjwatson@ubuntu.com> wrote: > On Sun, Feb 02, 2014 at 07:25:51PM +0000, Robert Parnell wrote: >> I was just looking at the grub2 source code and saw that >> https://bazaar.launchpad.net/~ubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/TODO >> <https://bazaar.launchpad.net/%7Eubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/TODO> >> says to contact with questions. >> I was looking at the source because I encountered a bug on my machine >> that has been previously reported >> (https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1027694) and >> thought I'd have a look to see if I could find the problem. >> So I have 2 questions: >> 1. Is the code on launchpad the up to date code? > > It's an automatically-imported version of the Ubuntu packaging. It's > up-to-date with what we're shipping, yes, but we do our work in git > these days. The upstream code is here: > > http://git.savannah.gnu.org/gitweb/?p=grub.git > > ... and the packaging used by both Debian and Ubuntu is here (the > "experimental" branch is most current): > > http://anonscm.debian.org/gitweb/?p=pkg-grub/grub.git > >> 2. Line 177 of >> https://bazaar.launchpad.net/~ubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/grub-core/boot/i386/pc/startup_raw.S >> <https://bazaar.launchpad.net/%7Eubuntu-branches/ubuntu/trusty/grub2/trusty/view/head:/grub-core/boot/i386/pc/startup_raw.S> >> is a function that flushes the keyboard buffer, could this be what is >> causing the bug? > > That seems unrelated. > > Ben's patch referenced in https://savannah.gnu.org/bugs/?34547 touches > code that I wrote and that I maintain as a patch, so this would > presumably be my problem to fix. But I don't understand why Ben's patch > fixes anything related to NumLock. The code being commented out is > essentially: > > movw $(GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR + 0x17), %bx > andb $3, (%bx) > > ... followed by something that tests the condition flags. But all that > does is test whether either Shift key is held down, not actually set the > NumLock state (see http://www.bioscentral.com/misc/bda.htm). That is, > it reads from the BIOS Data Area, but it doesn't write to it. > > Ben (CCed), could you shed some light on why your patch would fix this? Adding Roger Cruz, as he was the original author of this fix, where I was just trying to get it upstreamed - though, this was quite some time since he developed it (Oct 2011), so I'm sure this isn't exactly fresh in his mind. Roger, could you shed some light on this? Ben > > Thanks, > > -- > Colin Watson [cjwatson@ubuntu.com] ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: grub2 code 2014-02-03 12:42 ` Ben Guthro @ 2014-02-03 15:44 ` Roger Cruz 2014-02-03 15:50 ` Colin Watson 0 siblings, 1 reply; 8+ messages in thread From: Roger Cruz @ 2014-02-03 15:44 UTC (permalink / raw) To: Ben Guthro, Colin Watson; +Cc: grub-devel@gnu.org This was a bug that was introduced into GRUB when the code to detect if the SHIFT key is being pressed was added. The symptom was that customers were complaining that even though they had turn on NUMLOCK in the BIOS, once we booted our software, NUMLOCK was off. It was easy to "see" when the bug was happening because the NUMLOCK LED would turn off sometime during GRUB running. I tracked it down to that one line which ANDs the BIOS state area and blows away the NUMLOCK state in it (it stores the value in the address in BX via indirect addressing). Hope this helps. If it is not clear, please feel free to ask for more details. Roger R. Cruz -----Original Message----- From: ben.guthro@gmail.com [mailto:ben.guthro@gmail.com] On Behalf Of Ben Guthro Sent: Monday, February 03, 2014 7:42 AM To: Colin Watson Cc: grub-devel@gnu.org; Roger Cruz Subject: Re: grub2 code On Mon, Feb 3, 2014 at 6:57 AM, Colin Watson <cjwatson@ubuntu.com> wrote: > On Sun, Feb 02, 2014 at 07:25:51PM +0000, Robert Parnell wrote: >> I was just looking at the grub2 source code and saw that >> https://bazaar.launchpad.net/~ubuntu-branches/ubuntu/trusty/grub2/tru >> sty/view/head:/TODO >> <https://bazaar.launchpad.net/%7Eubuntu-branches/ubuntu/trusty/grub2/ >> trusty/view/head:/TODO> >> says to contact with questions. >> I was looking at the source because I encountered a bug on my machine >> that has been previously reported >> (https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1027694) and >> thought I'd have a look to see if I could find the problem. >> So I have 2 questions: >> 1. Is the code on launchpad the up to date code? > > It's an automatically-imported version of the Ubuntu packaging. It's > up-to-date with what we're shipping, yes, but we do our work in git > these days. The upstream code is here: > > http://git.savannah.gnu.org/gitweb/?p=grub.git > > ... and the packaging used by both Debian and Ubuntu is here (the > "experimental" branch is most current): > > http://anonscm.debian.org/gitweb/?p=pkg-grub/grub.git > >> 2. Line 177 of >> https://bazaar.launchpad.net/~ubuntu-branches/ubuntu/trusty/grub2/tru >> sty/view/head:/grub-core/boot/i386/pc/startup_raw.S >> <https://bazaar.launchpad.net/%7Eubuntu-branches/ubuntu/trusty/grub2/ >> trusty/view/head:/grub-core/boot/i386/pc/startup_raw.S> >> is a function that flushes the keyboard buffer, could this be what is >> causing the bug? > > That seems unrelated. > > Ben's patch referenced in https://savannah.gnu.org/bugs/?34547 touches > code that I wrote and that I maintain as a patch, so this would > presumably be my problem to fix. But I don't understand why Ben's > patch fixes anything related to NumLock. The code being commented out > is > essentially: > > movw $(GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR + 0x17), %bx > andb $3, (%bx) > > ... followed by something that tests the condition flags. But all > that does is test whether either Shift key is held down, not actually > set the NumLock state (see http://www.bioscentral.com/misc/bda.htm). > That is, it reads from the BIOS Data Area, but it doesn't write to it. > > Ben (CCed), could you shed some light on why your patch would fix this? Adding Roger Cruz, as he was the original author of this fix, where I was just trying to get it upstreamed - though, this was quite some time since he developed it (Oct 2011), so I'm sure this isn't exactly fresh in his mind. Roger, could you shed some light on this? Ben > > Thanks, > > -- > Colin Watson [cjwatson@ubuntu.com] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: grub2 code 2014-02-03 15:44 ` Roger Cruz @ 2014-02-03 15:50 ` Colin Watson 2014-02-03 15:53 ` Roger Cruz 0 siblings, 1 reply; 8+ messages in thread From: Colin Watson @ 2014-02-03 15:50 UTC (permalink / raw) To: Roger Cruz; +Cc: grub-devel@gnu.org, Ben Guthro On Mon, Feb 03, 2014 at 03:44:01PM +0000, Roger Cruz wrote: > This was a bug that was introduced into GRUB when the code to detect > if the SHIFT key is being pressed was added. The symptom was that > customers were complaining that even though they had turn on NUMLOCK > in the BIOS, once we booted our software, NUMLOCK was off. It was > easy to "see" when the bug was happening because the NUMLOCK LED would > turn off sometime during GRUB running. > > I tracked it down to that one line which ANDs the BIOS state area and > blows away the NUMLOCK state in it (it stores the value in the > address in BX via indirect addressing). Oh, of course, I missed the indirect store even on a second reading. I'm pretty sure that the correct fix isn't to remove the code, but rather to use testb instead of andb. I'll see about getting that fixed soon. -- Colin Watson [cjwatson@ubuntu.com] ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: grub2 code 2014-02-03 15:50 ` Colin Watson @ 2014-02-03 15:53 ` Roger Cruz 0 siblings, 0 replies; 8+ messages in thread From: Roger Cruz @ 2014-02-03 15:53 UTC (permalink / raw) To: Colin Watson; +Cc: grub-devel@gnu.org, Ben Guthro Yes, that certainly would be a better fix. Thanks for addressing it. Roger -----Original Message----- From: Colin Watson [mailto:cjwatson@ubuntu.com] Sent: Monday, February 03, 2014 10:51 AM To: Roger Cruz Cc: Ben Guthro; grub-devel@gnu.org Subject: Re: grub2 code On Mon, Feb 03, 2014 at 03:44:01PM +0000, Roger Cruz wrote: > This was a bug that was introduced into GRUB when the code to detect > if the SHIFT key is being pressed was added. The symptom was that > customers were complaining that even though they had turn on NUMLOCK > in the BIOS, once we booted our software, NUMLOCK was off. It was > easy to "see" when the bug was happening because the NUMLOCK LED would > turn off sometime during GRUB running. > > I tracked it down to that one line which ANDs the BIOS state area and > blows away the NUMLOCK state in it (it stores the value in the > address in BX via indirect addressing). Oh, of course, I missed the indirect store even on a second reading. I'm pretty sure that the correct fix isn't to remove the code, but rather to use testb instead of andb. I'll see about getting that fixed soon. -- Colin Watson [cjwatson@ubuntu.com] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: grub2 code 2014-02-03 11:57 ` Colin Watson 2014-02-03 12:42 ` Ben Guthro @ 2014-02-03 16:51 ` Vladimir 'φ-coder/phcoder' Serbinenko 1 sibling, 0 replies; 8+ messages in thread From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2014-02-03 16:51 UTC (permalink / raw) To: The development of GNU GRUB; +Cc: Ben Guthro [-- Attachment #1: Type: text/plain, Size: 341 bytes --] On 03.02.2014 12:57, Colin Watson wrote: > movw $(GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR + 0x17), %bx > andb $3, (%bx) andb $3, (%bx) is equivalent to C code: *(grub_uint8_t *)bx &= 3; andb modifies the value at (%bx). You probably meant to use testb which has same flag semantics but doesn't change operands [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 274 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-02-05 0:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-02 19:25 grub2 code Robert Parnell 2014-02-03 0:41 ` SevenBits 2014-02-03 11:57 ` Colin Watson 2014-02-03 12:42 ` Ben Guthro 2014-02-03 15:44 ` Roger Cruz 2014-02-03 15:50 ` Colin Watson 2014-02-03 15:53 ` Roger Cruz 2014-02-03 16:51 ` Vladimir 'φ-coder/phcoder' Serbinenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.