[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH target-arm v1 1/1] arm/xilinx_zynq: Always insta
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH target-arm v1 1/1] arm/xilinx_zynq: Always instantiate the GEMs |
Date: |
Sat, 4 Jan 2014 02:15:30 +1000 |
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
>