From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35627) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b5TxH-00072d-Gd for qemu-devel@nongnu.org; Wed, 25 May 2016 04:14:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b5TxD-0003lN-Du for qemu-devel@nongnu.org; Wed, 25 May 2016 04:14:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44732) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b5TxD-0003lB-5H for qemu-devel@nongnu.org; Wed, 25 May 2016 04:14:11 -0400 References: <573D3799.9090109@redhat.com> <573E0276.8040002@tribudubois.net> <573E77BE.4090609@redhat.com> <573F804B.4000608@tribudubois.net> <5743B96A.9060408@redhat.com> <5744AC9F.7010801@tribudubois.net> From: Jason Wang Message-ID: <57455ECD.7050304@redhat.com> Date: Wed, 25 May 2016 16:14:05 +0800 MIME-Version: 1.0 In-Reply-To: <5744AC9F.7010801@tribudubois.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jean-Christophe DUBOIS , qemu-devel@nongnu.org, peter.maydell@linaro.org On 2016=E5=B9=B405=E6=9C=8825=E6=97=A5 03:33, Jean-Christophe DUBOIS wrot= e: > Le 24/05/2016 04:16, Jason Wang a =C3=A9crit : >> >> >> On 2016=E5=B9=B405=E6=9C=8821=E6=97=A5 05:23, Jean-Christophe DUBOIS w= rote: >>> Le 20/05/2016 04:34, Jason Wang a =C3=A9crit : >>>> >>>> >>>> On 2016=E5=B9=B405=E6=9C=8820=E6=97=A5 02:14, Jean-Christophe DUBOIS= wrote: >>>>> Le 19/05/2016 05:48, Jason Wang a =C3=A9crit : >>>>>> >>>>>> >>>>>> On 2016=E5=B9=B405=E6=9C=8819=E6=97=A5 06:23, Jean-Christophe Dubo= is wrote: >>>>>>> The ENET device (present in i.MX6) is "derived" from FEC and=20 >>>>>>> backward >>>>>>> compatible with it. >>>>>>> >>>>>>> This patch adds the necessary support of the added feature in=20 >>>>>>> the ENET >>>>>>> device to allow Linux to use it (on supported processors). >>>>>>> >>>>>>> Signed-off-by: Jean-Christophe Dubois >>>>>>> --- >>>>>>> >>>>>>> Changes since v1: >>>>>>> * Not present on v1 >>>>>>> Changes since v2: >>>>>>> * Not present on v2 >>>>>>> Changes since v3: >>>>>>> * Separate and fix the 2 supported interrupts >>>>>>> >>>>>>> hw/arm/fsl-imx25.c | 3 + >>>>>>> hw/net/imx_fec.c | 713=20 >>>>>>> ++++++++++++++++++++++++++++++++++++++--------- >>>>>>> include/hw/net/imx_fec.h | 195 ++++++++++--- >>>>>>> 3 files changed, 745 insertions(+), 166 deletions(-) >>>>>> >>>>>> [...] >>>>>> >>>>>>> -static Property imx_fec_properties[] =3D { >>>>>>> +static Property imx_eth_properties[] =3D { >>>>>>> DEFINE_NIC_PROPERTIES(IMXFECState, conf), >>>>>>> + DEFINE_PROP_BOOL("is-fec", IMXFECState, is_fec, false), >>>>>>> DEFINE_PROP_END_OF_LIST(), >>>>>>> }; >>>>>> >>>>>> It's ok to decide with "is-fec", but is it better to use a new=20 >>>>>> type for that? >>>>> >>>>> Well, there is a lot of common code between FEC and ENET because=20 >>>>> ENET is basically backward compatible with FEC. So most/all of the=20 >>>>> FEC code needs to go in the ENET device. >>>>> >>>>> I thought this way of doing thing was the best way to avoid=20 >>>>> duplicating things. >>>> >>>> You can still share almost all the codes. E.g you can have a look=20 >>>> at e1000.c which have 3 types of rather similar devices. >>> >>> OK, I'll change it in next patch to match the pl110 model as=20 >>> proposed by Peter. No more additional property... >>> >>>> >>>> Another question not relate to this patch, I saw version were=20 >>>> bumped for vmstate version. Is it better to keep the vmstate for=20 >>>> FEC and using a new vmstate for ENET (register array). >>> >>> Well, as I moved the FEC to register array, the VMState structure=20 >>> has changed and therefore I believe the vmstate version needs to be=20 >>> bumped. >>> >>> Am I wrong?=20 >> >> You're right. But migration to old version does not work in this way,=20 >> even if there aren't any changes in FEC. >> >> I'm not sure the policy for arm, but we used to do lots of works in=20 >> the past to make migration to older version works. >> >> You can achieve this by using two different vmstates, and keep the=20 >> FEC's state (by just use parts of array). >> >> Perter, any thoughts on this? Should we try to keep the migration=20 >> compatibility here? > > I was thinking the migration feature was to migrate old VMSTATE (from=20 > previous version of Qemu) to the present (or more modern) version. > > I certainly understand the intent and it does make a lot of sense when=20 > you are migrating live guest image like for example when using KVM=20 > and/or Xen for x86. > > Now I am not sure it is worth the trouble in this case. FEC is only=20 > used by the i.MX25 PDK board (for now) and I would be surprised if=20 > anybody had build a guest image using this platform and that they have=20 > the need to migrate this guest from one Qemu version to another. > > I am not sure KVM/Xen for ARM is mainstream enough yet and in any case=20 > it would be surprising they used i.MX25 PDK as an emulated guest.=20 > Hopefully it might change someday ... > > I might be wrong and you could have another opinion ... > > JC=20 I get the points, then I think there's no need for doing the=20 compatibility stuffs. Thanks