linux-kernel - Re: [PATCH v3 5/5] xen: Put EFI machinery in place

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140326131214.GC24856@console-pimps.org>
Date:	Wed, 26 Mar 2014 13:12:14 +0000
From:	Matt Fleming <matt@...sole-pimps.org>
To:	Daniel Kiper <daniel.kiper@...cle.com>
Cc:	linux-efi@...r.kernel.org, linux-ia64@...r.kernel.org,
	linux-kernel@...r.kernel.org, x86@...nel.org,
	xen-devel@...ts.xenproject.org, boris.ostrovsky@...cle.com,
	david.vrabel@...rix.com, eshelton@...ox.com, fenghua.yu@...el.com,
	hpa@...or.com, ian.campbell@...rix.com, jbeulich@...e.com,
	jeremy@...p.org, konrad.wilk@...cle.com, matt.fleming@...el.com,
	mingo@...hat.com, stefano.stabellini@...citrix.com,
	tglx@...utronix.de, tony.luck@...el.com,
	Tang Liang <liang.tang@...cle.com>
Subject: Re: [PATCH v3 5/5] xen: Put EFI machinery in place

On Tue, 25 Mar, at 09:57:56PM, Daniel Kiper wrote:
> Put EFI machinery for Xen in place.
> 
> This patch is based on Jan Beulich and Tang Liang work.
> 
> Signed-off-by: Daniel Kiper <daniel.kiper@...cle.com>
> Signed-off-by: Tang Liang <liang.tang@...cle.com>
> Signed-off-by: Jan Beulich <jbeulich@...e.com>
> ---
>  arch/x86/xen/enlighten.c |   10 ++
>  drivers/xen/Kconfig      |    3 +
>  drivers/xen/Makefile     |    1 +
>  drivers/xen/efi.c        |  425 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 439 insertions(+)
>  create mode 100644 drivers/xen/efi.c

[...]

> +static void __init efi_init_xen(void)
> +{
> +	efi_char16_t vendor_c16[100];
> +	char vendor[ARRAY_SIZE(vendor_c16)];
> +	int ret, i;
> +	struct xen_platform_op op;
> +	union xenpf_efi_info *info = &op.u.firmware_info.u.efi_info;
> +
> +	efi = efi_xen;
> +	op.cmd = XENPF_firmware_info;
> +	op.u.firmware_info.type = XEN_FW_EFI_INFO;
> +
> +	/*
> +	 * Show what we know for posterity
> +	 */
> +	op.u.firmware_info.index = XEN_FW_EFI_VENDOR;
> +	info->vendor.bufsz = sizeof(vendor_c16);
> +	set_xen_guest_handle(info->vendor.name, vendor_c16);
> +	ret = HYPERVISOR_dom0_op(&op);
> +	if (!ret) {
> +		for (i = 0; i < sizeof(vendor) - 1 && vendor_c16[i]; ++i)
> +			vendor[i] = vendor_c16[i];
> +		vendor[i] = '\0';
> +	} else
> +		pr_err("Could not get the firmware vendor!\n");
> +

Is there a reason that you can't just populate an efi_system_table_t
object, which could be used by efi_init(), so that we can save you the
trouble of duplicating all of this code?

> +/*
> + * Convenience functions to obtain memory types and attributes
> + */
> +static u32 efi_mem_type_xen(unsigned long phys_addr)
> +{
> +	struct xen_platform_op op;
> +	union xenpf_efi_info *info = &op.u.firmware_info.u.efi_info;
> +
> +	op.cmd = XENPF_firmware_info;
> +	op.u.firmware_info.type = XEN_FW_EFI_INFO;
> +	op.u.firmware_info.index = XEN_FW_EFI_MEM_INFO;
> +	info->mem.addr = phys_addr;
> +	info->mem.size = 0;
> +	return HYPERVISOR_dom0_op(&op) ? 0 : info->mem.type;
> +}

Same idea here. Unless you expect the EFI memory map to change at runtime
(and it's not clear to me whether that wouldn't cause other things to
explode) you'd be better off building a struct efi_memory_map and using
the existing generic functions.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/