From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Mon, 14 Nov 2016 22:24:38 +0100 Subject: [U-Boot] [PATCH v3 7/8] x86: efi: Add a hello world test program In-Reply-To: References: <1476757754-1220-1-git-send-email-sjg@chromium.org> <1476757754-1220-7-git-send-email-sjg@chromium.org> <5805CBE5.8050509@suse.de> <303040d6-1394-fa83-6836-316f7725ef78@suse.de> <3D636533-5BE9-4029-BB71-ECB0C7605D12@suse.de> <44437559-36e5-1551-27f5-4f47edf71246@suse.de> Message-ID: <477cfddb-1d3c-ede3-d106-f71ab17a4a01@suse.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 14/11/2016 21:58, Simon Glass wrote: > Hi Alex, > > On 14 November 2016 at 13:46, Alexander Graf wrote: >> >> >> On 14/11/2016 21:44, Simon Glass wrote: >>> >>> Hi Alex, >>> >>> On 11 November 2016 at 23:23, Alexander Graf wrote: >>>> >>>> >>>> >>>>> Am 11.11.2016 um 17:17 schrieb Simon Glass : >>>>> >>>>> Hi Alex, >>>>> >>>>>> On 7 November 2016 at 09:32, Alexander Graf wrote: >>>>>> >>>>>> >>>>>>> On 07/11/2016 10:46, Simon Glass wrote: >>>>>>> >>>>>>> Hi Alex, >>>>>>> >>>>>>>> On 19 October 2016 at 01:09, Alexander Graf wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> On 18/10/2016 22:37, Simon Glass wrote: >>>>>>>>> >>>>>>>>> Hi Alex, >>>>>>>>> >>>>>>>>>> On 18 October 2016 at 01:14, Alexander Graf wrote: >>>>>>>>>> >>>>>>>>>>> On 10/18/2016 04:29 AM, Simon Glass wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> It is useful to have a basic sanity check for EFI loader support. >>>>>>>>>>> Add >>>>>>>>>>> a >>>>>>>>>>> 'bootefi hello' command which loads HelloWord.efi and runs it >>>>>>>>>>> under >>>>>>>>>>> U-Boot. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Simon Glass >>>>>>>>>>> --- >>>>>>>>>>> >>>>>>>>>>> Changes in v3: >>>>>>>>>>> - Include a link to the program instead of adding it to the tree >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> So, uh, where is the link? >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> I put it in the README (see the arm patch). >>>>>>>>> >>>>>>>>>> >>>>>>>>>> I'm really not convinced this command buys us anything yet. I do >>>>>>>>>> agree >>>>>>>>>> that >>>>>>>>>> we want automated testing - but can't we get that using QEMU and a >>>>>>>>>> downloadable image file that we pass in as disk and have the distro >>>>>>>>>> boot do >>>>>>>>>> its magic? >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> That seems very heavyweight as a sanity check, although I agree it >>>>>>>>> is >>>>>>>>> useful. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> It's not really much more heavy weight. The "image file" could simply >>>>>>>> contain your hello world binary. But with this we don't just verify >>>>>>>> whether "bootefi" works, but also whether the default boot path works >>>>>>>> ok. >>>>>>> >>>>>>> >>>>>>> >>>>>>> I don't think I understand what you mean by 'image file'. Is it >>>>>>> something other than the .efi file? Do you mean a disk image? >>>>>> >>>>>> >>>>>> >>>>>> Yes. For reasonable test coverage, we should also verify that the >>>>>> distro >>>>>> defaults wrote a sane boot script that automatically searches for a >>>>>> default >>>>>> EFI binary in /efi/boot/bootx86.efi on the first partition of all >>>>>> devices >>>>>> and runs it. >>>>>> >>>>>> So if we just provide an SD card image or hard disk image to QEMU which >>>>>> contains a hello world .efi binary as that default boot file, we don't >>>>>> only >>>>>> test whether the "bootefi" command works, but also whether the distro >>>>>> boot >>>>>> script works. >>>>> >>>>> >>>>> That's right. >>>>> >>>>>> >>>>>>> >>>>>>>> >>>>>>>>> Here I am just making sure that EFI programs can start, print output >>>>>>>>> and exit. It is a test that we can easily run without a lot of >>>>>>>>> overhead, much less than a full distro boot. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Again, I don't think it's much more overhead and I do believe it >>>>>>>> gives >>>>>>>> us much cleaner separation between responsibilities of code (tests go >>>>>>>> where tests are). >>>>>>> >>>>>>> >>>>>>> >>>>>>> You are talking about a functional test, something that tests things >>>>>>> end to end. I prefer to at least start with a smaller test. Granted it >>>>>>> takes a little more work but it means there are fewer things to hunt >>>>>>> through when something goes wrong. >>>>>> >>>>>> >>>>>> >>>>>> Yes, I personally find unit tests terribly annoying and unproductive >>>>>> and >>>>>> functional tests very helpful :). And in this case, the effort to write >>>>>> it >>>>>> is about the same for both, just that the functional test actually >>>>>> tells you >>>>>> that things work or don't work at the end of the day. >>>>>> >>>>>> With a code base like U-Boot, a simple functional test like the above >>>>>> plus >>>>>> git bisect should get you to an offending patch very quickly. >>>>> >>>>> >>>>> This is not a unit test - in fact the EFI stuff has no unit tests. I >>>>> suppose if we are trying to find a name this is a small functional >>>>> test since it exercises the general functionality. >>>>> >>>>> I am much keener on small tests than large ones for finding simple >>>>> bugs. Of course you can generally bisect to find a bug, but the more >>>>> layers of software you need to look for the harder this is. >>>>> >>>>> We could definitely use a pytest which checks an EFI boot into an >>>>> image, but I don't think this obviates the need for a smaller targeted >>>>> test like this one. >>>> >>>> >>>> I think arguing over this is moot :). More tests is usually a good thing, >>>> so whoever gets to write them gets to push them ;). As long as the licenses >>>> are sound at least. >>> >>> >>> OK good, well please can you review this at some point? >> >> >> Review what exactly? > > I mean the patches. There should be ~14 in your queue. Interesting. I see them on patchwork, but neither in my U-Boot folder, my inbox nor my spam folder. I wonder what happened there. Alex