| Message ID | 20240330041928.1555578-2-dlemoal@kernel.org (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Manivannan Sadhasivam |
| Headers | show |
| Series | Improve PCI memory mapping API | expand |
On Sat, Mar 30, 2024 at 01:19:11PM +0900, Damien Le Moal wrote: > Introduce the epc core helper function pci_epc_function_is_valid() to > verify that an epc pointer, a physical function number and a virtual > function number are all valid. This avoids repeating the code pattern: > > if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > return err; > > if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > return err; > > in many functions of the endpoint controller core code. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> One nit below. With that fixed, Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/pci/endpoint/pci-epc-core.c | 79 +++++++++++------------------ > 1 file changed, 31 insertions(+), 48 deletions(-) > > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c > index da3fc0795b0b..754afd115bbd 100644 > --- a/drivers/pci/endpoint/pci-epc-core.c > +++ b/drivers/pci/endpoint/pci-epc-core.c > @@ -126,6 +126,18 @@ enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features > } > EXPORT_SYMBOL_GPL(pci_epc_get_next_free_bar); > > +static inline bool pci_epc_function_is_valid(struct pci_epc *epc, > + u8 func_no, u8 vfunc_no) No need to add 'inline' keyword to function definitions in a .c file. Compiler will handle that. - Mani > +{ > + if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > + return false; > + > + if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + return false; > + > + return true; > +} > + > /** > * pci_epc_get_features() - get the features supported by EPC > * @epc: the features supported by *this* EPC device will be returned > @@ -143,10 +155,7 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc, > { > const struct pci_epc_features *epc_features; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return NULL; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return NULL; > > if (!epc->ops->get_features) > @@ -216,10 +225,7 @@ int pci_epc_raise_irq(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > { > int ret; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return -EINVAL; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > if (!epc->ops->raise_irq) > @@ -260,10 +266,7 @@ int pci_epc_map_msi_irq(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > { > int ret; > > - if (IS_ERR_OR_NULL(epc)) > - return -EINVAL; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > if (!epc->ops->map_msi_irq) > @@ -291,10 +294,7 @@ int pci_epc_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no) > { > int interrupt; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return 0; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return 0; > > if (!epc->ops->get_msi) > @@ -327,11 +327,10 @@ int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no, u8 interrupts) > int ret; > u8 encode_int; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || > - interrupts < 1 || interrupts > 32) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (interrupts < 1 || interrupts > 32) > return -EINVAL; > > if (!epc->ops->set_msi) > @@ -359,10 +358,7 @@ int pci_epc_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no) > { > int interrupt; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return 0; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return 0; > > if (!epc->ops->get_msix) > @@ -395,11 +391,10 @@ int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > { > int ret; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || > - interrupts < 1 || interrupts > 2048) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (interrupts < 1 || interrupts > 2048) > return -EINVAL; > > if (!epc->ops->set_msix) > @@ -426,10 +421,7 @@ EXPORT_SYMBOL_GPL(pci_epc_set_msix); > void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > phys_addr_t phys_addr) > { > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return; > > if (!epc->ops->unmap_addr) > @@ -457,10 +449,7 @@ int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > { > int ret; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return -EINVAL; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > if (!epc->ops->map_addr) > @@ -487,12 +476,11 @@ EXPORT_SYMBOL_GPL(pci_epc_map_addr); > void pci_epc_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > struct pci_epf_bar *epf_bar) > { > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || > - (epf_bar->barno == BAR_5 && > - epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64)) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return; > > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (epf_bar->barno == BAR_5 && > + epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) > return; > > if (!epc->ops->clear_bar) > @@ -519,18 +507,16 @@ int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > int ret; > int flags = epf_bar->flags; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || > - (epf_bar->barno == BAR_5 && > - flags & PCI_BASE_ADDRESS_MEM_TYPE_64) || > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > + return -EINVAL; > + > + if ((epf_bar->barno == BAR_5 && flags & PCI_BASE_ADDRESS_MEM_TYPE_64) || > (flags & PCI_BASE_ADDRESS_SPACE_IO && > flags & PCI_BASE_ADDRESS_IO_MASK) || > (upper_32_bits(epf_bar->size) && > !(flags & PCI_BASE_ADDRESS_MEM_TYPE_64))) > return -EINVAL; > > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > - return -EINVAL; > - > if (!epc->ops->set_bar) > return 0; > > @@ -559,10 +545,7 @@ int pci_epc_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > { > int ret; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return -EINVAL; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > /* Only Virtual Function #1 has deviceID */ > -- > 2.44.0 >
On Sat, Mar 30, 2024 at 01:19:11PM +0900, Damien Le Moal wrote: > Introduce the epc core helper function pci_epc_function_is_valid() to > verify that an epc pointer, a physical function number and a virtual > function number are all valid. This avoids repeating the code pattern: > > if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > return err; > > if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > return err; > > in many functions of the endpoint controller core code. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- Reviewed-by: Niklas Cassel <cassel@kernel.org>
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c index da3fc0795b0b..754afd115bbd 100644 --- a/drivers/pci/endpoint/pci-epc-core.c +++ b/drivers/pci/endpoint/pci-epc-core.c @@ -126,6 +126,18 @@ enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features } EXPORT_SYMBOL_GPL(pci_epc_get_next_free_bar); +static inline bool pci_epc_function_is_valid(struct pci_epc *epc, + u8 func_no, u8 vfunc_no) +{ + if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) + return false; + + if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + return false; + + return true; +} + /** * pci_epc_get_features() - get the features supported by EPC * @epc: the features supported by *this* EPC device will be returned @@ -143,10 +155,7 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc, { const struct pci_epc_features *epc_features; - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) - return NULL; - - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) return NULL; if (!epc->ops->get_features) @@ -216,10 +225,7 @@ int pci_epc_raise_irq(struct pci_epc *epc, u8 func_no, u8 vfunc_no, { int ret; - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) - return -EINVAL; - - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) return -EINVAL; if (!epc->ops->raise_irq) @@ -260,10 +266,7 @@ int pci_epc_map_msi_irq(struct pci_epc *epc, u8 func_no, u8 vfunc_no, { int ret; - if (IS_ERR_OR_NULL(epc)) - return -EINVAL; - - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) return -EINVAL; if (!epc->ops->map_msi_irq) @@ -291,10 +294,7 @@ int pci_epc_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no) { int interrupt; - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) - return 0; - - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) return 0; if (!epc->ops->get_msi) @@ -327,11 +327,10 @@ int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no, u8 interrupts) int ret; u8 encode_int; - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || - interrupts < 1 || interrupts > 32) + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) return -EINVAL; - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + if (interrupts < 1 || interrupts > 32) return -EINVAL; if (!epc->ops->set_msi) @@ -359,10 +358,7 @@ int pci_epc_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no) { int interrupt; - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) - return 0; - - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) return 0; if (!epc->ops->get_msix) @@ -395,11 +391,10 @@ int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no, { int ret; - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || - interrupts < 1 || interrupts > 2048) + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) return -EINVAL; - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + if (interrupts < 1 || interrupts > 2048) return -EINVAL; if (!epc->ops->set_msix) @@ -426,10 +421,7 @@ EXPORT_SYMBOL_GPL(pci_epc_set_msix); void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, phys_addr_t phys_addr) { - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) - return; - - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) return; if (!epc->ops->unmap_addr) @@ -457,10 +449,7 @@ int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, { int ret; - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) - return -EINVAL; - - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) return -EINVAL; if (!epc->ops->map_addr) @@ -487,12 +476,11 @@ EXPORT_SYMBOL_GPL(pci_epc_map_addr); void pci_epc_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, struct pci_epf_bar *epf_bar) { - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || - (epf_bar->barno == BAR_5 && - epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64)) + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) return; - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + if (epf_bar->barno == BAR_5 && + epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) return; if (!epc->ops->clear_bar) @@ -519,18 +507,16 @@ int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, int ret; int flags = epf_bar->flags; - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || - (epf_bar->barno == BAR_5 && - flags & PCI_BASE_ADDRESS_MEM_TYPE_64) || + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) + return -EINVAL; + + if ((epf_bar->barno == BAR_5 && flags & PCI_BASE_ADDRESS_MEM_TYPE_64) || (flags & PCI_BASE_ADDRESS_SPACE_IO && flags & PCI_BASE_ADDRESS_IO_MASK) || (upper_32_bits(epf_bar->size) && !(flags & PCI_BASE_ADDRESS_MEM_TYPE_64))) return -EINVAL; - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) - return -EINVAL; - if (!epc->ops->set_bar) return 0; @@ -559,10 +545,7 @@ int pci_epc_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no, { int ret; - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) - return -EINVAL; - - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) return -EINVAL; /* Only Virtual Function #1 has deviceID */
Introduce the epc core helper function pci_epc_function_is_valid() to verify that an epc pointer, a physical function number and a virtual function number are all valid. This avoids repeating the code pattern: if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) return err; if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) return err; in many functions of the endpoint controller core code. Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/pci/endpoint/pci-epc-core.c | 79 +++++++++++------------------ 1 file changed, 31 insertions(+), 48 deletions(-)