xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Anthony PERARD <anthony.perard@citrix.com>
Cc: EDK2 devel <edk2-devel@lists.sourceforge.net>,
	Xen Devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH RFC 00/18] Introducing Xen PV block driver to OVMF
Date: Wed, 16 Jul 2014 16:10:49 -0400	[thread overview]
Message-ID: <20140716201049.GA13085@laptop.dumpdata.com> (raw)
In-Reply-To: <1405523747-5024-1-git-send-email-anthony.perard@citrix.com>

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@lists.xen.org
> http://lists.xen.org/xen-devel

  parent reply	other threads:[~2014-07-16 20:10 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1405523747-5024-1-git-send-email-anthony.perard@citrix.com>
2014-07-16 15:15 ` [PATCH RFC 01/18] OvmfPkg: Add public headers from Xen Project Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 02/18] OvmfPkg: Add basic skeleton for the Xenbus driver Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 03/18] OvmfPkg/XenbusDxe: Add device state struct and create an ExitBoot services event Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 04/18] OvmfPkg/XenbusDxe: Add support to make Xen Hypercalls Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 05/18] OvmfPkg/XenbusDxe: Open PciIo protocol Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 06/18] OvmfPkg: Introduce Xenbus Protocol Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 07/18] OvmfPkg/XenbusDxe: Add InterlockedCompareExchange16 Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 08/18] OvmfPkg/XenbusDxe: Add Grant Table functions Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 09/18] OvmfPkg/XenbusDxe: Add Event Channel Notify Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 10/18] OvmfPkg/XenbusDxe: Add TestAndClearBit Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 11/18] OvmfPkg/XenbusDxe: Add XenStore client implementation Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 12/18] OvmfPkg/XenbusDxe: Add an helper AsciiStrDup Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 13/18] OvmfPkg/XenbusDxe: Add Xenstore function into the Xenbus protocol Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 14/18] OvmfPkg/XenbusDxe: Indroduce XenBus support itself Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 15/18] OvmfPkg/XenbusDxe: Add Event Channel into XenBus protocol Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 16/18] OvmfPkg/XenPvBlkDxe: Xen PV Block device, initial skeleton Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 17/18] OvmfPkg/XenPvBlkDxe: Add BlockFront client Anthony PERARD
2014-07-16 15:15 ` [PATCH RFC 18/18] OvmfPkg/XenPvBlkDxe: Add BlockIo Anthony PERARD
     [not found] ` <1405523747-5024-3-git-send-email-anthony.perard@citrix.com>
2014-07-16 17:25   ` [PATCH RFC 02/18] OvmfPkg: Add basic skeleton for the Xenbus driver Konrad Rzeszutek Wilk
2014-07-18 10:30     ` Anthony PERARD
     [not found] ` <1405523747-5024-4-git-send-email-anthony.perard@citrix.com>
2014-07-16 17:32   ` [PATCH RFC 03/18] OvmfPkg/XenbusDxe: Add device state struct and create an ExitBoot services event Konrad Rzeszutek Wilk
2014-07-18 10:32     ` Anthony PERARD
     [not found] ` <1405523747-5024-5-git-send-email-anthony.perard@citrix.com>
2014-07-16 17:37   ` [PATCH RFC 04/18] OvmfPkg/XenbusDxe: Add support to make Xen Hypercalls Konrad Rzeszutek Wilk
2014-07-17  9:43     ` Wei Liu
     [not found] ` <1405523747-5024-6-git-send-email-anthony.perard@citrix.com>
2014-07-16 17:39   ` [PATCH RFC 05/18] OvmfPkg/XenbusDxe: Open PciIo protocol Konrad Rzeszutek Wilk
2014-07-18 10:36     ` Anthony PERARD
     [not found] ` <1405523747-5024-7-git-send-email-anthony.perard@citrix.com>
2014-07-16 17:42   ` [PATCH RFC 06/18] OvmfPkg: Introduce Xenbus Protocol Konrad Rzeszutek Wilk
2014-07-18 10:40     ` Anthony PERARD
     [not found] ` <1405523747-5024-9-git-send-email-anthony.perard@citrix.com>
2014-07-16 17:48   ` [PATCH RFC 08/18] OvmfPkg/XenbusDxe: Add Grant Table functions Konrad Rzeszutek Wilk
2014-07-18 10:53     ` Anthony PERARD
     [not found] ` <1405523747-5024-15-git-send-email-anthony.perard@citrix.com>
2014-07-16 19:04   ` [PATCH RFC 14/18] OvmfPkg/XenbusDxe: Indroduce XenBus support itself Konrad Rzeszutek Wilk
2014-07-18 11:04     ` Anthony PERARD
     [not found] ` <1405523747-5024-18-git-send-email-anthony.perard@citrix.com>
2014-07-16 19:41   ` [PATCH RFC 17/18] OvmfPkg/XenPvBlkDxe: Add BlockFront client Konrad Rzeszutek Wilk
2014-07-18 11:58     ` Anthony PERARD
     [not found]     ` <20140718115817.GG1582@perard.uk.xensource.com>
2014-07-18 18:36       ` Konrad Rzeszutek Wilk
     [not found] ` <1405523747-5024-19-git-send-email-anthony.perard@citrix.com>
2014-07-16 19:57   ` [PATCH RFC 18/18] OvmfPkg/XenPvBlkDxe: Add BlockIo Konrad Rzeszutek Wilk
2014-07-18 12:10     ` Anthony PERARD
2014-07-16 20:10 ` Konrad Rzeszutek Wilk [this message]
2014-07-18 12:12   ` [PATCH RFC 00/18] Introducing Xen PV block driver to OVMF Anthony PERARD
2014-07-16 15:15 Anthony PERARD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140716201049.GA13085@laptop.dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=anthony.perard@citrix.com \
    --cc=edk2-devel@lists.sourceforge.net \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).