> From: "Theo de Raadt" <dera...@openbsd.org> > Date: Fri, 31 May 2019 15:34:18 -0600 > > > On arm64, pci_intr_handle_t is a pointer to an opaque struct. > > That's a subtle trap. How would someone realize the order is wrong... > > Would it not be better if this was done like the other architectures, > where the pci_intr_handle_t is a structure, not a pointer. > > On arm64, the pci subsystems seem to use structures which are > the same, and additional fields could be added easily if there > was a need later.
Yes. I suppose we anticipated that we would need different structs for different host bridge drivers, but so far that hasn't happened. I'm fairly confident that what we have now is sufficient for most other hardware that we would like to support. This actually allows me to remove some duplicated code. ok? Index: dev/fdt/dwpcie.c =================================================================== RCS file: /cvs/src/sys/dev/fdt/dwpcie.c,v retrieving revision 1.13 diff -u -p -r1.13 dwpcie.c --- dev/fdt/dwpcie.c 31 May 2019 10:35:49 -0000 1.13 +++ dev/fdt/dwpcie.c 1 Jun 2019 18:00:30 -0000 @@ -224,9 +224,6 @@ pcireg_t dwpcie_conf_read(void *, pcitag void dwpcie_conf_write(void *, pcitag_t, int, pcireg_t); int dwpcie_intr_map(struct pci_attach_args *, pci_intr_handle_t *); -int dwpcie_intr_map_msi(struct pci_attach_args *, pci_intr_handle_t *); -int dwpcie_intr_map_msix(struct pci_attach_args *, int, - pci_intr_handle_t *); const char *dwpcie_intr_string(void *, pci_intr_handle_t); void *dwpcie_intr_establish(void *, pci_intr_handle_t, int, int (*)(void *), void *, char *); @@ -453,8 +450,8 @@ dwpcie_attach(struct device *parent, str sc->sc_pc.pc_intr_v = sc; sc->sc_pc.pc_intr_map = dwpcie_intr_map; - sc->sc_pc.pc_intr_map_msi = dwpcie_intr_map_msi; - sc->sc_pc.pc_intr_map_msix = dwpcie_intr_map_msix; + sc->sc_pc.pc_intr_map_msi = _pci_intr_map_msi; + sc->sc_pc.pc_intr_map_msix = _pci_intr_map_msix; sc->sc_pc.pc_intr_string = dwpcie_intr_string; sc->sc_pc.pc_intr_establish = dwpcie_intr_establish; sc->sc_pc.pc_intr_disestablish = dwpcie_intr_disestablish; @@ -903,21 +900,9 @@ dwpcie_conf_write(void *v, pcitag_t tag, sc->sc_io_bus_addr, sc->sc_io_size); } -#define PCI_INTX 0 -#define PCI_MSI 1 -#define PCI_MSIX 2 - -struct dwpcie_intr_handle { - pci_chipset_tag_t ih_pc; - pcitag_t ih_tag; - int ih_intrpin; - int ih_type; -}; - int dwpcie_intr_map(struct pci_attach_args *pa, pci_intr_handle_t *ihp) { - struct dwpcie_intr_handle *ih; int pin = pa->pa_rawintrpin; if (pin == 0 || pin > PCI_INTERRUPT_PIN_MAX) @@ -926,68 +911,18 @@ dwpcie_intr_map(struct pci_attach_args * if (pa->pa_tag == 0) return -1; - ih = malloc(sizeof(struct dwpcie_intr_handle), M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_intrtag; - ih->ih_intrpin = pa->pa_intrpin; - ih->ih_type = PCI_INTX; - *ihp = (pci_intr_handle_t)ih; - - return 0; -} - -int -dwpcie_intr_map_msi(struct pci_attach_args *pa, pci_intr_handle_t *ihp) -{ - pci_chipset_tag_t pc = pa->pa_pc; - pcitag_t tag = pa->pa_tag; - struct dwpcie_intr_handle *ih; - - if ((pa->pa_flags & PCI_FLAGS_MSI_ENABLED) == 0 || - pci_get_capability(pc, tag, PCI_CAP_MSI, NULL, NULL) == 0) - return -1; - - ih = malloc(sizeof(struct dwpcie_intr_handle), M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_tag; - ih->ih_type = PCI_MSI; - *ihp = (pci_intr_handle_t)ih; - - return 0; -} - -int -dwpcie_intr_map_msix(struct pci_attach_args *pa, int vec, - pci_intr_handle_t *ihp) -{ - pci_chipset_tag_t pc = pa->pa_pc; - pcitag_t tag = pa->pa_tag; - struct dwpcie_intr_handle *ih; - pcireg_t reg; - - if ((pa->pa_flags & PCI_FLAGS_MSI_ENABLED) == 0 || - pci_get_capability(pc, tag, PCI_CAP_MSIX, NULL, ®) == 0) - return -1; - - if (vec > PCI_MSIX_MC_TBLSZ(reg)) - return -1; - - ih = malloc(sizeof(struct dwpcie_intr_handle), M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_tag; - ih->ih_intrpin = vec; - ih->ih_type = PCI_MSIX; - *ihp = (pci_intr_handle_t)ih; + ihp->ih_pc = pa->pa_pc; + ihp->ih_tag = pa->pa_intrtag; + ihp->ih_intrpin = pa->pa_intrpin; + ihp->ih_type = PCI_INTX; return 0; } const char * -dwpcie_intr_string(void *v, pci_intr_handle_t ihp) +dwpcie_intr_string(void *v, pci_intr_handle_t ih) { - struct dwpcie_intr_handle *ih = (struct dwpcie_intr_handle *)ihp; - - switch (ih->ih_type) { + switch (ih.ih_type) { case PCI_MSI: return "msi"; case PCI_MSIX: @@ -998,18 +933,19 @@ dwpcie_intr_string(void *v, pci_intr_han } void * -dwpcie_intr_establish(void *v, pci_intr_handle_t ihp, int level, +dwpcie_intr_establish(void *v, pci_intr_handle_t ih, int level, int (*func)(void *), void *arg, char *name) { struct dwpcie_softc *sc = v; - struct dwpcie_intr_handle *ih = (struct dwpcie_intr_handle *)ihp; void *cookie; - if (ih->ih_type != PCI_INTX) { + KASSERT(ih.ih_type != PCI_NONE); + + if (ih.ih_type != PCI_INTX) { uint64_t addr, data; /* Assume hardware passes Requester ID as sideband data. */ - data = pci_requester_id(ih->ih_pc, ih->ih_tag); + data = pci_requester_id(ih.ih_pc, ih.ih_tag); cookie = fdt_intr_establish_msi(sc->sc_node, &addr, &data, level, func, arg, (void *)name); if (cookie == NULL) @@ -1017,26 +953,25 @@ dwpcie_intr_establish(void *v, pci_intr_ /* TODO: translate address to the PCI device's view */ - if (ih->ih_type == PCI_MSIX) { - pci_msix_enable(ih->ih_pc, ih->ih_tag, - &sc->sc_bus_memt, ih->ih_intrpin, addr, data); + if (ih.ih_type == PCI_MSIX) { + pci_msix_enable(ih.ih_pc, ih.ih_tag, + &sc->sc_bus_memt, ih.ih_intrpin, addr, data); } else - pci_msi_enable(ih->ih_pc, ih->ih_tag, addr, data); + pci_msi_enable(ih.ih_pc, ih.ih_tag, addr, data); } else { int bus, dev, fn; uint32_t reg[4]; - dwpcie_decompose_tag(sc, ih->ih_tag, &bus, &dev, &fn); + dwpcie_decompose_tag(sc, ih.ih_tag, &bus, &dev, &fn); reg[0] = bus << 16 | dev << 11 | fn << 8; reg[1] = reg[2] = 0; - reg[3] = ih->ih_intrpin; + reg[3] = ih.ih_intrpin; cookie = fdt_intr_establish_imap(sc->sc_node, reg, sizeof(reg), level, func, arg, name); } - free(ih, M_DEVBUF, sizeof(struct dwpcie_intr_handle)); return cookie; } Index: dev/fdt/rkpcie.c =================================================================== RCS file: /cvs/src/sys/dev/fdt/rkpcie.c,v retrieving revision 1.7 diff -u -p -r1.7 rkpcie.c --- dev/fdt/rkpcie.c 31 May 2019 21:23:34 -0000 1.7 +++ dev/fdt/rkpcie.c 1 Jun 2019 18:00:30 -0000 @@ -137,9 +137,6 @@ pcireg_t rkpcie_conf_read(void *, pcitag void rkpcie_conf_write(void *, pcitag_t, int, pcireg_t); int rkpcie_intr_map(struct pci_attach_args *, pci_intr_handle_t *); -int rkpcie_intr_map_msi(struct pci_attach_args *, pci_intr_handle_t *); -int rkpcie_intr_map_msix(struct pci_attach_args *, int, - pci_intr_handle_t *); const char *rkpcie_intr_string(void *, pci_intr_handle_t); void *rkpcie_intr_establish(void *, pci_intr_handle_t, int, int (*)(void *), void *, char *); @@ -289,8 +286,8 @@ rkpcie_attach(struct device *parent, str sc->sc_pc.pc_intr_v = sc; sc->sc_pc.pc_intr_map = rkpcie_intr_map; - sc->sc_pc.pc_intr_map_msi = rkpcie_intr_map_msi; - sc->sc_pc.pc_intr_map_msix = rkpcie_intr_map_msix; + sc->sc_pc.pc_intr_map_msi = _pci_intr_map_msi; + sc->sc_pc.pc_intr_map_msix = _pci_intr_map_msix; sc->sc_pc.pc_intr_string = rkpcie_intr_string; sc->sc_pc.pc_intr_establish = rkpcie_intr_establish; sc->sc_pc.pc_intr_disestablish = rkpcie_intr_disestablish; @@ -475,21 +472,9 @@ rkpcie_conf_write(void *v, pcitag_t tag, } } -#define PCI_INTX 0 -#define PCI_MSI 1 -#define PCI_MSIX 2 - -struct rkpcie_intr_handle { - pci_chipset_tag_t ih_pc; - pcitag_t ih_tag; - int ih_intrpin; - int ih_type; -}; - int rkpcie_intr_map(struct pci_attach_args *pa, pci_intr_handle_t *ihp) { - struct rkpcie_intr_handle *ih; int pin = pa->pa_rawintrpin; if (pin == 0 || pin > PCI_INTERRUPT_PIN_MAX) @@ -498,69 +483,18 @@ rkpcie_intr_map(struct pci_attach_args * if (pa->pa_tag == 0) return -1; - ih = malloc(sizeof(struct rkpcie_intr_handle), M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_intrtag; - ih->ih_intrpin = pa->pa_intrpin; - ih->ih_type = PCI_INTX; - *ihp = (pci_intr_handle_t)ih; - - return 0; -} - -int -rkpcie_intr_map_msi(struct pci_attach_args *pa, pci_intr_handle_t *ihp) -{ - pci_chipset_tag_t pc = pa->pa_pc; - pcitag_t tag = pa->pa_tag; - struct rkpcie_intr_handle *ih; - - if ((pa->pa_flags & PCI_FLAGS_MSI_ENABLED) == 0 || - pci_get_capability(pc, tag, PCI_CAP_MSI, NULL, NULL) == 0) - return -1; - - ih = malloc(sizeof(struct rkpcie_intr_handle), M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_tag; - ih->ih_intrpin = pa->pa_intrpin; - ih->ih_type = PCI_MSI; - *ihp = (pci_intr_handle_t)ih; - - return 0; -} - -int -rkpcie_intr_map_msix(struct pci_attach_args *pa, int vec, - pci_intr_handle_t *ihp) -{ - pci_chipset_tag_t pc = pa->pa_pc; - pcitag_t tag = pa->pa_tag; - struct rkpcie_intr_handle *ih; - pcireg_t reg; - - if ((pa->pa_flags & PCI_FLAGS_MSI_ENABLED) == 0 || - pci_get_capability(pc, tag, PCI_CAP_MSIX, NULL, ®) == 0) - return -1; - - if (vec > PCI_MSIX_MC_TBLSZ(reg)) - return -1; - - ih = malloc(sizeof(struct rkpcie_intr_handle), M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_tag; - ih->ih_intrpin = vec; - ih->ih_type = PCI_MSIX; - *ihp = (pci_intr_handle_t)ih; + ihp->ih_pc = pa->pa_pc; + ihp->ih_tag = pa->pa_intrtag; + ihp->ih_intrpin = pa->pa_intrpin; + ihp->ih_type = PCI_INTX; return 0; } const char * -rkpcie_intr_string(void *v, pci_intr_handle_t ihp) +rkpcie_intr_string(void *v, pci_intr_handle_t ih) { - struct rkpcie_intr_handle *ih = (struct rkpcie_intr_handle *)ihp; - - switch (ih->ih_type) { + switch (ih.ih_type) { case PCI_MSI: return "msi"; case PCI_MSIX: @@ -571,18 +505,19 @@ rkpcie_intr_string(void *v, pci_intr_han } void * -rkpcie_intr_establish(void *v, pci_intr_handle_t ihp, int level, +rkpcie_intr_establish(void *v, pci_intr_handle_t ih, int level, int (*func)(void *), void *arg, char *name) { struct rkpcie_softc *sc = v; - struct rkpcie_intr_handle *ih = (struct rkpcie_intr_handle *)ihp; void *cookie; - if (ih->ih_type != PCI_INTX) { + KASSERT(ih.ih_type != PCI_NONE); + + if (ih.ih_type != PCI_INTX) { uint64_t addr, data; /* Assume hardware passes Requester ID as sideband data. */ - data = pci_requester_id(ih->ih_pc, ih->ih_tag); + data = pci_requester_id(ih.ih_pc, ih.ih_tag); cookie = fdt_intr_establish_msi(sc->sc_node, &addr, &data, level, func, arg, name); if (cookie == NULL) @@ -590,11 +525,11 @@ rkpcie_intr_establish(void *v, pci_intr_ /* TODO: translate address to the PCI device's view */ - if (ih->ih_type == PCI_MSIX) { - pci_msix_enable(ih->ih_pc, ih->ih_tag, - sc->sc_iot, ih->ih_intrpin, addr, data); + if (ih.ih_type == PCI_MSIX) { + pci_msix_enable(ih.ih_pc, ih.ih_tag, + sc->sc_iot, ih.ih_intrpin, addr, data); } else - pci_msi_enable(ih->ih_pc, ih->ih_tag, addr, data); + pci_msi_enable(ih.ih_pc, ih.ih_tag, addr, data); } else { /* Unmask legacy interrupts. */ HWRITE4(sc, PCIE_CLIENT_INT_MASK, @@ -605,7 +540,6 @@ rkpcie_intr_establish(void *v, pci_intr_ func, arg, name); } - free(ih, M_DEVBUF, sizeof(struct rkpcie_intr_handle)); return cookie; } Index: arch/arm64/dev/acpipci.c =================================================================== RCS file: /cvs/src/sys/arch/arm64/dev/acpipci.c,v retrieving revision 1.9 diff -u -p -r1.9 acpipci.c --- arch/arm64/dev/acpipci.c 31 May 2019 08:02:04 -0000 1.9 +++ arch/arm64/dev/acpipci.c 1 Jun 2019 18:00:30 -0000 @@ -107,9 +107,6 @@ pcireg_t acpipci_conf_read(void *, pcita void acpipci_conf_write(void *, pcitag_t, int, pcireg_t); int acpipci_intr_map(struct pci_attach_args *, pci_intr_handle_t *); -int acpipci_intr_map_msi(struct pci_attach_args *, pci_intr_handle_t *); -int acpipci_intr_map_msix(struct pci_attach_args *, int, - pci_intr_handle_t *); const char *acpipci_intr_string(void *, pci_intr_handle_t); void *acpipci_intr_establish(void *, pci_intr_handle_t, int, int (*)(void *), void *, char *); @@ -183,8 +180,8 @@ acpipci_attach(struct device *parent, st sc->sc_pc->pc_intr_v = sc; sc->sc_pc->pc_intr_map = acpipci_intr_map; - sc->sc_pc->pc_intr_map_msi = acpipci_intr_map_msi; - sc->sc_pc->pc_intr_map_msix = acpipci_intr_map_msix; + sc->sc_pc->pc_intr_map_msi = _pci_intr_map_msi; + sc->sc_pc->pc_intr_map_msix = _pci_intr_map_msix; sc->sc_pc->pc_intr_string = acpipci_intr_string; sc->sc_pc->pc_intr_establish = acpipci_intr_establish; sc->sc_pc->pc_intr_disestablish = acpipci_intr_disestablish; @@ -334,36 +331,20 @@ acpipci_conf_write(void *v, pcitag_t tag bus_space_write_4(am->am_iot, am->am_ioh, tag | reg, data); } -#define PCI_INTX 0 -#define PCI_MSI 1 -#define PCI_MSIX 2 - -struct acpipci_intr_handle { - pci_chipset_tag_t ih_pc; - pcitag_t ih_tag; - int ih_intrpin; - int ih_type; -}; - int acpipci_intr_swizzle(struct pci_attach_args *pa, pci_intr_handle_t *ihp) { - struct acpipci_intr_handle *ih; int dev, swizpin; - if (pa->pa_bridgetag == NULL) + if (pa->pa_bridgeih == NULL) return -1; pci_decompose_tag(pa->pa_pc, pa->pa_tag, NULL, &dev, NULL); swizpin = PPB_INTERRUPT_SWIZZLE(pa->pa_rawintrpin, dev); - if ((void *)pa->pa_bridgeih[swizpin - 1] == NULL) + if (pa->pa_bridgeih[swizpin - 1].ih_type == PCI_NONE) return -1; - ih = malloc(sizeof(struct acpipci_intr_handle), M_DEVBUF, M_WAITOK); - memcpy(ih, (void *)pa->pa_bridgeih[swizpin - 1], - sizeof(struct acpipci_intr_handle)); - *ihp = (pci_intr_handle_t)ih; - + *ihp = pa->pa_bridgeih[swizpin - 1]; return 0; } @@ -374,7 +355,6 @@ acpipci_intr_map(struct pci_attach_args struct aml_node *node = sc->sc_node; struct aml_value res; uint64_t addr, pin, source, index; - struct acpipci_intr_handle *ih; int i; /* @@ -417,13 +397,10 @@ acpipci_intr_map(struct pci_attach_args pin != pa->pa_intrpin - 1 || source != 0) continue; - ih = malloc(sizeof(struct acpipci_intr_handle), - M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_tag; - ih->ih_intrpin = index; - ih->ih_type = PCI_INTX; - *ihp = (pci_intr_handle_t)ih; + ihp->ih_pc = pa->pa_pc; + ihp->ih_tag = pa->pa_tag; + ihp->ih_intrpin = index; + ihp->ih_type = PCI_INTX; return 0; } @@ -431,75 +408,27 @@ acpipci_intr_map(struct pci_attach_args return -1; } -int -acpipci_intr_map_msi(struct pci_attach_args *pa, pci_intr_handle_t *ihp) -{ - pci_chipset_tag_t pc = pa->pa_pc; - pcitag_t tag = pa->pa_tag; - struct acpipci_intr_handle *ih; - - if ((pa->pa_flags & PCI_FLAGS_MSI_ENABLED) == 0 || - pci_get_capability(pc, tag, PCI_CAP_MSI, NULL, NULL) == 0) - return -1; - - ih = malloc(sizeof(struct acpipci_intr_handle), M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_tag; - ih->ih_type = PCI_MSI; - *ihp = (pci_intr_handle_t)ih; - - return 0; -} - -int -acpipci_intr_map_msix(struct pci_attach_args *pa, int vec, - pci_intr_handle_t *ihp) -{ - pci_chipset_tag_t pc = pa->pa_pc; - pcitag_t tag = pa->pa_tag; - struct acpipci_intr_handle *ih; - pcireg_t reg; - - if ((pa->pa_flags & PCI_FLAGS_MSI_ENABLED) == 0 || - pci_get_capability(pc, tag, PCI_CAP_MSIX, NULL, ®) == 0) - return -1; - - if (vec > PCI_MSIX_MC_TBLSZ(reg)) - return -1; - - ih = malloc(sizeof(struct acpipci_intr_handle), M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_tag; - ih->ih_intrpin = vec; - ih->ih_type = PCI_MSIX; - *ihp = (pci_intr_handle_t)ih; - - return 0; -} - const char * -acpipci_intr_string(void *v, pci_intr_handle_t ihp) +acpipci_intr_string(void *v, pci_intr_handle_t ih) { - struct acpipci_intr_handle *ih = (struct acpipci_intr_handle *)ihp; static char irqstr[32]; - switch (ih->ih_type) { + switch (ih.ih_type) { case PCI_MSI: return "msi"; case PCI_MSIX: return "msix"; } - snprintf(irqstr, sizeof(irqstr), "irq %d", ih->ih_intrpin); + snprintf(irqstr, sizeof(irqstr), "irq %d", ih.ih_intrpin); return irqstr; } void * -acpipci_intr_establish(void *v, pci_intr_handle_t ihp, int level, +acpipci_intr_establish(void *v, pci_intr_handle_t ih, int level, int (*func)(void *), void *arg, char *name) { struct acpipci_softc *sc = v; - struct acpipci_intr_handle *ih = (struct acpipci_intr_handle *)ihp; struct interrupt_controller *ic; void *cookie; @@ -511,11 +440,13 @@ acpipci_intr_establish(void *v, pci_intr if (ic == NULL) return NULL; - if (ih->ih_type != PCI_INTX) { + KASSERT(ih.ih_type != PCI_NONE); + + if (ih.ih_type != PCI_INTX) { uint64_t addr, data; /* Map Requester ID through IORT to get sideband data. */ - data = acpipci_iort_map_msi(ih->ih_pc, ih->ih_tag); + data = acpipci_iort_map_msi(ih.ih_pc, ih.ih_tag); cookie = ic->ic_establish_msi(ic->ic_cookie, &addr, &data, level, func, arg, name); if (cookie == NULL) @@ -523,17 +454,16 @@ acpipci_intr_establish(void *v, pci_intr /* TODO: translate address to the PCI device's view */ - if (ih->ih_type == PCI_MSIX) { - pci_msix_enable(ih->ih_pc, ih->ih_tag, - &sc->sc_bus_memt, ih->ih_intrpin, addr, data); + if (ih.ih_type == PCI_MSIX) { + pci_msix_enable(ih.ih_pc, ih.ih_tag, + &sc->sc_bus_memt, ih.ih_intrpin, addr, data); } else - pci_msi_enable(ih->ih_pc, ih->ih_tag, addr, data); + pci_msi_enable(ih.ih_pc, ih.ih_tag, addr, data); } else { - cookie = acpi_intr_establish(ih->ih_intrpin, 0, level, + cookie = acpi_intr_establish(ih.ih_intrpin, 0, level, func, arg, name); } - free(ih, M_DEVBUF, sizeof(struct acpipci_intr_handle)); return cookie; } Index: arch/arm64/dev/pci_machdep.c =================================================================== RCS file: /cvs/src/sys/arch/arm64/dev/pci_machdep.c,v retrieving revision 1.1 diff -u -p -r1.1 pci_machdep.c --- arch/arm64/dev/pci_machdep.c 31 May 2019 08:02:04 -0000 1.1 +++ arch/arm64/dev/pci_machdep.c 1 Jun 2019 18:00:30 -0000 @@ -83,3 +83,44 @@ pci_msix_enable(pci_chipset_tag_t pc, pc pci_conf_write(pc, tag, off, reg | PCI_MSIX_MC_MSIXE); } + +int +_pci_intr_map_msi(struct pci_attach_args *pa, pci_intr_handle_t *ihp) +{ + pci_chipset_tag_t pc = pa->pa_pc; + pcitag_t tag = pa->pa_tag; + + if ((pa->pa_flags & PCI_FLAGS_MSI_ENABLED) == 0 || + pci_get_capability(pc, tag, PCI_CAP_MSI, NULL, NULL) == 0) + return -1; + + ihp->ih_pc = pa->pa_pc; + ihp->ih_tag = pa->pa_tag; + ihp->ih_type = PCI_MSI; + + return 0; +} + +int +_pci_intr_map_msix(struct pci_attach_args *pa, int vec, + pci_intr_handle_t *ihp) +{ + pci_chipset_tag_t pc = pa->pa_pc; + pcitag_t tag = pa->pa_tag; + pcireg_t reg; + + if ((pa->pa_flags & PCI_FLAGS_MSI_ENABLED) == 0 || + pci_get_capability(pc, tag, PCI_CAP_MSIX, NULL, ®) == 0) + return -1; + + if (vec > PCI_MSIX_MC_TBLSZ(reg)) + return -1; + + ihp->ih_pc = pa->pa_pc; + ihp->ih_tag = pa->pa_tag; + ihp->ih_intrpin = vec; + ihp->ih_type = PCI_MSIX; + + return 0; +} + Index: arch/arm64/dev/pciecam.c =================================================================== RCS file: /cvs/src/sys/arch/arm64/dev/pciecam.c,v retrieving revision 1.8 diff -u -p -r1.8 pciecam.c --- arch/arm64/dev/pciecam.c 31 May 2019 10:36:32 -0000 1.8 +++ arch/arm64/dev/pciecam.c 1 Jun 2019 18:00:30 -0000 @@ -101,8 +101,6 @@ int pciecam_conf_size(void *, pcitag_t); pcireg_t pciecam_conf_read(void *, pcitag_t, int); void pciecam_conf_write(void *, pcitag_t, int, pcireg_t); int pciecam_intr_map(struct pci_attach_args *, pci_intr_handle_t *); -int pciecam_intr_map_msi(struct pci_attach_args *, pci_intr_handle_t *); -int pciecam_intr_map_msix(struct pci_attach_args *, int, pci_intr_handle_t *); const char *pciecam_intr_string(void *, pci_intr_handle_t); void *pciecam_intr_establish(void *, pci_intr_handle_t, int, int (*func)(void *), void *, char *); void pciecam_intr_disestablish(void *, void *); @@ -230,8 +228,8 @@ pciecam_attach(struct device *parent, st sc->sc_pc.pc_intr_v = sc; sc->sc_pc.pc_intr_map = pciecam_intr_map; - sc->sc_pc.pc_intr_map_msi = pciecam_intr_map_msi; - sc->sc_pc.pc_intr_map_msix = pciecam_intr_map_msix; + sc->sc_pc.pc_intr_map_msi = _pci_intr_map_msi; + sc->sc_pc.pc_intr_map_msix = _pci_intr_map_msix; sc->sc_pc.pc_intr_string = pciecam_intr_string; sc->sc_pc.pc_intr_establish = pciecam_intr_establish; sc->sc_pc.pc_intr_disestablish = pciecam_intr_disestablish; @@ -318,85 +316,21 @@ pciecam_conf_write(void *v, pcitag_t tag HWRITE4(sc, PCIE_ADDR_OFFSET(bus, dev, fn, reg & ~0x3), data); } -#define PCI_INTX 0 -#define PCI_MSI 1 -#define PCI_MSIX 2 - -struct pciecam_intr_handle { - pci_chipset_tag_t ih_pc; - pcitag_t ih_tag; - int ih_intrpin; - int ih_type; -}; - int -pciecam_intr_map(struct pci_attach_args *pa, - pci_intr_handle_t *ihp) +pciecam_intr_map(struct pci_attach_args *pa, pci_intr_handle_t *ihp) { - struct pciecam_intr_handle *ih; - - ih = malloc(sizeof(struct pciecam_intr_handle), M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_intrtag; - ih->ih_intrpin = pa->pa_intrpin; - ih->ih_type = PCI_INTX; - *ihp = (pci_intr_handle_t)ih; - return 0; -} - -int -pciecam_intr_map_msi(struct pci_attach_args *pa, - pci_intr_handle_t *ihp) -{ - pci_chipset_tag_t pc = pa->pa_pc; - pcitag_t tag = pa->pa_tag; - struct pciecam_intr_handle *ih; - - if ((pa->pa_flags & PCI_FLAGS_MSI_ENABLED) == 0 || - pci_get_capability(pc, tag, PCI_CAP_MSI, NULL, NULL) == 0) - return 1; - - ih = malloc(sizeof(struct pciecam_intr_handle), M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_tag; - ih->ih_type = PCI_MSI; - *ihp = (pci_intr_handle_t)ih; - - return 0; -} - -int -pciecam_intr_map_msix(struct pci_attach_args *pa, - int vec, pci_intr_handle_t *ihp) -{ - pci_chipset_tag_t pc = pa->pa_pc; - pcitag_t tag = pa->pa_tag; - struct pciecam_intr_handle *ih; - pcireg_t reg; - - if ((pa->pa_flags & PCI_FLAGS_MSI_ENABLED) == 0 || - pci_get_capability(pc, tag, PCI_CAP_MSIX, NULL, ®) == 0) - return -1; - - if (vec > PCI_MSIX_MC_TBLSZ(reg)) - return -1; - - ih = malloc(sizeof(struct pciecam_intr_handle), M_DEVBUF, M_WAITOK); - ih->ih_pc = pa->pa_pc; - ih->ih_tag = pa->pa_tag; - ih->ih_intrpin = vec; - ih->ih_type = PCI_MSIX; - *ihp = (pci_intr_handle_t)ih; + ihp->ih_pc = pa->pa_pc; + ihp->ih_tag = pa->pa_intrtag; + ihp->ih_intrpin = pa->pa_intrpin; + ihp->ih_type = PCI_INTX; return 0; } const char * -pciecam_intr_string(void *sc, pci_intr_handle_t ihp) +pciecam_intr_string(void *sc, pci_intr_handle_t ih) { - struct pciecam_intr_handle *ih = (struct pciecam_intr_handle *)ihp; - - switch (ih->ih_type) { + switch (ih.ih_type) { case PCI_MSI: return "msi"; case PCI_MSIX: @@ -407,18 +341,19 @@ pciecam_intr_string(void *sc, pci_intr_h } void * -pciecam_intr_establish(void *self, pci_intr_handle_t ihp, int level, +pciecam_intr_establish(void *self, pci_intr_handle_t ih, int level, int (*func)(void *), void *arg, char *name) { struct pciecam_softc *sc = (struct pciecam_softc *)self; - struct pciecam_intr_handle *ih = (struct pciecam_intr_handle *)ihp; void *cookie; - if (ih->ih_type != PCI_INTX) { + KASSERT(ih.ih_type != PCI_NONE); + + if (ih.ih_type != PCI_INTX) { uint64_t addr, data; /* Assume hardware passes Requester ID as sideband data. */ - data = pci_requester_id(ih->ih_pc, ih->ih_tag); + data = pci_requester_id(ih.ih_pc, ih.ih_tag); cookie = arm_intr_establish_fdt_msi(sc->sc_node, &addr, &data, level, func, arg, (void *)name); if (cookie == NULL) @@ -426,26 +361,25 @@ pciecam_intr_establish(void *self, pci_i /* TODO: translate address to the PCI device's view */ - if (ih->ih_type == PCI_MSIX) { - pci_msix_enable(ih->ih_pc, ih->ih_tag, - &sc->sc_bus, ih->ih_intrpin, addr, data); + if (ih.ih_type == PCI_MSIX) { + pci_msix_enable(ih.ih_pc, ih.ih_tag, + &sc->sc_bus, ih.ih_intrpin, addr, data); } else - pci_msi_enable(ih->ih_pc, ih->ih_tag, addr, data); + pci_msi_enable(ih.ih_pc, ih.ih_tag, addr, data); } else { int bus, dev, fn; uint32_t reg[4]; - pciecam_decompose_tag(sc, ih->ih_tag, &bus, &dev, &fn); + pciecam_decompose_tag(sc, ih.ih_tag, &bus, &dev, &fn); reg[0] = bus << 16 | dev << 11 | fn << 8; reg[1] = reg[2] = 0; - reg[3] = ih->ih_intrpin; + reg[3] = ih.ih_intrpin; cookie = arm_intr_establish_fdt_imap(sc->sc_node, reg, sizeof(reg), level, func, arg, name); } - free(ih, M_DEVBUF, sizeof(struct pciecam_intr_handle)); return cookie; } Index: arch/arm64/include/pci_machdep.h =================================================================== RCS file: /cvs/src/sys/arch/arm64/include/pci_machdep.h,v retrieving revision 1.4 diff -u -p -r1.4 pci_machdep.h --- arch/arm64/include/pci_machdep.h 31 May 2019 08:02:04 -0000 1.4 +++ arch/arm64/include/pci_machdep.h 1 Jun 2019 18:00:30 -0000 @@ -28,7 +28,19 @@ typedef struct arm64_pci_chipset *pci_chipset_tag_t; typedef u_long pcitag_t; -typedef u_long pci_intr_handle_t; + +/* Supported interrupt types. */ +#define PCI_NONE 0 +#define PCI_INTX 1 +#define PCI_MSI 2 +#define PCI_MSIX 3 + +typedef struct { + pci_chipset_tag_t ih_pc; + pcitag_t ih_tag; + int ih_intrpin; + int ih_type; +} pci_intr_handle_t; struct pci_attach_args; @@ -103,3 +115,5 @@ pci_chipset_tag_t pci_lookup_segment(int void pci_msi_enable(pci_chipset_tag_t, pcitag_t, bus_addr_t, uint32_t); void pci_msix_enable(pci_chipset_tag_t, pcitag_t, bus_space_tag_t, int, bus_addr_t, uint32_t); +int _pci_intr_map_msi(struct pci_attach_args *, pci_intr_handle_t *); +int _pci_intr_map_msix(struct pci_attach_args *, int, pci_intr_handle_t *);