* Re: [PATCH] PCI: mvebu: Fix endianity when accessing pci emul bridge members
2022-08-12 9:40 [PATCH] PCI: mvebu: Fix endianity when accessing pci emul bridge members Pali Rohár
@ 2022-08-12 10:32 ` Robin Murphy
2022-08-12 13:58 ` Pali Rohár
2022-08-12 14:11 ` [PATCH v2] " Pali Rohár
2022-08-12 21:12 ` [PATCH] " kernel test robot
2 siblings, 1 reply; 6+ messages in thread
From: Robin Murphy @ 2022-08-12 10:32 UTC (permalink / raw)
To: Pali Rohár, Lorenzo Pieralisi, Thomas Petazzoni,
Krzysztof Wilczyński, Bjorn Helgaas
Cc: linux-pci, linux-arm-kernel, linux-kernel
On 2022-08-12 10:40, Pali Rohár wrote:
> PCI emul bridge members iolimitupper, iobaseupper, memlimit and membase are
> of type __le16, so correctly access these members via le16_to_cpu() macros.
>
> Fixes: 4ded69473adb ("PCI: mvebu: Propagate errors when updating PCI_IO_BASE and PCI_MEM_BASE registers")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> drivers/pci/controller/pci-mvebu.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> index c1ffdb06c971..00ea0836b81a 100644
> --- a/drivers/pci/controller/pci-mvebu.c
> +++ b/drivers/pci/controller/pci-mvebu.c
> @@ -523,7 +523,7 @@ static int mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
>
> /* Are the new iobase/iolimit values invalid? */
> if (conf->iolimit < conf->iobase ||
> - conf->iolimitupper < conf->iobaseupper)
> + le16_to_cpu(conf->iolimitupper) < le16_to_cpu(conf->iobaseupper))
> return mvebu_pcie_set_window(port, port->io_target, port->io_attr,
> &desired, &port->iowin);
>
> @@ -535,10 +535,10 @@ static int mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
> * is the CPU address.
> */
> desired.remap = ((conf->iobase & 0xF0) << 8) |
> - (conf->iobaseupper << 16);
> + le16_to_cpu(conf->iobaseupper << 16);
This will always give 0, even when natively LE.
> desired.base = port->pcie->io.start + desired.remap;
> desired.size = ((0xFFF | ((conf->iolimit & 0xF0) << 8) |
> - (conf->iolimitupper << 16)) -
> + le16_to_cpu(conf->iolimitupper << 16)) -
Similarly here.
Robin.
> desired.remap) +
> 1;
>
> @@ -552,7 +552,7 @@ static int mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
> struct pci_bridge_emul_conf *conf = &port->bridge.conf;
>
> /* Are the new membase/memlimit values invalid? */
> - if (conf->memlimit < conf->membase)
> + if (le16_to_cpu(conf->memlimit) < le16_to_cpu(conf->membase))
> return mvebu_pcie_set_window(port, port->mem_target, port->mem_attr,
> &desired, &port->memwin);
>
> @@ -562,8 +562,8 @@ static int mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
> * window to setup, according to the PCI-to-PCI bridge
> * specifications.
> */
> - desired.base = ((conf->membase & 0xFFF0) << 16);
> - desired.size = (((conf->memlimit & 0xFFF0) << 16) | 0xFFFFF) -
> + desired.base = ((le16_to_cpu(conf->membase) & 0xFFF0) << 16);
> + desired.size = (((le16_to_cpu(conf->memlimit) & 0xFFF0) << 16) | 0xFFFFF) -
> desired.base + 1;
>
> return mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, &desired,
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] PCI: mvebu: Fix endianity when accessing pci emul bridge members
2022-08-12 10:32 ` Robin Murphy
@ 2022-08-12 13:58 ` Pali Rohár
0 siblings, 0 replies; 6+ messages in thread
From: Pali Rohár @ 2022-08-12 13:58 UTC (permalink / raw)
To: Robin Murphy
Cc: Lorenzo Pieralisi, Thomas Petazzoni, Krzysztof Wilczyński,
Bjorn Helgaas, linux-pci, linux-arm-kernel, linux-kernel
On Friday 12 August 2022 11:32:59 Robin Murphy wrote:
> On 2022-08-12 10:40, Pali Rohár wrote:
> > PCI emul bridge members iolimitupper, iobaseupper, memlimit and membase are
> > of type __le16, so correctly access these members via le16_to_cpu() macros.
> >
> > Fixes: 4ded69473adb ("PCI: mvebu: Propagate errors when updating PCI_IO_BASE and PCI_MEM_BASE registers")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> > drivers/pci/controller/pci-mvebu.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> > index c1ffdb06c971..00ea0836b81a 100644
> > --- a/drivers/pci/controller/pci-mvebu.c
> > +++ b/drivers/pci/controller/pci-mvebu.c
> > @@ -523,7 +523,7 @@ static int mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
> > /* Are the new iobase/iolimit values invalid? */
> > if (conf->iolimit < conf->iobase ||
> > - conf->iolimitupper < conf->iobaseupper)
> > + le16_to_cpu(conf->iolimitupper) < le16_to_cpu(conf->iobaseupper))
> > return mvebu_pcie_set_window(port, port->io_target, port->io_attr,
> > &desired, &port->iowin);
> > @@ -535,10 +535,10 @@ static int mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
> > * is the CPU address.
> > */
> > desired.remap = ((conf->iobase & 0xF0) << 8) |
> > - (conf->iobaseupper << 16);
> > + le16_to_cpu(conf->iobaseupper << 16);
>
> This will always give 0, even when natively LE.
You are right, I overlooked it and I put closing parenthesis at wrong
place. Bit shifting should be applied after le to cpu conversion. I will
fix it in V2.
> > desired.base = port->pcie->io.start + desired.remap;
> > desired.size = ((0xFFF | ((conf->iolimit & 0xF0) << 8) |
> > - (conf->iolimitupper << 16)) -
> > + le16_to_cpu(conf->iolimitupper << 16)) -
>
> Similarly here.
>
> Robin.
>
> > desired.remap) +
> > 1;
> > @@ -552,7 +552,7 @@ static int mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
> > struct pci_bridge_emul_conf *conf = &port->bridge.conf;
> > /* Are the new membase/memlimit values invalid? */
> > - if (conf->memlimit < conf->membase)
> > + if (le16_to_cpu(conf->memlimit) < le16_to_cpu(conf->membase))
> > return mvebu_pcie_set_window(port, port->mem_target, port->mem_attr,
> > &desired, &port->memwin);
> > @@ -562,8 +562,8 @@ static int mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
> > * window to setup, according to the PCI-to-PCI bridge
> > * specifications.
> > */
> > - desired.base = ((conf->membase & 0xFFF0) << 16);
> > - desired.size = (((conf->memlimit & 0xFFF0) << 16) | 0xFFFFF) -
> > + desired.base = ((le16_to_cpu(conf->membase) & 0xFFF0) << 16);
> > + desired.size = (((le16_to_cpu(conf->memlimit) & 0xFFF0) << 16) | 0xFFFFF) -
> > desired.base + 1;
> > return mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, &desired,
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] PCI: mvebu: Fix endianity when accessing pci emul bridge members
2022-08-12 9:40 [PATCH] PCI: mvebu: Fix endianity when accessing pci emul bridge members Pali Rohár
2022-08-12 10:32 ` Robin Murphy
@ 2022-08-12 14:11 ` Pali Rohár
2022-08-23 10:23 ` Lorenzo Pieralisi
2022-08-12 21:12 ` [PATCH] " kernel test robot
2 siblings, 1 reply; 6+ messages in thread
From: Pali Rohár @ 2022-08-12 14:11 UTC (permalink / raw)
To: Lorenzo Pieralisi, Thomas Petazzoni, Krzysztof Wilczyński,
Bjorn Helgaas, Robin Murphy
Cc: linux-pci, linux-arm-kernel, linux-kernel
PCI emul bridge members iolimitupper, iobaseupper, memlimit and membase are
of type __le16, so correctly access these members via le16_to_cpu() macros.
Fixes: 4ded69473adb ("PCI: mvebu: Propagate errors when updating PCI_IO_BASE and PCI_MEM_BASE registers")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Pali Rohár <pali@kernel.org>
---
Changes in v2:
* Fix parenthesis around le16_to_cpu() calls
---
drivers/pci/controller/pci-mvebu.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
index 0798ed182a96..b04b9bbe9217 100644
--- a/drivers/pci/controller/pci-mvebu.c
+++ b/drivers/pci/controller/pci-mvebu.c
@@ -587,7 +587,7 @@ static int mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
/* Are the new iobase/iolimit values invalid? */
if (conf->iolimit < conf->iobase ||
- conf->iolimitupper < conf->iobaseupper)
+ le16_to_cpu(conf->iolimitupper) < le16_to_cpu(conf->iobaseupper))
return mvebu_pcie_set_window(port, port->io_target, port->io_attr,
&desired, &port->iowin);
@@ -599,10 +599,10 @@ static int mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
* is the CPU address.
*/
desired.remap = ((conf->iobase & 0xF0) << 8) |
- (conf->iobaseupper << 16);
+ (le16_to_cpu(conf->iobaseupper) << 16);
desired.base = port->pcie->io.start + desired.remap;
desired.size = ((0xFFF | ((conf->iolimit & 0xF0) << 8) |
- (conf->iolimitupper << 16)) -
+ (le16_to_cpu(conf->iolimitupper) << 16)) -
desired.remap) +
1;
@@ -616,7 +616,7 @@ static int mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
struct pci_bridge_emul_conf *conf = &port->bridge.conf;
/* Are the new membase/memlimit values invalid? */
- if (conf->memlimit < conf->membase)
+ if (le16_to_cpu(conf->memlimit) < le16_to_cpu(conf->membase))
return mvebu_pcie_set_window(port, port->mem_target, port->mem_attr,
&desired, &port->memwin);
@@ -626,8 +626,8 @@ static int mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
* window to setup, according to the PCI-to-PCI bridge
* specifications.
*/
- desired.base = ((conf->membase & 0xFFF0) << 16);
- desired.size = (((conf->memlimit & 0xFFF0) << 16) | 0xFFFFF) -
+ desired.base = ((le16_to_cpu(conf->membase) & 0xFFF0) << 16);
+ desired.size = (((le16_to_cpu(conf->memlimit) & 0xFFF0) << 16) | 0xFFFFF) -
desired.base + 1;
return mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, &desired,
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] PCI: mvebu: Fix endianity when accessing pci emul bridge members
2022-08-12 14:11 ` [PATCH v2] " Pali Rohár
@ 2022-08-23 10:23 ` Lorenzo Pieralisi
0 siblings, 0 replies; 6+ messages in thread
From: Lorenzo Pieralisi @ 2022-08-23 10:23 UTC (permalink / raw)
To: Robin Murphy, Bjorn Helgaas, Thomas Petazzoni, Pali Rohár,
Krzysztof Wilczyński
Cc: Lorenzo Pieralisi, linux-pci, linux-arm-kernel, linux-kernel
On Fri, 12 Aug 2022 16:11:15 +0200, Pali Rohár wrote:
> PCI emul bridge members iolimitupper, iobaseupper, memlimit and membase are
> of type __le16, so correctly access these members via le16_to_cpu() macros.
>
>
Applied to pci/mvebu, thanks!
[1/1] PCI: mvebu: Fix endianity when accessing pci emul bridge members
https://git.kernel.org/lpieralisi/pci/c/2e379ac66d4b
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: mvebu: Fix endianity when accessing pci emul bridge members
2022-08-12 9:40 [PATCH] PCI: mvebu: Fix endianity when accessing pci emul bridge members Pali Rohár
2022-08-12 10:32 ` Robin Murphy
2022-08-12 14:11 ` [PATCH v2] " Pali Rohár
@ 2022-08-12 21:12 ` kernel test robot
2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2022-08-12 21:12 UTC (permalink / raw)
To: Pali Rohár, Lorenzo Pieralisi, Thomas Petazzoni,
Krzysztof Wilczyński, Bjorn Helgaas
Cc: kbuild-all, linux-pci, linux-arm-kernel, linux-kernel
Hi "Pali,
I love your patch! Perhaps something to improve:
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on linus/master v5.19 next-20220812]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Pali-Roh-r/PCI-mvebu-Fix-endianity-when-accessing-pci-emul-bridge-members/20220812-174306
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arm-randconfig-s041-20220812 (https://download.01.org/0day-ci/archive/20220813/202208130542.WpzYnXZ3-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/525898e0a8389f8bb560edbce3f07a3ec2550968
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Pali-Roh-r/PCI-mvebu-Fix-endianity-when-accessing-pci-emul-bridge-members/20220812-174306
git checkout 525898e0a8389f8bb560edbce3f07a3ec2550968
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm SHELL=/bin/bash drivers/pci/controller/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
drivers/pci/controller/pci-mvebu.c:538:25: sparse: sparse: restricted __le16 degrades to integer
>> drivers/pci/controller/pci-mvebu.c:538:25: sparse: sparse: cast to restricted __le16
drivers/pci/controller/pci-mvebu.c:541:26: sparse: sparse: restricted __le16 degrades to integer
drivers/pci/controller/pci-mvebu.c:541:26: sparse: sparse: cast to restricted __le16
vim +538 drivers/pci/controller/pci-mvebu.c
518
519 static int mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
520 {
521 struct mvebu_pcie_window desired = {};
522 struct pci_bridge_emul_conf *conf = &port->bridge.conf;
523
524 /* Are the new iobase/iolimit values invalid? */
525 if (conf->iolimit < conf->iobase ||
526 le16_to_cpu(conf->iolimitupper) < le16_to_cpu(conf->iobaseupper))
527 return mvebu_pcie_set_window(port, port->io_target, port->io_attr,
528 &desired, &port->iowin);
529
530 /*
531 * We read the PCI-to-PCI bridge emulated registers, and
532 * calculate the base address and size of the address decoding
533 * window to setup, according to the PCI-to-PCI bridge
534 * specifications. iobase is the bus address, port->iowin_base
535 * is the CPU address.
536 */
537 desired.remap = ((conf->iobase & 0xF0) << 8) |
> 538 le16_to_cpu(conf->iobaseupper << 16);
539 desired.base = port->pcie->io.start + desired.remap;
540 desired.size = ((0xFFF | ((conf->iolimit & 0xF0) << 8) |
541 le16_to_cpu(conf->iolimitupper << 16)) -
542 desired.remap) +
543 1;
544
545 return mvebu_pcie_set_window(port, port->io_target, port->io_attr, &desired,
546 &port->iowin);
547 }
548
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 6+ messages in thread