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

Re: [Xen-devel] [PATCH RFC 14/18] OvmfPkg/XenbusDxe: Indroduce XenBus support itself.



On Wed, Jul 16, 2014 at 03:04:50PM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Jul 16, 2014 at 04:15:43PM +0100, Anthony PERARD wrote:
> > This is a bus-like on top of Xenstore. It will look for advertised
> > ParaVirtualized devices and initialize them by producing XenBus
> > protocol.
> > 
> > Origin: FreeBSD 10.0
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > ---
> >  OvmfPkg/Include/Protocol/Xenbus.h |   9 +
> >  OvmfPkg/XenbusDxe/XenBus.c        | 360 
> > ++++++++++++++++++++++++++++++++++++++
> >  OvmfPkg/XenbusDxe/XenBus.h        |  76 ++++++++
> >  OvmfPkg/XenbusDxe/XenbusDxe.c     |  61 +++++++
> >  OvmfPkg/XenbusDxe/XenbusDxe.h     |  18 ++
> >  OvmfPkg/XenbusDxe/XenbusDxe.inf   |   3 +
> >  6 files changed, 527 insertions(+)
> >  create mode 100644 OvmfPkg/XenbusDxe/XenBus.c
> >  create mode 100644 OvmfPkg/XenbusDxe/XenBus.h
> > 
> > diff --git a/OvmfPkg/Include/Protocol/Xenbus.h 
> > b/OvmfPkg/Include/Protocol/Xenbus.h
> > index ef4d0c2..25408c1 100644
> > --- a/OvmfPkg/Include/Protocol/Xenbus.h
> > +++ b/OvmfPkg/Include/Protocol/Xenbus.h
> > @@ -113,6 +113,14 @@ XENSTORE_STATUS
> >    );
> >  
> >  typedef
> > +XENSTORE_STATUS
> > +(EFIAPI *XENBUS_SET_STATE)(
> > +  IN XENBUS_PROTOCOL        *This,
> > +  IN XENSTORE_TRANSACTION   Transaction,
> > +  IN XenbusState            State
> > +  );
> > +
> > +typedef
> >  EFI_STATUS
> >  (EFIAPI *XENBUS_GRANT_ACCESS)(
> >    IN  XENBUS_PROTOCOL *This,
> > @@ -171,6 +179,7 @@ struct _XENBUS_PROTOCOL {
> >    XENBUS_XS_REMOVE              XsRemove;
> >    XENBUS_XS_TRANSACTION_START   XsTransactionStart;
> >    XENBUS_XS_TRANSACTION_END     XsTransactionEnd;
> > +  XENBUS_SET_STATE              SetState;
> >  
> >    XENBUS_GRANT_ACCESS           GrantAccess;
> >    XENBUS_GRANT_END_ACCESS       GrantEndAccess;
> > diff --git a/OvmfPkg/XenbusDxe/XenBus.c b/OvmfPkg/XenbusDxe/XenBus.c
> > new file mode 100644
> > index 0000000..b0cf1ba
> > --- /dev/null
> > +++ b/OvmfPkg/XenbusDxe/XenBus.c
> > @@ -0,0 +1,360 @@
> > +/******************************************************************************
> > + * Copyright (C) 2010 Spectra Logic Corporation
> > + * Copyright (C) 2008 Doug Rabson
> > + * Copyright (C) 2005 Rusty Russell, IBM Corporation
> > + * Copyright (C) 2005 Mike Wray, Hewlett-Packard
> > + * Copyright (C) 2005 XenSource Ltd
> > + *
> > + * This file may be distributed separately from the Linux kernel, or
> > + * incorporated into other software packages, subject to the following 
> > license:
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > + * of this source file (the "Software"), to deal in the Software without
> > + * restriction, including without limitation the rights to use, copy, 
> > modify,
> > + * merge, publish, distribute, sublicense, and/or sell copies of the 
> > Software,
> > + * and to permit persons to whom the Software is furnished to do so, 
> > subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included 
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> > THE
> > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> > DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> > +
> > +#include "XenbusDxe.h"
> > +
> > +#include <Library/PrintLib.h>
> > +
> > +#include "XenBus.h"
> > +#include "GrantTable.h"
> > +#include "Xenstore.h"
> > +#include "EventChannel.h"
> > +
> > +#include <IndustryStandard/Xen/io/xenbus.h>
> > +
> > +STATIC XENBUS_PRIVATE_DATA gXenbusPrivateData;
> > +
> > +STATIC XENBUS_DEVICE_PATH gXenbusDevicePathTemplate = {
> > +  .Vendor.Header.Type = HARDWARE_DEVICE_PATH,
> > +  .Vendor.Header.SubType = HW_VENDOR_DP,
> > +  .Vendor.Header.Length[0] = (UINT8) sizeof (XENBUS_DEVICE_PATH),
> > +  .Vendor.Header.Length[1] = (UINT8) (sizeof (XENBUS_DEVICE_PATH) >> 8),
> > +  .Vendor.Guid = XENBUS_PROTOCOL_GUID,
> > +  .Type = 0,
> > +  .DeviceId = 0
> > +};
> > +
> > +
> > +/**
> > + * Search our internal record of configured devices (not the XenStore)
> > + * to determine if the XenBus device indicated by \a node is known to
> > + * the system.
> > + *
> > + * \param Dev   The XenBus bus instance to search for device children.
> > + * \param node  The XenStore node path for the device to find.
> > + *
> > + * \return  The device_t of the found device if any, or NULL.
> > + *
> > + * \note device_t is a pointer type, so it can be compared against
> > + *       NULL for validity.
> > + */
> > +STATIC
> > +XENBUS_PRIVATE_DATA *
> > +XenbusDeviceInitialized (
> > +  IN XENBUS_DEVICE *Dev,
> > +  IN CONST CHAR8 *Node
> > +  )
> > +{
> > +  LIST_ENTRY *Entry;
> > +  XENBUS_PRIVATE_DATA *Child;
> > +  XENBUS_PRIVATE_DATA *Result;
> > +
> > +  if (IsListEmpty (&Dev->ChildList)) {
> > +    return NULL;
> > +  }
> > +
> > +  Result = NULL;
> > +  for (Entry = GetFirstNode (&Dev->ChildList);
> > +       !IsNodeAtEnd (&Dev->ChildList, Entry);
> > +       Entry = GetNextNode (&Dev->ChildList, Entry)) {
> > +    Child = XENBUS_PRIVATE_DATA_FROM_LINK (Entry);
> > +    if (!AsciiStrCmp (Child->XenbusIo.Node, Node)) {
> > +      Result = Child;
> > +      break;
> > +    }
> > +  }
> > +
> > +  return (Result);
> > +}
> > +
> > +STATIC
> > +XenbusState
> > +XenbusReadDriverState (
> > +  IN CONST CHAR8 *Path
> > +  )
> > +{
> > +  XenbusState State;
> > +  CHAR8 *Ptr;
> > +  XENSTORE_STATUS Status;
> > +
> > +  Status = XenstoreRead (XST_NIL, Path, "state", NULL, (VOID **)&Ptr);
> > +  if (Status != XENSTORE_STATUS_SUCCESS) {
> > +    State = XenbusStateClosed;
> > +  } else {
> > +    State = AsciiStrDecimalToUintn (Ptr);
> > +    FreePool (Ptr);
> > +  }
> 
> I think the FreePool(Ptr) should be here. You want to it
> irregardless of what the error might be (thought the implemenation
> in XenstoreRead et.all is pretty clear that we won't initialize
> *Ptr to anything if !Status. But just in case this changes in 
> the future..

OK, will do. But I might have to check for NULL.

> > +
> > +  return State;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +XenbusAddDevice (
> > +  XENBUS_DEVICE *Dev,
> > +  CONST CHAR8 *Type,
> > +  CONST CHAR8 *Id)
> > +{
> > +  CHAR8 DevicePath[XENSTORE_ABS_PATH_MAX];
> > +  XENSTORE_STATUS StatusXenstore;
> > +  XENBUS_PRIVATE_DATA *Private;
> > +  EFI_STATUS Status;
> > +  XENBUS_DEVICE_PATH *TempXenbusPath;
> > +  VOID *ChildPciIo;
> > +
> > +  AsciiSPrint (DevicePath, sizeof (DevicePath),
> > +               XENBUS_XENSTORE_NODE "/%a/%a", Type, Id);
> > +
> > +  Status = EFI_NOT_STARTED; //ENXIO; // Device not configured
> > +
> > +  if (XenstorePathExists (XST_NIL, DevicePath, "")) {
> > +    XENBUS_PRIVATE_DATA *Child;
> > +    enum xenbus_state State;
> > +    CHAR8 *BackendPath;
> > +
> > +    Child = XenbusDeviceInitialized (Dev, DevicePath);
> > +    if (Child != NULL) {
> > +      /*
> > +       * We are already tracking this node
> > +       */
> > +      Status = EFI_SUCCESS;
> > +      goto out;
> > +    }
> > +
> > +    State = XenbusReadDriverState (DevicePath);
> > +    if (State != XenbusStateInitialising) {
> > +      /*
> > +       * Device is not new, so ignore it. This can
> > +       * happen if a device is going away after
> > +       * switching to Closed.
> > +       */
> > +      DEBUG ((EFI_D_INFO, "Xenbus: Device %a ignored. "
> > +              "State %d\n", DevicePath, State));
> > +      Status = EFI_SUCCESS;
> > +      goto out;
> > +    }
> > +
> > +    StatusXenstore = XenstoreRead (XST_NIL, DevicePath, "backend",
> > +                                   NULL, (VOID **) &BackendPath);
> > +    if (StatusXenstore != XENSTORE_STATUS_SUCCESS) {
> > +      DEBUG ((EFI_D_ERROR, "xenbus: %a no backend path.\n", DevicePath));
> > +      Status = EFI_NOT_FOUND;
> > +      goto out;
> > +    }
> > +
> > +    Private = AllocateCopyPool (sizeof *Private, &gXenbusPrivateData);
> > +    InsertTailList (&Dev->ChildList, &Private->Link);
> > +
> > +    Private->XenbusIo.Type = AsciiStrDup (Type);
> > +    Private->XenbusIo.Node = AsciiStrDup (DevicePath);
> > +    Private->XenbusIo.Backend = BackendPath;
> > +    Private->XenbusIo.DeviceId = AsciiStrDecimalToUintn (Id);
> > +    Private->Dev = Dev;
> > +
> > +    TempXenbusPath = AllocateCopyPool (sizeof (XENBUS_DEVICE_PATH),
> > +                                       &gXenbusDevicePathTemplate);
> > +    if (!AsciiStrCmp (Private->XenbusIo.Type, "vbd")) {
> > +      TempXenbusPath->Type = XENBUS_DEVICE_PATH_TYPE_VBD;
> > +    }
> > +    TempXenbusPath->DeviceId = Private->XenbusIo.DeviceId;
> > +    Private->DevicePath = (XENBUS_DEVICE_PATH *)AppendDevicePathNode (
> > +                            Dev->DevicePath,
> > +                            &TempXenbusPath->Vendor.Header);
> 
> Could you provide a comment explaining why you do that please?

Will do.

I think bus drivers need to do that. It is used to specify wich device
to boot on. Without it done here, it would be hard to differentiate two
PV block drivers from each other.

> > +
> > +    Status = gBS->InstallMultipleProtocolInterfaces (
> > +               &Private->Handle,
> > +               &gEfiDevicePathProtocolGuid, Private->DevicePath,
> > +               &gXenbusProtocolGuid, &Private->XenbusIo,
> > +               NULL);
> > +    if (EFI_ERROR (Status)) {
> 
> Shouldn't you free 'Private' and 'BackendPath' ? And 
> also de-link yourself from 'Dev->ChildList' ?

Yes, I need to fix the error handling.

> > +      return Status;
> > +    }
> > +
> > +    Status = gBS->OpenProtocol (Dev->ControllerHandle,
> > +               &gEfiPciIoProtocolGuid,
> > +               &ChildPciIo, Dev->This->DriverBindingHandle,
> > +               Private->Handle,
> > +               EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER);
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((EFI_D_ERROR, "open by child controller fail (%r)\n",
> > +              Status));
> > +    }
> > +  } else {
> > +    DEBUG ((EFI_D_INFO, "Xenbus: does not exist: %a\n", DevicePath));
> > +  }
> > +
> > +out:
> > +  return Status;
> > +}
> > +
> > +/**
> > + * \brief Enumerate all devices of the given type on this bus.
> > + *
> > + * \param Dev   NewBus device_t for this XenBus front bus instance.
> > + * \param type  String indicating the device sub-tree (e.g. "vfb", "vif")
> > + *              to enumerate.
> > + *
> > + * Devices that are found are entered into the NewBus hierarchy via
> > + * xenbusb_add_device().  xenbusb_add_device() ignores duplicate detects
> > + * and ignores duplicate devices, so it can be called unconditionally
> > + * for any device found in the XenStore.
> > + */
> > +STATIC
> > +VOID
> > +XenbusEnumerateDeviceType (
> > +  XENBUS_DEVICE *Dev,
> > +  CONST CHAR8 *Type
> > +  )
> > +{
> > +  CONST CHAR8 **Directory;
> > +  UINTN Index;
> > +  UINT32 Count;
> > +  XENSTORE_STATUS Status;
> > +
> > +  Status = XenstoreListDirectory (XST_NIL,
> > +                                  XENBUS_XENSTORE_NODE, Type,
> > +                                  &Count, &Directory);
> > +  if (Status != XENSTORE_STATUS_SUCCESS) {
> > +    return;
> > +  }
> > +  for (Index = 0; Index < Count; Index++) {
> > +    XenbusAddDevice (Dev, Type, Directory[Index]);
> > +  }
> > +
> > +  FreePool (Directory);
> > +}
> > +
> > +
> > +/**
> > + * \brief Enumerate the devices on a XenBus bus and register them with
> > + *        the NewBus device tree.
> > + *
> > + * xenbusb_enumerate_bus() will create entries (in state DS_NOTPRESENT)
> > + * for nodes that appear in the XenStore, but will not invoke probe/attach
> > + * operations on drivers.  Probe/Attach processing must be separately
> > + * performed via an invocation of xenbusb_probe_children().  This is 
> > usually
> > + * done via the xbs_probe_children task.
> > + *
> > + * \param Dev  XenBus Bus device softc of the owner of the bus to 
> > enumerate.
> > + *
> > + * \return  On success, 0. Otherwise an errno value indicating the
> > + *          type of failure.
> > + */
> > +XENSTORE_STATUS
> > +XenbusEnumerateBus (
> > +  XENBUS_DEVICE *Dev
> > +  )
> > +{
> > +  CONST CHAR8 **Types;
> > +  UINTN Index;
> > +  UINT32 Count;
> > +  XENSTORE_STATUS Status;
> > +
> > +  Status = XenstoreListDirectory (XST_NIL,
> > +                                  XENBUS_XENSTORE_NODE, "",
> > +                                  &Count, &Types);
> > +  if (Status != XENSTORE_STATUS_SUCCESS) {
> > +    return Status;
> > +  }
> > +
> > +  for (Index = 0; Index < Count; Index++) {
> > +    XenbusEnumerateDeviceType (Dev, Types[Index]);
> > +  }
> > +
> > +  FreePool (Types);
> > +
> > +  return XENSTORE_STATUS_SUCCESS;
> > +}
> > +
> > +STATIC
> > +XENSTORE_STATUS
> > +EFIAPI
> > +XenbusSetState (
> > +  IN XENBUS_PROTOCOL      *This,
> > +  IN XENSTORE_TRANSACTION Transaction,
> > +  IN enum xenbus_state    NewState
> > +  )
> > +{
> > +  XENBUS_PRIVATE_DATA *Private;
> > +  enum xenbus_state CurrentState;
> > +  XENSTORE_STATUS Status;
> > +  CHAR8 *Temp;
> > +
> > +  DEBUG ((EFI_D_INFO, "Xenbus: Set state to %d\n", NewState));
> > +
> > +  Private = XENBUS_PRIVATE_DATA_FROM_THIS (This);
> > +
> > +  Status = XenstoreRead (Transaction, This->Node, "state", NULL, (VOID 
> > **)&Temp);
> > +  if (Status != XENSTORE_STATUS_SUCCESS) {
> > +    goto Error;
> > +  }
> > +  CurrentState = AsciiStrDecimalToUintn (Temp);
> > +  FreePool (Temp);
> > +  if (CurrentState == NewState) {
> > +    DEBUG ((EFI_D_INFO, "%a %d, same state\n", __func__, __LINE__));
> > +    goto Error;
> > +  }
> > +
> > +  do {
> > +    Status = XenstoreSPrint (Transaction, This->Node, "state", "%d", 
> > NewState);
> > +  } while (Status == XENSTORE_STATUS_EAGAIN);
> > +  if (Status != XENSTORE_STATUS_SUCCESS) {
> > +    DEBUG ((EFI_D_ERROR, "Xenbus: fail to writing new state\n"));
> 
> failed to write new state

OK.

> > +    goto Error;
> > +  }
> > +  DEBUG ((EFI_D_INFO, "Xenbus: Set state to %d, done\n", NewState));
> > +
> > +Error:
> 
> s/Error/Out/
> 
> as it is OK for to set to the same state twice.

OK.

> > +  return Status;
> > +}
> > +
> > +STATIC XENBUS_PRIVATE_DATA gXenbusPrivateData = {
> > +  .Signature = XENBUS_PRIVATE_DATA_SIGNATURE,
> > +
> > +  .XenbusIo.XsRead = XenbusXenstoreRead,
> > +  .XenbusIo.XsBackendRead = XenbusXenstoreBackendRead,
> > +  .XenbusIo.XsPrintf = XenbusXenstoreSPrint,
> > +  .XenbusIo.XsRemove = XenbusXenstoreRemove,
> > +  .XenbusIo.XsTransactionStart = XenbusXenstoreTransactionStart,
> > +  .XenbusIo.XsTransactionEnd = XenbusXenstoreTransactionEnd,
> > +  .XenbusIo.SetState = XenbusSetState,
> > +  .XenbusIo.GrantAccess = XenbusGrantAccess,
> > +  .XenbusIo.GrantEndAccess = XenbusGrantEndAccess,
> > +  .XenbusIo.RegisterWatch = XenbusRegisterWatch,
> > +  .XenbusIo.RegisterWatchBackend = XenbusRegisterWatchBackend,
> > +  .XenbusIo.UnregisterWatch = XenbusUnregisterWatch,
> > +  .XenbusIo.WaitForWatch = XenbusWaitForWatch,
> > +
> > +  .XenbusIo.Type = NULL,
> > +  .XenbusIo.Node = NULL,
> > +  .XenbusIo.Backend = NULL,
> > +
> > +  .Dev = NULL
> > +};
> > diff --git a/OvmfPkg/XenbusDxe/XenBus.h b/OvmfPkg/XenbusDxe/XenBus.h
> > new file mode 100644
> > index 0000000..63d0815
> > --- /dev/null
> > +++ b/OvmfPkg/XenbusDxe/XenBus.h
> > @@ -0,0 +1,76 @@
> > +/*-
> > + * Core definitions and data structures shareable across OS platforms.
> > + *
> > + * Copyright (c) 2010 Spectra Logic Corporation
> > + * Copyright (C) 2008 Doug Rabson
> > + * All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions, and the following disclaimer,
> > + *    without modification.
> > + * 2. Redistributions in binary form must reproduce at minimum a disclaimer
> > + *    substantially similar to the "NO WARRANTY" disclaimer below
> > + *    ("Disclaimer") and any redistribution must be conditioned upon
> > + *    including a substantially similar Disclaimer requirement for further
> > + *    binary redistribution.
> > + *
> > + * NO WARRANTY
> > + * 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 MERCHANTIBILITY AND FITNESS FOR
> > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> > + * HOLDERS OR CONTRIBUTORS BE LIABLE FOR 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 DAMAGES.
> > + *
> > + * $FreeBSD: release/10.0.0/sys/xen/xenbus/xenbusb.h 222975 2011-06-11 
> > 04:59:01Z gibbs $
> > + */
> > +#ifndef _XEN_XENBUS_XENBUSB_H
> > +#define _XEN_XENBUS_XENBUSB_H
> > +
> > +#include "XenbusDxe.h"
> > +
> > +#define XENBUS_DEVICE_PATH_TYPE_VBD 0x1
> > +struct _XENBUS_DEVICE_PATH {
> > +  VENDOR_DEVICE_PATH  Vendor;
> > +  UINT8               Type;
> > +  UINT16              DeviceId;
> > +};
> > +
> > +/**
> > + * The VM relative path to the XenStore subtree this
> > + * bus attachment manages.
> > + *
> > + * OR
> > + *
> > + * The XenStore path to the XenStore subtree for this XenBus bus.
> > + */
> > +#define XENBUS_XENSTORE_NODE "device"
> > +
> > +
> > +/**
> > + * \brief Perform common XenBus bus attach processing.
> > + *
> > + * \param Dev            The NewBus device representing this XenBus bus.
> > + *
> > + * \return  On success, 0. Otherwise an errno value indicating the
> > + *          type of failure.
> > + *
> > + * Intiailizes the softc for this bus, installs an interrupt driven
> > + * configuration hook to block boot processing until XenBus devices fully
> > + * configure, performs an initial probe/attach of the bus, and registers
> > + * a XenStore watch so we are notified when the bus topology changes.
> > + */
> > +XENSTORE_STATUS
> > +XenbusEnumerateBus (
> > +  XENBUS_DEVICE *Dev
> > +  );
> > +
> > +#endif /* _XEN_XENBUS_XENBUSB_H */
> > diff --git a/OvmfPkg/XenbusDxe/XenbusDxe.c b/OvmfPkg/XenbusDxe/XenbusDxe.c
> > index 2c85d5e..56225c4 100644
> > --- a/OvmfPkg/XenbusDxe/XenbusDxe.c
> > +++ b/OvmfPkg/XenbusDxe/XenbusDxe.c
> > @@ -19,6 +19,7 @@
> >  #include "XenHypercall.h"
> >  #include "GrantTable.h"
> >  #include "Xenstore.h"
> > +#include "XenBus.h"
> >  
> >  ///
> >  /// Driver Binding Protocol instance
> > @@ -295,6 +296,7 @@ XenbusDxeDriverBindingStart (
> >    EFI_PCI_IO_PROTOCOL *PciIo;
> >    EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *BarDesc;
> >    UINT64 MmioAddr;
> > +  EFI_DEVICE_PATH_PROTOCOL *DevicePath;
> >  
> >    Status = gBS->OpenProtocol (
> >                       ControllerHandle,
> > @@ -308,11 +310,26 @@ XenbusDxeDriverBindingStart (
> >      return Status;
> >    }
> >  
> > +  Status = gBS->OpenProtocol (
> > +                  ControllerHandle,
> > +                  &gEfiDevicePathProtocolGuid,
> > +                  (VOID **) &DevicePath,
> > +                  This->DriverBindingHandle,
> > +                  ControllerHandle,
> > +                  EFI_OPEN_PROTOCOL_BY_DRIVER
> > +                  );
> > +
> > +  if (EFI_ERROR (Status)) {
> > +    return Status;
> > +  }
> > +
> >    Dev = AllocateZeroPool (sizeof (*Dev));
> >    Dev->Signature = XENBUS_DEVICE_SIGNATURE;
> >    Dev->This = This;
> >    Dev->ControllerHandle = ControllerHandle;
> >    Dev->PciIo = PciIo;
> > +  Dev->DevicePath = DevicePath;
> > +  InitializeListHead (&Dev->ChildList);
> >  
> >    Status = PciIo->GetBarAttributes (PciIo, PCI_BAR_IDX1, NULL, (VOID**) 
> > &BarDesc);
> >    ASSERT_EFI_ERROR (Status);
> > @@ -334,6 +351,8 @@ XenbusDxeDriverBindingStart (
> >    Status = XenstoreInit (Dev);
> >    ASSERT_EFI_ERROR (Status);
> >  
> > +  XenbusEnumerateBus (Dev);
> > +
> >    Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
> >                               NotifyExitBoot,
> >                               (VOID*) Dev,
> > @@ -379,12 +398,54 @@ XenbusDxeDriverBindingStop (
> >    IN EFI_HANDLE                   *ChildHandleBuffer OPTIONAL
> >    )
> >  {
> > +  UINTN Index;
> > +  XENBUS_PROTOCOL *XenbusIo;
> > +  XENBUS_PRIVATE_DATA *ChildData;
> > +  EFI_STATUS Status;
> >    XENBUS_DEVICE *Dev = mMyDevice;
> >  
> > +  for (Index = 0; Index < NumberOfChildren; Index++) {
> > +    Status = gBS->OpenProtocol (
> > +               ChildHandleBuffer[Index],
> > +               &gXenbusProtocolGuid,
> > +               (VOID **) &XenbusIo,
> > +               This->DriverBindingHandle,
> > +               ControllerHandle,
> > +               EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((EFI_D_ERROR, "XenbusDxe: get childre proto: %r\n", Status));
> 
> get children protocol failed: %r

Will fix.

> > +      continue;
> > +    }
> > +    ChildData = XENBUS_PRIVATE_DATA_FROM_THIS(XenbusIo);
> > +    Status = gBS->DisconnectController(ChildData->Handle, NULL, NULL);
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((EFI_D_ERROR, "XenbusDxe: error disconnecting child: %r\n",
> > +              Status));
> > +      continue;
> > +    }
> > +
> > +    Status = gBS->UninstallMultipleProtocolInterfaces (
> > +               ChildData->Handle,
> > +               &gEfiDevicePathProtocolGuid, ChildData->DevicePath,
> > +               &gXenbusProtocolGuid, &ChildData->XenbusIo,
> > +               NULL);
> > +    ASSERT_EFI_ERROR (Status);
> > +
> > +    FreePool ((VOID*)ChildData->XenbusIo.Type);
> > +    FreePool ((VOID*)ChildData->XenbusIo.Node);
> > +    FreePool ((VOID*)ChildData->XenbusIo.Backend);
> > +    FreePool (ChildData->DevicePath);
> > +    RemoveEntryList (&ChildData->Link);
> > +    FreePool (ChildData);
> > +  }
> > +
> >    gBS->CloseEvent (Dev->ExitBootEvent);
> > +  // XXX xenbus cleanup
> >    // XXX xenstore cleanup
> >    XenGrantTableDeinit (Dev);
> >  
> > +  gBS->CloseProtocol(ControllerHandle, &gEfiDevicePathProtocolGuid,
> > +         This->DriverBindingHandle, ControllerHandle);
> >    gBS->CloseProtocol(ControllerHandle, &gEfiPciIoProtocolGuid,
> >           This->DriverBindingHandle, ControllerHandle);
> >    return EFI_SUCCESS;
> > diff --git a/OvmfPkg/XenbusDxe/XenbusDxe.h b/OvmfPkg/XenbusDxe/XenbusDxe.h
> > index 4d9db72..3ff08dd 100644
> > --- a/OvmfPkg/XenbusDxe/XenbusDxe.h
> > +++ b/OvmfPkg/XenbusDxe/XenbusDxe.h
> > @@ -80,6 +80,7 @@ extern EFI_COMPONENT_NAME_PROTOCOL  
> > gXenbusDxeComponentName;
> >  #define PCI_DEVICE_ID_XEN_PLATFORM       0x0001
> >  
> >  
> > +typedef struct _XENBUS_DEVICE_PATH XENBUS_DEVICE_PATH;
> >  typedef struct _XENBUS_DEVICE XENBUS_DEVICE;
> >  
> >  #define XENBUS_DEVICE_SIGNATURE SIGNATURE_32 ('X','B','b','d')
> > @@ -89,11 +90,28 @@ struct _XENBUS_DEVICE {
> >    EFI_HANDLE                    ControllerHandle;
> >    EFI_PCI_IO_PROTOCOL           *PciIo;
> >    EFI_EVENT                     ExitBootEvent;
> > +  EFI_DEVICE_PATH_PROTOCOL      *DevicePath;
> > +  LIST_ENTRY                    ChildList;
> >  
> >    VOID                          *Hyperpage;
> >    shared_info_t                 *SharedInfo;
> >  };
> >  
> > +#define XENBUS_PRIVATE_DATA_SIGNATURE SIGNATURE_32 ('X', 'B', 'u', 's')
> > +typedef struct {
> > +    UINTN Signature;
> > +    LIST_ENTRY Link;
> > +    EFI_HANDLE Handle;
> > +    XENBUS_PROTOCOL XenbusIo;
> > +    XENBUS_DEVICE *Dev;
> > +    XENBUS_DEVICE_PATH *DevicePath;
> > +} XENBUS_PRIVATE_DATA;
> > +
> > +#define XENBUS_PRIVATE_DATA_FROM_THIS(a) \
> > +  CR (a, XENBUS_PRIVATE_DATA, XenbusIo, XENBUS_PRIVATE_DATA_SIGNATURE)
> > +#define XENBUS_PRIVATE_DATA_FROM_LINK(a) \
> > +  CR (a, XENBUS_PRIVATE_DATA, Link, XENBUS_PRIVATE_DATA_SIGNATURE)
> > +
> >  /*
> >   * Helpers
> >   */
> > diff --git a/OvmfPkg/XenbusDxe/XenbusDxe.inf 
> > b/OvmfPkg/XenbusDxe/XenbusDxe.inf
> > index 6c8260f..708fae6 100644
> > --- a/OvmfPkg/XenbusDxe/XenbusDxe.inf
> > +++ b/OvmfPkg/XenbusDxe/XenbusDxe.inf
> > @@ -40,6 +40,9 @@
> >    EventChannel.h
> >    Xenstore.c
> >    Xenstore.h
> > +  XenBus.c
> > +  XenBus.h
> > +  Helpers.c
> >  
> >  [Sources.X64]
> >    X64/hypercall.S
> > -- 
> > Anthony PERARD
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel

-- 
Anthony PERARD

_______________________________________________
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®.