linux-kernel - RE: [RFC PATCH 1/3] arch/x86: add ACRN hypervisor guest

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 25 Mar 2019 03:24:28 +0000
From:   "Zhao, Yakui" <yakui.zhao@...el.com>
To:     Thomas Gleixner <tglx@...utronix.de>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "Chen, Jason CJ" <jason.cj.chen@...el.com>
Subject: RE: [RFC PATCH 1/3] arch/x86: add ACRN hypervisor guest

Hi,  Tglx

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