Re: [Xen-devel] [PATCH 4/8] xen/pvh: Bootstrap PVH guest

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 10/14/2016 03:14 PM, Konrad Rzeszutek Wilk wrote:
>
>> +
>> +    memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
>> +
>> +    memmap.nr_entries = ARRAY_SIZE(pvh_bootparams.e820_map);
>> +    set_xen_guest_handle(memmap.buffer, pvh_bootparams.e820_map);
>> +    if (HYPERVISOR_memory_op(XENMEM_memory_map, &memmap)) {
>> +            xen_raw_console_write("XENMEM_memory_map failed\n");
> Should we print the error value at least?

I will have to check again but IIRC there was something about not being
able to format strings properly this early. But if we can --- sure.

>> +            BUG();
>> +    }
>> +
>> +    pvh_bootparams.e820_map[memmap.nr_entries].addr =
>> +            ISA_START_ADDRESS;
> What if nr_entries is 128? Should we double-check for that?
>

OK.



>> + */
>> +void __init xen_prepare_pvh(void)
>> +{
>> +    u32 eax, ecx, edx, msr;
> msr = 0 ?

Won't cpuid() (or cpuid_ebx()) overwrite it anyway?

>> +    u64 pfn;
>> +
>> +    xen_pvh = 1;
>> +
>> +    cpuid(xen_cpuid_base() + 2, &eax, &msr, &ecx, &edx);
> cpuid_ebx ? And that way you don't have have ecx and edx?



>> +    cli
>> +    cld
>> +
>> +    mov $_pa(gdt), %eax
>> +    lgdt (%eax)
>> +
>> +    movl $(__BOOT_DS),%eax
>> +    movl %eax,%ds
>> +    movl %eax,%es
>> +    movl %eax,%ss
>> +
>> +    /* Stash hvm_start_info */
>> +    mov $_pa(pvh_start_info), %edi
>> +    mov %ebx, %esi
> Should we derference the first byte or such to check for the magic
> string? Actually I am not even seeing the check in the C code?


Yes, good idea.


>> +    .code64
>> +1:
>> +    call xen_prepare_pvh
>> +
>> +    /* startup_64 expects boot_params in %rsi */
> ..
>> +    mov $_pa(pvh_bootparams), %rsi
>> +    movq $_pa(startup_64), %rax
>> +    jmp *%rax
>> +
>> +#else /* CONFIG_X86_64 */
>> +
>> +    call setup_pgtable_32
>> +
>> +    mov $_pa(initial_page_table), %eax
>> +    movl %eax, %cr3
>> +
>> +    movl %cr0, %eax
>> +    orl $(X86_CR0_PG | X86_CR0_PE), %eax
>> +    movl %eax, %cr0
>> +
>> +    ljmp $__BOOT_CS,$1f
>> +1:
>> +    call xen_prepare_pvh
>> +    mov $_pa(pvh_bootparams), %esi
>> +
>> +    /* startup_32 doesn't expect paging and PAE to be on */
> Should 'startup_32' be documented with this?

It is documented in Documentation/x86/boot.txt and in the startup_64 code.


-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel