Re: [Qemu-devel] [PATCH target-arm v1 1/1] arm/xilinx_zynq: Always instantiate the GEMs

[Peter Crosthwaite]

On Fri, Jan 3, 2014 at 8:51 PM, Peter Maydell <address@hidden> wrote:
> On 3 January 2014 03:21, Peter Crosthwaite <address@hidden> wrote:
>> Don't conditionalise GEM instantiation on networking attachments. The
>> device should always be present even if not attached to a network.
>>
>> This allows for probing of the device by expectant guests (such as
>> OS's).  This is needed because sysbus (or AXI in Xilinx's real hw case)
>> is not self identifying so the guest has no dynamic way of detecting
>> device absence.
>
> Agreed that this is the right thing.
>
> Some day I might try to look into how to update our handling
> of embedded NICs to work with non-legacy command lines...
>
>
>> -    for (n = 0; n < nb_nics; n++) {
>> -        nd = &nd_table[n];
>> +    for (n = 0; n < 2; n++) {
>> +        NICInfo *nd = n < nb_nics ? &nd_table[n] : NULL;
>>          if (n == 0) {
>>              gem_init(nd, 0xE000B000, pic[54-IRQ_OFFSET]);
>>          } else if (n == 1) {
>
> This is now a rather odd loop which goes round exactly twice
> and uses an if() statement to make the body different each time.

Yeh, I think I was going for minimum diff when I did this one. Will fix.

>
> Instead you can just say:
>      gem_init(nd_table[0], 0xe000b000, pic[54 - IRQ_OFFSET]);
>      gem_init(nd_table[1], 0xe000c000, pic[54 - IRQ_OFFSET]);
>
> and have gem_init() condition the calls to qdev_set_nic_properties

One subtle question before respinning - should the
qdev_set_nic_properties actually be conditional? Setting it to an
unused NIC should be harmless, so perhaps it should be outside the
if(nd->used). Other boards/MAC combos that dont do the
qdev_check_nic_model can then just dispose of the if (nd->used) check
altogether.

Regards,
Peter

> and qdev_check_nic_model on "if (nd->used)".
>
> thanks
> -- PMM
>