| Message ID | 20241010103203.382898-4-michal.winiarski@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: VF resizable BAR | expand |
On Thu, 10 Oct 2024, Michał Winiarski wrote: > Similar to regular resizable BAR, VF BAR can also be resized. > The structures are very similar, which means we can reuse most of the > implementation. See PCIe r4.0, sec 9.3.7.4. > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> > --- > drivers/pci/iov.c | 20 ++++++++++++++++++++ > drivers/pci/pci.c | 9 ++++++++- > drivers/pci/pci.h | 8 ++++++++ > drivers/pci/setup-res.c | 33 ++++++++++++++++++++++++++++----- > 4 files changed, 64 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c > index fd5c059b29c13..591a3eae1618a 100644 > --- a/drivers/pci/iov.c > +++ b/drivers/pci/iov.c > @@ -154,6 +154,26 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno) > return dev->sriov->barsz[resno - PCI_IOV_RESOURCES]; > } > > +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size) > +{ > + if (!pci_resource_is_iov(resno)) { > + pci_warn(dev, "%s is not an IOV resource\n", > + pci_resource_name(dev, resno)); > + return; > + } > + > + dev->sriov->barsz[resno - PCI_IOV_RESOURCES] = size; > +} > + > +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev) > +{ > + u16 cmd; > + > + pci_read_config_word(dev, dev->sriov->pos + PCI_SRIOV_CTRL, &cmd); > + > + return cmd & PCI_SRIOV_CTRL_MSE; > +} > + > static void pci_read_vf_config_common(struct pci_dev *virtfn) > { > struct pci_dev *physfn = virtfn->physfn; > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 7d85c04fbba2a..788ae61731213 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -3718,10 +3718,17 @@ void pci_acs_init(struct pci_dev *dev) > */ > static int pci_rebar_find_pos(struct pci_dev *pdev, int bar) > { > + int cap = PCI_EXT_CAP_ID_REBAR; > unsigned int pos, nbars, i; > u32 ctrl; > > - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR); > +#ifdef CONFIG_PCI_IOV > + if (pci_resource_is_iov(bar)) { > + cap = PCI_EXT_CAP_ID_VF_REBAR; > + bar -= PCI_IOV_RESOURCES; > + } > +#endif Perhaps abstracting bar -= PCI_IOV_RESOURCES too into some static inline function would be useful so you could drop the ifdefs. That calculation seems to be done in few places besides this one. > + pos = pci_find_ext_capability(pdev, cap); > if (!pos) > return -ENOTSUPP; > > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > index c55f2d7a4f37e..e15fd8fe0f81f 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -584,6 +584,8 @@ static inline bool pci_resource_is_iov(int resno) > { > return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END; > } > +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size); > +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev); > extern const struct attribute_group sriov_pf_dev_attr_group; > extern const struct attribute_group sriov_vf_dev_attr_group; > #else > @@ -607,6 +609,12 @@ static inline bool pci_resource_is_iov(int resno) > { > return false; > } > +static inline void pci_iov_resource_set_size(struct pci_dev *dev, int resno, > + resource_size_t size) { } > +static inline bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev) > +{ > + return false; > +} > #endif /* CONFIG_PCI_IOV */ > > #ifdef CONFIG_PCIE_PTM > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c > index e2cf79253ebda..95a13a5fa379c 100644 > --- a/drivers/pci/setup-res.c > +++ b/drivers/pci/setup-res.c > @@ -425,13 +425,37 @@ void pci_release_resource(struct pci_dev *dev, int resno) > } > EXPORT_SYMBOL(pci_release_resource); > > +static bool pci_resize_is_memory_decoding_enabled(struct pci_dev *dev, int resno) > +{ > + u16 cmd; > + > + if (pci_resource_is_iov(resno)) > + return pci_iov_is_memory_decoding_enabled(dev); > + > + pci_read_config_word(dev, PCI_COMMAND, &cmd); > + > + return cmd & PCI_COMMAND_MEMORY; > +} > + > +static void pci_resize_resource_set_size(struct pci_dev *dev, int resno, int size) > +{ > + resource_size_t res_size = pci_rebar_size_to_bytes(size); > + struct resource *res = dev->resource + resno; > + > + if (!pci_resource_is_iov(resno)) { > + res->end = res->start + res_size - 1; > + } else { > + res->end = res->start + res_size * pci_sriov_get_totalvfs(dev) - 1; I wish Bjorn would pick up my resource_set_{range,size}() series [1] so we wouldn't need to open-code this resource size calculation everywhere. > + pci_iov_resource_set_size(dev, resno, res_size); > + } > +} > + > int pci_resize_resource(struct pci_dev *dev, int resno, int size) > { > struct resource *res = dev->resource + resno; > struct pci_host_bridge *host; > int old, ret; > u32 sizes; > - u16 cmd; > > /* Check if we must preserve the firmware's resource assignment */ > host = pci_find_host_bridge(dev->bus); > @@ -442,8 +466,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size) > if (!(res->flags & IORESOURCE_UNSET)) > return -EBUSY; > > - pci_read_config_word(dev, PCI_COMMAND, &cmd); > - if (cmd & PCI_COMMAND_MEMORY) > + if (pci_resize_is_memory_decoding_enabled(dev, resno)) > return -EBUSY; > > sizes = pci_rebar_get_possible_sizes(dev, resno); > @@ -461,7 +484,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size) > if (ret) > return ret; > > - res->end = res->start + pci_rebar_size_to_bytes(size) - 1; > + pci_resize_resource_set_size(dev, resno, size); > > /* Check if the new config works by trying to assign everything. */ > if (dev->bus->self) { > @@ -473,7 +496,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size) > > error_resize: > pci_rebar_set_size(dev, resno, old); > - res->end = res->start + pci_rebar_size_to_bytes(old) - 1; > + pci_resize_resource_set_size(dev, resno, old); > return ret; > } > EXPORT_SYMBOL(pci_resize_resource); > [1] https://patchwork.kernel.org/project/linux-pci/patch/20240614100606.15830-2-ilpo.jarvinen@linux.intel.com/
On Thu, Oct 10, 2024 at 02:17:11PM +0300, Ilpo Järvinen wrote: > On Thu, 10 Oct 2024, Michał Winiarski wrote: > > > Similar to regular resizable BAR, VF BAR can also be resized. > > The structures are very similar, which means we can reuse most of the > > implementation. See PCIe r4.0, sec 9.3.7.4. > > > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> > > --- > > drivers/pci/iov.c | 20 ++++++++++++++++++++ > > drivers/pci/pci.c | 9 ++++++++- > > drivers/pci/pci.h | 8 ++++++++ > > drivers/pci/setup-res.c | 33 ++++++++++++++++++++++++++++----- > > 4 files changed, 64 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c > > index fd5c059b29c13..591a3eae1618a 100644 > > --- a/drivers/pci/iov.c > > +++ b/drivers/pci/iov.c > > @@ -154,6 +154,26 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno) > > return dev->sriov->barsz[resno - PCI_IOV_RESOURCES]; > > } > > > > +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size) > > +{ > > + if (!pci_resource_is_iov(resno)) { > > + pci_warn(dev, "%s is not an IOV resource\n", > > + pci_resource_name(dev, resno)); > > + return; > > + } > > + > > + dev->sriov->barsz[resno - PCI_IOV_RESOURCES] = size; > > +} > > + > > +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev) > > +{ > > + u16 cmd; > > + > > + pci_read_config_word(dev, dev->sriov->pos + PCI_SRIOV_CTRL, &cmd); > > + > > + return cmd & PCI_SRIOV_CTRL_MSE; > > +} > > + > > static void pci_read_vf_config_common(struct pci_dev *virtfn) > > { > > struct pci_dev *physfn = virtfn->physfn; > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index 7d85c04fbba2a..788ae61731213 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -3718,10 +3718,17 @@ void pci_acs_init(struct pci_dev *dev) > > */ > > static int pci_rebar_find_pos(struct pci_dev *pdev, int bar) > > { > > + int cap = PCI_EXT_CAP_ID_REBAR; > > unsigned int pos, nbars, i; > > u32 ctrl; > > > > - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR); > > +#ifdef CONFIG_PCI_IOV > > + if (pci_resource_is_iov(bar)) { > > + cap = PCI_EXT_CAP_ID_VF_REBAR; > > + bar -= PCI_IOV_RESOURCES; > > + } > > +#endif > > Perhaps abstracting bar -= PCI_IOV_RESOURCES too into some static inline > function would be useful so you could drop the ifdefs. That calculation > seems to be done in few places besides this one. I have a version of this series with helpers for conversion in both ways: pci_iov_resource_to_vf_bar (which is this one) and pci_vf_bar_to_iov_resource (+= PCI_IOV_RESOURCES) But decided to leave it out for now. I can include it in v4 (or send it separately, depending on the direction in which this revision goes). > > > + pos = pci_find_ext_capability(pdev, cap); > > if (!pos) > > return -ENOTSUPP; > > > > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > > index c55f2d7a4f37e..e15fd8fe0f81f 100644 > > --- a/drivers/pci/pci.h > > +++ b/drivers/pci/pci.h > > @@ -584,6 +584,8 @@ static inline bool pci_resource_is_iov(int resno) > > { > > return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END; > > } > > +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size); > > +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev); > > extern const struct attribute_group sriov_pf_dev_attr_group; > > extern const struct attribute_group sriov_vf_dev_attr_group; > > #else > > @@ -607,6 +609,12 @@ static inline bool pci_resource_is_iov(int resno) > > { > > return false; > > } > > +static inline void pci_iov_resource_set_size(struct pci_dev *dev, int resno, > > + resource_size_t size) { } > > +static inline bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev) > > +{ > > + return false; > > +} > > #endif /* CONFIG_PCI_IOV */ > > > > #ifdef CONFIG_PCIE_PTM > > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c > > index e2cf79253ebda..95a13a5fa379c 100644 > > --- a/drivers/pci/setup-res.c > > +++ b/drivers/pci/setup-res.c > > @@ -425,13 +425,37 @@ void pci_release_resource(struct pci_dev *dev, int resno) > > } > > EXPORT_SYMBOL(pci_release_resource); > > > > +static bool pci_resize_is_memory_decoding_enabled(struct pci_dev *dev, int resno) > > +{ > > + u16 cmd; > > + > > + if (pci_resource_is_iov(resno)) > > + return pci_iov_is_memory_decoding_enabled(dev); > > + > > + pci_read_config_word(dev, PCI_COMMAND, &cmd); > > + > > + return cmd & PCI_COMMAND_MEMORY; > > +} > > + > > +static void pci_resize_resource_set_size(struct pci_dev *dev, int resno, int size) > > +{ > > + resource_size_t res_size = pci_rebar_size_to_bytes(size); > > + struct resource *res = dev->resource + resno; > > + > > + if (!pci_resource_is_iov(resno)) { > > + res->end = res->start + res_size - 1; > > + } else { > > + res->end = res->start + res_size * pci_sriov_get_totalvfs(dev) - 1; > > I wish Bjorn would pick up my resource_set_{range,size}() series [1] so we > wouldn't need to open-code this resource size calculation everywhere. Yeah - that looks useful indeed. Thanks, -Michał > > > + pci_iov_resource_set_size(dev, resno, res_size); > > + } > > +} > > + > > int pci_resize_resource(struct pci_dev *dev, int resno, int size) > > { > > struct resource *res = dev->resource + resno; > > struct pci_host_bridge *host; > > int old, ret; > > u32 sizes; > > - u16 cmd; > > > > /* Check if we must preserve the firmware's resource assignment */ > > host = pci_find_host_bridge(dev->bus); > > @@ -442,8 +466,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size) > > if (!(res->flags & IORESOURCE_UNSET)) > > return -EBUSY; > > > > - pci_read_config_word(dev, PCI_COMMAND, &cmd); > > - if (cmd & PCI_COMMAND_MEMORY) > > + if (pci_resize_is_memory_decoding_enabled(dev, resno)) > > return -EBUSY; > > > > sizes = pci_rebar_get_possible_sizes(dev, resno); > > @@ -461,7 +484,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size) > > if (ret) > > return ret; > > > > - res->end = res->start + pci_rebar_size_to_bytes(size) - 1; > > + pci_resize_resource_set_size(dev, resno, size); > > > > /* Check if the new config works by trying to assign everything. */ > > if (dev->bus->self) { > > @@ -473,7 +496,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size) > > > > error_resize: > > pci_rebar_set_size(dev, resno, old); > > - res->end = res->start + pci_rebar_size_to_bytes(old) - 1; > > + pci_resize_resource_set_size(dev, resno, old); > > return ret; > > } > > EXPORT_SYMBOL(pci_resize_resource); > > > > [1] https://patchwork.kernel.org/project/linux-pci/patch/20240614100606.15830-2-ilpo.jarvinen@linux.intel.com/ > > -- > i.
On Thu, Oct 10, 2024 at 12:32:01PM +0200, Michał Winiarski wrote: > Similar to regular resizable BAR, VF BAR can also be resized. > The structures are very similar, which means we can reuse most of the > implementation. See PCIe r4.0, sec 9.3.7.4. Add blank line between paragraphs. Add "()" after function name in subject. Add what the patch does in the commit log, not just what can be done. > static int pci_rebar_find_pos(struct pci_dev *pdev, int bar) > { > + int cap = PCI_EXT_CAP_ID_REBAR; > unsigned int pos, nbars, i; > u32 ctrl; > > - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR); > +#ifdef CONFIG_PCI_IOV > + if (pci_resource_is_iov(bar)) { > + cap = PCI_EXT_CAP_ID_VF_REBAR; > + bar -= PCI_IOV_RESOURCES; > + } > +#endif Personal preference, but I'd rather set "cap" directly here instead of setting a default and then overriding it in some cases. Setting it here means both settings are in the same place. > + pos = pci_find_ext_capability(pdev, cap); > if (!pos) > return -ENOTSUPP; > > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > index c55f2d7a4f37e..e15fd8fe0f81f 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -584,6 +584,8 @@ static inline bool pci_resource_is_iov(int resno) > { > return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END; > } > +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size); > +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev); > extern const struct attribute_group sriov_pf_dev_attr_group; > extern const struct attribute_group sriov_vf_dev_attr_group; > #else > @@ -607,6 +609,12 @@ static inline bool pci_resource_is_iov(int resno) > { > return false; > } > +static inline void pci_iov_resource_set_size(struct pci_dev *dev, int resno, > + resource_size_t size) { } > +static inline bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev) > +{ > + return false; > +} > #endif /* CONFIG_PCI_IOV */ > > #ifdef CONFIG_PCIE_PTM > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c > index e2cf79253ebda..95a13a5fa379c 100644 > --- a/drivers/pci/setup-res.c > +++ b/drivers/pci/setup-res.c > @@ -425,13 +425,37 @@ void pci_release_resource(struct pci_dev *dev, int resno) > } > EXPORT_SYMBOL(pci_release_resource); > > +static bool pci_resize_is_memory_decoding_enabled(struct pci_dev *dev, int resno) Wrap this (and others) to fit in 80 columns like the rest of the file. Bjorn
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index fd5c059b29c13..591a3eae1618a 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -154,6 +154,26 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno) return dev->sriov->barsz[resno - PCI_IOV_RESOURCES]; } +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size) +{ + if (!pci_resource_is_iov(resno)) { + pci_warn(dev, "%s is not an IOV resource\n", + pci_resource_name(dev, resno)); + return; + } + + dev->sriov->barsz[resno - PCI_IOV_RESOURCES] = size; +} + +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev) +{ + u16 cmd; + + pci_read_config_word(dev, dev->sriov->pos + PCI_SRIOV_CTRL, &cmd); + + return cmd & PCI_SRIOV_CTRL_MSE; +} + static void pci_read_vf_config_common(struct pci_dev *virtfn) { struct pci_dev *physfn = virtfn->physfn; diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 7d85c04fbba2a..788ae61731213 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3718,10 +3718,17 @@ void pci_acs_init(struct pci_dev *dev) */ static int pci_rebar_find_pos(struct pci_dev *pdev, int bar) { + int cap = PCI_EXT_CAP_ID_REBAR; unsigned int pos, nbars, i; u32 ctrl; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR); +#ifdef CONFIG_PCI_IOV + if (pci_resource_is_iov(bar)) { + cap = PCI_EXT_CAP_ID_VF_REBAR; + bar -= PCI_IOV_RESOURCES; + } +#endif + pos = pci_find_ext_capability(pdev, cap); if (!pos) return -ENOTSUPP; diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index c55f2d7a4f37e..e15fd8fe0f81f 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -584,6 +584,8 @@ static inline bool pci_resource_is_iov(int resno) { return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END; } +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size); +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev); extern const struct attribute_group sriov_pf_dev_attr_group; extern const struct attribute_group sriov_vf_dev_attr_group; #else @@ -607,6 +609,12 @@ static inline bool pci_resource_is_iov(int resno) { return false; } +static inline void pci_iov_resource_set_size(struct pci_dev *dev, int resno, + resource_size_t size) { } +static inline bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev) +{ + return false; +} #endif /* CONFIG_PCI_IOV */ #ifdef CONFIG_PCIE_PTM diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c index e2cf79253ebda..95a13a5fa379c 100644 --- a/drivers/pci/setup-res.c +++ b/drivers/pci/setup-res.c @@ -425,13 +425,37 @@ void pci_release_resource(struct pci_dev *dev, int resno) } EXPORT_SYMBOL(pci_release_resource); +static bool pci_resize_is_memory_decoding_enabled(struct pci_dev *dev, int resno) +{ + u16 cmd; + + if (pci_resource_is_iov(resno)) + return pci_iov_is_memory_decoding_enabled(dev); + + pci_read_config_word(dev, PCI_COMMAND, &cmd); + + return cmd & PCI_COMMAND_MEMORY; +} + +static void pci_resize_resource_set_size(struct pci_dev *dev, int resno, int size) +{ + resource_size_t res_size = pci_rebar_size_to_bytes(size); + struct resource *res = dev->resource + resno; + + if (!pci_resource_is_iov(resno)) { + res->end = res->start + res_size - 1; + } else { + res->end = res->start + res_size * pci_sriov_get_totalvfs(dev) - 1; + pci_iov_resource_set_size(dev, resno, res_size); + } +} + int pci_resize_resource(struct pci_dev *dev, int resno, int size) { struct resource *res = dev->resource + resno; struct pci_host_bridge *host; int old, ret; u32 sizes; - u16 cmd; /* Check if we must preserve the firmware's resource assignment */ host = pci_find_host_bridge(dev->bus); @@ -442,8 +466,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size) if (!(res->flags & IORESOURCE_UNSET)) return -EBUSY; - pci_read_config_word(dev, PCI_COMMAND, &cmd); - if (cmd & PCI_COMMAND_MEMORY) + if (pci_resize_is_memory_decoding_enabled(dev, resno)) return -EBUSY; sizes = pci_rebar_get_possible_sizes(dev, resno); @@ -461,7 +484,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size) if (ret) return ret; - res->end = res->start + pci_rebar_size_to_bytes(size) - 1; + pci_resize_resource_set_size(dev, resno, size); /* Check if the new config works by trying to assign everything. */ if (dev->bus->self) { @@ -473,7 +496,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size) error_resize: pci_rebar_set_size(dev, resno, old); - res->end = res->start + pci_rebar_size_to_bytes(old) - 1; + pci_resize_resource_set_size(dev, resno, old); return ret; } EXPORT_SYMBOL(pci_resize_resource);
Similar to regular resizable BAR, VF BAR can also be resized. The structures are very similar, which means we can reuse most of the implementation. See PCIe r4.0, sec 9.3.7.4. Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> --- drivers/pci/iov.c | 20 ++++++++++++++++++++ drivers/pci/pci.c | 9 ++++++++- drivers/pci/pci.h | 8 ++++++++ drivers/pci/setup-res.c | 33 ++++++++++++++++++++++++++++----- 4 files changed, 64 insertions(+), 6 deletions(-)