Re: [PATCH v2 18/18] OvmfPkg/XenPvBlkDxe: Add BlockIo.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Sep 04, 2014 at 05:51:13PM +0100, Anthony PERARD wrote:
> Install the BlockIo protocol.

Implement. Perhaps point out that the blkif.h is a good literature
to understand it?

> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> ---
> Change in V2:
> - Remove blockIo2 headers.
> - Fix few comment.
> - file header, copyright
> - Rewrite few comment and error messages
> - No more callback
> - Improving block read/write, increase to the max size in one request
>   (instead of only 8pages)
> - Fix lastblock when it's a cdrom
> - Do uninitialisation when fail to install fail
> - few comment
> - Licenses
> ---
>  OvmfPkg/XenPvBlkDxe/BlockIo.c       | 292 ++++++++++++++++++++++++++++++++++++
>  OvmfPkg/XenPvBlkDxe/BlockIo.h       | 124 +++++++++++++++
>  OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c   |  64 ++++++++
>  OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h   |   1 +
>  OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf |   2 +
>  5 files changed, 483 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..f08339b
> --- /dev/null
> +++ b/OvmfPkg/XenPvBlkDxe/BlockIo.c
> @@ -0,0 +1,292 @@
> +/** @file
> +  BlockIo implementation for Xen PV Block driver.
> +
> +  This file is implementing the interface between the actual driver in
> +  BlockFront.c to the BlockIo protocol.
> +
> +  Copyright (C) 2014, Citrix Ltd.
> +
> +  Redistribution and use in source and binary forms, with or without
> +  modification, are permitted provided that the following conditions
> +  are met:
> +
> +  * Redistributions of source code must retain the above copyright
> +    notice, this list of conditions and the following disclaimer.
> +  * Redistributions in binary form must reproduce the above copyright
> +    notice, this list of conditions and the following disclaimer in
> +    the documentation and/or other materials provided with the
> +    distribution.
> +
> +  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> +  FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> +  COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> +  INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
> +  BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> +  LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
> +  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> +  LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
> +  ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> +  POSSIBILITY OF SUCH DAMAGE.
> +
> +**/
> +
> +#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, BlockFront does not support less than 512 bits-aligned.
> +  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
> +  XenPvBlkDxeBlockIoReset,                  // Reset
> +  XenPvBlkDxeBlockIoReadBlocks,             // ReadBlocks
> +  XenPvBlkDxeBlockIoWriteBlocks,            // WriteBlocks
> +  XenPvBlkDxeBlockIoFlushBlocks             // FlushBlocks
> +};
> +
> +
> +
> +
> +/**
> +  Read/Write BufferSize bytes from Lba into Buffer.
> +
> +  This function is commun to XenPvBlkDxeBlockIoReadBlocks and
> +  XenPvBlkDxeBlockIoWriteBlocks.
> +
> +  @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/write to.
> +  @param  BufferSize Size of Buffer, must be a multiple of device block size.
> +  @param  Buffer     A pointer to the destination/source buffer for the data.
> +  @param  IsWrite    Indicate if the operation is write or read.
> +
> +  @return See description of XenPvBlkDxeBlockIoReadBlocks and
> +          XenPvBlkDxeBlockIoWriteBlocks.
> +**/
> +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;
> +  EFI_STATUS Status;
> +
> +  if (Buffer == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  if (BufferSize == 0) {
> +    return EFI_SUCCESS;
> +  }
> +
> +  Media = This->Media;
> +
> +  if (BufferSize % Media->BlockSize != 0) {
> +    DEBUG ((EFI_D_ERROR, "XenPvBlkDxe: Bad buffer size: 0x%X\n", BufferSize));
> +    return EFI_BAD_BUFFER_SIZE;
> +  }
> +
> +  if (Lba > Media->LastBlock ||
> +      (BufferSize / Media->BlockSize) - 1 > Media->LastBlock - Lba) {
> +    DEBUG ((EFI_D_ERROR, "XenPvBlkDxe: %a with invalid LBA: 0x%LX, size: 0x%x\n",
> +            IsWrite ? "Write" : "Read", Lba, BufferSize));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (IsWrite && Media->ReadOnly) {
> +    return EFI_WRITE_PROTECTED;
> +  }
> +
> +  if ((Media->IoAlign > 1) && (UINTN)Buffer & (Media->IoAlign - 1)) {
> +    //
> +    // Grub2 does not appear to respect IoAlign of 512, so reallocate the
> +    // buffer here.
> +    //
> +    VOID *NewBuffer;
> +
> +    //
> +    // 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;
> +  }
> +
> +  IoData.Dev = XEN_BLOCK_FRONT_FROM_BLOCK_IO (This);
> +  NextOffset = Lba * Media->BlockSize;

Perhaps instead of calling it 'Offset' could it be Sector? SAy 'NextSector' ?
[edit: Just ditch this]
> +
> +  while (BufferSize > 0) {
> +    if (((UINTN)Buffer & EFI_PAGE_MASK) == 0) {
> +      IoData.Size = MIN (BLKIF_MAX_SEGMENTS_PER_REQUEST * EFI_PAGE_SIZE,
> +                         BufferSize);
> +    } else {
> +      IoData.Size = MIN ((BLKIF_MAX_SEGMENTS_PER_REQUEST - 1) * EFI_PAGE_SIZE,
> +                         BufferSize);
> +    }
> +
> +    IoData.Buffer = Buffer;
> +    IoData.Offset = NextOffset;

And this would be 'IoData.Lba = NextSector' or perhaps to make obvious:
IoData.Lba = Lba * Media->BlockSize;

Thought why even bother with the multiplication of BlockSize when you are going to
divide it in the XenPvBlockIo again? Perhaps just do

IoData.Lba = Lba;
[edit: Use that]

And modify the 'Lba' value ?

> +    BufferSize -= IoData.Size;
> +    Buffer = (VOID*) ((UINTN) Buffer + IoData.Size);
> +    NextOffset += IoData.Size;

So this would be 'Lba += IoData.Size'

That would make this easier - and you can ditch the NextOffset altogther?
[edit: And Dev.Lba would have the LBA value instead of the odd 'Offset' thing]

> +    Status = XenPvBlockIo (&IoData, IsWrite);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((EFI_D_ERROR, "XenPvBlkDxe: Error durring %a operation.\n",
> +              IsWrite ? "write" : "read"));
> +      return Status;
> +    }
> +  }
> +  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 Not used.
> +
> +  @retval EFI_SUCCESS          The device was reset.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +XenPvBlkDxeBlockIoReset (
> +  IN EFI_BLOCK_IO_PROTOCOL   *This,
> +  IN BOOLEAN                 ExtendedVerification
> +  )
> +{
> +  //
> +  // Since the initialization of the devices is done, then the device is
> +  // working correctly.
> +  //
> +  return EFI_SUCCESS;
> +}
> diff --git a/OvmfPkg/XenPvBlkDxe/BlockIo.h b/OvmfPkg/XenPvBlkDxe/BlockIo.h
> new file mode 100644
> index 0000000..22b6de2
> --- /dev/null
> +++ b/OvmfPkg/XenPvBlkDxe/BlockIo.h
> @@ -0,0 +1,124 @@
> +/** @file
> +  BlockIo function declaration for Xen PV block driver.
> +
> +  Copyright (C) 2014, Citrix Ltd.
> +
> +  Redistribution and use in source and binary forms, with or without
> +  modification, are permitted provided that the following conditions
> +  are met:
> +
> +  * Redistributions of source code must retain the above copyright
> +    notice, this list of conditions and the following disclaimer.
> +  * Redistributions in binary form must reproduce the above copyright
> +    notice, this list of conditions and the following disclaimer in
> +    the documentation and/or other materials provided with the
> +    distribution.
> +
> +  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> +  FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> +  COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> +  INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
> +  BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> +  LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
> +  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> +  LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
> +  ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> +  POSSIBILITY OF SUCH DAMAGE.
> +
> +**/
> +
> +/**
> +  Read BufferSize bytes from Lba into Buffer.

The caller has to ensure that the  XenPvBlkDxeBlockIoFlushBlocks or
Reset are not called when this is executing.
> +
> +  @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.

The caller has to ensure that the  XenPvBlkDxeBlockIoFlushBlocks or
Reset are not called when this is executing.
> +
> +  @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.

The caller has to ensure that there are no other calls to read/write to the
device when this is executued.

> +
> +  @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.

Ditto.

> +
> +  @param[in]  This                 Indicates a pointer to the calling context.
> +  @param[in]  ExtendedVerification Not used.
> +
> +  @retval EFI_SUCCESS          The device was reset.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +XenPvBlkDxeBlockIoReset (
> +  IN EFI_BLOCK_IO_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 930333f..d530600 100644
> --- a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c
> +++ b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c
> @@ -281,6 +281,7 @@ XenPvBlkDxeDriverBindingStart (
>    EFI_STATUS Status;
>    XENBUS_PROTOCOL *XenBusIo;
>    XEN_BLOCK_FRONT_DEVICE *Dev;
> +  EFI_BLOCK_IO_MEDIA *Media;
>  
>    Status = gBS->OpenProtocol (
>                  ControllerHandle,
> @@ -299,8 +300,44 @@ XenPvBlkDxeDriverBindingStart (
>      goto CloseProtocol;
>    }
>  
> +  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;
> +    Media->LastBlock = DivU64x32 (Dev->MediaInfo.Sectors,
> +                                  Media->BlockSize / Dev->MediaInfo.SectorSize) - 1;
> +  } else {
> +    Media->BlockSize = Dev->MediaInfo.SectorSize;
> +    Media->LastBlock = Dev->MediaInfo.Sectors - 1;
> +  }
> +  Dev->BlockIo.Media = Media;
> +
> +  Status = gBS->InstallMultipleProtocolInterfaces (
> +                    &ControllerHandle,
> +                    &gEfiBlockIoProtocolGuid, &Dev->BlockIo,
> +                    NULL
> +                    );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "XenPvBlk: install protocol fail: %r\n", Status));
> +    goto UninitBlockFront;
> +  }
> +
>    return EFI_SUCCESS;
>  
> +UninitBlockFront:
> +  FreePool (Media);
> +  XenPvBlockFrontShutdown (Dev);
>  CloseProtocol:
>    gBS->CloseProtocol (ControllerHandle, &gXenBusProtocolGuid,
>                        This->DriverBindingHandle, ControllerHandle);
> @@ -342,6 +379,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 14c65b2..420e751 100644
> --- a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h
> +++ b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h
> @@ -93,6 +93,7 @@ extern EFI_COMPONENT_NAME_PROTOCOL  gXenPvBlkDxeComponentName;
>  //
>  #include "DriverBinding.h"
>  #include "ComponentName.h"
> +#include "BlockIo.h"
>  
>  
>  #endif
> diff --git a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> index 619ed8f..b7c4a1f 100644
> --- a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> +++ b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> @@ -52,6 +52,8 @@
>    ComponentName.h
>    BlockFront.c
>    BlockFront.h
> +  BlockIo.c
> +  BlockIo.h
>  
>  
>  [LibraryClasses]
> -- 
> Anthony PERARD
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel