From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965339AbcAUNrO (ORCPT ); Thu, 21 Jan 2016 08:47:14 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:47805 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965064AbcAUNrL (ORCPT ); Thu, 21 Jan 2016 08:47:11 -0500 Subject: Re: [RFC v1 4/8] x86/init: add linker table support To: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= , "Luis R. Rodriguez" , Konrad Rzeszutek Wilk , Stefano Stabellini References: <1450217797-19295-1-git-send-email-mcgrof@do-not-panic.com> <1450217797-19295-5-git-send-email-mcgrof@do-not-panic.com> <20160120210014.GF4769@char.us.oracle.com> <56A0990C.9090601@citrix.com> Cc: "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Rusty Russell , Andy Lutomirski , mcb30@ipxe.org, Juergen Gross , Jan Beulich , joro@8bytes.org, Andrey Ryabinin , andreyknvl@google.com, long.wanglong@huawei.com, qiuxishi@huawei.com, aryabinin@virtuozzo.com, Mauro Carvalho Chehab , Valentin Rothberg , Peter Senna Tschudin , X86 ML , "xen-devel@lists.xensource.com" , "linux-kernel@vger.kernel.org" From: Boris Ostrovsky Message-ID: <56A0E0FB.6020809@oracle.com> Date: Thu, 21 Jan 2016 08:45:31 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <56A0990C.9090601@citrix.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Source-IP: aserv0022.oracle.com [141.146.126.234] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/21/2016 03:38 AM, Roger Pau Monné wrote: > El 20/01/16 a les 22.33, Luis R. Rodriguez ha escrit: >> On Wed, Jan 20, 2016 at 1:00 PM, Konrad Rzeszutek Wilk >> wrote: >>>> +static bool x86_init_fn_supports_subarch(struct x86_init_fn *fn) >>>> +{ >>>> + if (!fn->supp_hardware_subarch) { >>>> + pr_err("Init sequence fails to declares any supported subarchs: %pF\n", fn->early_init); >>>> + WARN_ON(1); >>>> + } >>>> + if (BIT(boot_params.hdr.hardware_subarch) & fn->supp_hardware_subarch) >>>> + return true; >>>> + return false; >>>> +} >>> So the logic for this working is that boot_params.hdr.hardware_subarch >>> >>> And the macros define two: BIT(X86_SUBARCH_PC) or BIT(X86_SUBARCH_XEN). >>> >>> But hardware_subarch by default is set to zero. Which means if GRUB2, PXELinux, Xen multiboot1 >>> don't set it - then the X86_SUBARCH_PC is choosen right? >>> >>> 1 << 0 & 1 << X86_SUBARCH_PC (which is zero). >>> >>> For this to nicely work with Xen it ought to do this: >>> >>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c >>> index 993b7a7..6cf9afd 100644 >>> --- a/arch/x86/xen/enlighten.c >>> +++ b/arch/x86/xen/enlighten.c >>> @@ -1676,6 +1676,7 @@ asmlinkage __visible void __init xen_start_kernel(void) >>> boot_params.hdr.ramdisk_image = initrd_start; >>> boot_params.hdr.ramdisk_size = xen_start_info->mod_len; >>> boot_params.hdr.cmd_line_ptr = __pa(xen_start_info->cmd_line); >>> + boot_params.hdr.hardware_subarch = X86_SUBARCH_XEN; >>> >>> if (!xen_initial_domain()) { >>> add_preferred_console("xenboot", 0, NULL); >>> >>> >>> ? >> That's correct for PV and PVH, likewise when qemu is required for HVM >> qemu could set it. I have the qemu change done but that should only >> cover HVM. A common place to set this as well could be the hypervisor, >> but currently the hypervisor doesn't set any boot_params, instead a >> generic struct is passed and the kernel code (for any OS) is expected >> to interpret this and then set the required values for the OS in the >> init path. Long term though if we wanted to merge init further one way >> could be to have the hypervisor just set the zero page cleanly for the >> different modes. If we needed more data other than the >> hardware_subarch we also have the hardware_subarch_data, that's a u64 >> , and how that is used would be up to the subarch. In Xen's case it >> could do what it wants with it. That would still mean perhaps defining >> as part of a Xen boot protocol a place where xen specific code can >> count on finding more Xen data passed by the hypervisor, the >> xen_start_info. That is, if we wanted to merge init paths this is >> something to consider. >> >> One thing I considered on the question of who should set the zero page >> for Xen with the prospect of merging inits, or at least this subarch >> for both short term and long term are the obvious implications in >> terms of hypervisor / kernel / qemu combination requirements if the >> subarch is needed. Having it set in the kernel is an obvious immediate >> choice for PV / PVH but it means we can't merge init paths completely >> (down to asm inits), we'd still be able to merge some C init paths >> though, the first entry would still be different. Having the zero page >> set on the hypervisor would go long ways but it would mean a >> hypervisor change required. > I don't think the hypervisor should be setting Linux specific boot > related parameters, the boot ABI should be OS agnostic. IMHO, a small > shim should be added to Linux in order to set what Linux requires when > entering from a Xen entry point. And that's exactly what HVMlite does. Most of this shim layer is setting up boot_params, after which we jump to standard x86 boot path (i.e. startup_{32|64}). With hardware_subarch set to zero. -boris