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

[Peter Crosthwaite]

On Sat, Jan 4, 2014 at 3:24 AM, Peter Maydell <address@hidden> wrote:
> On 3 January 2014 16:56, Peter Crosthwaite <address@hidden> wrote:
>> On Sat, Jan 4, 2014 at 2:22 AM, Peter Maydell <address@hidden> wrote:
>>> On 3 January 2014 16:15, Peter Crosthwaite <address@hidden> wrote:
>>>> 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.
>>>
>>> Well qdev_set_nic_properties as it stands today will unconditionally
>>> assume you've passed it a valid NICInfo, so we have to a void calling
>>> it if nd->used isn't set.
>>
>> So I guess I'm thinking that the unusued nics should be a "valid"
>> NICInfo - just one that doesn't connect to anything. Currently,
>> qdev_set_nic_properties() sets three props - "mac", "netdev" and
>> "vectors". "mac" looks dubious to me, I'm not sure why the net layer
>> is telling devices what their MAC addresses are - it should be the
>> other way round.
>
> The device doesn't (in the old -net model) know what its MAC address
> is, because that's something you specify in the -net command line option.
> So this is the passing on of that info to the device.
>
>> Setting of "netdev" is already protected against NULL
>> setting so that one is handled. "vectors" looks like it has a sanity
>> check guard on its setting as well via the DEV_NVECTORS_UNSPECIFIED
>> check. Although that value may rely on some net.c code paths for
>> non-NULL NICs.
>
> I think it would probably be better to look into properly cleaning up
> the legacy -net nic support and moving embedded devices over to
> work with some variant of the new command line model, rather
> than tweaking the legacy stuff. nd_table eventually should ideally
> just go away I think.
>

Ok, you've talked me out of it. Respun with the if.

Regards,
Peter

> thanks
> -- PMM
>