Re: [Xen-devel] [PATCH 1 of 3] Enable UEFI BIOS(OVMF) support in Xen-unstable HVM

[Bei Guan <gbtju85@xxxxxxxxx>]

Hi Keir,

Thank you for your comments.

2011/7/23 Keir Fraser <keir@xxxxxxx>

On 23/07/2011 07:53, "Keir Fraser" <keir.xen@xxxxxxxxx> wrote:

> On 22/07/2011 19:38, "Andrei Warkentin" <andreiw@xxxxxxxxxxxx> wrote:
>
>> On Fri, Jul 22, 2011 at 11:38 AM, Keir Fraser <keir.xen@xxxxxxxxx> wrote:
>>
>>> Looks pretty decent. I wonder why you need to change get_shared_info() --
>>> the existing mapping location is unused at the time hvmloader runs, and you
>>> instead map it over the top of a page of RAM. If you want shared_info mapped
>>> elsewhere, you can map it wherever you like as soon as your BIOS payload
>>> takes over.
>>>
>>
>> The problem is that this page lies in an unsafe for OVMF area (right
>> below 4GB). In a typical PC environment,
>> you have the firmware ROM decoding the physical address space right
>> below 4GB, and it also has a chunk (~64-128k) shadowed below 1MB for
>> legacy reasons. The EFI firmware bootstrap code is written with the
>> assumption that it can transfer control to code < 4GB that will
>> finalize the 16->PM-(>LM if 64) transitions and call C code. The
>> get_shared_info page overlaps code we copy up below 4GB. This is why
>> it was moved to a safer region.
>
> Okay, we can work with this easily enough.

I've applied the util.c changes, or equivalent, to xen-unstable.

Further comments on the rest of the hvmloader patch:

The added pci_load hook should be removed or used.

Ok, I  have removed this hook.

The printk change on non-detection of a VGA adapter should be removed and
proposed as a standalone fix if it makes sense.

I also removed this change. 

The bios_load hook is fine, but should replace the ugly bios_relocate hook
that was previously hacked in at around the same place. This will simply
involve rombios loading both low and high portions of itself, which is
really cleaner than what happens currently. This avoids adding yet another
hook.

Do you mean that put the bios_relocate hook in the "else" statement? Just like this:

    if (bios->load) {
        bios->load(bios);
    } else {
        printf("Loading %s ...\n", bios->name);
        memcpy((void *)bios->bios_address, bios->image,
               bios->image_size);

        if (bios->bios_relocate)
            bios->bios_relocate(); 

   }
 

Thanks,

Bei Guan

 

 -- Keir

>  -- Keir
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel