From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751040AbdBCQTZ convert rfc822-to-8bit (ORCPT <rfc822;w@1wt.eu>);
        Fri, 3 Feb 2017 11:19:25 -0500
Received: from aserp1040.oracle.com ([141.146.126.69]:39903 "EHLO
        aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1750891AbdBCQTX (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 3 Feb 2017 11:19:23 -0500
Subject: Re: [PATCH v2 4/9] xen/pvh: Bootstrap PVH guest
To: Juergen Gross <jgross@suse.com>
References: <1485459692-23961-1-git-send-email-boris.ostrovsky@oracle.com>
 <1485459692-23961-5-git-send-email-boris.ostrovsky@oracle.com>
 <2d9887fb-15ad-e665-bcd7-df69a93380a3@suse.com>
Cc: roger.pau@citrix.com, xen-devel@lists.xenproject.org,
        linux-kernel@vger.kernel.org
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Message-ID: <0ca64861-0695-df0b-6b86-a41f3e170722@oracle.com>
Date: Fri, 3 Feb 2017 11:20:35 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.6.0
MIME-Version: 1.0
In-Reply-To: <2d9887fb-15ad-e665-bcd7-df69a93380a3@suse.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8BIT
X-Source-IP: aserv0021.oracle.com [141.146.126.233]
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


>> +
>> +	__HEAD
>> +
>> +/* Entry point for PVH guests. */
> Could you add some comments about register conetnts at entry?

Reference to Xen's docs/misc/hvmlite.markdown would be sifficient?




>> +gdt:
>> +	.word	gdt_end - gdt
>> +	.long	_pa(gdt)
> This is a rather strange construct: the NULL descriptor of the
> GDT being used as space for lgdt operand.
>
>> +	.word	0
>> +	.quad	0x0000000000000000 /* NULL descriptor */
> And this comment is wrong: the NULL descriptor is at "gdt:".

I'll change it to:

gdt:
        .word   gdt_end - gdt_start
        .long   _pa(gdt_start)
        .word   0
gdt_start:
        .quad   0x0000000000000000 /* NULL descriptor */
        .quad   0x0000000000000000 /* reserved */
#ifdef CONFIG_X86_64
        .quad   0x00af9a000000ffff /* __KERNEL_CS */
#else
        .quad   0x00cf9a000000ffff /* __KERNEL_CS */
#endif
        .quad   0x00cf92000000ffff /* __KERNEL_DS */
gdt_end:


>
>> +#ifdef CONFIG_X86_64
>> +	.quad	0x00af9a000000ffff /* __KERNEL_CS */
> Mind adding comments about the semantics of those constants?
> Or use GDT_ENTRY() macro?
>
>> +#else
>> +	.quad	0x00cf9a000000ffff /* __KERNEL_CS */
>> +#endif
>> +	.quad	0x00cf92000000ffff /* __KERNEL_DS */
>> +gdt_end:
>> +
>> +	.bss
>> +	.balign 4
>> +early_stack:
>> +	.fill 16, 1, 0
> Is the stack size large enough? With a hypercall being executed in
> xen_prepare_pvh() I doubt this will be okay.

What do you think it should be then?

-boris