Re: [Xen-devel] [PATCH v4 3/5] xen: Put EFI machinery in place

[David Vrabel <david.vrabel@xxxxxxxxxx>]

On 16/05/14 21:41, Daniel Kiper wrote:
> Put EFI machinery for Xen in place.

Put what machinery to do what?

> @@ -1714,6 +1725,21 @@ asmlinkage __visible void __init xen_start_kernel(void)
>  
>       xen_setup_runstate_info(0);
>  
> +     efi_systab_xen = xen_efi_probe();
> +
> +     if (efi_systab_xen) {
> +             strncpy((char *)&boot_params.efi_info.efi_loader_signature, 
> "Xen",
> +                             
> sizeof(boot_params.efi_info.efi_loader_signature));
> +             boot_params.efi_info.efi_systab = 
> (__u32)((__u64)efi_systab_xen);
> +             boot_params.efi_info.efi_systab_hi = 
> (__u32)((__u64)efi_systab_xen >> 32);
> +
> +             x86_platform.get_wallclock = efi_get_time;

x86_platform.get_wallclock should always be xen_get_wallclock().

> +             x86_platform.set_wallclock = efi_set_rtc_mmss;
> +
> +             set_bit(EFI_BOOT, &efi.flags);
> +             set_bit(EFI_64BIT, &efi.flags);
> +     }
> +
>       /* Start the world */
>  #ifdef CONFIG_X86_32
>       i386_start_kernel();
> --- /dev/null
> +++ b/drivers/xen/efi.c
> @@ -0,0 +1,374 @@
> + * Copyright (c) 2014 Daniel Kiper, Oracle Corporation

Is this really copyright by you personally and not Oracle?


> +#define call (op.u.efi_runtime_call)
> +#define DECLARE_CALL(what) \
> +     struct xen_platform_op op; \
> +     op.cmd = XENPF_efi_runtime_call; \
> +     call.function = XEN_EFI_##what; \
> +     call.misc = 0

Macros that declare local variables are awful.

Use what Andrew suggested and something like

struct xen_blah *call = &op.u.efi_runtime_call;


> +static const struct efi efi_xen __initconst = {
> +     .systab                   = NULL, /* Initialized later. */
> +     .runtime_version          = 0,    /* Initialized later. */
> +     .mps                      = EFI_INVALID_TABLE_ADDR,
> +     .acpi                     = EFI_INVALID_TABLE_ADDR,
> +     .acpi20                   = EFI_INVALID_TABLE_ADDR,
> +     .smbios                   = EFI_INVALID_TABLE_ADDR,
> +     .sal_systab               = EFI_INVALID_TABLE_ADDR,
> +     .boot_info                = EFI_INVALID_TABLE_ADDR,
> +     .hcdp                     = EFI_INVALID_TABLE_ADDR,
> +     .uga                      = EFI_INVALID_TABLE_ADDR,
> +     .uv_systab                = EFI_INVALID_TABLE_ADDR,
> +     .fw_vendor                = EFI_INVALID_TABLE_ADDR,
> +     .runtime                  = EFI_INVALID_TABLE_ADDR,
> +     .config_table             = EFI_INVALID_TABLE_ADDR,
> +     .get_time                 = xen_efi_get_time,
> +     .set_time                 = xen_efi_set_time,
> +     .get_wakeup_time          = xen_efi_get_wakeup_time,
> +     .set_wakeup_time          = xen_efi_set_wakeup_time,
> +     .get_variable             = xen_efi_get_variable,
> +     .get_next_variable        = xen_efi_get_next_variable,
> +     .set_variable             = xen_efi_set_variable,
> +     .query_variable_info      = xen_efi_query_variable_info,
> +     .update_capsule           = xen_efi_update_capsule,
> +     .query_capsule_caps       = xen_efi_query_capsule_caps,
> +     .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
> +     .reset_system             = NULL, /* Functionality provided by Xen. */

Xen provides functionality to reset (just maybe not via an EFI call).
Should an implementation be provided that does this?

> +     .set_virtual_address_map  = NULL, /* Not used under Xen. */
> +     .memmap                   = NULL, /* Not used under Xen. */
> +     .flags                    = 0     /* Initialized later. */
> +};
> +
> +efi_system_table_t __init *xen_efi_probe(void)
> +{
> +     struct xen_platform_op op = {
> +             .cmd = XENPF_firmware_info,
> +             .u.firmware_info = {
> +                     .type = XEN_FW_EFI_INFO,
> +                     .index = XEN_FW_EFI_CONFIG_TABLE
> +             }
> +     };
> +     union xenpf_efi_info *info = &op.u.firmware_info.u.efi_info;
> +
> +     if (!xen_initial_domain() || HYPERVISOR_dom0_op(&op))
> +             return NULL;

        if (!xen_initial_domain())
                return NULL;

        if (HYPERVISOR_dom0_op(&op) < 0)
                return NULL;

> +
> +     /* Here we know that Xen runs on EFI platform. */
> +
> +     efi = efi_xen;
> +
> +     op.cmd = XENPF_firmware_info;
> +     op.u.firmware_info.type = XEN_FW_EFI_INFO;
> +     op.u.firmware_info.index = XEN_FW_EFI_VENDOR;
> +     info->vendor.bufsz = sizeof(vendor);
> +     set_xen_guest_handle(info->vendor.name, vendor);
> +
> +     if (!HYPERVISOR_dom0_op(&op)) {

if (HYPERVISOR_dom0_op(&op) == 0)

David

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