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: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.

> > >  on a second thought I
> > > see how this may affect people using a non-default bios, but I guess 3
> > > boot devices is better than only one that was possible without this
> > > patch.
> >
> > For most emulation targets, there still is a limit to 1. And the global
> > limit to 3 is not even related to the PC spec, according to the code
> > commited in pc.c. Then, imho, it cannot be better as it's inconsistent
> > for the PC case and provides nothing in most cases.
> 
> The limit of three is the max boot sequence length and the same (or
> lower) limit is already hardcoded in the machine initialisations where
> they deal with passing the sequence to the BIOS.

Sure. And here is the only place you can hardcode / check anything like
this, because this is _the_ place where you know what physical devices
are actually emulated, and maybe (not always) what are the BIOS
features, ...

> > What is the explanation of a global define to 1 for most target when you
> > cannot globally know how the information will be exploited ?
> 
> Yes, you can, it all sits in one repository and is part of the same
> project. vl.c doesn't have to deal with cases where, say, hw/pc.c is
> modified. In such case the author is supposed to update vl.c too.

There is no information about the emulated target reaching vl.c. In
fact, in a ideal world, there should be not even a single #ifdef
TARGET_xxx in that code. All the hardware emulation part is in the
hardware library and the generic code has no idea about the actual
emulated hardware. It even does not know if a family of device is
supported or not by the target. In microcontrollers, you often have no
bloc device. But nothing prevents you to add '-hda <my_disk> -boot c' to
the command line...

> > It would
> > seem really more logical to allow the user to give all defined possible
> > boot devices to the -boot parameter, then it's up to the target
> > initialisation code or the BIOS (some target may use different BIOS with
> > different ABIs for different usages...) to determine if the information
> > can be used totally, partially or not at all. Let me give an example:
> > what is the meaning of the -boot parameter for embedded board that can
> > only boot from a flash device (see the ppc405_boards.c, for
> > example...) ? My answer is that the user can always give the -boot
> > parameter but it will just be ignored by the target specific code.
> 
> This is exactly how it works in current CVS and also how it was before
> the commit.
> 
> I will remove some of the check in vl.c though, so that machine code
> is free to allow the combinations that it likes.
> 
> > And
> > the number of boot devices that may be usefull for a target is target or
> > BIOS dependant. It's not in any way CPU architecture dependant,
> 
> In qemu scope target arch is strictly connected with the target
> machines and BIOSes available.

I guess you're wrong, here. i386 CPU is the special case for which you
have only 1 target available and only 1 BIOS ABI. For ARM, Mips, Sparc,
PowerPC, you've got several targets with different BIOS ABIs (this last
point, maybe not for Sparc). Using the -L and/or the -bios option, one
can boot on different BIOS, maybe proprietary ones (which often happens
on embedded targets) with specific ABIs. The vl.c code have no way to
know anything about this, the way Qemu is implemented.

Regards.

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