Re: [Qemu-devel] [PATCH v3 2/2] hw/vexpress.c: Add model of ARM Versatile Express board

[Aurelien Jarno <aurelien@aurel32.net>]

On Mon, Mar 21, 2011 at 09:19:56PM +0000, Peter Maydell wrote:
> On 21 March 2011 20:41, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > On Mon, Mar 07, 2011 at 11:10:32AM +0000, Peter Maydell wrote:
> >> +    /* 0x4e000000 LAN9118 Ethernet */
> >> +    if (nd_table[0].vlan) {
> >> +        lan9118_init(&nd_table[0], 0x4e000000, pic[15]);
> >> +    }
> >
> > It basically means we silently ignore all other network cards the user
> > asked. Shouldn't we but a warning/error in that case?
> 
> Well, if you ask for one network card that's the wrong kind
> you get an error:
> qemu-system-arm: qemu: Unsupported NIC model: ne2k_pci

Which looks fine.

> but you're right that if you ask for several cards we ignore
> the other ones.
> 
> I just copied this code pattern from hw/integratorcp.c ; if you
> want to suggest a better code fragment I'm happy to revise
> the patch.

Something like that maybe:

if (nb_nics > 1) {
    fprintf(stderr, "Only one network card supported\n");
    exit(1);
}

> (hw/versatilepb.c is even worse, if I'm reading the code right
> it creates rtl8139 cards regardless of what model you ask for...)

IIRC it's because versatilepb doesn't support PCI I/O ports, so this is
the only network card working. In this kind of situations, a comment
would clearly help, also I agree it should simply fail if the user
requested another network card.

> Conceptually, though, it seems to me that the right place to
> put "the user specified a network card/a USB device/whatever
> but nothing in the board model used it" checks is in the
> generic code. Otherwise even boards with no network support
> at all need to have boilerplate "reject any network card
> specification" code, for instance, all boards without USB need
> code to reject attempts to define USB devices, and so on...

I agree that it the long term we should have a better infrastructure for
that. Looks like a bit similar to the maximum memory patch you posted.
Maybe it should be merged in something providing the architectural
limits of an emulated machine (number of CPU also comes to my mind).

> > For having the possibility to add more than one card, I guess it's
> > possible to simulated the PCI controller? Well if the board has any PCI
> > controller.
> 
> Sadly PCI is not supported by the A9MPx4 daughterboard.

:(

> > Otherwise the patch looks fine to me, it's very nice being able to boot
> > an ARM board with more than 256MB of RAM.
> 
> PS: PBX-A9 also supports 1GB RAM :-) (but for different reasons
> doesn't have PCI support and also the model is decidedly inaccurate).
> 

If a distribution wants to choose between providing PBX-A9 and VExpress
kernels, what would you advice?

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net