[Lguest] [patch] u32 pointer nightmare .....

[Jes Sorensen]

Rusty Russell wrote:
>> -static u32 *dma2iov(unsigned long dma, struct iovec iov[], unsigned *num)
>> +static unsigned long *
>> +dma2iov(unsigned long dma, struct iovec iov[], unsigned *num)
>>  {
> 
> This one really is a u32*, even on 64-bit platforms (it's where we put
> the length written/read).

Problem is when you have a struct that has multiple of them in it, you
can end up with badly aligned fields which needs kernel or firmware
support to handle - ia64 doesn't handle 64 bit loads on misaligned
boundaries in hardware :(

Thats why I thought it was better to just stick to long for those and
get proper alignment at all times.

> IRQ is also handed as a 32 bit, even on 64 bit platforms.
> 
>>  		= ((struct e820entry) { 0, mem, E820_RAM });
>>  	/* The boot header contains a command line pointer: we put the command
>>  	 * line after the boot header (at address 4096) */
>> -	*(u32 *)(boot + 0x228) = 4096;
>> +	*(unsigned long *)(boot + 0x228) = 4096;
>>  	concat(boot + 4096, argv+optind+2);
> 
> I think the boot header is i386 specific anyway, but it's definitely
> defined to be a 32 bit field.

I see, I thought it was an ELF header field.

>> -static ssize_t write(struct file *file, const char __user *input,
>> +static ssize_t write(struct file *file, const char __user *__input,
>>  		     size_t size, loff_t *off)
>>  {
>>  	/* Once the guest is initialized, we hold the "struct lguest" in the
>>  	 * file private data. */
>>  	struct lguest *lg = file->private_data;
>> -	u32 req;
>> +	const unsigned long __user *input  =
>> +		(const unsigned long __user *)__input;
>> +	unsigned long req;
> 
> This is a nice change, but we do an "input += sizeof(req)" later, and
> that needed changing too.

I see, problem is I don't have a setup to boot test on, so all I can do
is to build test it :-(

Cheers,
Jes