Re: [PATCH] pcictl: simplify its usage

[Christoph Egger <Christoph_Egger%gmx.de@localhost>]

Manuel Bouyer wrote:
> On Thu, Jun 04, 2009 at 11:05:21PM +0200, Christoph Egger wrote:
>> Manuel Bouyer wrote:
>>> On Thu, Jun 04, 2009 at 10:28:18PM +0200, Christoph Egger wrote:
>>>>> But what we have right now does work !
>>>>> You just need to use the autoconf index instead of the bus number.
>>>>>
>>>>> Also, minor(dev) is not the PCI bus domain but the autoconf index.
>>>> Can I assume, that the 'i' iterator matches the autoconf index?
>>>> If yes, then I can compare 'i' with minor(dev).
>>> Yes, and device_lookup() and device_lookup_private() does it for you.
>>> But with this we're back to the old code. You have to use different
>>> /dev/pciN for the different busses. I don't see this as a problem;
>>> I think all what you need is the list of PCI busses (list of autoconf 
>>> indexes)
>>> which you could get by a specific ioctl (either a "get the list" ioctl,
>>> or a "get the next bus" ioctl).
>> At work, there's a 4-way Opteron machine having two primary PCI roots.
>> You find the second one via ACPI. The pciN starts with 128 on that
>> machine. Should we have 255 /dev/pciN device files then?
> 
> You mean that you have more than 128 PCI busses on this machine ?
> Or just that you have
> pci1 at xxx bus 128 ?
> In this case, you'll access it with /dev/pci1 and not /dev/pci128
> It if's really pci128 at xxx bus 128 then there's something I don't
> understand in autoconf, and I'd like to see the dmesg for this machine !
> 

I still haven't access to that machine yet. So I still have to owe you
the answer.

In this discussion, it has been proposed more than once to extend the
pciio ioctl. Also the real root issue has been identified: We don't
support PCI domains.

In attached diff I propose three new ioctl which deprecate
PCI_IOC_BDF_CFGREAD, PCI_IOC_BDF_CFGWRITE and PCI_IOC_BUSINFO.

I also would like to introduce a new argument which specifies
the PCI domain. But I am unsure how to name it since -d is already
in use for 'device'. If we can use -p for 'pcidomain' then we
go do:

pcictl list -p 0

to enlist all busses and devices from pci domain 0.

pcictl list

enlists all domains, busses and devices.
This requires to change/extend pci_conf_print(3) how to specify
the domain and how to print the pci domain. Any proposals?
I am not sure about the other functions in pci(3).

On the kernel side, autoconf index will be used for PCI domains
until we have them for real.
Whenever we get PCI domains, we have to go this step anyway.
Doing it now, will save us on changing the kernel ABI/API another time.

Christoph

Index: sys/dev/pci/pciio.h
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/pciio.h,v
retrieving revision 1.3
diff -u -p -r1.3 pciio.h
--- sys/dev/pci/pciio.h 6 Jun 2009 12:56:43 -0000       1.3
+++ sys/dev/pci/pciio.h 11 Jun 2009 08:49:35 -0000
@@ -93,4 +93,37 @@ struct pciio_businfo {
 
 #define        PCI_IOC_BUSINFO          _IOR('P', 4, struct pciio_businfo)
 
+/*
+ * pciio_dbdf_cfgreg {
+ *
+ *     Like pciio_bdf_cfgreg, except for any domain/bus/dev/func
+ */
+struct pciio_dbdf_cfgreg {
+       u_int   domain;
+       u_int   bus;
+       u_int   device;
+       u_int   function;
+       struct pciio_cfgreg cfgreg;
+};
+
+/*
+ * Read and write PCI configuration space registers on any
+ * device within a given PCI domain.
+ */
+#define        PCI_IOC_DBDF_CFGREAD    _IOWR('P', 5, struct pciio_dbdf_cfgreg)
+#define        PCI_IOC_DBDF_CFGWRITE    _IOW('P', 6, struct pciio_dbdf_cfgreg)
+
+/*
+ * pciio_domaininfo:
+ *
+ *     Information for a PCI domain (autoconfiguration node) instance.
+ */
+struct pciio_domaininfo {
+       u_int   domain;         /* pci domain */
+       u_int   busno;          /* bus number */
+       u_int   maxdevs;        /* max devices on bus */
+};
+
+#define        PCI_IOC_DOMAININFO      _IOR('P', 7, struct pciio_domaininfo)
+
 #endif /* _DEV_PCI_PCIIO_H_ */