Re: [Xen-devel] [PATCH 01/15] x86/cpu: Create Hygon Dhyana architecture support file

[Pu Wen <puwen@xxxxxxxx>]

On 2018/12/28 5:11, Andrew Cooper wrote:
> On 26/12/2018 11:42, Pu Wen wrote:
>> On 2018/12/21 18:20, Andrew Cooper wrote:
>>> Is there anything which is actually unique to Hygon here?  I ask,
>>> because this looks like a lot of duplicate code, considering that the
>>> processor base is the same.
>> Right now these codes are necessary for Hygon Dhyana processor even though
>> they are duplicated. As Hygon Dhyana support many CPU features such as ITSC
>> and EFRO, so I think the "if cpu_has" determine should be removed to make
>> the code clear and essential.
>>
>> Keeping the codes into a separate compilation unit(hygon.c) at least has
>> two advantages:
>> 1) Make the code flow more clear. Hygon is a new joint venture which has no
>>     historical old architectures, so I'm afraid that there are sufficient
>>     motivations to keep a clear new processor init flow.
>> 2) Beneficial for the future maintaining. AMD and Hygon may maintain their
>>     respective architecture related codes with no interaction with each
>>     other.
>>
>> For these reasons, we choose to keep the architecture initialization codes
>> in hygon.c.
>
> The most important question here is how likely is it to diverge in the
> future?> > Where possible, duplicate code should be kept to a minimum, 
> because of
> the risk of it being modified in only one of the places.

Yes, we are trying our best to make the duplicated code minimum but
essential for Hygon Dhyana processor.

> If Hygon is expected to diverge substantially in the future, then
> perhaps the duplication is fine.  If Hygon is unlikely to diverge far
> from Zen (particularly if you intend to use newer Zen cores as new Hygon
> bases), then perhaps it would be worth making a common amd_base.c file,
> and restrict amd.c and hygon.c to unique features.

Your point is correct, for future version, Hygon CPU will diverge from
AMD Zen and do its own modification. So we lay the ground for the future
code maintenance.

Actually we have discussed the same topic with the linux community[1],
and finally reached some agreement that to keep hygon.c separated from
AMD even though there are some duplicated codes at the moment[2].

We understand the difficulty to find a balance point between sharing
codes and maintenance effort.
Any suggestion is welcome.

Reference:
[1] https://lore.kernel.org/lkml/65c91fb2-4b57-8ddb-3363-ac6fe69957b9@xxxxxxxx/
[2] https://lore.kernel.org/lkml/20180910163821.GD4386@xxxxxxx/

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel