svn commit: r271202 - head/sys/dev/ofw

[Nathan Whitehorn]

On 09/06/14 12:40, Ian Lepore wrote:
> On Sat, 2014-09-06 at 12:16 -0700, Nathan Whitehorn wrote:
>> Not looking at the code: what happens if you ask for the node
>> corresponding to a phandle but the device corresponding to that phandle
>> has not registered yet and it has an implicit crossreference mapping?
>> -Nathan
>>
> The node<->xref behavior is the same as it has always been: if there
> isn't a phandle property in a node to supply an xref handle then the
> node and xref handles are the synonyms.
>
> The only thing that has changed is that with my first implementation an
> xref handle had exist (meaning it had to have been discovered as a
> phandle property during the init-time scan of the tree) before you could
> associate a device_t with it.  Now you can make associations on the fly
> whether the data author intended for such associations to exist or not.
> I can't decide whether that's a good or a bad thing.
>
> -- Ian

Thanks for the explanation! That seems like the most reasonable approach.
-Nathan

>> On 09/06/14 11:43, Ian Lepore wrote:
>>> Author: ian
>>> Date: Sat Sep  6 18:43:17 2014
>>> New Revision: 271202
>>> URL: http://svnweb.freebsd.org/changeset/base/271202
>>>
>>> Log:
>>>     When registering an association between a device and an xref phandle, create
>>>     an entry in the xref list if one doesn't already exist for the given handle.
>>>     
>>>     On a system that uses phandle properties, the init-time scan of the tree
>>>     which builds the xref list will pre-create entries for every xref handle
>>>     that exists in the data.  On systems where the xref and node handles are
>>>     synonymous there is no phandle property in referenced nodes, and the xref
>>>     list will initialize to an empty state.  In the latter case, we still need
>>>     to be able to associate a device_t with an xref handle, so we create list
>>>     entries on the fly as needed.  Since the node and xref handles are
>>>     synonymous, we have all the info needed to create a list entry at device
>>>     registration time.
>>>     
>>>     The downside to this change is that it basically allows on the fly creation
>>>     of xref handles as synonyms of node handles, and the association of a
>>>     device_t with them.  Whether this is a bug or a feature is in the eye of
>>>     the beholder, I guess.
>>>
>>> Modified:
>>>     head/sys/dev/ofw/openfirm.c
>>>
>>> Modified: head/sys/dev/ofw/openfirm.c
>>> ==============================================================================
>>> --- head/sys/dev/ofw/openfirm.c	Sat Sep  6 18:20:50 2014	(r271201)
>>> +++ head/sys/dev/ofw/openfirm.c	Sat Sep  6 18:43:17 2014	(r271202)
>>> @@ -62,7 +62,10 @@ __FBSDID("$FreeBSD$");
>>>    
>>>    #include <sys/param.h>
>>>    #include <sys/kernel.h>
>>> +#include <sys/lock.h>
>>>    #include <sys/malloc.h>
>>> +#include <sys/mutex.h>
>>> +#include <sys/queue.h>
>>>    #include <sys/systm.h>
>>>    #include <sys/endian.h>
>>>    
>>> @@ -92,6 +95,7 @@ struct xrefinfo {
>>>    };
>>>    
>>>    static SLIST_HEAD(, xrefinfo) xreflist = SLIST_HEAD_INITIALIZER(xreflist);
>>> +static struct mtx xreflist_lock;
>>>    static boolean_t xref_init_done;
>>>    
>>>    #define	FIND_BY_XREF	0
>>> @@ -138,6 +142,12 @@ static void
>>>    xrefinfo_init(void *unsed)
>>>    {
>>>    
>>> +	/*
>>> +	 * There is no locking during this init because it runs much earlier
>>> +	 * than any of the clients/consumers of the xref list data, but we do
>>> +	 * initialize the mutex that will be used for access later.
>>> +	 */
>>> +	mtx_init(&xreflist_lock, "OF xreflist lock", NULL, MTX_DEF);
>>>    	xrefinfo_create(OF_peer(0));
>>>    	xref_init_done = true;
>>>    }
>>> @@ -146,17 +156,35 @@ SYSINIT(xrefinfo, SI_SUB_KMEM, SI_ORDER_
>>>    static struct xrefinfo *
>>>    xrefinfo_find(phandle_t phandle, int find_by)
>>>    {
>>> -	struct xrefinfo * xi;
>>> +	struct xrefinfo *rv, *xi;
>>>    
>>> +	rv = NULL;
>>> +	mtx_lock(&xreflist_lock);
>>>    	SLIST_FOREACH(xi, &xreflist, next_entry) {
>>> -		if (find_by == FIND_BY_XREF && phandle == xi->xref)
>>> -			return (xi);
>>> -		else if (find_by == FIND_BY_NODE && phandle == xi->node)
>>> -			return (xi);
>>> -		else if (find_by == FIND_BY_DEV && phandle == (uintptr_t)xi->dev)
>>> -			return (xi);
>>> +		if ((find_by == FIND_BY_XREF && phandle == xi->xref) ||
>>> +		    (find_by == FIND_BY_NODE && phandle == xi->node) ||
>>> +		    (find_by == FIND_BY_DEV && phandle == (uintptr_t)xi->dev)) {
>>> +			rv = xi;
>>> +			break;
>>> +		}
>>>    	}
>>> -	return (NULL);
>>> +	mtx_unlock(&xreflist_lock);
>>> +	return (rv);
>>> +}
>>> +
>>> +static struct xrefinfo *
>>> +xrefinfo_add(phandle_t node, phandle_t xref, device_t dev)
>>> +{
>>> +	struct xrefinfo *xi;
>>> +
>>> +	xi = malloc(sizeof(*xi), M_OFWPROP, M_WAITOK);
>>> +	xi->node = node;
>>> +	xi->xref = xref;
>>> +	xi->dev  = dev;
>>> +	mtx_lock(&xreflist_lock);
>>> +	SLIST_INSERT_HEAD(&xreflist, xi, next_entry);
>>> +	mtx_unlock(&xreflist_lock);
>>> +	return (xi);
>>>    }
>>>    
>>>    /*
>>> @@ -605,10 +633,17 @@ OF_device_register_xref(phandle_t xref,
>>>    {
>>>    	struct xrefinfo *xi;
>>>    
>>> +	/*
>>> +	 * If the given xref handle doesn't already exist in the list then we
>>> +	 * add a list entry.  In theory this can only happen on a system where
>>> +	 * nodes don't contain phandle properties and xref and node handles are
>>> +	 * synonymous, so the xref handle is added as the node handle as well.
>>> +	 */
>>>    	if (xref_init_done) {
>>>    		if ((xi = xrefinfo_find(xref, FIND_BY_XREF)) == NULL)
>>> -			return (ENXIO);
>>> -		xi->dev = dev;
>>> +			xrefinfo_add(xref, xref, dev);
>>> +		else
>>> +			xi->dev = dev;
>>>    		return (0);
>>>    	}
>>>    	panic("Attempt to register device before xreflist_init");
>>>
>