Re: [PATCH] x86/xen: populate boot_params with EDD data

[David Vrabel <david.vrabel@citrix.com>]

On 03/04/13 15:53, Konrad Rzeszutek Wilk wrote:
> On Wed, Apr 03, 2013 at 12:10:49PM +0100, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> During early setup of a dom0 kernel, populate boot_params with the
>> Enhanced Disk Drive (EDD) and MBR signature data.  This makes
>> information on the BIOS boot device available in /sys/firmware/edd/.
[...]
>> --- a/arch/x86/xen/enlighten.c
>> +++ b/arch/x86/xen/enlighten.c
>> @@ -1306,6 +1307,60 @@ static const struct machine_ops xen_machine_ops __initconst = {
>>  	.emergency_restart = xen_emergency_restart,
>>  };
>>  
>> +#if defined(CONFIG_EDD) || defined(CONFIG_EDD_MODULE)
>> +static void __init load_edd(void)
>> +{
>> +	struct xen_platform_op op;
>> +	struct edd_info *edd_info;
>> +	u32 *mbr_signature;
>> +	unsigned nr;
>> +	int ret;
>> +
>> +	edd_info = boot_params.eddbuf;
>> +	mbr_signature = boot_params.edd_mbr_sig_buffer;
>> +
>> +	op.cmd = XENPF_firmware_info;
>> +
>> +	op.u.firmware_info.type = XEN_FW_DISK_INFO;
>> +	for (nr = 0; nr < EDDMAXNR; nr++) {
>> +		struct edd_info *info = edd_info + nr;
>> +
>> +		op.u.firmware_info.index = nr;
>> +		info->params.length = sizeof(info->params);
>> +		set_xen_guest_handle(op.u.firmware_info.u.disk_info.edd_params,
>> +				     &info->params);
>> +		ret = HYPERVISOR_dom0_op(&op);
>> +		if (ret)
>> +			break;
>> +
>> +#define C(x) info->x = op.u.firmware_info.u.disk_info.x
>> +		C(device);
>> +		C(version);
>> +		C(interface_support);
>> +		C(legacy_max_cylinder);
>> +		C(legacy_max_head);
>> +		C(legacy_sectors_per_track);
>> +#undef C
>> +	}
>> +	boot_params.eddbuf_entries = nr;
>> +
>> +	op.u.firmware_info.type = XEN_FW_DISK_MBR_SIGNATURE;
>> +	for (nr = 0; nr < EDD_MBR_SIG_MAX; nr++) {
>> +		op.u.firmware_info.index = nr;
>> +		ret = HYPERVISOR_dom0_op(&op);
>> +		if (ret)
>> +			break;
>> +		mbr_signature[nr] = op.u.firmware_info.u.disk_mbr_signature.mbr_signature;
>> +	}
>> +	boot_params.edd_mbr_sig_buf_entries = nr;
> 
> If those two loops end up terminating at different spots (say first
> one ends at nr=1, and the second at nr=5), is that going to present
> a problem?
> 
> Should we have some form of 'min(nr_earlier, nr)' to clamp down in
> case of weird oddities?

The edd firmware driver looks to correctly handle this -- it will only
show attributes for things that exist.

David