Re: [PATCH v2 15/18] OvmfPkg/XenBusDxe: Add Event Channel into XenBus protocol.

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

On Thu, Sep 04, 2014 at 05:51:10PM +0100, Anthony PERARD wrote:
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

While the code is fine, please expand the description here a bit.

You can even just copy and paste the function comments - and
start it with: "Adding three event channel related functions:
a) Allocate (copy-n-paste of the oneline from the function
   comment).
b) Notify..

and so on.

Thank you.
> 
> ---
> Change in V2:
> - coding style
> - adding comment to functions
> - Rename Xenbus to XenBus.
> ---
>  OvmfPkg/Include/Protocol/XenBus.h | 27 +++++++++++++++++++
>  OvmfPkg/XenBusDxe/EventChannel.c  | 55 +++++++++++++++++++++++++++++++++++++++
>  OvmfPkg/XenBusDxe/EventChannel.h  | 48 ++++++++++++++++++++++++++++++++++
>  OvmfPkg/XenBusDxe/XenBus.c        |  3 +++
>  4 files changed, 133 insertions(+)
> 
> diff --git a/OvmfPkg/Include/Protocol/XenBus.h b/OvmfPkg/Include/Protocol/XenBus.h
> index f223be7..f404a81 100644
> --- a/OvmfPkg/Include/Protocol/XenBus.h
> +++ b/OvmfPkg/Include/Protocol/XenBus.h
> @@ -76,6 +76,7 @@ typedef enum {
>  
>  
>  #include <IndustryStandard/Xen/grant_table.h>
> +#include <IndustryStandard/Xen/event_channel.h>
>  
>  ///
>  /// Function prototypes
> @@ -163,6 +164,28 @@ EFI_STATUS
>    );
>  
>  typedef
> +UINT32
> +(EFIAPI *XENBUS_EVENT_CHANNEL_ALLOCATE) (
> +  IN  XENBUS_PROTOCOL *This,
> +  IN  domid_t         DomainId,
> +  OUT evtchn_port_t   *Port
> +  );
> +
> +typedef
> +VOID
> +(EFIAPI *XENBUS_EVENT_CHANNEL_NOTIFY) (
> +  IN XENBUS_PROTOCOL  *This,
> +  IN evtchn_port_t    Port
> +  );
> +
> +typedef
> +UINT32
> +(EFIAPI *XENBUS_EVENT_CHANNEL_CLOSE) (
> +  IN XENBUS_PROTOCOL  *This,
> +  IN evtchn_port_t    Port
> +  );
> +
> +typedef
>  XENSTORE_STATUS
>  (EFIAPI *XENBUS_REGISTER_WATCH) (
>    IN  XENBUS_PROTOCOL *This,
> @@ -209,6 +232,10 @@ struct _XENBUS_PROTOCOL {
>    XENBUS_GRANT_ACCESS           GrantAccess;
>    XENBUS_GRANT_END_ACCESS       GrantEndAccess;
>  
> +  XENBUS_EVENT_CHANNEL_ALLOCATE EventChannelAllocate;
> +  XENBUS_EVENT_CHANNEL_NOTIFY   EventChannelNotify;
> +  XENBUS_EVENT_CHANNEL_CLOSE    EventChannelClose;
> +
>    XENBUS_REGISTER_WATCH         RegisterWatch;
>    XENBUS_REGISTER_WATCH_BACKEND RegisterWatchBackend;
>    XENBUS_UNREGISTER_WATCH       UnregisterWatch;
> diff --git a/OvmfPkg/XenBusDxe/EventChannel.c b/OvmfPkg/XenBusDxe/EventChannel.c
> index f34f9b8..82c4f4b 100644
> --- a/OvmfPkg/XenBusDxe/EventChannel.c
> +++ b/OvmfPkg/XenBusDxe/EventChannel.c
> @@ -47,3 +47,58 @@ XenEventChannelNotify (
>    ReturnCode = XenHypercallEventChannelOp (Dev, EVTCHNOP_send, &Send);
>    ASSERT (ReturnCode == 0);
>  }
> +
> +UINT32
> +EFIAPI
> +XenBusEventChannelAllocate (
> +  IN  XENBUS_PROTOCOL *This,
> +  IN  domid_t         DomainId,
> +  OUT evtchn_port_t   *Port
> +  )
> +{
> +  XENBUS_PRIVATE_DATA *Private;
> +  evtchn_alloc_unbound_t Parameter;
> +  UINT32 ReturnCode;
> +
> +  Private = XENBUS_PRIVATE_DATA_FROM_THIS (This);
> +
> +  Parameter.dom = DOMID_SELF;
> +  Parameter.remote_dom = DomainId;
> +  ReturnCode = XenHypercallEventChannelOp (Private->Dev,
> +                                   EVTCHNOP_alloc_unbound,
> +                                   &Parameter);
> +  if (ReturnCode != 0) {
> +    DEBUG ((EFI_D_ERROR, "ERROR: alloc_unbound failed with rc=%d", ReturnCode));
> +    return ReturnCode;
> +  }
> +  *Port = Parameter.port;
> +  return ReturnCode;
> +}
> +
> +VOID
> +EFIAPI
> +XenBusEventChannelNotify (
> +  IN XENBUS_PROTOCOL *This,
> +  IN evtchn_port_t   Port
> +  )
> +{
> +  XENBUS_PRIVATE_DATA *Private;
> +
> +  Private = XENBUS_PRIVATE_DATA_FROM_THIS(This);
> +  XenEventChannelNotify (Private->Dev, Port);
> +}
> +
> +UINT32
> +EFIAPI
> +XenBusEventChannelClose (
> +  IN XENBUS_PROTOCOL *This,
> +  IN evtchn_port_t   Port
> +  )
> +{
> +  XENBUS_PRIVATE_DATA *Private;
> +  evtchn_close_t Close;
> +
> +  Private = XENBUS_PRIVATE_DATA_FROM_THIS (This);
> +  Close.port = Port;
> +  return XenHypercallEventChannelOp (Private->Dev, EVTCHNOP_close, &Close);
> +}
> diff --git a/OvmfPkg/XenBusDxe/EventChannel.h b/OvmfPkg/XenBusDxe/EventChannel.h
> index fb2845b..2cc8266 100644
> --- a/OvmfPkg/XenBusDxe/EventChannel.h
> +++ b/OvmfPkg/XenBusDxe/EventChannel.h
> @@ -47,4 +47,52 @@ XenEventChannelNotify (
>    IN evtchn_port_t Port
>    );
>  
> +/*
> + * XenBus protocol
> + */
> +
> +/**
> +  Allocate a port that can be bind from domain DomainId.
> +
> +  @param This       A pointer to the XENBUS_PROTOCOL.
> +  @param DomainId   The domain ID that can bind the newly allocated port.
> +  @param Port       A pointer to a evtchn_port_t that will contain the newly
> +                    allocated port.
> +
> +  @retval UINT32    The return value from the hypercall, 0 if success.
> +**/
> +UINT32
> +EFIAPI
> +XenBusEventChannelAllocate (
> +  IN  XENBUS_PROTOCOL *This,
> +  IN  domid_t         DomainId,
> +  OUT evtchn_port_t   *Port
> +  );
> +
> +/**
> +  Send an event to the remote end of the channel whose local endpoint is Port.
> +
> +  @param This       A pointer to the XENBUS_PROTOCOL.
> +  @param Port       Local port to the the event from.
> +**/
> +VOID
> +EFIAPI
> +XenBusEventChannelNotify (
> +  IN XENBUS_PROTOCOL *This,
> +  IN evtchn_port_t   Port
> +  );
> +
> +/**
> +  Close a local event channel Port.
> +
> +  @param This       A pointer to the XENBUS_PROTOCOL.
> +  @param Port       The event channel to close.
> +**/
> +UINT32
> +EFIAPI
> +XenBusEventChannelClose (
> +  IN XENBUS_PROTOCOL *This,
> +  IN evtchn_port_t   Port
> +  );
> +
>  #endif
> diff --git a/OvmfPkg/XenBusDxe/XenBus.c b/OvmfPkg/XenBusDxe/XenBus.c
> index 2e1f939..8c68809 100644
> --- a/OvmfPkg/XenBusDxe/XenBus.c
> +++ b/OvmfPkg/XenBusDxe/XenBus.c
> @@ -355,6 +355,9 @@ STATIC XENBUS_PRIVATE_DATA gXenBusPrivateData = {
>    .XenBusIo.SetState = XenBusSetState,
>    .XenBusIo.GrantAccess = XenBusGrantAccess,
>    .XenBusIo.GrantEndAccess = XenBusGrantEndAccess,
> +  .XenBusIo.EventChannelAllocate = XenBusEventChannelAllocate,
> +  .XenBusIo.EventChannelNotify = XenBusEventChannelNotify,
> +  .XenBusIo.EventChannelClose = XenBusEventChannelClose,
>    .XenBusIo.RegisterWatch = XenBusRegisterWatch,
>    .XenBusIo.RegisterWatchBackend = XenBusRegisterWatchBackend,
>    .XenBusIo.UnregisterWatch = XenBusUnregisterWatch,
> -- 
> Anthony PERARD
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel