[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH RFC 00/18] Introducing Xen PV block driver to OVMF



On Wed, Jul 16, 2014 at 04:15:29PM +0100, Anthony PERARD wrote:
> Hi all,
> 
> This patch series is implementing the necessary in order to access a PV block
> device. For that, one need a XenStore client, a XenBus client, and the PV 
> block
> driver.
> 
> There are two new drivers, XenbusDxe and XenPvBlkDxe. The first one implement 
> a
> bus drivers, and the second is a block drivers.
> 
> There are still a bit of work to be done on this series, especially the 
> comment
> in the code, but I'd like your comment on this patch series.

Went through it - I had some questions and spotted some issues that are
pretty easy to fix.

Otherwise I think you just need to flesh it out with more comments, links
to the specs or just copy the relevant parts.

And little puzzled by the usage of 8 pages instead of doing it via 11.
But that is probably not a big deal since you are doing each I/O request
synchronously anyhow.

Thank you for posting this and developing it!
> 
> Anthony PERARD (18):
>   OvmfPkg: Add public headers from Xen Project.
>   OvmfPkg: Add basic skeleton for the Xenbus driver.
>   OvmfPkg/XenbusDxe: Add device state struct and create an ExitBoot
>     services event.
>   OvmfPkg/XenbusDxe: Add support to make Xen Hypercalls.
>   OvmfPkg/XenbusDxe: Open PciIo protocol.
>   OvmfPkg: Introduce Xenbus Protocol.
>   OvmfPkg/XenbusDxe: Add InterlockedCompareExchange16.
>   OvmfPkg/XenbusDxe: Add Grant Table functions.
>   OvmfPkg/XenbusDxe: Add Event Channel Notify.
>   OvmfPkg/XenbusDxe: Add TestAndClearBit.
>   OvmfPkg/XenbusDxe: Add XenStore client implementation
>   OvmfPkg/XenbusDxe: Add an helper AsciiStrDup.
>   OvmfPkg/XenbusDxe: Add Xenstore function into the Xenbus protocol
>   OvmfPkg/XenbusDxe: Indroduce XenBus support itself.
>   OvmfPkg/XenbusDxe: Add Event Channel into XenBus protocol.
>   OvmfPkg/XenPvBlkDxe: Xen PV Block device, initial skeleton
>   OvmfPkg/XenPvBlkDxe: Add BlockFront client.
>   OvmfPkg/XenPvBlkDxe: Add BlockIo.
> 
>  .../IndustryStandard/Xen/arch-x86/xen-x86_32.h     |  171 +++
>  .../IndustryStandard/Xen/arch-x86/xen-x86_64.h     |  202 +++
>  .../Include/IndustryStandard/Xen/arch-x86/xen.h    |  273 ++++
>  .../Include/IndustryStandard/Xen/event_channel.h   |  381 +++++
>  OvmfPkg/Include/IndustryStandard/Xen/grant_table.h |  662 +++++++++
>  OvmfPkg/Include/IndustryStandard/Xen/hvm/hvm_op.h  |  275 ++++
>  OvmfPkg/Include/IndustryStandard/Xen/hvm/params.h  |  150 ++
>  OvmfPkg/Include/IndustryStandard/Xen/io/blkif.h    |  608 ++++++++
>  .../Include/IndustryStandard/Xen/io/protocols.h    |   40 +
>  OvmfPkg/Include/IndustryStandard/Xen/io/ring.h     |  312 +++++
>  OvmfPkg/Include/IndustryStandard/Xen/io/xenbus.h   |   80 ++
>  OvmfPkg/Include/IndustryStandard/Xen/io/xs_wire.h  |  138 ++
>  OvmfPkg/Include/IndustryStandard/Xen/memory.h      |  480 +++++++
>  OvmfPkg/Include/IndustryStandard/Xen/sched.h       |  174 +++
>  OvmfPkg/Include/IndustryStandard/Xen/trace.h       |  310 +++++
>  OvmfPkg/Include/IndustryStandard/Xen/xen-compat.h  |   44 +
>  OvmfPkg/Include/IndustryStandard/Xen/xen.h         |  897 ++++++++++++
>  OvmfPkg/Include/Protocol/Xenbus.h                  |  231 +++
>  OvmfPkg/OvmfPkg.dec                                |    2 +
>  OvmfPkg/OvmfPkgX64.dsc                             |    2 +
>  OvmfPkg/OvmfPkgX64.fdf                             |    2 +
>  OvmfPkg/XenPvBlkDxe/BlockFront.c                   |  583 ++++++++
>  OvmfPkg/XenPvBlkDxe/BlockFront.h                   |   88 ++
>  OvmfPkg/XenPvBlkDxe/BlockIo.c                      |  256 ++++
>  OvmfPkg/XenPvBlkDxe/BlockIo.h                      |  109 ++
>  OvmfPkg/XenPvBlkDxe/ComponentName.c                |  160 +++
>  OvmfPkg/XenPvBlkDxe/ComponentName.h                |   91 ++
>  OvmfPkg/XenPvBlkDxe/DriverBinding.h                |  139 ++
>  OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c                  |  388 ++++++
>  OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h                  |   83 ++
>  OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf                |   61 +
>  OvmfPkg/XenbusDxe/ComponentName.c                  |  160 +++
>  OvmfPkg/XenbusDxe/ComponentName.h                  |   91 ++
>  OvmfPkg/XenbusDxe/DriverBinding.h                  |  139 ++
>  OvmfPkg/XenbusDxe/EventChannel.c                   |   71 +
>  OvmfPkg/XenbusDxe/EventChannel.h                   |   40 +
>  OvmfPkg/XenbusDxe/GrantTable.c                     |  204 +++
>  OvmfPkg/XenbusDxe/GrantTable.h                     |   34 +
>  OvmfPkg/XenbusDxe/Helpers.c                        |    9 +
>  OvmfPkg/XenbusDxe/InterlockedCompareExchange16.h   |    7 +
>  .../XenbusDxe/X64/InterlockedCompareExchange16.c   |   41 +
>  OvmfPkg/XenbusDxe/X64/TestAndClearBit.S            |   10 +
>  OvmfPkg/XenbusDxe/X64/hypercall.S                  |   16 +
>  OvmfPkg/XenbusDxe/X64/hypercall.asm                |   19 +
>  OvmfPkg/XenbusDxe/XenBus.c                         |  363 +++++
>  OvmfPkg/XenbusDxe/XenBus.h                         |   76 +
>  OvmfPkg/XenbusDxe/XenHypercall.c                   |  101 ++
>  OvmfPkg/XenbusDxe/XenHypercall.h                   |   44 +
>  OvmfPkg/XenbusDxe/XenbusDxe.c                      |  452 ++++++
>  OvmfPkg/XenbusDxe/XenbusDxe.h                      |  131 ++
>  OvmfPkg/XenbusDxe/XenbusDxe.inf                    |   75 +
>  OvmfPkg/XenbusDxe/Xenstore.c                       | 1472 
> ++++++++++++++++++++
>  OvmfPkg/XenbusDxe/Xenstore.h                       |  367 +++++
>  53 files changed, 11314 insertions(+)
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/arch-x86/xen-x86_32.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/arch-x86/xen-x86_64.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/arch-x86/xen.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/event_channel.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/grant_table.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/hvm/hvm_op.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/hvm/params.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/io/blkif.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/io/protocols.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/io/ring.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/io/xenbus.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/io/xs_wire.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/memory.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/sched.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/trace.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/xen-compat.h
>  create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/xen.h
>  create mode 100644 OvmfPkg/Include/Protocol/Xenbus.h
>  create mode 100644 OvmfPkg/XenPvBlkDxe/BlockFront.c
>  create mode 100644 OvmfPkg/XenPvBlkDxe/BlockFront.h
>  create mode 100644 OvmfPkg/XenPvBlkDxe/BlockIo.c
>  create mode 100644 OvmfPkg/XenPvBlkDxe/BlockIo.h
>  create mode 100644 OvmfPkg/XenPvBlkDxe/ComponentName.c
>  create mode 100644 OvmfPkg/XenPvBlkDxe/ComponentName.h
>  create mode 100644 OvmfPkg/XenPvBlkDxe/DriverBinding.h
>  create mode 100644 OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c
>  create mode 100644 OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h
>  create mode 100644 OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
>  create mode 100644 OvmfPkg/XenbusDxe/ComponentName.c
>  create mode 100644 OvmfPkg/XenbusDxe/ComponentName.h
>  create mode 100644 OvmfPkg/XenbusDxe/DriverBinding.h
>  create mode 100644 OvmfPkg/XenbusDxe/EventChannel.c
>  create mode 100644 OvmfPkg/XenbusDxe/EventChannel.h
>  create mode 100644 OvmfPkg/XenbusDxe/GrantTable.c
>  create mode 100644 OvmfPkg/XenbusDxe/GrantTable.h
>  create mode 100644 OvmfPkg/XenbusDxe/Helpers.c
>  create mode 100644 OvmfPkg/XenbusDxe/InterlockedCompareExchange16.h
>  create mode 100644 OvmfPkg/XenbusDxe/X64/InterlockedCompareExchange16.c
>  create mode 100644 OvmfPkg/XenbusDxe/X64/TestAndClearBit.S
>  create mode 100644 OvmfPkg/XenbusDxe/X64/hypercall.S
>  create mode 100644 OvmfPkg/XenbusDxe/X64/hypercall.asm
>  create mode 100644 OvmfPkg/XenbusDxe/XenBus.c
>  create mode 100644 OvmfPkg/XenbusDxe/XenBus.h
>  create mode 100644 OvmfPkg/XenbusDxe/XenHypercall.c
>  create mode 100644 OvmfPkg/XenbusDxe/XenHypercall.h
>  create mode 100644 OvmfPkg/XenbusDxe/XenbusDxe.c
>  create mode 100644 OvmfPkg/XenbusDxe/XenbusDxe.h
>  create mode 100644 OvmfPkg/XenbusDxe/XenbusDxe.inf
>  create mode 100644 OvmfPkg/XenbusDxe/Xenstore.c
>  create mode 100644 OvmfPkg/XenbusDxe/Xenstore.h
> 
> -- 
> 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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.