|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 18/18] OvmfPkg/XenPvBlkDxe: Add BlockIo.
On Wed, Jul 16, 2014 at 04:15:47PM +0100, Anthony PERARD wrote:
> Install the BlockIo protocol.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
> OvmfPkg/XenPvBlkDxe/BlockIo.c | 256
> ++++++++++++++++++++++++++++++++++++
> OvmfPkg/XenPvBlkDxe/BlockIo.h | 109 +++++++++++++++
> OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c | 59 +++++++++
> OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h | 1 +
> OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf | 2 +
> 5 files changed, 427 insertions(+)
> create mode 100644 OvmfPkg/XenPvBlkDxe/BlockIo.c
> create mode 100644 OvmfPkg/XenPvBlkDxe/BlockIo.h
>
> diff --git a/OvmfPkg/XenPvBlkDxe/BlockIo.c b/OvmfPkg/XenPvBlkDxe/BlockIo.c
> new file mode 100644
> index 0000000..4ac2411
> --- /dev/null
> +++ b/OvmfPkg/XenPvBlkDxe/BlockIo.c
> @@ -0,0 +1,256 @@
> +
> +/** @file
> + TODO: Brief Description of UEFI Driver XenPvBlkDxe
> +
> + TODO: Detailed Description of UEFI Driver XenPvBlkDxe
> +
> + TODO: Copyright for UEFI Driver XenPvBlkDxe
> +
> + TODO: License for UEFI Driver XenPvBlkDxe
> +
> +**/
> +
> +#include "XenPvBlkDxe.h"
> +
> +#include "BlockFront.h"
> +
> +///
> +/// Block I/O Media structure
> +///
> +GLOBAL_REMOVE_IF_UNREFERENCED
> +EFI_BLOCK_IO_MEDIA gXenPvBlkDxeBlockIoMedia = {
> + 0, // MediaId
> + FALSE, // RemovableMedia
> + FALSE, // MediaPresent
> + FALSE, // LogicalPartition
> + TRUE, // ReadOnly
> + FALSE, // WriteCaching
> + 512, // BlockSize
> + 512, // IoAlign
> + 0, // LastBlock
> + 0, // LowestAlignedLba
> + 0, // LogicalBlocksPerPhysicalBlock
> + 0 // OptimalTransferLengthGranularity
> +};
> +
> +///
> +/// Block I/O Protocol instance
> +///
> +GLOBAL_REMOVE_IF_UNREFERENCED
> +EFI_BLOCK_IO_PROTOCOL gXenPvBlkDxeBlockIo = {
> + EFI_BLOCK_IO_PROTOCOL_REVISION3, // Revision
> + &gXenPvBlkDxeBlockIoMedia, // Media
> + (EFI_BLOCK_RESET)XenPvBlkDxeBlockIoReset, // Reset
> + XenPvBlkDxeBlockIoReadBlocks, // ReadBlocks
> + XenPvBlkDxeBlockIoWriteBlocks, // WriteBlocks
> + XenPvBlkDxeBlockIoFlushBlocks // FlushBlocks
> +};
> +
> +
> +
> +
> +STATIC
> +EFI_STATUS
> +XenPvBlkDxeBlockIoReadWriteBlocks (
> + IN EFI_BLOCK_IO_PROTOCOL *This,
> + IN UINT32 MediaId,
> + IN EFI_LBA Lba,
> + IN UINTN BufferSize,
> + IN OUT VOID *Buffer,
> + IN BOOLEAN IsWrite
> + )
> +{
> + XEN_BLOCK_FRONT_IO IoData;
> + EFI_BLOCK_IO_MEDIA *Media;
> + UINTN NextOffset;
> +
> + if (Buffer == NULL) {
> + DEBUG ((EFI_D_ERROR, "%a %d, buffer null\n", __func__, __LINE__));
Heheh.
> + return EFI_INVALID_PARAMETER;
> + }
> + if (BufferSize == 0) {
> + return EFI_SUCCESS;
> + }
> +
> + Media = This->Media;
> +
> + if (BufferSize % Media->BlockSize != 0) {
> + DEBUG ((EFI_D_ERROR, "%a %d, bad buffer size\n", __func__, __LINE__));
> + return EFI_BAD_BUFFER_SIZE;
> + }
> +
> + if (Lba > Media->LastBlock ||
> + (BufferSize / Media->BlockSize) - 1 > Media->LastBlock - Lba) {
> + DEBUG ((EFI_D_ERROR, "%a %d, read too large\n", __func__, __LINE__));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (IsWrite && Media->ReadOnly) {
> + DEBUG ((EFI_D_ERROR, "%a %d, read only\n", __func__, __LINE__));
> + return EFI_WRITE_PROTECTED;
> + }
> +
> + if ((Media->IoAlign > 1) && (UINTN)Buffer & (Media->IoAlign - 1)) {
> + // This happen often when using grub2
> + VOID *NewBuffer;
> + EFI_STATUS Status;
> +
> + /* Try again with a properly aligned buffer. */
> + NewBuffer = AllocateAlignedPages((BufferSize + EFI_PAGE_SIZE) /
> EFI_PAGE_SIZE,
> + Media->IoAlign);
> + if (!IsWrite) {
> + Status = XenPvBlkDxeBlockIoReadBlocks (This, MediaId,
> + Lba, BufferSize, NewBuffer);
> + CopyMem (Buffer, NewBuffer, BufferSize);
> + } else {
> + CopyMem (NewBuffer, Buffer, BufferSize);
> + Status = XenPvBlkDxeBlockIoWriteBlocks (This, MediaId,
> + Lba, BufferSize, NewBuffer);
> + }
> + FreeAlignedPages (NewBuffer, (BufferSize + EFI_PAGE_SIZE) /
> EFI_PAGE_SIZE);
> + return Status;
> + }
> +
> +
> + // XXX Check MediaId
> +
> + IoData.Dev = XEN_BLOCK_FRONT_FROM_BLOCK_IO (This);
> + IoData.Offset = Lba * Media->BlockSize;
> + IoData.Callback = NULL;
> + IoData.Data = NULL;
> + IoData.Size = BufferSize;
> + IoData.Buffer = Buffer;
> +
> + NextOffset = Lba * Media->BlockSize;
Or just = IoData.Offset ?
> + while (BufferSize > 0) {
> + // This should work if every Buffer is page aligned
> + // but block accept sector aligned, so FIXME
Isn't the work-around for GRUB2 above doing this?
> + IoData.Size = MIN(EFI_PAGE_SIZE * 8,//BLKIF_MAX_SEGMENTS_PER_REQUEST,
> + BufferSize);
Why 8 ?
> + IoData.Buffer = Buffer;
> + IoData.Offset = NextOffset;
> + BufferSize -= IoData.Size;
> + Buffer = (VOID*) ((UINTN) Buffer + IoData.Size);
> + NextOffset += IoData.Size;
> + XenPvBlockIo (&IoData, IsWrite);
> + if (!IoData.Data) {
Well that wouldn't be possible as the XenPvBlockIo loops forever until that
is done.
However, you should probably have a comment here:
/* XXX If we move to a different mechanism for CallBack this might
go away */
> + DEBUG ((EFI_D_ERROR, "%a %d, XenPvBlockIo no data...\n", __func__,
> __LINE__));
> + return EFI_DEVICE_ERROR;
> + }
> + }
> + return EFI_SUCCESS;
> +}
> +
> +
> +/**
> + Read BufferSize bytes from Lba into Buffer.
> +
> + @param This Indicates a pointer to the calling context.
> + @param MediaId Id of the media, changes every time the media is
> replaced.
> + @param Lba The starting Logical Block Address to read from
> + @param BufferSize Size of Buffer, must be a multiple of device block size.
> + @param Buffer A pointer to the destination buffer for the data. The
> caller is
> + responsible for either having implicit or explicit
> ownership of the buffer.
> +
> + @retval EFI_SUCCESS The data was read correctly from the device.
> + @retval EFI_DEVICE_ERROR The device reported an error while
> performing the read.
> + @retval EFI_NO_MEDIA There is no media in the device.
> + @retval EFI_MEDIA_CHANGED The MediaId does not matched the current
> device.
> + @retval EFI_BAD_BUFFER_SIZE The Buffer was not a multiple of the block
> size of the device.
> + @retval EFI_INVALID_PARAMETER The read request contains LBAs that are not
> valid,
> + or the buffer is not on proper alignment.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +XenPvBlkDxeBlockIoReadBlocks (
> + IN EFI_BLOCK_IO_PROTOCOL *This,
> + IN UINT32 MediaId,
> + IN EFI_LBA Lba,
> + IN UINTN BufferSize,
> + OUT VOID *Buffer
> + )
> +{
> + return XenPvBlkDxeBlockIoReadWriteBlocks (This,
> + MediaId, Lba, BufferSize, Buffer, FALSE);
> +}
> +
> +/**
> + Write BufferSize bytes from Lba into Buffer.
> +
> + @param This Indicates a pointer to the calling context.
> + @param MediaId The media ID that the write request is for.
> + @param Lba The starting logical block address to be written. The
> caller is
> + responsible for writing to only legitimate locations.
> + @param BufferSize Size of Buffer, must be a multiple of device block size.
> + @param Buffer A pointer to the source buffer for the data.
> +
> + @retval EFI_SUCCESS The data was written correctly to the device.
> + @retval EFI_WRITE_PROTECTED The device can not be written to.
> + @retval EFI_DEVICE_ERROR The device reported an error while
> performing the write.
> + @retval EFI_NO_MEDIA There is no media in the device.
> + @retval EFI_MEDIA_CHNAGED The MediaId does not matched the current
> device.
> + @retval EFI_BAD_BUFFER_SIZE The Buffer was not a multiple of the block
> size of the device.
> + @retval EFI_INVALID_PARAMETER The write request contains LBAs that are not
> valid,
> + or the buffer is not on proper alignment.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +XenPvBlkDxeBlockIoWriteBlocks (
> + IN EFI_BLOCK_IO_PROTOCOL *This,
> + IN UINT32 MediaId,
> + IN EFI_LBA Lba,
> + IN UINTN BufferSize,
> + IN VOID *Buffer
> + )
> +{
> + return XenPvBlkDxeBlockIoReadWriteBlocks (This,
> + MediaId, Lba, BufferSize, Buffer, TRUE);
> +}
> +
> +/**
> + Flush the Block Device.
> +
> + @param This Indicates a pointer to the calling context.
> +
> + @retval EFI_SUCCESS All outstanding data was written to the device
> + @retval EFI_DEVICE_ERROR The device reported an error while writting back
> the data
> + @retval EFI_NO_MEDIA There is no media in the device.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +XenPvBlkDxeBlockIoFlushBlocks (
> + IN EFI_BLOCK_IO_PROTOCOL *This
> + )
> +{
> + XenPvBlockSync (XEN_BLOCK_FRONT_FROM_BLOCK_IO (This));
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Reset the block device hardware.
> +
> + @param[in] This Indicates a pointer to the calling
> context.
> + @param[in] ExtendedVerification Indicates that the driver may perform a
> more
> + exhausive verfication operation of the
> device
> + during reset.
> +
> + @retval EFI_SUCCESS The device was reset.
> + @retval EFI_DEVICE_ERROR The device is not functioning properly and
> could
> + not be reset.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +XenPvBlkDxeBlockIoReset (
> + IN EFI_BLOCK_IO2_PROTOCOL *This,
> + IN BOOLEAN ExtendedVerification
> + )
> +{
> + // Is there anything to do ?
> + // since initialisation is done in Start (driver binding)
Won't we leak memory? Or will it after an Reset do Stop, then Start?
> + DEBUG ((EFI_D_INFO, "%a %d\n", __func__, __LINE__));
> + return EFI_SUCCESS;
> +}
> diff --git a/OvmfPkg/XenPvBlkDxe/BlockIo.h b/OvmfPkg/XenPvBlkDxe/BlockIo.h
> new file mode 100644
> index 0000000..ec0fe2b
> --- /dev/null
> +++ b/OvmfPkg/XenPvBlkDxe/BlockIo.h
> @@ -0,0 +1,109 @@
> +
> +/** @file
> + TODO: Brief Description of UEFI Driver XenPvBlkDxe
> +
> + TODO: Detailed Description of UEFI Driver XenPvBlkDxe
> +
> + TODO: Copyright for UEFI Driver XenPvBlkDxe
> +
> + TODO: License for UEFI Driver XenPvBlkDxe
> +
> +**/
> +
> +/**
> + Read BufferSize bytes from Lba into Buffer.
> +
> + @param This Indicates a pointer to the calling context.
> + @param MediaId Id of the media, changes every time the media is
> replaced.
> + @param Lba The starting Logical Block Address to read from
> + @param BufferSize Size of Buffer, must be a multiple of device block size.
> + @param Buffer A pointer to the destination buffer for the data. The
> caller is
> + responsible for either having implicit or explicit
> ownership of the buffer.
> +
> + @retval EFI_SUCCESS The data was read correctly from the device.
> + @retval EFI_DEVICE_ERROR The device reported an error while
> performing the read.
> + @retval EFI_NO_MEDIA There is no media in the device.
> + @retval EFI_MEDIA_CHANGED The MediaId does not matched the current
> device.
> + @retval EFI_BAD_BUFFER_SIZE The Buffer was not a multiple of the block
> size of the device.
> + @retval EFI_INVALID_PARAMETER The read request contains LBAs that are not
> valid,
> + or the buffer is not on proper alignment.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +XenPvBlkDxeBlockIoReadBlocks (
> + IN EFI_BLOCK_IO_PROTOCOL *This,
> + IN UINT32 MediaId,
> + IN EFI_LBA Lba,
> + IN UINTN BufferSize,
> + OUT VOID *Buffer
> + );
> +
> +/**
> + Write BufferSize bytes from Lba into Buffer.
> +
> + @param This Indicates a pointer to the calling context.
> + @param MediaId The media ID that the write request is for.
> + @param Lba The starting logical block address to be written. The
> caller is
> + responsible for writing to only legitimate locations.
> + @param BufferSize Size of Buffer, must be a multiple of device block size.
> + @param Buffer A pointer to the source buffer for the data.
> +
> + @retval EFI_SUCCESS The data was written correctly to the device.
> + @retval EFI_WRITE_PROTECTED The device can not be written to.
> + @retval EFI_DEVICE_ERROR The device reported an error while
> performing the write.
> + @retval EFI_NO_MEDIA There is no media in the device.
> + @retval EFI_MEDIA_CHNAGED The MediaId does not matched the current
> device.
> + @retval EFI_BAD_BUFFER_SIZE The Buffer was not a multiple of the block
> size of the device.
> + @retval EFI_INVALID_PARAMETER The write request contains LBAs that are not
> valid,
> + or the buffer is not on proper alignment.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +XenPvBlkDxeBlockIoWriteBlocks (
> + IN EFI_BLOCK_IO_PROTOCOL *This,
> + IN UINT32 MediaId,
> + IN EFI_LBA Lba,
> + IN UINTN BufferSize,
> + IN VOID *Buffer
> + );
> +
> +/**
> + Flush the Block Device.
> +
> + @param This Indicates a pointer to the calling context.
> +
> + @retval EFI_SUCCESS All outstanding data was written to the device
> + @retval EFI_DEVICE_ERROR The device reported an error while writting back
> the data
> + @retval EFI_NO_MEDIA There is no media in the device.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +XenPvBlkDxeBlockIoFlushBlocks (
> + IN EFI_BLOCK_IO_PROTOCOL *This
> + );
> +
> +/**
> + Reset the block device hardware.
> +
> + @param[in] This Indicates a pointer to the calling
> context.
> + @param[in] ExtendedVerification Indicates that the driver may perform a
> more
> + exhausive verfication operation of the
> device
> + during reset.
> +
> + @retval EFI_SUCCESS The device was reset.
> + @retval EFI_DEVICE_ERROR The device is not functioning properly and
> could
> + not be reset.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +XenPvBlkDxeBlockIoReset (
> + IN EFI_BLOCK_IO2_PROTOCOL *This,
> + IN BOOLEAN ExtendedVerification
> + );
> +
> +extern EFI_BLOCK_IO_MEDIA gXenPvBlkDxeBlockIoMedia;
> +extern EFI_BLOCK_IO_PROTOCOL gXenPvBlkDxeBlockIo;
> diff --git a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c
> b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c
> index 7ec7d60..3991e83 100644
> --- a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c
> +++ b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c
> @@ -266,6 +266,7 @@ XenPvBlkDxeDriverBindingStart (
> EFI_STATUS Status;
> XENBUS_PROTOCOL *XenbusIo;
> XEN_BLOCK_FRONT_DEVICE *Dev;
> + EFI_BLOCK_IO_MEDIA *Media;
>
> Status = gBS->OpenProtocol (
> ControllerHandle,
> @@ -284,6 +285,37 @@ XenPvBlkDxeDriverBindingStart (
> return Status;
> }
>
> + CopyMem (&Dev->BlockIo, &gXenPvBlkDxeBlockIo, sizeof
> (EFI_BLOCK_IO_PROTOCOL));
> + Media = AllocateCopyPool (sizeof (EFI_BLOCK_IO_MEDIA),
> + &gXenPvBlkDxeBlockIoMedia);
> + if (Dev->MediaInfo.VDiskInfo & VDISK_REMOVABLE) {
> + Media->RemovableMedia = TRUE;
> + }
> + Media->MediaPresent = TRUE;
> + Media->ReadOnly = !Dev->MediaInfo.ReadWrite;
> + if (Dev->MediaInfo.CdRom) {
> + // If it's a cdrom, the blocksize value need to be 2048 for OVMF to
> + // recognize it as a cdrom:
> + // MdeModulePkg/Universal/Disk/PartitionDxe/ElTorito.c
> + Media->BlockSize = 2048;
> + } else {
> + Media->BlockSize = Dev->MediaInfo.SectorSize;
> + }
> + Media->LastBlock = Dev->MediaInfo.Sectors - 1;
> + // XXX What about LowestAlignedLba, LogicalBlocksPerPhysicalBlock and
> OptimalTransferLengthGranularity?
I think you can skip LogicalBlocksPerPhysicalBlock and
OptimalTransferLengthGranularity
as they are used for discard operations.
The LowestAlignedLba .. no idea.
> + Dev->BlockIo.Media = Media;
> +
> + Status = gBS->InstallMultipleProtocolInterfaces (
> + &ControllerHandle,
> + &gEfiBlockIoProtocolGuid, &Dev->BlockIo,
> + NULL
> + );
> + if (EFI_ERROR (Status)) {
> + // XXX: uninit everythings
XenPvBlockFrontShutdown (Dev); ?
FreePool(Media) ?
> + DEBUG ((EFI_D_ERROR, "XenPvBlk: install protocol fail: %r\n", Status));
> + return Status;
> + }
> +
> return EFI_SUCCESS;
> }
>
> @@ -322,6 +354,33 @@ XenPvBlkDxeDriverBindingStop (
> IN EFI_HANDLE *ChildHandleBuffer OPTIONAL
> )
> {
> + EFI_BLOCK_IO_PROTOCOL *BlockIo;
> + XEN_BLOCK_FRONT_DEVICE *Dev;
> + EFI_BLOCK_IO_MEDIA *Media;
> + EFI_STATUS Status;
> +
> + Status = gBS->OpenProtocol (
> + ControllerHandle, &gEfiBlockIoProtocolGuid,
> + (VOID **)&BlockIo,
> + This->DriverBindingHandle, ControllerHandle,
> + EFI_OPEN_PROTOCOL_GET_PROTOCOL
> + );
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + Status = gBS->UninstallProtocolInterface (ControllerHandle,
> + &gEfiBlockIoProtocolGuid, BlockIo);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + Media = BlockIo->Media;
> + Dev = XEN_BLOCK_FRONT_FROM_BLOCK_IO (BlockIo);
> + XenPvBlockFrontShutdown (Dev);
> +
> + FreePool (Media);
> +
> gBS->CloseProtocol (ControllerHandle, &gXenbusProtocolGuid,
> This->DriverBindingHandle, ControllerHandle);
>
> diff --git a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h
> b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h
> index 186d0ee..3120137 100644
> --- a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h
> +++ b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h
> @@ -76,6 +76,7 @@ extern EFI_COMPONENT_NAME_PROTOCOL
> gXenPvBlkDxeComponentName;
> //
> #include "DriverBinding.h"
> #include "ComponentName.h"
> +#include "BlockIo.h"
>
>
>
> diff --git a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> index a43d006..b379020 100644
> --- a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> +++ b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> @@ -32,6 +32,8 @@
> ComponentName.h
> BlockFront.c
> BlockFront.h
> + BlockIo.c
> + BlockIo.h
>
>
> [LibraryClasses]
> --
> Anthony PERARD
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |