[gPXE-devel] [PATCH] EFI Simple Network Protocol driver

[Michael Brown]

On Saturday 29 May 2010 04:58:16 Geoff Lywood wrote:
> > 2. SNP TX completions are potentially unsafe; see the comments in
> > efi_snp.c
> > about the interesting design choices in the SNP TX completion reporting
> > mechanism.  As far as I am aware, you are essentially limited to having
> > only
> > one outstanding TX buffer at any one time, otherwise you hit undefined
> > behaviour.
> 
> All of the EFI specifications that I can find, from 1.02 to 2.3, say that
>  GetStatus returns a single TX buffer each time it is called, and the
>  implementation is free to queue TX buffers if it pleases. I think that the
>  code in efi_snp.c is actually incorrect in this case.

Strange.  I can no longer find any trace of the documentation I remember 
reading (reflected in the comments in efi_snp.c) that TxBuf is supposed to 
represent a list of buffers.  It certainly makes a lot more sense that it 
should return a single buffer address.

Will update efi_snp.c.

> > 7. You have a call to OpenProtocol() for EfiLoadedImageProtocol which
> > isn't
> > balanced by any call to CloseProtocol.  More generally, would it be
> > possible
> > to use either only OpenProtocol or only LocateProtocol in order to obtain
> > all
> > the required protocols?
> 
> Since I use EFI_OPEN_PROTOCOL_GET_PROTOCOL, I don't have to match it with a
>  CloseProtocol. It says so in the spec. I think it is safe to assume that
>  the current image will not go away while it is executing.
> 
> The problem with LocateProtocol is that it will give you any instance of
>  the protocol, not necessarily the one associated with the handle that you
>  care about. In this particular case, I want the loaded image protocol
>  associated with my own image, not just any image.
> 
> In fact, I'm not convinced that the current uses of LocateProtocol are
>  actually correct. For example, a system with multiple PCI root bridges
>  will have multiple PCI root bridge I/O protocols, and there is nothing in
>  the gPXE code to select the correct one.

OK.  Would it be possible/sensible to use OpenProtocol with 
EFI_OPEN_PROTOCOL_GET_PROTOCOL for all required protocols (i.e. do not use 
LocateProtocol at all)?  (This wouldn't solve the multiple PCI root bridge 
issue, but it would make the code neater, if it's allowable; I don't know 
whether or not it would work for protocols that aren't directly related to the 
image.)

Michael