On Thu, Aug 24, 2017 at 10:44:46AM +0100, Ard Biesheuvel wrote: > >> Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDriver.c | 346 ++++++++++++++++++++ > >> Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDriver.h | 61 ++++ > >> Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDxe.inf | 48 +++ > >> Silicon/Openmoko/ChaosKeyDxe/ComponentName.c | 205 ++++++++++++ > >> Silicon/Openmoko/ChaosKeyDxe/DriverBinding.c | 256 +++++++++++++++ > >> 5 files changed, 916 insertions(+) > > > > This is all a bit new territory, so I don't know what the best way of > > dealing with this is, but I think we could do with a .dsc somewhere to > > enable building this driver standalone. My gut feel is that an > > Openmoko/Openmoko.dsc would be a reasonable candidate. > > Yeah, I can add one. Also useful for building this driver as an EBC binary.
Yes, indeed. Thanks. > >> diff --git a/Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDriver.c > >> b/Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDriver.c > >> new file mode 100644 > >> index 000000000000..1870080d2c70 > >> --- /dev/null > >> +++ b/Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDriver.c > >> @@ -0,0 +1,346 @@ > >> +/** @file > >> + Device driver for the ChaosKey hardware random number generator. > >> + > >> + Copyright (c) 2016 - 2017, Linaro Ltd. All rights reserved.<BR> > >> + > >> + This program and the accompanying materials > >> + are licensed and made available under the terms and conditions of the > >> BSD > >> + License which accompanies this distribution. The full text of the > >> license may > >> + be found at http://opensource.org/licenses/bsd-license.php. > >> + > >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > >> IMPLIED. > >> + > >> +**/ > >> + > >> +#include "ChaosKeyDriver.h" > >> + > >> +#include <Library/BaseMemoryLib.h> > >> +#include <Library/DebugLib.h> > >> +#include <Library/MemoryAllocationLib.h> > >> + > >> +STATIC > >> +BOOLEAN > >> +IsBulkInEndpoint ( > >> + IN EFI_USB_ENDPOINT_DESCRIPTOR *Endpoint > >> + ) > >> +{ > >> + if ((Endpoint->Attributes & USB_ENDPOINT_TYPE_MASK) == > >> USB_ENDPOINT_BULK) { > >> + if (Endpoint->EndpointAddress & USB_ENDPOINT_DIR_IN) { > >> + return TRUE; > >> + } > >> + } > >> + return FALSE; > >> +} > >> + > >> + > >> +STATIC > >> +EFI_STATUS > >> +FindEndpoint ( > >> + IN CHAOSKEY_DEV *ChaosKey > >> + ) > >> +{ > >> + EFI_USB_IO_PROTOCOL *UsbIo; > >> + EFI_STATUS Status; > >> + UINTN Index; > >> + EFI_USB_INTERFACE_DESCRIPTOR InterfaceDescriptor; > >> + > >> + UsbIo = ChaosKey->UsbIo; > >> + > >> + // > >> + // Get interface & endpoint descriptor > >> + // > >> + Status = UsbIo->UsbGetInterfaceDescriptor (UsbIo, &InterfaceDescriptor); > >> + if (EFI_ERROR (Status)) { > >> + return Status; > >> + } > >> + > >> + // > >> + // The ChaosKey provides two endpoints: > >> + // - The first one is the 'cooked' one, to be used as random data input > >> + // - The second one is the raw bitstream from the generator, higher > >> + // throughput, but lower randomness. > >> + // So locate the first bulk IN endpoint and save it for later use. > >> + // > >> + for (Index = 0; Index < InterfaceDescriptor.NumEndpoints; Index++) { > >> + EFI_USB_ENDPOINT_DESCRIPTOR Endpoint; > >> + > >> + Status = UsbIo->UsbGetEndpointDescriptor (UsbIo, Index, &Endpoint); > >> + if (EFI_ERROR (Status)) { > >> + DEBUG ((DEBUG_ERROR, "UsbGetEndPointDescriptor(%d) failed!\n", > >> Index)); > >> + return Status; > >> + } > >> + > >> + if (IsBulkInEndpoint(&Endpoint)) { > >> + ChaosKey->EndpointAddress = Endpoint.EndpointAddress; > >> + ChaosKey->EndpointSize = Endpoint.MaxPacketSize; > >> + return EFI_SUCCESS; > >> + } > >> + } > >> + > >> + DEBUG ((DEBUG_ERROR, "Failed to locate suitable BULK IN USB > >> endpoint!\n")); > >> + return EFI_DEVICE_ERROR; > >> +} > >> + > >> + > >> +/** > >> + Returns information about the random number generation implementation. > >> + > >> + @param[in] This A pointer to the EFI_RNG_PROTOCOL > >> instance. > >> + @param[in,out] AlgorithmListSize On input, the size in bytes of > >> AlgorithmList > >> + On output with a return code of > >> EFI_SUCCESS, > >> + the size in bytes of the data > >> returned in > >> + AlgorithmList. On output with a return > >> + code of EFI_BUFFER_TOO_SMALL, the > >> size of > >> + AlgorithmList required to obtain the > >> list. > >> + @param[out] AlgorithmList A caller-allocated memory buffer > >> filled by > >> + the driver with one EFI_RNG_ALGORITHM > >> + element for each supported RNG > >> algorithm. > >> + The list must not change across > >> multiple > >> + calls to the same driver. The first > >> + algorithm in the list is the default > >> + algorithm for the driver. > >> + > >> + @retval EFI_SUCCESS The RNG algorithm list was returned > >> + successfully. > >> + @retval EFI_UNSUPPORTED The services is not supported by this > >> driver > >> + @retval EFI_DEVICE_ERROR The list of algorithms could not be > >> + retrieved due to a hardware or > >> firmware > >> + error. > >> + @retval EFI_INVALID_PARAMETER One or more of the parameters are > >> incorrect. > >> + @retval EFI_BUFFER_TOO_SMALL The buffer RNGAlgorithmList is too > >> small to > >> + hold the result. > >> + > >> +**/ > >> +STATIC > >> +EFI_STATUS > >> +EFIAPI > >> +GetInfo ( > >> + IN EFI_RNG_PROTOCOL *This, > >> + IN OUT UINTN *AlgorithmListSize, > >> + OUT EFI_RNG_ALGORITHM *AlgorithmList > >> +) > >> +{ > >> + UINTN Size; > >> + > >> + // > >> + // We only implement the raw algorithm > >> + // > >> + Size = sizeof gEfiRngAlgorithmRaw; > >> + > >> + if (*AlgorithmListSize < Size) { > >> + *AlgorithmListSize = Size; > >> + return EFI_BUFFER_TOO_SMALL; > >> + } > >> + > >> + gBS->CopyMem (AlgorithmList, &gEfiRngAlgorithmRaw, Size); > >> + *AlgorithmListSize = Size; > >> + > >> + return EFI_SUCCESS; > >> +} > >> + > >> + > >> +/** > >> + Produces and returns an RNG value using either the default or specified > >> RNG > >> + algorithm. > >> + > >> + @param[in] This A pointer to the EFI_RNG_PROTOCOL > >> instance. > >> + @param[in] Algorithm A pointer to the EFI_RNG_ALGORITHM that > >> + identifies the RNG algorithm to use. > >> May be > >> + NULL in which case the function will > >> use its > >> + default RNG algorithm. > >> + @param[in] ValueLength The length in bytes of the memory buffer > >> + pointed to by RNGValue. The driver shall > >> + return exactly this numbers of bytes. > >> + @param[out] Value A caller-allocated memory buffer filled > >> by the > >> + driver with the resulting RNG value. > >> + > >> + @retval EFI_SUCCESS The RNG value was returned successfully. > >> + @retval EFI_UNSUPPORTED The algorithm specified by RNGAlgorithm > >> is not > >> + supported by this driver. > >> + @retval EFI_DEVICE_ERROR An RNG value could not be retrieved due > >> to a > >> + hardware or firmware error. > >> + @retval EFI_NOT_READY There is not enough random data > >> available to > >> + satisfy the length requested by > >> + RNGValueLength. > >> + @retval EFI_INVALID_PARAMETER RNGValue is NULL or RNGValueLength is > >> zero. > >> + > >> +**/ > >> +STATIC > >> +EFI_STATUS > >> +EFIAPI > >> +GetRNG ( > >> + IN EFI_RNG_PROTOCOL *This, > >> + IN EFI_RNG_ALGORITHM *Algorithm OPTIONAL, > >> + IN UINTN ValueLength, > >> + OUT UINT8 *Value > >> +) > >> +{ > >> + EFI_STATUS Status; > >> + CHAOSKEY_DEV *ChaosKey; > >> + EFI_TPL Tpl; > >> + UINT8 Buffer[CHAOSKEY_MAX_EP_SIZE]; > >> + UINT8 *OutPointer; > >> + UINTN OutSize; > >> + UINT32 Result; > >> + > >> + if (Algorithm != NULL && !CompareGuid (Algorithm, > >> &gEfiRngAlgorithmRaw)) { > >> + return EFI_UNSUPPORTED; > >> + } > >> + > >> + ChaosKey = CHAOSKEY_DEV_FROM_THIS (This); > >> + > >> + while (ValueLength > 0) { > >> + // > >> + // If more data is requested than the endpoint can deliver in a single > >> + // transfer, put it straight into the caller's buffer. > >> + // > >> + if (ValueLength >= ChaosKey->EndpointSize) { > >> + OutPointer = Value; > >> + } else { > >> + OutPointer = Buffer; > >> + } > >> + OutSize = ChaosKey->EndpointSize; > >> + > >> + Tpl = gBS->RaiseTPL (TPL_NOTIFY); > > > > You mentioned to me on IRC that this should not be necessary, and that > > UsbIo should adjust tpl as necessary. > > > > It is quite likely I misinterpreted behaviour I encountered when using > > USB passthrough in QEMU for testing (and managed to break the host > > system's driver). > > I will remove it. We are dealing with an abstract protocol here, not > hardware, and if completing a single bulk transfer does not tolerate > interruptions, it should be up to the USB I/O protocol implementation > to deal with that. Yes, sounds good. > >> diff --git a/Silicon/Openmoko/ChaosKeyDxe/DriverBinding.c > >> b/Silicon/Openmoko/ChaosKeyDxe/DriverBinding.c > >> new file mode 100644 > >> index 000000000000..a1a5a796d38d > >> --- /dev/null > >> +++ b/Silicon/Openmoko/ChaosKeyDxe/DriverBinding.c > >> @@ -0,0 +1,256 @@ > >> +/** @file > >> + Device driver for the ChaosKey hardware random number generator. > >> + > >> + Copyright (c) 2016 - 2017, Linaro Ltd. All rights reserved.<BR> > >> + > >> + This program and the accompanying materials > >> + are licensed and made available under the terms and conditions of the > >> BSD > >> + License which accompanies this distribution. The full text of the > >> license may > >> + be found at http://opensource.org/licenses/bsd-license.php. > >> + > >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > >> IMPLIED. > >> + > >> +**/ > >> + > >> +#include <Library/UefiDriverEntryPoint.h> > >> + > >> +#include "ChaosKeyDriver.h" > >> + > >> +/** > >> + Tests to see if this driver supports a given controller. > >> + > >> + @param This[in] A pointer to the > >> EFI_DRIVER_BINDING_PROTOCOL > >> + instance. > >> + @param ControllerHandle[in] The handle of the controller to test. > >> + @param RemainingDevicePath[in] The remaining device path. > >> + (Ignored - this is not a bus driver.) > >> + > >> + @retval EFI_SUCCESS The driver supports this controller. > >> + @retval EFI_ALREADY_STARTED The device specified by > >> ControllerHandle is > >> + already being managed by the driver > >> specified > >> + by This. > >> + @retval EFI_UNSUPPORTED The device specified by > >> ControllerHandle is > >> + not supported by the driver specified > >> by This. > >> + > >> +**/ > >> +EFI_STATUS > >> +EFIAPI > >> +UsbHwrngDriverBindingSupported ( > >> + IN EFI_DRIVER_BINDING_PROTOCOL *This, > >> + IN EFI_HANDLE ControllerHandle, > >> + IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath > >> + ) > >> +{ > >> + EFI_USB_DEVICE_DESCRIPTOR Device; > >> + EFI_USB_IO_PROTOCOL *UsbIo; > >> + EFI_STATUS Status; > >> + > >> + // > >> + // Connect to the USB stack > >> + // > >> + Status = gBS->OpenProtocol (ControllerHandle, > >> + &gEfiUsbIoProtocolGuid, > >> + (VOID **) &UsbIo, > >> + This->DriverBindingHandle, > >> + ControllerHandle, > >> + EFI_OPEN_PROTOCOL_BY_DRIVER); > >> + if (EFI_ERROR (Status)) { > >> + return Status; > >> + } > >> + > >> + // > >> + // Get the interface descriptor to check the USB class and find a > >> transport > >> + // protocol handler. > >> + // > >> + Status = UsbIo->UsbGetDeviceDescriptor (UsbIo, &Device); > >> + if (!EFI_ERROR (Status)) { > >> + // > >> + // Validate the adapter > >> + // > >> + if ((Device.IdVendor != CHAOSKEY_VENDOR_ID) || > >> + (Device.IdProduct != CHAOSKEY_PRODUCT_ID)) { > >> + Status = EFI_UNSUPPORTED; > >> + } else { > >> + DEBUG ((DEBUG_INIT | DEBUG_INFO, > >> + "ChaosKey (0x%04x:0x%04x) is my homeboy!\n", > >> + Device.IdVendor, Device.IdProduct)); > > > > That was intended as a cute remark in my original blog post. Perhaps a > > more descriptive message would be better. > > "Detected ChaosKey RNG device." or such. > > > > OK, will change that. Thanks. > >> + Status = EFI_SUCCESS; > >> + } > >> + } > >> + > >> + // > >> + // Clean up. > >> + // > >> + gBS->CloseProtocol (ControllerHandle, > >> + &gEfiUsbIoProtocolGuid, > >> + This->DriverBindingHandle, > >> + ControllerHandle); > >> + > >> + return Status; > >> +} > >> + > >> + > >> +/** > >> + Starts a device controller or a bus controller. > >> + > >> + @param[in] This A pointer to the > >> EFI_DRIVER_BINDING_PROTOCOL > >> + instance. > >> + @param[in] ControllerHandle The handle of the device to start. This > >> + handle must support a protocol > >> interface that > >> + supplies an I/O abstraction to the > >> driver. > >> + @param[in] RemainingDevicePath The remaining portion of the device > >> path. > >> + (Ignored - this is not a bus driver.) > >> + > >> + @retval EFI_SUCCESS The device was started. > >> + @retval EFI_DEVICE_ERROR The device could not be started due to > >> a > >> + device error. > >> + @retval EFI_OUT_OF_RESOURCES The request could not be completed due > >> to a > >> + lack of resources. > >> + > >> +**/ > >> +EFI_STATUS > >> +EFIAPI > >> +UsbHwrngDriverBindingStart ( > >> + IN EFI_DRIVER_BINDING_PROTOCOL *This, > >> + IN EFI_HANDLE ControllerHandle, > >> + IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL > >> + ) > >> +{ > >> + return ChaosKeyInit (This->DriverBindingHandle, ControllerHandle); > >> +} > >> + > >> + > >> +/** > >> + Stops a device controller or a bus controller. > >> + > >> + @param[in] This A pointer to the > >> EFI_DRIVER_BINDING_PROTOCOL > >> + instance. > >> + @param[in] ControllerHandle A handle to the device being stopped. The > >> handle > >> + must support a bus specific I/O protocol > >> for the > >> + driver to use to stop the device. > >> + @param[in] NumberOfChildren The number of child device handles in > >> + ChildHandleBuffer. > >> + @param[in] ChildHandleBuffer An array of child handles to be freed. > >> May be > >> + NULL if NumberOfChildren is 0. > >> + > >> + @retval EFI_SUCCESS The device was stopped. > >> + @retval EFI_DEVICE_ERROR The device could not be stopped due to a > >> device > >> + error. > >> + > >> +**/ > >> +EFI_STATUS > >> +EFIAPI > >> +UsbHwrngDriverBindingStop ( > >> + IN EFI_DRIVER_BINDING_PROTOCOL *This, > >> + IN EFI_HANDLE ControllerHandle, > >> + IN UINTN NumberOfChildren, > >> + IN EFI_HANDLE *ChildHandleBuffer OPTIONAL > >> + ) > >> +{ > >> + return ChaosKeyRelease (This->DriverBindingHandle, ControllerHandle); > >> +} > >> + > >> + > >> +STATIC > >> +EFI_DRIVER_BINDING_PROTOCOL gUsbDriverBinding = { > >> + UsbHwrngDriverBindingSupported, > >> + UsbHwrngDriverBindingStart, > >> + UsbHwrngDriverBindingStop, > >> + 0xa, > >> + NULL, > >> + NULL > >> +}; > >> + > >> + > >> +/** > >> + The entry point of ChaosKey UEFI Driver. > >> + > >> + @param ImageHandle The image handle of the UEFI Driver. > >> + @param SystemTable A pointer to the EFI System Table. > >> + > >> + @retval EFI_SUCCESS The Driver or UEFI Driver exited > >> normally. > >> + @retval EFI_INCOMPATIBLE_VERSION _gUefiDriverRevision is greater than > >> + SystemTable->Hdr.Revision. > >> + > >> +**/ > >> +EFI_STATUS > >> +EFIAPI > >> +EntryPoint ( > >> + IN EFI_HANDLE ImageHandle, > >> + IN EFI_SYSTEM_TABLE *SystemTable > >> + ) > >> +{ > >> + EFI_STATUS Status; > >> + > >> + // > >> + // Add the driver to the list of drivers > >> + // > >> + Status = EfiLibInstallDriverBindingComponentName2 ( > >> + ImageHandle, SystemTable, &gUsbDriverBinding, ImageHandle, > >> + &gChaosKeyDriverComponentName, > >> &gChaosKeyDriverComponentName2); > > > > Random question ... do we _want_ to provide > > EFI_COMPONENT_NAME_PROTOCOL, in addition to > > EFI_COMPONENT_NAME2_PROTOCOL, for new drivers? > > > > If this is supposed to be a UEFI Driver Model driver, I'd suggest we > keep it for compatibility with older firmwares. I am not strongly opposed to it, but I think the following statement from UEFI 2.7 suggests that it may be worth starting to consider leaving it out of new drivers: "To provide backwards compatibility with preexisting EFI 1.10 drivers, a UEFI platforms may support deprecated protocols which represent languages in the ISO 639-2 format. This includes the following protocols: UNICODE_COLLATION_INTERFACE, EFI_DRIVER_CONFIGURATION_PROTOCOL, EFI_DRIVER_DIAGNOSTICS_PROTOCOL, and EFI_COMPONENT_NAME_PROTOCOL." The protocol is mentioned in only one other location in the specification. > > I should also point out that I don't actually see a DRIVER NAME when > > executing the "drivers" command from UEFI Shell. > > That worked fine for me, both for 'drivers' and 'devices'. Which > platform? Does it display other driver names correctly? *cough* No. Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc: gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable|TRUE Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc: gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable|TRUE And this has been cargo-culted across to other platforms. Or, well, the FAT driver does show up (how!?). Ah, FatBinPkg. Ok, the world makes sense again, I will delete these illogical overrides on all ARM platforms in edk2-platforms. And probably from BeagleBoardPkh as well - I guess this is where this originated, together with Ebl. In an effort to reduce image size. Since BeagleBoardPkg now includes UefiShell, I don't think we need to worry about that as much. It is time for me to start deduplicating the common bits of platform configuration, originally prototyped by Ryan a couple of years back, in order to avoid this kind of spread in future. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
