From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Mon, 14 Nov 2016 21:46:50 +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> Message-ID: <44437559-36e5-1551-27f5-4f47edf71246@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: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? > Also, are you > planning to write the 'larger' test? How do you test this all in suse? Planning yes, but I'm very good at not writing tests :). Currently I'm testing this all in suse by running systems which rely on the code to work. Alex