#3029 assigned defect

PCI libary hard configured for auto-configuration code failure

Reported by: Jeffrey Hill Owned by: Daniel Hellstrom
Priority: normal Milestone:
Component: score Version: 4.11
Severity: normal Keywords: PCI
Cc: Blocked By:
Blocking:

Description

Executine Summary:
1) The PCI_CFG_AUTO_LIB macro needs to be defined in both "pci_cfg_auto.c" and also "pci_dev_create.c" otherwise the "pci_bus" structure is allocated too small in size in "pci_dev_create.c". Currently, PCI_CFG_AUTO_LIB is set only within "pci_cfg_auto.c" and therefore it sees random values in field "busres" of "struct pci_bus".

2) I can identify two potential issues with the PCI library documentation. See below.

After some quality time in the debugger the user discovers the following code in "pci/cfg.h".

struct pci_bus {
	struct pci_dev	dev; /* PCI Bridge */
	struct pci_dev	*devs; /* Devices on child (secondary) Bus */
	unsigned int	flags;

	/* Bridge Information */
	int num;	/* Bus number (0=Root-PCI-bus) */
	int pri;	/* Primary Bus Number */
	int sord;	/* Subordinate Buses (Child bus count) */

#if defined(PCI_CFG_AUTO_LIB)
	/* Resources of devices on bus. USED INTERNALLY IN AUTO-CFG LIBRARY.
	 *
	 * BUS_RES_IO    = 0:  I/O resources
	 * BUS_RES_MEMIO = 1:  Prefetchable memory resources
	 * BUS_RES_MEM   = 2:  Non-Prefetchable memory resources
	 */
	struct pci_res	*busres[3];
#endif
};

And the following in "pci_dev_create.c" which explains why the PCI auto configuration code crashes using random values in the "busres" field.

struct pci_dev *pci_dev_create(int isbus)
{
	void *ptr;
	int size;

	if (isbus)
		size = sizeof(struct pci_bus);
	else
		size = sizeof(struct pci_dev);

	ptr = malloc(size);
	if (!ptr)
		rtems_fatal_error_occurred(RTEMS_NO_MEMORY);
	memset(ptr, 0, size);
	return ptr;
}

Furthermore, I had to modify the following in the cpukit configure.ac to build the PCI library for architecture nios2, but perhaps this should be accomplished in some other way by configuring the rtems build or by specifying options in the BSP, but its unclear, at least to me, how that might be done. So perhaps there is a nit against the documentation on this point.

# Filter libpci to only build for architectures that have support for it
AC_MSG_CHECKING([whether CPU supports libpci])
case $RTEMS_CPU in
  sparc | nios2)
   HAVE_LIBPCI=yes ;;
  *)
   HAVE_LIBPCI=no ;;
esac
AM_CONDITIONAL(LIBPCI,[test x"$HAVE_LIBPCI" = x"yes"])
AC_MSG_RESULT([$HAVE_LIBPCI])

# Filter libdrvmgr to only build for architectures that have support for it
AC_MSG_CHECKING([whether CPU supports libdrvmgr])
case $RTEMS_CPU in
  sparc | nios2)
   HAVE_LIBDRVMGR=yes ;;
  *)
   HAVE_LIBDRVMGR=no ;;
esac
AM_CONDITIONAL(LIBDRVMGR,[test x"$HAVE_LIBDRVMGR" = x"yes"])
AC_MSG_RESULT([$HAVE_LIBDRVMGR])

Furthermore, it was necessary to add some code like this to the application, but perhaps the documentation could be updated to indicate that this is required, or if not how it might be configured to run auto-magically.

#   if defined ( RTEMS_PCI_CONFIG_LIB ) 
      if ( pci_config_init () ) { 
        printk ( "PCI Configuration Failed\n" );
      }
#   endif

Change History (4)

comment:1 Changed on Jun 1, 2017 at 6:42:42 PM by Jeffrey Hill

After another look at the documentation I do see that it mentions that a function "pci_config_init ()" exists. I suspect that on SPARC architecture it is setup to be called by the driver library, but otherwise it must be explicitly called by the application, and perhaps the doc should say that.

comment:2 Changed on Jun 1, 2017 at 7:24:13 PM by Gedare Bloom

Owner: changed from joel.sherrill@… to Daniel Hellstrom
Status: newassigned

Currently the drvmgr is only known to work for sparc.

comment:3 Changed on Jun 21, 2017 at 1:13:19 AM by Jeffrey Hill

Perhaps it was a safe conclusion of mine that the PCI library is intended to become a future unified approach for PCI on RTEMS based on the new PCI Library documentation in RTEMS 4.11, but there is also some confusion given that there are other BSP specific PCI libraries in the distribution, and also given that the PCI library currently only builds for SPARC unless one modifies the distribution. Of course unification takes time, and may never be 100% complete, but perhaps the documentation could include a short paragraph describing at least the future goals for supporting, and moving to, the new library on multiple platforms.

comment:4 Changed on Sep 29, 2017 at 11:38:30 AM by Daniel Hellstrom <daniel@…>

In f9fbb333/rtems:

libpci: fix pci device allocation

The refactoring of pci_dev_create() was incorrect since the code relied on
different defines before including pci/cfg.h. This reverts back to the
original code having two pci_dev_create() one in auto and one in read library.
confdefs.h selectes between the two libraries so both there is no link
conflict.

Updates #3029

Note: See TracTickets for help on using tickets.