From owner-freebsd-hackers@FreeBSD.ORG Mon Jul 9 04:37:12 2012 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 881A51065670 for ; Mon, 9 Jul 2012 04:37:12 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from mail-pb0-f54.google.com (mail-pb0-f54.google.com [209.85.160.54]) by mx1.freebsd.org (Postfix) with ESMTP id 4D02B8FC0A for ; Mon, 9 Jul 2012 04:37:12 +0000 (UTC) Received: by pbbro2 with SMTP id ro2so19888466pbb.13 for ; Sun, 08 Jul 2012 21:37:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=sender:subject:mime-version:content-type:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to:x-mailer :x-gm-message-state; bh=jojy+TpzE3UZwtEJ/jX0QxlF/++XKd4pwlt2h/66jqk=; b=BN3mXZw1KT3W7f1uZkHDCZZCq1Qjl1K5VtPX65FMzT2Fsevevd1gBkgjoP1RDJpBcT Fku/f1H1TdpYU7JHLmrNO5REzJI9Jw0WP/4sNaT4OyCCUq3ECuuQHSAXI7j/sgVYTgEt E1AihStmbu+/pnif8g5GE1PRHgdHjwk+oJ/0LuX7xFByv0BKRI+wgHnSp8UaQOapqPV/ B7/8oXb7MQGx8HVOHpl9JebnOSwJexrpnS31w6wLMYnE9FozR4ZrSRz5JT/BBKiL/DSA tmcu9fElaxjm/kLJ9U1Ns3F4YzrN7Ggx4irwg+mgjvWxSmN2WFjqxH8WDBuzjAoOCq2N hbbA== Received: by 10.68.232.197 with SMTP id tq5mr56266726pbc.53.1341808631827; Sun, 08 Jul 2012 21:37:11 -0700 (PDT) Received: from [10.0.0.63] (50-78-194-198-static.hfc.comcastbusiness.net. [50.78.194.198]) by mx.google.com with ESMTPS id nj4sm12980987pbc.5.2012.07.08.21.37.10 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 08 Jul 2012 21:37:11 -0700 (PDT) Sender: Warner Losh Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: Warner Losh In-Reply-To: Date: Sun, 8 Jul 2012 22:37:07 -0600 Content-Transfer-Encoding: quoted-printable Message-Id: <8048FFC5-6952-49FC-849D-EA1A5675ACBE@bsdimp.com> References: <31A0DCE7-3B93-41BC-805A-E0B163892112@bsdimp.com> <5C18109D-E7A8-4868-BEA9-26B63360BB24@bsdimp.com> To: Arnaud Lacombe X-Mailer: Apple Mail (2.1084) X-Gm-Message-State: ALoCoQlb6C92yxvcUlvqY1qXoYX1fQlTgyqnQishOfxeFY0PxUkUGNm9czYpoceCAF/kNaaBXgtB Cc: FreeBSD Hackers , FreeBSD Current Subject: Re: newbus' ivar's limitation.. X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Jul 2012 04:37:12 -0000 On Jul 8, 2012, at 9:46 PM, Arnaud Lacombe wrote: > On Sun, Jul 8, 2012 at 11:31 PM, Warner Losh wrote: >>=20 >> On Jul 8, 2012, at 9:26 PM, Arnaud Lacombe wrote: >>=20 >>> Hi, >>>=20 >>> On Sun, Jul 8, 2012 at 10:07 PM, Warner Losh wrote: >>>>=20 >>>> On Jul 8, 2012, at 7:22 PM, Arnaud Lacombe wrote: >>>>> Ok, yet another Newbus' limitation. Assuming a device exports more >>>>> than one interface, and one of its child has need to use more than = one >>>>> interface, each interfaces cannot register, concurrently, its own >>>>> ivar. While I try to always have a single child per >>>>> interface/resource, I need to keep some compatibility with the old = way >>>>> of doing thing (POLA wrt. drivers I cannot/will not convert and >>>>> userland). So, it would have been nice if ivar had been = per-interface, >>>>> not global and unique to one device. >>>>=20 >>>> There's one pointer for the ivars. The bus code gets to determine = what the ivar looks like, because the interface is totally private to = the bus. So long as it returns the right thing for any key that's = presented, it doesn't matter quite how things are done. >>>>=20 >>>> So I'm not sure I understand what you are saying here. >>>>=20 >>> dev0 implements two interfaces: A and B. dev1, child of dev0, needs = to >>> use both interfaces. There is no generic way for dev0 to export >>> independent ivars for both interface. For now, I restricted the >>> function of the second interface not to need ivar, but that's kind = of >>> hackish. >>=20 >> Only if the IVARs for interface A and interface B have overlapping = values. If the Ivar keys don't overlap, then there's no problems at = all. Certainly less hackish than not using them at all. Since dev0 = knows the layout of the ivar that it set on its child, this presents no = problems at all. It would return the values from A from the right part = of the ivar, and those from B in the right part. Apart from the = coordination of Ivar numbers, as I outlined in my last post, there's no = issue here. >>=20 > I think we should not be talking about the same API here. I have no > idea what you mean by "the key to value translation", nor "Ivar > numbers". What I refer to is that device_set_ivars() / > device_get_ivars() acts on a single instance variables from `struct > device': `ivars'. In that case, I do not really see how to set that > specific field to two distinct values for each interfaces. We are talking about the ivar interface. You are just misunderstanding = how it is used. Let us say that dev0 wants to implement interface A and B. The = interface it implements is the device_read _ivar method. That method = looks like: int pci_read_ivar(device_t dev, device_t child, int which, uintptr_t = *result) { struct pci_ivar *iv =3D device_get_ivar(child); switch (which) { case PCI_IVAR_SUBVENDOR: *result =3D iv->subvendor; break; ... default: return EINVAL; } return 0; } So, if you wanted to implement interface A and interface B, you would = have to support reading and writing A_IVAR_* and B_IVAR_*. The above = routine would look like: struct mydev_ivar { int a_1; int a_2; int b_1; int b_2; }; int mydev_read_ivar(device_t dev, device child, int which, uintpr_t = *result) { struct mydev_ivar *iv =3D device_get_ivar(child); switch (which) { case A_IVAR_1: *result =3D iv->a_1; break; case A_IVAR_2: *result =3D iv->a_2; break; case B_IVAR_1: *result =3D iv->b_1; break; case B_IVAR_2: *result =3D iv->b_2; break; default: return EINVAL; } return 0; } and similar for the write routine. Now, there are some busses that = provide convenience structures and functions for dealing with the ivars. = In that case, you'd be able to use the wrapper structures and routines = like so: struct mdev_ivar { struct a_ivar a; struct b_ivar b; }; int mydev_read_ivar(device_t dev, device child, int which, uintpr_t = *result) { struct mydev_ivar *iv =3D device_get_ivar(child); switch (which) { case A_IVAR_1: case A_IVAR_2: return a_helper_read_ivar(&iv->a, which, result); case B_IVAR_1: case B_IVAR_2: return b_helper_read_ivar(&iv->b, which, result); default: return EINVAL; } return 0; } Both of these work only if the A_IVAR values are disjoint from the = B_IVAR values. Your mydev device is the one that sets the ivar on dev1, = in your example, so it would know how to malloc and create the = mydev_ivar structure, even if the interfaces it is implementing provide = helper functions, like I've outlined above. Warner > - Arnaud >=20 >> Warner >>=20 >>> - Arnaud >>>=20 >>>> The problem, more basically, is that the ivar keys are not unique. = Currently, there's no bits used in the key to define the values to be = non-overlapping. For example: >>>> enum pci_device_ivars { >>>> PCI_IVAR_SUBVENDOR, >>>> PCI_IVAR_SUBDEVICE, >>>> PCI_IVAR_VENDOR, >>>> .... >>>> }; >>>>=20 >>>> We could easily reserve the upper 16-bits of this field to be that = key. This value could then be used to differentiate them. But this = wouldn't scale too well. Given that there's only about a dozen or two = in the tree, that's right at the moment, it wouldn't be hard to do = something like: >>>>=20 >>>> enum ivar_namespace { >>>> IVAR_PCI =3D 1, >>>> IVAR_PCCARD, >>>> IVAR_USB, >>>> etc >>>> }; >>>> #define IVAR_SHIFT 16 >>>>=20 >>>> and the above could be changed to: >>>>=20 >>>> enum pci_device_ivars { >>>> PCI_IVAR_SUBVENDOR =3D IVAR_PCI << IVAR_SHIFT, >>>> PCI_IVAR_SUBDEVICE, >>>> PCI_IVAR_VENDOR, >>>> .... >>>> }; >>>>=20 >>>> and then we'd have an unambiguous key, and the bus could easily = implement multiple interfaces. >>>>=20 >>>> but then again, most of the existing interfaces in the kernel are = mutually exclusive, so you could implement this just for your new = interfaces. >>>>=20 >>>>> Unless I am mistaken, ivar are the only way for a parent can = transmit >>>>> information to a child. I can not simply implement a new METHOD to = get >>>>> that ivar as the device implements multiple time the same function >>>>> (actually, up to 4 time for one, 3 for the other, with possible >>>>> crossovers...), each one physically distinct. Each child is being = tied >>>>> to a pair. Thus, I need to pass each child discriminator(s) for = each >>>>> interfaces right after having been *created*, which cannot be done >>>>> later on. Of course, it is out-of-question to have crossover in = the >>>>> interfaces definitions. >>>>=20 >>>> ivars are but one way to communicate this. However, they are the = generic way to convert a key to a value and store a key on a value. I = don't really understand what you are trying to say here, perhaps an = example would help illustrate what you are trying to do, since I don't = quite understand the problem here. >>>>=20 >>>>> The best way I could achieve this currently is to pass the child's >>>>> device to its parent, and do a lookup based on that pointer to get >>>>> information I need, but erk.... >>>>=20 >>>> That doesn't make any sense. The child's parent already sets that = child's ivar when the child is created. The child's parent already gets = a pointer to the child when asked to do the key to value translation. = Again, perhaps an example would help here. >>>>=20 >>>> Warner >>=20