Hi, Tglx
Thanks for your nice review.
>-----Original Message-----
>From: Thomas Gleixner [mailto:t...@linutronix.de]
>Sent: Friday, March 22, 2019 11:20 PM
>To: Zhao, Yakui <yakui.z...@intel.com>
>Cc: linux-kernel@vger.kernel.org; x...@kernel.org; Chen, Jason CJ
><jason.cj.c...@intel.com>
>Subject: Re: [RFC PATCH 1/3] arch/x86: add ACRN hypervisor guest
>
>Zhao,
>
>On Thu, 7 Mar 2019, Zhao Yakui wrote:
>
>> ACRN is one open-source hypervisour, which is maintained by Linux
>> foundation. This is to add the para-virtualization support so that it
>> allows to enable the Linux guest on acrn-hypervisor.
>>
>> This adds x86_hyper_acrn into supported hypervisors array, which
>> enables ACRN services guest running on ACRN hypervisor. It is
>
>What is a ACRN services guest?
It should be "ACRN guest".
>
>> restricted to X86_64.
>>
>> Signed-off-by: Jason Chen CJ <jason.cj.c...@intel.com>
>> Signed-off-by: Zhao Yakui <yakui.z...@intel.com>
>
>This SOB chain is wrong. If Jason wrote the patch, then there should be a
>'From: Jason ..' line at the top of the change log. If you both wrote it then
>Jason's SOB wants to be preceeded by a 'Co-developed-by: Jason....'
>line. See Documentation/process/
Sure. It will be updated to "Co-developed-by: Jason" in next version.
>
>> +config ACRN
>> + bool "Enable services on ACRN hypervisor"
>> + depends on X86_64 && PARAVIRT
>> + help
>> + This option allows to run Linux as guest in ACRN hypervisor.
>> + It is needed if you want to run ACRN services linux on top of
>> + ACRN hypervisor.
>
>This help text does not make any sense to me.
"Enable Services on ACRN hypervisor" is a little confusing.
In next version it will be changed to "ACRN_GUEST". How about the below:
config ACRN_GUEST
bool "ACRN Guest Support"
depends on X86_64 && PARAVIRT
help
This option allows to run Linux as guest in ACRN hypervisor. Enabling
this will allow the
kernel to boot in a paravirtualized environment under the ACRN
hypervisor
>
>> +const struct hypervisor_x86 x86_hyper_acrn = {
>> + .name = "ACRN",
>> + .detect = acrn_detect,
>> + .type = X86_HYPER_ACRN,
>> + .init.init_platform = acrn_init_platform,
>> + .init.x2apic_available = acrn_x2apic_available, };
>> +EXPORT_SYMBOL(x86_hyper_acrn);
>
>Whay is this exported? The only user of this is hypervisor.c and that is built
>in.
>Please remove.
You are right.
It will be removed and the x86_hyper_acrn will also be added into "__initconst"
section.
>
>Thanks,
>
> tglx
