| Message ID | 20210708181900.8047-1-pali@kernel.org |
|---|---|
| State | Accepted |
| Commit | a83149509131b3c5cd1811f953cf96c213308eef |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [1/3] arm: a37xx: pci: Extend validation for PCIe resources and oubound windows | expand |
> -----Original Message----- > From: Pali Rohár <pali@kernel.org> > Sent: Thursday, July 8, 2021 21:19 > To: Stefan Roese <sr@denx.de>; Kostya Porotchkin <kostap@marvell.com> > Cc: Marek Behún <marek.behun@nic.cz>; u-boot@lists.denx.de > Subject: [EXT] [PATCH 1/3] arm: a37xx: pci: Extend validation for PCIe > resources and oubound windows > > External Email > > ---------------------------------------------------------------------- > Remapped address of PCIe outbound window may have set only bits from the > mask. Add additional check that remapped address which is calculated from > PCIe bus address specified in DTS file is valid. > > Remove also useless clearing of low 16 bits in win_mask. As win_size is power > of two and is at least 0x10000 it means that it always has zero low > 16 bits. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > drivers/pci/pci-aardvark.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c index > 96aa039bdc26..77ce9d0c8d87 100644 > --- a/drivers/pci/pci-aardvark.c > +++ b/drivers/pci/pci-aardvark.c > @@ -609,21 +609,22 @@ static void pcie_advk_set_ob_region(struct > pcie_advk *pcie, int *wins, > * match with given mask. > * So every PCIe window size must be a power of two and every start > * address must be aligned to window size. Minimal size is 64 KiB > - * because lower 16 bits of mask must be zero. > + * because lower 16 bits of mask must be zero. Remapped address > + * may have set only bits from the mask. > */ > while (*wins < OB_WIN_COUNT && size > 0) { > /* Calculate the largest aligned window size */ > win_size = (1ULL << (fls64(size) - 1)) | > (phys_start ? (1ULL << __ffs64(phys_start)) : 0); > win_size = 1ULL << __ffs64(win_size); > - if (win_size < 0x10000) > + win_mask = ~(win_size - 1); > + if (win_size < 0x10000 || (bus_start & ~win_mask)) > break; > > dev_dbg(pcie->dev, > "Configuring PCIe window %d: [0x%llx-0x%llx] as > 0x%x\n", > *wins, (u64)phys_start, (u64)phys_start + win_size, > actions); > - win_mask = ~(win_size - 1) & ~0xffff; > pcie_advk_set_ob_win(pcie, *wins, phys_start, bus_start, > win_mask, actions); > > -- > 2.20.1 Reviewed-by: Konstantin Porotchkin <kostap@marvell.com>
On 08.07.21 20:18, Pali Rohár wrote: > Remapped address of PCIe outbound window may have set only bits from the > mask. Add additional check that remapped address which is calculated from > PCIe bus address specified in DTS file is valid. > > Remove also useless clearing of low 16 bits in win_mask. As win_size is > power of two and is at least 0x10000 it means that it always has zero low > 16 bits. > > Signed-off-by: Pali Rohár <pali@kernel.org> Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > --- > drivers/pci/pci-aardvark.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c > index 96aa039bdc26..77ce9d0c8d87 100644 > --- a/drivers/pci/pci-aardvark.c > +++ b/drivers/pci/pci-aardvark.c > @@ -609,21 +609,22 @@ static void pcie_advk_set_ob_region(struct pcie_advk *pcie, int *wins, > * match with given mask. > * So every PCIe window size must be a power of two and every start > * address must be aligned to window size. Minimal size is 64 KiB > - * because lower 16 bits of mask must be zero. > + * because lower 16 bits of mask must be zero. Remapped address > + * may have set only bits from the mask. > */ > while (*wins < OB_WIN_COUNT && size > 0) { > /* Calculate the largest aligned window size */ > win_size = (1ULL << (fls64(size) - 1)) | > (phys_start ? (1ULL << __ffs64(phys_start)) : 0); > win_size = 1ULL << __ffs64(win_size); > - if (win_size < 0x10000) > + win_mask = ~(win_size - 1); > + if (win_size < 0x10000 || (bus_start & ~win_mask)) > break; > > dev_dbg(pcie->dev, > "Configuring PCIe window %d: [0x%llx-0x%llx] as 0x%x\n", > *wins, (u64)phys_start, (u64)phys_start + win_size, > actions); > - win_mask = ~(win_size - 1) & ~0xffff; > pcie_advk_set_ob_win(pcie, *wins, phys_start, bus_start, > win_mask, actions); > > Viele Grüße, Stefan
On 08.07.21 20:18, Pali Rohár wrote: > Remapped address of PCIe outbound window may have set only bits from the > mask. Add additional check that remapped address which is calculated from > PCIe bus address specified in DTS file is valid. > > Remove also useless clearing of low 16 bits in win_mask. As win_size is > power of two and is at least 0x10000 it means that it always has zero low > 16 bits. > > Signed-off-by: Pali Rohár <pali@kernel.org> All patches: Applied to u-boot-marvell/master Thanks, Stefan > --- > drivers/pci/pci-aardvark.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c > index 96aa039bdc26..77ce9d0c8d87 100644 > --- a/drivers/pci/pci-aardvark.c > +++ b/drivers/pci/pci-aardvark.c > @@ -609,21 +609,22 @@ static void pcie_advk_set_ob_region(struct pcie_advk *pcie, int *wins, > * match with given mask. > * So every PCIe window size must be a power of two and every start > * address must be aligned to window size. Minimal size is 64 KiB > - * because lower 16 bits of mask must be zero. > + * because lower 16 bits of mask must be zero. Remapped address > + * may have set only bits from the mask. > */ > while (*wins < OB_WIN_COUNT && size > 0) { > /* Calculate the largest aligned window size */ > win_size = (1ULL << (fls64(size) - 1)) | > (phys_start ? (1ULL << __ffs64(phys_start)) : 0); > win_size = 1ULL << __ffs64(win_size); > - if (win_size < 0x10000) > + win_mask = ~(win_size - 1); > + if (win_size < 0x10000 || (bus_start & ~win_mask)) > break; > > dev_dbg(pcie->dev, > "Configuring PCIe window %d: [0x%llx-0x%llx] as 0x%x\n", > *wins, (u64)phys_start, (u64)phys_start + win_size, > actions); > - win_mask = ~(win_size - 1) & ~0xffff; > pcie_advk_set_ob_win(pcie, *wins, phys_start, bus_start, > win_mask, actions); > > Viele Grüße, Stefan
diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c index 96aa039bdc26..77ce9d0c8d87 100644 --- a/drivers/pci/pci-aardvark.c +++ b/drivers/pci/pci-aardvark.c @@ -609,21 +609,22 @@ static void pcie_advk_set_ob_region(struct pcie_advk *pcie, int *wins, * match with given mask. * So every PCIe window size must be a power of two and every start * address must be aligned to window size. Minimal size is 64 KiB - * because lower 16 bits of mask must be zero. + * because lower 16 bits of mask must be zero. Remapped address + * may have set only bits from the mask. */ while (*wins < OB_WIN_COUNT && size > 0) { /* Calculate the largest aligned window size */ win_size = (1ULL << (fls64(size) - 1)) | (phys_start ? (1ULL << __ffs64(phys_start)) : 0); win_size = 1ULL << __ffs64(win_size); - if (win_size < 0x10000) + win_mask = ~(win_size - 1); + if (win_size < 0x10000 || (bus_start & ~win_mask)) break; dev_dbg(pcie->dev, "Configuring PCIe window %d: [0x%llx-0x%llx] as 0x%x\n", *wins, (u64)phys_start, (u64)phys_start + win_size, actions); - win_mask = ~(win_size - 1) & ~0xffff; pcie_advk_set_ob_win(pcie, *wins, phys_start, bus_start, win_mask, actions);
Remapped address of PCIe outbound window may have set only bits from the mask. Add additional check that remapped address which is calculated from PCIe bus address specified in DTS file is valid. Remove also useless clearing of low 16 bits in win_mask. As win_size is power of two and is at least 0x10000 it means that it always has zero low 16 bits. Signed-off-by: Pali Rohár <pali@kernel.org> --- drivers/pci/pci-aardvark.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)