Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1573265

Re: [PATCH v2 4/9] xen/pvh: Bootstrap PVH guest

From Juergen Gross <jgross@suse.com>
Newsgroups linux.kernel
Subject Re: [PATCH v2 4/9] xen/pvh: Bootstrap PVH guest
Date 2017-02-03 17:50 +0100
Message-ID <t6PdE-4aP-23@gated-at.bofh.it> (permalink)
References <t3Ydu-2X9-77@gated-at.bofh.it> <t3Z9w-3wx-7@gated-at.bofh.it> <t6GtI-6ST-5@gated-at.bofh.it> <t6OKB-40r-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 03/02/17 17:20, Boris Ostrovsky wrote:
> 
>>> +
>>> +	__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?

I think the corresponding lines should be copied to this source
file. It is inconvenient to have to get the Xen repostory for
this information.

>>> +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 */

Much better. :-)

> #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?

I didn't check the disassembly, but even if it is okay right now
the needed stack size will depend on the compiler used. I'd rather
use a larger size (e.g. 256 bytes).

Maybe its even possible to reuse initial_stack, but I haven't
verified that.


Juergen

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH v2 4/9] xen/pvh: Bootstrap PVH guest Juergen Gross <jgross@suse.com> - 2017-02-03 08:30 +0100
  Re: [PATCH v2 4/9] xen/pvh: Bootstrap PVH guest Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-02-03 17:20 +0100
    Re: [PATCH v2 4/9] xen/pvh: Bootstrap PVH guest Juergen Gross <jgross@suse.com> - 2017-02-03 17:50 +0100
      Re: [Xen-devel] [PATCH v2 4/9] xen/pvh: Bootstrap PVH guest Andrew Cooper <andrew.cooper3@citrix.com> - 2017-02-03 19:10 +0100

csiph-web