Re: [PATCH 3/9] clean root dir of all x86-ness

[Christoffer Dall <christoffer.dall@linaro.org>]

On Thu, Jan 02, 2014 at 04:00:17PM +0100, Andrew Jones wrote:
> On Sat, Dec 28, 2013 at 10:30:12PM -0800, Christoffer Dall wrote:
> > >  
> > >  clean: arch_clean
> > > -	$(RM) *.o *.a .*.d lib/.*.d $(libcflat) $(cflatobjs)
> > > +	$(RM) lib/.*.d $(libcflat) $(cflatobjs)
> > 
> > so above you're referencing */.*.d and */*/.*.d but in make arch_clean
> > we're only removing lib/.*.d ?
> 
> There are no longer any source files in the root dir, allowing us to
> remove the 'rm *.o *.a .*.d' from this top-level makefile. arch_clean
> doesn't remove lib/.*.d. It only removes its arch-specific files, e.g.
> $(TEST_DIR)/* and lib/$(ARCH)/*.
> 

ah ok.

> > >  
> > >  	Options include:
> > > +	    --test-dir=DIR         the main directory for tests ($arch)
> > 
> > huh? why would you want to specify something different from arch here?
> 
> $(TEST_DIR) isn't new with this patch, only the ability to specify it
> is. As a separate tidyup patch we could s/r TEST_DIR with ARCH to get
> rid of it, but it wouldn't gain us much, and would lose the ability to
> have a second testdir for the same arch (for what I don't know - maybe
> just for a scratch dir?)
> 

hmm, ok, I don't know what the original intention with this patch was
then.

> > > +Services supplied by the testdev device:
> > > +
> > > +serial output: write only, on io port 0xf1
> > > +exit process: write only, on io port 0xf4, value used as exit code
> > > +ram size: read-only, on io port 0xd1, 4 bytes' size
> > > +irq line setting: write only, on io ports 0x2000 - 0x2018, value to set/clear
> > > +simple io: read/write, on io port 0xe0, 1/2/4 bytes
> > > +
> > > +Test device used a char device for actual output
> > 
> > what do you mean by this last sentence?
> 
> Don't shoot the file-mover :-) I didn't write it, but it looks like
> it's referring to how testdev (as opposed to pc-testdev) works, i.e.
> '-device testdev,chardev=testlog -chardev file,id=testlog,path=msr.out'
> 

ok, I'll let you off on that one then :)

> > > +# As it happens, config.mak is valid shell script code, too :-)
> > > +source config.mak
> > 
> > is this a well-established method of doing things or a hack?  If the
> > latter, sems like something that could quickly come and bite us.
> 
> As long as config.mak is only ever a list of 'key=value's, then I
> guess we're pretty safe.
> 
> > >  
> > > -Set the environment variable QEMU=/path/to/qemu-system-x86_64 to allow the
> > > -internally used x86-run to pick up the right qemu binary.
> > > +Set the environment variable QEMU=/path/to/qemu-system-ARCH to allow the
> > > +internally used ARCH-run to pick up the right qemu binary.
> > 
> > this message may not make a lot of sense to non kvm-unit-test
> > devevelopers, why not say that it allows the user to specify which QEMU
> > binary to use?
> 
> OK, tweaked it to
>   Set the environment variable QEMU=/path/to/qemu-system-ARCH to specify
>   the appropriate qemu binary for ARCH-run.

thanks!

> 
> > 
> > >  
> > >  EOF
> > >  }
> > >  
> > > -# As it happens, config.mak is valid shell script code, too :-)
> > > -source config.mak
> > > -
> > 
> > ah, I see, this hack/method was used before too, still relevant to
> > consider if it's a good approach though...
> 
> yup, I think it's OK.
> 
> > > +
> > > +Tests for x86 architecture are run as kernel images for qemu that supports
> > > +multiboot format. Tests uses an infrastructure called from the bios code.
> > > +The infrastructure initialize the system/cpu's, switch to long-mode and calls
> > > +the 'main' function of the individual test. Tests uses a qemu's virtual test
> > 
> > Tests use
> 
> fixed
> 
> drew

-- 
Christoffer