[ipxe-devel] [gPXE-devel] [PATCH 1/4] [io] Move get_memmap() to the I/O API group

[Geoff Lywood]

I don't want my comments to block the work that you are doing. I'm not against your change: I think it is a definite improvement, I just wanted to throw some ideas around.

If you decide to go with a stub efi_get_memmap(), can you at least fix the b44 driver so that it errors out if memmap.count == 0?

Thanks,
Geoff


> -----Original Message-----
> From: Piotr Jaroszyński [mailto:p.jaroszynski at gmail.com]
> Sent: Monday, August 16, 2010 3:34 PM
> To: Geoff Lywood
> Cc: gpxe-devel at etherboot.org; Michael Brown; ipxe-devel at lists.ipxe.org
> Subject: Re: [gPXE-devel] [PATCH 1/4] [io] Move get_memmap() to the I/O
> API group
> 
> (CCing interested parties)
> 
> 2010/8/16 Geoff Lywood <glywood at vmware.com>:
> > I'd rather see the EFI build fail if someone tries to call get_memmap,
> instead of seeing a strange runtime failure when it gives incorrect
> results.
> 
> I was actually after making the all-drivers EFI build work so new
> problems are easily spotted, but that's a valid concern I think.
> Although there is just a single driver using it currently and I made
> it catch the error.
> 
> > Two ideas for other ways to do this:
> > - Move b44 to the arch/i386 directory.
> 
> That's easy :)
> 
> > - Find some way to get rid of the call to get_memmap in the b44 driver,
> perhaps by guaranteeing that malloc_dma always allocates memory below 1GB.
> You could provide an implementation of this on EFI, using BS-
> >AllocatePages. This would also be beneficial for other drivers on EFI,
> because most are not 64-bit clean.
> 
> I think this was brought up a few times. My patch [1] from the
> userspace bunch might help with that but I wouldn't want rest of the
> changes to block on implementing it as it doesn't sound
> straight-forward (I don't even have any EFI hardware to test).
> 
> [1] -
> http://git.etherboot.org/?p=people/peper/gpxe.git&a=search&h=refs/heads/dr
> ivers&st=commit&s=Introduce+memory+pools+and+hide+internal+API
> 
> --
> Piotr