lkml.org 
[lkml]   [2019]   [Mar]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [RFC PATCH 1/3] arch/x86: add ACRN hypervisor guest
Date
Hi,  Tglx

Thanks for your nice review.
>-----Original Message-----
>From: Thomas Gleixner [mailto:tglx@linutronix.de]
>Sent: Friday, March 22, 2019 11:20 PM
>To: Zhao, Yakui <yakui.zhao@intel.com>
>Cc: linux-kernel@vger.kernel.org; x86@kernel.org; Chen, Jason CJ
><jason.cj.chen@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.chen@intel.com>
>> Signed-off-by: Zhao Yakui <yakui.zhao@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

\
 
 \ /
  Last update: 2019-03-25 04:28    [W:0.087 / U:4.068 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site