On Sun, Apr 02, 2023 at 07:09:40PM +0000, Miod Vallat wrote:
> pci_{io,mem}_find() have intentionally been left undocumented,
> developers being (rightfully) advised to use pci_mapreg_info() instead.
>
> The following diff removes these undocumented functions and converts
> their last few users.
>
> Bonus changes introduced while looking into this:
> - correctly cope with 64-bit memory bars in ppb, should there ever
> exist (I honestly believe that, should that be the case, this would
> have been noticed with failures to detect or attach devices in the
> past).
> - do not hardcode memory type for rtsx (although the fact the driver
> works implies that this isn't likely to ever be an I/O bar).
pci.c part should be a different commit
ok jsg@
>
> Index: share/man/man9/pci_mapreg_map.9
> ===================================================================
> RCS file: /cvs/src/share/man/man9/pci_mapreg_map.9,v
> retrieving revision 1.1
> diff -u -p -u -p -r1.1 pci_mapreg_map.9
> --- share/man/man9/pci_mapreg_map.9 23 Feb 2019 04:54:25 -0000 1.1
> +++ share/man/man9/pci_mapreg_map.9 1 Apr 2023 17:15:36 -0000
> @@ -23,8 +23,6 @@
> .Nm pci_mapreg_info ,
> .Nm pci_mapreg_probe ,
> .Nm pci_mapreg_type
> -.\" .Nm pci_mem_find ,
> -.\" .Nm pci_io_find
> .Nd PCI register mappings
> .Sh SYNOPSIS
> .In dev/pci/pcivar.h
> @@ -63,24 +61,6 @@
> .Fa "pcitag_t tag"
> .Fa "int reg"
> .Fc
> -.\" .Ft int
> -.\" .Fo pci_mem_find
> -.\" .Fa "pci_chipset_tag_t pc"
> -.\" .Fa "pcitag_t pcitag"
> -.\" .Fa "int reg"
> -.\" .Fa "bus_addr_t *basep"
> -.\" .Fa "bus_size_t *sizep"
> -.\" .Fa "int *cacheablep"
> -.\" .Fc
> -.\" .Ft int
> -.\" .Fo pci_io_find
> -.\" .Fa "pci_chipset_tag_t pc"
> -.\" .Fa "pcitag_t pcitag"
> -.\" .Fa "int reg"
> -.\" .Fa "bus_addr_t *basep"
> -.\" .Fa "bus_size_t *sizep"
> -.\" .Fa "int *cacheablep"
> -.\" .Fc
> .Sh DESCRIPTION
> These functions provide wrappers and helpers around
> .Xr bus_space 9
> Index: sys/arch/hppa/dev/sti_pci_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/dev/sti_pci_machdep.c,v
> retrieving revision 1.2
> diff -u -p -u -p -r1.2 sti_pci_machdep.c
> --- sys/arch/hppa/dev/sti_pci_machdep.c 10 Apr 2009 17:11:27 -0000
> 1.2
> +++ sys/arch/hppa/dev/sti_pci_machdep.c 1 Apr 2023 17:15:37 -0000
> @@ -51,22 +51,13 @@ sti_pci_is_console(struct pci_attach_arg
> * matches what PAGE0 says, then we are the console, and it
> * doesn't matter which BAR matched.
> */
> - for (bar = PCI_MAPREG_START; bar <= PCI_MAPREG_PPB_END; ) {
> + for (bar = PCI_MAPREG_START; bar <= PCI_MAPREG_PPB_END; bar += 4) {
> cf = pci_conf_read(paa->pa_pc, paa->pa_tag, bar);
> -
> - if (PCI_MAPREG_TYPE(cf) == PCI_MAPREG_TYPE_IO) {
> - rc = pci_io_find(paa->pa_pc, paa->pa_tag, bar, &addr,
> - NULL);
> + rc = pci_mapreg_info(paa->pa_pc, paa->pa_tag, bar,
> + _PCI_MAPREG_TYPEBITS(cf), &addr, NULL, NULL);
> + if (PCI_MAPREG_TYPE(cf) == PCI_MAPREG_TYPE_MEM &&
> + PCI_MAPREG_MEM_TYPE(cf) == PCI_MAPREG_MEM_TYPE_64BIT)
> bar += 4;
> - } else {
> - rc = pci_mem_find(paa->pa_pc, paa->pa_tag, bar, &addr,
> - NULL, NULL);
> - if (PCI_MAPREG_MEM_TYPE(cf) ==
> - PCI_MAPREG_MEM_TYPE_64BIT)
> - bar += 8;
> - else
> - bar += 4;
> - }
>
> if (rc == 0 &&
> (hppa_hpa_t)addr == (hppa_hpa_t)PAGE0->mem_cons.pz_hpa)
> Index: sys/arch/macppc/pci/vgafb.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/macppc/pci/vgafb.c,v
> retrieving revision 1.64
> diff -u -p -u -p -r1.64 vgafb.c
> --- sys/arch/macppc/pci/vgafb.c 31 Dec 2022 05:06:18 -0000 1.64
> +++ sys/arch/macppc/pci/vgafb.c 1 Apr 2023 17:15:37 -0000
> @@ -508,7 +508,7 @@ vgafb_mapregs(struct vgafb_softc *sc, st
> bus_addr_t ba;
> bus_size_t bs;
> int hasmem = 0, hasmmio = 0;
> - uint32_t i, cf;
> + uint32_t bar, cf;
> int rv;
>
> /*
> @@ -517,12 +517,12 @@ vgafb_mapregs(struct vgafb_softc *sc, st
> * For nvidia, this finds mmio 0x10 and frame memory 0x14.
> * Some nvidias have a 3rd mem region 0x18, which we ignore.
> */
> - for (i = PCI_MAPREG_START; i <= PCI_MAPREG_PPB_END; i += 4) {
> - cf = pci_conf_read(pa->pa_pc, pa->pa_tag, i);
> + for (bar = PCI_MAPREG_START; bar <= PCI_MAPREG_PPB_END; bar += 4) {
> + cf = pci_conf_read(pa->pa_pc, pa->pa_tag, bar);
> if (PCI_MAPREG_TYPE(cf) == PCI_MAPREG_TYPE_MEM) {
> /* Memory mapping... frame memory or mmio? */
> - rv = pci_mem_find(pa->pa_pc, pa->pa_tag, i,
> - &ba, &bs, NULL);
> + rv = pci_mapreg_info(pa->pa_pc, pa->pa_tag, bar,
> + _PCI_MAPREG_TYPEBITS(cf), &ba, &bs, NULL);
> if (rv != 0)
> continue;
>
> @@ -558,6 +558,9 @@ vgafb_mapregs(struct vgafb_softc *sc, st
> /* Ignore any other mem region. */
> break;
> }
> + if (PCI_MAPREG_MEM_TYPE(cf) ==
> + PCI_MAPREG_MEM_TYPE_64BIT)
> + bar += 4;
> }
> }
>
> Index: sys/arch/sparc64/dev/vgafb.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/sparc64/dev/vgafb.c,v
> retrieving revision 1.68
> diff -u -p -u -p -r1.68 vgafb.c
> --- sys/arch/sparc64/dev/vgafb.c 15 Jul 2022 17:57:26 -0000 1.68
> +++ sys/arch/sparc64/dev/vgafb.c 1 Apr 2023 17:15:37 -0000
> @@ -377,31 +377,33 @@ vgafb_mapregs(struct vgafb_softc *sc, st
> bus_addr_t ba;
> bus_size_t bs;
> int hasio = 0, hasmem = 0, hasmmio = 0;
> - u_int32_t i, cf;
> + u_int32_t bar, cf;
> int rv;
>
> - for (i = PCI_MAPREG_START; i <= PCI_MAPREG_PPB_END; i += 4) {
> - cf = pci_conf_read(pa->pa_pc, pa->pa_tag, i);
> + for (bar = PCI_MAPREG_START; bar <= PCI_MAPREG_PPB_END; bar += 4) {
> + cf = pci_conf_read(pa->pa_pc, pa->pa_tag, bar);
> if (PCI_MAPREG_TYPE(cf) == PCI_MAPREG_TYPE_IO) {
> if (hasio)
> continue;
> - rv = pci_io_find(pa->pa_pc, pa->pa_tag, i,
> - &sc->sc_io_addr, &sc->sc_io_size);
> + rv = pci_mapreg_info(pa->pa_pc, pa->pa_tag, bar,
> + _PCI_MAPREG_TYPEBITS(cf),
> + &sc->sc_io_addr, &sc->sc_io_size, NULL);
> if (rv != 0) {
> if (rv != ENOENT)
> printf("%s: failed to find io at
> 0x%x\n",
> - sc->sc_sunfb.sf_dev.dv_xname, i);
> + sc->sc_sunfb.sf_dev.dv_xname, bar);
> continue;
> }
> hasio = 1;
> + bar += 4;
> } else {
> /* Memory mapping... frame memory or mmio? */
> - rv = pci_mem_find(pa->pa_pc, pa->pa_tag, i,
> - &ba, &bs, NULL);
> + rv = pci_mapreg_info(pa->pa_pc, pa->pa_tag, bar,
> + _PCI_MAPREG_TYPEBITS(cf), &ba, &bs, NULL);
> if (rv != 0) {
> if (rv != ENOENT)
> printf("%s: failed to find mem at
> 0x%x\n",
> - sc->sc_sunfb.sf_dev.dv_xname, i);
> + sc->sc_sunfb.sf_dev.dv_xname, bar);
> continue;
> }
>
> @@ -435,6 +437,9 @@ vgafb_mapregs(struct vgafb_softc *sc, st
> sc->sc_mem_size = bs;
> }
> }
> + if (PCI_MAPREG_MEM_TYPE(cf) ==
> + PCI_MAPREG_MEM_TYPE_64BIT)
> + bar += 4;
> }
> }
>
> Index: sys/dev/pci/pci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/pci.c,v
> retrieving revision 1.126
> diff -u -p -u -p -r1.126 pci.c
> --- sys/dev/pci/pci.c 27 Nov 2022 22:55:31 -0000 1.126
> +++ sys/dev/pci/pci.c 1 Apr 2023 17:15:38 -0000
> @@ -929,7 +929,7 @@ pci_reserve_resources(struct pci_attach_
> #ifdef __sparc64__
> /*
> * Certain SPARC T5 systems assign
> - * non-prefetchable 64-bit BARs of its onboard
> + * non-prefetchable 64-bit BARs of their onboard
> * mpii(4) controllers addresses in the
> * prefetchable memory range. This is
> * (probably) safe, as reads from the device
> Index: sys/dev/pci/pci_map.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/pci_map.c,v
> retrieving revision 1.32
> diff -u -p -u -p -r1.32 pci_map.c
> --- sys/dev/pci/pci_map.c 17 Jun 2019 11:04:06 -0000 1.32
> +++ sys/dev/pci/pci_map.c 1 Apr 2023 17:15:38 -0000
> @@ -252,21 +252,6 @@ obsd_pci_mem_find(pci_chipset_tag_t pc,
> BUS_SPACE_MAP_PREFETCHABLE : 0;
>
> return (0);
> -}
> -
> -int
> -pci_io_find(pci_chipset_tag_t pc, pcitag_t pcitag, int reg,
> - bus_addr_t *iobasep, bus_size_t *iosizep)
> -{
> - return (obsd_pci_io_find(pc, pcitag, reg, 0, iobasep, iosizep, 0));
> -}
> -
> -int
> -pci_mem_find(pci_chipset_tag_t pc, pcitag_t pcitag, int reg,
> - bus_addr_t *membasep, bus_size_t *memsizep, int *cacheablep)
> -{
> - return (obsd_pci_mem_find(pc, pcitag, reg, -1, membasep, memsizep,
> - cacheablep));
> }
>
> pcireg_t
> Index: sys/dev/pci/pcivar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/pcivar.h,v
> retrieving revision 1.77
> diff -u -p -u -p -r1.77 pcivar.h
> --- sys/dev/pci/pcivar.h 4 Jan 2022 20:43:44 -0000 1.77
> +++ sys/dev/pci/pcivar.h 1 Apr 2023 17:15:38 -0000
> @@ -231,11 +231,6 @@ int pci_mapreg_map(struct pci_attach_arg
> bus_space_tag_t *, bus_space_handle_t *, bus_addr_t *,
> bus_size_t *, bus_size_t);
>
> -int pci_io_find(pci_chipset_tag_t, pcitag_t, int, bus_addr_t *,
> - bus_size_t *);
> -int pci_mem_find(pci_chipset_tag_t, pcitag_t, int, bus_addr_t *,
> - bus_size_t *, int *);
> -
> int pci_get_capability(pci_chipset_tag_t, pcitag_t, int,
> int *, pcireg_t *);
> int pci_get_ht_capability(pci_chipset_tag_t, pcitag_t, int,
> Index: sys/dev/pci/ppb.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/ppb.c,v
> retrieving revision 1.71
> diff -u -p -u -p -r1.71 ppb.c
> --- sys/dev/pci/ppb.c 20 Jan 2023 15:11:44 -0000 1.71
> +++ sys/dev/pci/ppb.c 1 Apr 2023 17:15:38 -0000
> @@ -637,6 +637,10 @@ ppb_alloc_resources(struct ppb_softc *sc
> io_count++;
> else
> mem_count++;
> +
> + if (type == (PCI_MAPREG_TYPE_MEM |
> + PCI_MAPREG_MEM_TYPE_64BIT))
> + reg += 4;
> }
>
> if (reg_rom != 0) {
> Index: sys/dev/pci/rtsx_pci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/rtsx_pci.c,v
> retrieving revision 1.15
> diff -u -p -u -p -r1.15 rtsx_pci.c
> --- sys/dev/pci/rtsx_pci.c 11 Mar 2022 18:00:51 -0000 1.15
> +++ sys/dev/pci/rtsx_pci.c 1 Apr 2023 17:15:38 -0000
> @@ -83,6 +83,7 @@ rtsx_pci_attach(struct device *parent, s
> bus_size_t size;
> int flags;
> int bar = RTSX_PCI_BAR;
> + pcireg_t type;
>
> if ((pci_conf_read(pa->pa_pc, pa->pa_tag, RTSX_CFG_PCI)
> & RTSX_CFG_ASIC) != 0) {
> @@ -121,13 +122,13 @@ rtsx_pci_attach(struct device *parent, s
> break;
> }
>
> - if (pci_mem_find(pa->pa_pc, pa->pa_tag, bar, NULL, NULL, NULL) != 0) {
> + type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, bar);
> + if (pci_mapreg_info(pa->pa_pc, pa->pa_tag, bar, type, NULL, NULL,
> + NULL) != 0) {
> printf("%s: can't find registers\n", sc->sc.sc_dev.dv_xname);
> return;
> }
> -
> - if (pci_mapreg_map(pa, bar, PCI_MAPREG_TYPE_MEM, 0, &iot, &ioh, NULL,
> - &size, 0)) {
> + if (pci_mapreg_map(pa, bar, type, 0, &iot, &ioh, NULL, &size, 0)) {
> printf("%s: can't map registers\n", sc->sc.sc_dev.dv_xname);
> return;
> }
> Index: sys/dev/pci/sti_pci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/sti_pci.c,v
> retrieving revision 1.11
> diff -u -p -u -p -r1.11 sti_pci.c
> --- sys/dev/pci/sti_pci.c 20 Feb 2023 11:31:16 -0000 1.11
> +++ sys/dev/pci/sti_pci.c 1 Apr 2023 17:15:38 -0000
> @@ -316,7 +316,7 @@ sti_readbar(struct sti_softc *sc, struct
> {
> bus_addr_t addr;
> bus_size_t size;
> - u_int32_t cf;
> + pcireg_t type;
> int rc;
>
> if (bar == 0) {
> @@ -332,15 +332,9 @@ sti_readbar(struct sti_softc *sc, struct
> return EINVAL;
> }
>
> - cf = pci_conf_read(pa->pa_pc, pa->pa_tag, bar);
> -
> - if (PCI_MAPREG_TYPE(cf) == PCI_MAPREG_TYPE_IO) {
> - rc = pci_io_find(pa->pa_pc, pa->pa_tag, bar,
> - &addr, &size);
> - } else {
> - rc = pci_mem_find(pa->pa_pc, pa->pa_tag, bar,
> - &addr, &size, NULL);
> - }
> + type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, bar);
> + rc = pci_mapreg_info(pa->pa_pc, pa->pa_tag, bar, type, &addr, &size,
> + NULL);
> if (rc != 0) {
> printf("%s: invalid bar %02x for region %d\n",
> sc->sc_dev.dv_xname, bar, region);
>
>