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

[Pu Wen <puwen@xxxxxxxx>]

On 2019/3/18 16:55, Jan Beulich wrote:
> On 16.03.19 at 10:57, <puwen@xxxxxxxx> wrote:
>> On 2019/3/15 19:18, Jan Beulich wrote:
>>> On 15.03.19 at 11:17, <puwen@xxxxxxxx> wrote:
>>>> On 2019/3/15 1:11, Jan Beulich wrote:
>>>>> This is a lot of duplicated code with only minor differences. I think
>>>>> you would be better off calling into the AMD original functions.
>>>> These functions and AMD original ones are static. So Hygon cannot directly
>>>> call into them. Or we can put them into the common cpu code, but I think
>>>> it's not good for future maintenance.
>>> Just make non-static what needs to be, add an amd_ prefix, and
>>> call it from your code.
>> That's OK. With this method only init_levelling in amd.c should be added
>> an amd_ prefix and called by hygon.c.
>> But I'm afraid Hygon should have its own init functions and not call the
>> AMD ones. The current Hygon init functions have been heavily stripped
>> from the original AMD's.
> Let me give you this rule of thumb (subject to discussion): If you can
> safely re-use any non-trivial current AMD function with at most minor
> adjustments (and irrespective of certain code there being unreachable
> on Hygon), then I think it would be better to re-use it than to duplicate
> it.

I tried, but it will add 0x18 to init_amd(). It's strange because AMD
does not have family 18h now. So it becomes unwieldy to maintain
init_amd() just at this time. The same situation for amd_get_topology().

So I think it's better to carve out init_hygon() and hygon_get_topology()
(which also need Hygon's own adjustment for computing phys_proc_id).

I think it's time to develop a new patch series for your review.

-- 
Regards,
Pu Wen

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