[PATCH] ntb: idt: Set PCIe bus address to BARLIMITx

[PATCH] ntb: idt: Set PCIe bus address to BARLIMITx

[Serge Semin]

IDT NTB driver sets the upper limit of actual translation address
being set to the corresponding memory window. It is achieved by
BARLIMITx register initialization. Needless to say, that the register
works within PCIe bus address space.

In general CPU and PCIe address spaces are different. It means,
that addresses used for Memory TLPs routine can be different from
CPU addresses. While in most of cases they are the same, there are
exceptions when the proper mapping must be performed to have the
portable driver code. There used to be a virt_to_bus()/bus_to_virt()
interface for this purpose. But it's deprecated now. It was also a
mistake to use pci_resource_start() since the return address of the
method is at the CPU address space. In order to achieve the desired
purpose we need to use pcibios_resource_to_bus(). This method shall
return a PCIe bus address region of the corresponding BAR resources.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
 drivers/ntb/hw/idt/ntb_hw_idt.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c
index dbe72f116017..0f4f5e7e4ff8 100644
--- a/drivers/ntb/hw/idt/ntb_hw_idt.c
+++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
@@ -1311,6 +1311,7 @@ static int idt_ntb_peer_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx,
 	/* DIR and LUT based translations are initialized differently */
 	if (mw_cfg->type == IDT_MW_DIR) {
 		const struct idt_ntb_bar *bar = &ntdata_tbl.bars[mw_cfg->bar];
+		struct pci_bus_region region;
 		u64 limit;
 		/* Set destination partition of translation */
 		data = idt_nt_read(ndev, bar->setup);
@@ -1320,7 +1321,9 @@ static int idt_ntb_peer_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx,
 		idt_nt_write(ndev, bar->ltbase, (u32)addr);
 		idt_nt_write(ndev, bar->utbase, (u32)(addr >> 32));
 		/* Set the custom BAR aperture limit */
-		limit = pci_resource_start(ntb->pdev, mw_cfg->bar) + size;
+		pcibios_resource_to_bus(ntb->pdev->bus, &region,
+					&ntb->pdev->resource[mw_cfg->bar]);
+		limit = region.start + size;
 		idt_nt_write(ndev, bar->limit, (u32)limit);
 		if (IS_FLD_SET(BARSETUP_TYPE, data, 64))
 			idt_nt_write(ndev, (bar + 1)->limit, (limit >> 32));
-- 
2.12.0

Re: [PATCH] ntb: idt: Set PCIe bus address to BARLIMITx

[Logan Gunthorpe]

On 11/07/18 10:32 AM, Serge Semin wrote:
> @@ -1320,7 +1321,9 @@ static int idt_ntb_peer_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx,
>  		idt_nt_write(ndev, bar->ltbase, (u32)addr);
>  		idt_nt_write(ndev, bar->utbase, (u32)(addr >> 32));
>  		/* Set the custom BAR aperture limit */
> -		limit = pci_resource_start(ntb->pdev, mw_cfg->bar) + size;
> +		pcibios_resource_to_bus(ntb->pdev->bus, &region,
> +					&ntb->pdev->resource[mw_cfg->bar]);
> +		limit = region.start + size;

You should use the pci_bus_address() helper instead. It's much simpler
and you don't need to delve into pci_dev internals to use it.

Logan

Re: [PATCH] ntb: idt: Set PCIe bus address to BARLIMITx

[Fancer's opinion]

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

Oh, thanks man. I failed to find it.

-Sergey


On Wed, 11 Jul 2018, 19:52 Logan Gunthorpe, <logang@deltatee.com> wrote:

>
>
> On 11/07/18 10:32 AM, Serge Semin wrote:
> > @@ -1320,7 +1321,9 @@ static int idt_ntb_peer_mw_set_trans(struct
> ntb_dev *ntb, int pidx, int widx,
> >               idt_nt_write(ndev, bar->ltbase, (u32)addr);
> >               idt_nt_write(ndev, bar->utbase, (u32)(addr >> 32));
> >               /* Set the custom BAR aperture limit */
> > -             limit = pci_resource_start(ntb->pdev, mw_cfg->bar) + size;
> > +             pcibios_resource_to_bus(ntb->pdev->bus, &region,
> > +                                     &ntb->pdev->resource[mw_cfg->bar]);
> > +             limit = region.start + size;
>
> You should use the pci_bus_address() helper instead. It's much simpler
> and you don't need to delve into pci_dev internals to use it.
>
> Logan
>
> --
> You received this message because you are subscribed to the Google Groups
> "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to linux-ntb+unsubscribe@googlegroups.com.
> To post to this group, send email to linux-ntb@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/linux-ntb/58db6cb1-c62e-bedd-6542-9a0e40e3dd86%40deltatee.com
> .
> For more options, visit https://groups.google.com/d/optout.
>

[-- Attachment #2: Type: text/html, Size: 2357 bytes --]

[PATCH v2] ntb: idt: Set PCIe bus address to BARLIMITx

[Serge Semin]

IDT NTB driver sets the upper limit of actual translation address
being written to the corresponding memory window setup. It is achieved
by BARLIMITx register initialization. Needless to say, that the register
works within PCIe bus address space.

In general CPU and PCIe address spaces are different. It means,
that addresses used for Memory TLPs routine can be different from
CPU addresses. While in most of cases they are the same, there are
exceptions when the proper mapping must be performed to have the
portable driver code. There used to be a virt_to_bus()/bus_to_virt()
interface for this purpose. But it's deprecated now. It was also a
mistake to use pci_resource_start() since the return address of the
method is at the CPU address space. In order to achieve the desired
purpose we need to use pci_bus_address() helper. This method shall
return a PCIe bus base address of the corresponding BAR resource.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>

---

Changelog v2:
- Replace pcibios_resource_to_bus() with pci_bus_address() helper.

 drivers/ntb/hw/idt/ntb_hw_idt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c
index dbe72f116017..fb2c44ac9c69 100644
--- a/drivers/ntb/hw/idt/ntb_hw_idt.c
+++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
@@ -1320,7 +1320,7 @@ static int idt_ntb_peer_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx,
 		idt_nt_write(ndev, bar->ltbase, (u32)addr);
 		idt_nt_write(ndev, bar->utbase, (u32)(addr >> 32));
 		/* Set the custom BAR aperture limit */
-		limit = pci_resource_start(ntb->pdev, mw_cfg->bar) + size;
+		limit = pci_bus_address(ntb->pdev, mw_cfg->bar) + size;
 		idt_nt_write(ndev, bar->limit, (u32)limit);
 		if (IS_FLD_SET(BARSETUP_TYPE, data, 64))
 			idt_nt_write(ndev, (bar + 1)->limit, (limit >> 32));
-- 
2.12.0

Re: [PATCH v2] ntb: idt: Set PCIe bus address to BARLIMITx

[Allen Hubbe]

On Wed, Jul 11, 2018 at 4:40 PM Serge Semin <fancer.lancer@gmail.com> wrote:
> IDT NTB driver sets the upper limit of actual translation address
> being written to the corresponding memory window setup. It is achieved
> by BARLIMITx register initialization. Needless to say, that the register
> works within PCIe bus address space.
>
> In general CPU and PCIe address spaces are different. It means,
> that addresses used for Memory TLPs routine can be different from
> CPU addresses. While in most of cases they are the same, there are
> exceptions when the proper mapping must be performed to have the
> portable driver code. There used to be a virt_to_bus()/bus_to_virt()
> interface for this purpose. But it's deprecated now. It was also a
> mistake to use pci_resource_start() since the return address of the
> method is at the CPU address space. In order to achieve the desired
> purpose we need to use pci_bus_address() helper. This method shall
> return a PCIe bus base address of the corresponding BAR resource.
>
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>

Acked-by: Allen Hubbe <allenbh@gmail.com>

>
> ---
>
> Changelog v2:
> - Replace pcibios_resource_to_bus() with pci_bus_address() helper.
>
>  drivers/ntb/hw/idt/ntb_hw_idt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c
> index dbe72f116017..fb2c44ac9c69 100644
> --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
> +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
> @@ -1320,7 +1320,7 @@ static int idt_ntb_peer_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx,
>                 idt_nt_write(ndev, bar->ltbase, (u32)addr);
>                 idt_nt_write(ndev, bar->utbase, (u32)(addr >> 32));
>                 /* Set the custom BAR aperture limit */
> -               limit = pci_resource_start(ntb->pdev, mw_cfg->bar) + size;
> +               limit = pci_bus_address(ntb->pdev, mw_cfg->bar) + size;
>                 idt_nt_write(ndev, bar->limit, (u32)limit);
>                 if (IS_FLD_SET(BARSETUP_TYPE, data, 64))
>                         idt_nt_write(ndev, (bar + 1)->limit, (limit >> 32));
> --
> 2.12.0

Re: [PATCH v2] ntb: idt: Set PCIe bus address to BARLIMITx

[Jon Mason]

On Fri, Jul 13, 2018 at 02:31:43PM -0400, Allen Hubbe wrote:
> On Wed, Jul 11, 2018 at 4:40 PM Serge Semin <fancer.lancer@gmail.com> wrote:
> > IDT NTB driver sets the upper limit of actual translation address
> > being written to the corresponding memory window setup. It is achieved
> > by BARLIMITx register initialization. Needless to say, that the register
> > works within PCIe bus address space.
> >
> > In general CPU and PCIe address spaces are different. It means,
> > that addresses used for Memory TLPs routine can be different from
> > CPU addresses. While in most of cases they are the same, there are
> > exceptions when the proper mapping must be performed to have the
> > portable driver code. There used to be a virt_to_bus()/bus_to_virt()
> > interface for this purpose. But it's deprecated now. It was also a
> > mistake to use pci_resource_start() since the return address of the
> > method is at the CPU address space. In order to achieve the desired
> > purpose we need to use pci_bus_address() helper. This method shall
> > return a PCIe bus base address of the corresponding BAR resource.
> >
> > Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> 
> Acked-by: Allen Hubbe <allenbh@gmail.com>

Applied to the ntb-next branch

Thanks,
Jon

> 
> >
> > ---
> >
> > Changelog v2:
> > - Replace pcibios_resource_to_bus() with pci_bus_address() helper.
> >
> >  drivers/ntb/hw/idt/ntb_hw_idt.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c
> > index dbe72f116017..fb2c44ac9c69 100644
> > --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
> > +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
> > @@ -1320,7 +1320,7 @@ static int idt_ntb_peer_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx,
> >                 idt_nt_write(ndev, bar->ltbase, (u32)addr);
> >                 idt_nt_write(ndev, bar->utbase, (u32)(addr >> 32));
> >                 /* Set the custom BAR aperture limit */
> > -               limit = pci_resource_start(ntb->pdev, mw_cfg->bar) + size;
> > +               limit = pci_bus_address(ntb->pdev, mw_cfg->bar) + size;
> >                 idt_nt_write(ndev, bar->limit, (u32)limit);
> >                 if (IS_FLD_SET(BARSETUP_TYPE, data, 64))
> >                         idt_nt_write(ndev, (bar + 1)->limit, (limit >> 32));
> > --
> > 2.12.0