Re: [Xen-devel] [PATCH v2 09/18] OvmfPkg/XenBusDxe: Add Event Channel Notify.

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Sep 10, 2014 at 04:15:03PM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 04, 2014 at 05:51:04PM +0100, Anthony PERARD wrote:
> > This first function is used to notify the other side that there is
> > something to do. The other side is another Xen domain.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > 
> > ---
> > Change in V2:
> > - file header
> > - coding style
> > - adding comment to functions
> > - Licenses
> > ---
> >  OvmfPkg/XenBusDxe/EventChannel.c | 49 
> > +++++++++++++++++++++++++++++++++++++++
> >  OvmfPkg/XenBusDxe/EventChannel.h | 50 
> > ++++++++++++++++++++++++++++++++++++++++
> >  OvmfPkg/XenBusDxe/XenBusDxe.inf  |  2 ++
> >  3 files changed, 101 insertions(+)
> >  create mode 100644 OvmfPkg/XenBusDxe/EventChannel.c
> >  create mode 100644 OvmfPkg/XenBusDxe/EventChannel.h
> > 
> > diff --git a/OvmfPkg/XenBusDxe/EventChannel.c 
> > b/OvmfPkg/XenBusDxe/EventChannel.c
> > new file mode 100644
> > index 0000000..f34f9b8
> > --- /dev/null
> > +++ b/OvmfPkg/XenBusDxe/EventChannel.c
> > @@ -0,0 +1,49 @@
> > +/** @file
> > +  Event Channel function implementation.
> > +
> > +  Event channel are use to notify of an event that happend in a shared
> > +  structure for example.
> > +
> > +  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 "EventChannel.h"
> > +#include "XenHypercall.h"
> > +
> > +VOID
> > +XenEventChannelNotify (
> > +  IN XENBUS_DEVICE *Dev,
> > +  IN evtchn_port_t Port
> > +  )
> > +{
> > +  INTN ReturnCode;
> > +  evtchn_send_t Send;
> > +
> > +  Send.port = Port;
> > +  ReturnCode = XenHypercallEventChannelOp (Dev, EVTCHNOP_send, &Send);
> > +  ASSERT (ReturnCode == 0);
> 
> Ouch. How about returning the return code. For example it might be -EINVAL
> due to the port value being wrong (or not yet set). Shouldn't the caller
> decided whether to ASSERT at that point?

Yes. I will fix that.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel