Re: [Qemu-devel] qemu vl.c vl.h hw/an5206.c hw/etraxfs.c hw/inte...

[J. Mayer]

On Wed, 2007-10-31 at 11:22 +0100, J. Mayer wrote:
> On Wed, 2007-10-31 at 11:01 +0100, andrzej zaborowski wrote:
> > On 31/10/2007, J. Mayer <address@hidden> wrote:
> > >
> > > On Wed, 2007-10-31 at 03:35 +0100, andrzej zaborowski wrote:
> > > > Hi,
> > > >
> > > > On 31/10/2007, J. Mayer <address@hidden> wrote:
> > > > >
> > > > > On Wed, 2007-10-31 at 01:54 +0000, Andrzej Zaborowski wrote:
> > > > > > CVSROOT:      /sources/qemu
> > > > > > Module name:  qemu
> > > > > > Changes by:   Andrzej Zaborowski <balrog>     07/10/31 01:54:05
> > > > > >
> > > > > > Modified files:
> > > > > >       .              : vl.c vl.h
> > > > > >       hw             : an5206.c etraxfs.c integratorcp.c mcf5208.c
> > > > > >                        mips_malta.c mips_mipssim.c mips_pica61.c
> > > > > >                        mips_r4k.c palm.c pc.c ppc405_boards.c
> > > > > >                        ppc_chrp.c ppc_oldworld.c ppc_prep.c r2d.c
> > > > > >                        realview.c shix.c spitz.c sun4m.c sun4u.c
> > > > > >                        versatilepb.c
> > > > > >
> > > > > > Log message:
> > > > > >       Set boot sequence from command line (Dan Kenigsberg).
> > > > >
> > > > > There have been remarks about this patch that have not been addressed
> > > > > (not even answered, in fact).  For example, the MAX_BOOT_DEVICES is 
> > > > > set
> > > > > to 3 when more than 3 boot devices are possible to select (see the
> > > > > BOOTCHARS definition), which clearly shows the patch is not 
> > > > > consistent.
> > > >
> > > > I double-checked to make sure all remarks made on qemu-devel were
> > > > addressed, but I may have missed something. It was explained that the
> > > > default bios supports only three boot devices,
> > >
> > > Then just take a look at the function boot_device2nibble in hw/pc.c. You
> > > can see 4 possibilities implemented here. And I think I've never seen a
> > > PC BIOS (on real machines, I mean) that don't allow more than 4 choices
> > > in last 5 years (and maybe much more...)
> > 
> > MAX_BOOT_DEVICES doesn't limit the number of possible boot devices, it
> > is only a limit for the length of the sequence given on command-line.
> > 
> > > The second point is that, as the legacy PC-BIOS is maybe the less
> > > versatile architecture that can be, putting limitations to the emulation
> > > model based on this spec seems to be a nonsense in Qemu, which is
> > > supposed to emulate _a lot_ of different architectures. As a matter of
> > > fact, a specific implementation (ie legacy PC target) should not lead to
> > > have hardcoded limits that would affect all other emulated targets.
> > 
> > I personally wouldn't hardcode any limit but this code was submitted
> > this way and doesn't limit any current functionality in any way, it
> > extends it. I prefer the GNU/Hurd style code where no software limits
> > are ever imposed and even the standard unix limits are undefined (e.g.
> > no MAXPATHLEN), sometimes at significant cost.
> 
> Imho, in that case, the only thing that can be check is that the given
> string contains only characters that can be valid devices in Qemu. Then,
> making boot_device a pointer directly assigned to optarg then check that
> all chars are >= 'a' and < 'c' + MAX_DISKS || chars == 'n' would greatly
> simplify the code. And this kind of check is the only valid one you can
> do in the generic code.

Here's a generic implementation that checks only the boot devices known
to be supported, ie 'a', 'c', 'd' and 'n', thus need no change in the
machine emulation code to work. When the machines will be able to check
properly if the boot devices match the emulated hardware and the BIOS
ABI, then it can be easily extended, changing one line, to allow boot
from more devices. I think that this code should allow choosing to (try
to...) boot from at least the 2 floppies and the 4 possible IDE devices.
The consistency test could also be changed to add more drives if it
seems to be needed.
For consistency, I also made the boot_devices variable local to the main
routine, as it's never used anywhere else.

This patch does not make the code simpler (in fact it's even more
complicated as it does more generic consistency checks) but is generic
and extensible, not breaking the previous patch and being consistent
with the i386 machine BIOS features, as implemented now.

The machine specific checks can be added later, for each target that
need some. Another solution could be that every machine implements a
callback that return a features bitmap, then the generic code could
check if the given command line arguments (including the -boot option,
but not only) are consistent with the emulated hardware platform.

[...]

-- 
J. Mayer <address@hidden>
Never organized

boot_devices.diff
Description: Text Data