This patchset introduces PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute in
pci_dev_flags which informs the pci subsystem to suppress all realignment
messages.
Powerpc has aligned all of its PCI resources to its PAGE_SIZE, and this
feature can be easily used on any other arch through the __weak
pcibios_default_alignment() interface.
However, since all PCI resources will be now realigned during boot, the
following messages will not only flood the system logs, but can be inter-
preted as a false positive for total PCI failure on the system.
[root@system user]# dmesg | grep -i disabling
[ 0.692270] pci 0000:00:00.0: Disabling memory decoding and releasing memory resources
[ 0.692324] pci 0000:00:00.0: disabling bridge mem windows
[ 0.729134] pci 0001:00:00.0: Disabling memory decoding and releasing memory resources
[ 0.737352] pci 0001:00:00.0: disabling bridge mem windows
[ 0.776295] pci 0002:00:00.0: Disabling memory decoding and releasing memory resources
[ 0.784509] pci 0002:00:00.0: disabling bridge mem windows
... and goes on for all PCI devices ...
Thus, this patchset provides a way for drivers to silent these messages if
the PCI_DEV_FLAGS_QUIET_PCI_REALIGN is turned on. Moreover, it also
tweaks this flag during boot on powerpc.
Fixes: 38274637699 ("powerpc/powernv: Override pcibios_default_alignment() to force PCI devices to be page aligned")
Signed-off-by: Desnes A. Nunes do Rosario <[email protected]>
Desnes A. Nunes do Rosario (2):
pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI
subsystem
powerpc/powernv: Tweak PCI_DEV_FLAGS_QUIET_PCI_REALIGN on/off during
boot
arch/powerpc/platforms/powernv/pci.c | 14 ++++++++++++++
drivers/pci/pci.c | 3 ++-
drivers/pci/setup-res.c | 3 ++-
include/linux/pci.h | 2 ++
4 files changed, 20 insertions(+), 2 deletions(-)
--
2.14.3
Turn on PCI_DEV_FLAGS_QUIET_PCI_REALIGN flag on powernv's pci driver
to silent PCI realignment messages through a early stage hook, and turn
it off right before the pci device is enabled with a final stage hook.
Fixes: 38274637699 ("powerpc/powernv: Override pcibios_default_alignment() to force PCI devices to be page aligned")
Signed-off-by: Desnes A. Nunes do Rosario <[email protected]>
---
arch/powerpc/platforms/powernv/pci.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 69d102cbf48f..d28ce0899487 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -1108,6 +1108,20 @@ static void pnv_p7ioc_rc_quirk(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_IBM, 0x3b9, pnv_p7ioc_rc_quirk);
+/* Fixup that disables PCI realignment menssages for all PCI devices */
+static void disable_realign_menssages(struct pci_dev *dev)
+{
+ dev->dev_flags |= PCI_DEV_FLAGS_QUIET_PCI_REALIGN;
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_ANY_ID, PCI_ANY_ID, disable_realign_menssages);
+
+/* Fixup that enables PCI realignment messages for all PCI devices */
+static void enable_realign_menssages(struct pci_dev *dev)
+{
+ dev->dev_flags ^= PCI_DEV_FLAGS_QUIET_PCI_REALIGN;
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, enable_realign_menssages);
+
void __init pnv_pci_init(void)
{
struct device_node *np;
--
2.14.3
Add PCI_DEV_FLAGS_QUIET_PCI_REALIGN to pci_dev_flags and use it to
silent PCI realignment messages if the flag is turned on by a driver.
Signed-off-by: Desnes A. Nunes do Rosario <[email protected]>
---
drivers/pci/pci.c | 3 ++-
drivers/pci/setup-res.c | 3 ++-
include/linux/pci.h | 2 ++
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8c71d1a66cdd..be197c944e5f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5505,7 +5505,8 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)
return;
}
- pci_info(dev, "Disabling memory decoding and releasing memory resources\n");
+ if (!(dev->dev_flags & PCI_DEV_FLAGS_QUIET_PCI_REALIGN))
+ pci_info(dev, "Disabling memory decoding and releasing memory resources\n");
pci_read_config_word(dev, PCI_COMMAND, &command);
command &= ~PCI_COMMAND_MEMORY;
pci_write_config_word(dev, PCI_COMMAND, command);
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 369d48d6c6f1..00a538def763 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -172,7 +172,8 @@ EXPORT_SYMBOL(pci_claim_resource);
void pci_disable_bridge_window(struct pci_dev *dev)
{
- pci_info(dev, "disabling bridge mem windows\n");
+ if (!(dev->dev_flags & PCI_DEV_FLAGS_QUIET_PCI_REALIGN))
+ pci_info(dev, "disabling bridge mem windows\n");
/* MMIO Base/Limit */
pci_write_config_dword(dev, PCI_MEMORY_BASE, 0x0000fff0);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e057e8cc63e7..993f9c7dcc00 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -205,6 +205,8 @@ enum pci_dev_flags {
PCI_DEV_FLAGS_NO_FLR_RESET = (__force pci_dev_flags_t) (1 << 10),
/* Don't use Relaxed Ordering for TLPs directed at this device */
PCI_DEV_FLAGS_NO_RELAXED_ORDERING = (__force pci_dev_flags_t) (1 << 11),
+ /* Silent PCI resource realignment messages */
+ PCI_DEV_FLAGS_QUIET_PCI_REALIGN = (__force pci_dev_flags_t) (1 << 12),
};
enum pci_irq_reroute_variant {
--
2.14.3
On Wed, Mar 14, 2018 at 6:34 PM, Desnes A. Nunes do Rosario
<[email protected]> wrote:
> Add PCI_DEV_FLAGS_QUIET_PCI_REALIGN to pci_dev_flags and use it to
> silent PCI realignment messages if the flag is turned on by a driver.
It doesn't explain "Why?"
Why the driver needs that?
Another approach is to increase level of the message. Would it be
accepted by you (in case Bjorn agrees)?
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -205,6 +205,8 @@ enum pci_dev_flags {
> PCI_DEV_FLAGS_NO_FLR_RESET = (__force pci_dev_flags_t) (1 << 10),
> /* Don't use Relaxed Ordering for TLPs directed at this device */
> PCI_DEV_FLAGS_NO_RELAXED_ORDERING = (__force pci_dev_flags_t) (1 << 11),
> + /* Silent PCI resource realignment messages */
> + PCI_DEV_FLAGS_QUIET_PCI_REALIGN = (__force pci_dev_flags_t) (1 << 12),
I would rather name it _NO_PCI_REALIGN_MSG
> };
--
With Best Regards,
Andy Shevchenko
On Wed, Mar 14, 2018 at 7:41 PM, Andy Shevchenko
<[email protected]> wrote:
> I would rather name it _NO_PCI_REALIGN_MSG
I meant _NO_REALIGN_MSG of course (PCI is already at the beginning).
--
With Best Regards,
Andy Shevchenko
On Wed, Mar 14, 2018 at 01:34:54PM -0300, Desnes A. Nunes do Rosario wrote:
> Add PCI_DEV_FLAGS_QUIET_PCI_REALIGN to pci_dev_flags and use it to
> silent PCI realignment messages if the flag is turned on by a driver.
>
> Signed-off-by: Desnes A. Nunes do Rosario <[email protected]>
> ---
> drivers/pci/pci.c | 3 ++-
> drivers/pci/setup-res.c | 3 ++-
> include/linux/pci.h | 2 ++
> 3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8c71d1a66cdd..be197c944e5f 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5505,7 +5505,8 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)
> return;
> }
>
> - pci_info(dev, "Disabling memory decoding and releasing memory resources\n");
> + if (!(dev->dev_flags & PCI_DEV_FLAGS_QUIET_PCI_REALIGN))
> + pci_info(dev, "Disabling memory decoding and releasing memory resources\n");
> pci_read_config_word(dev, PCI_COMMAND, &command);
> command &= ~PCI_COMMAND_MEMORY;
> pci_write_config_word(dev, PCI_COMMAND, command);
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 369d48d6c6f1..00a538def763 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -172,7 +172,8 @@ EXPORT_SYMBOL(pci_claim_resource);
>
> void pci_disable_bridge_window(struct pci_dev *dev)
> {
> - pci_info(dev, "disabling bridge mem windows\n");
> + if (!(dev->dev_flags & PCI_DEV_FLAGS_QUIET_PCI_REALIGN))
> + pci_info(dev, "disabling bridge mem windows\n");
As far as I'm concerned, we can just remove these messages completely.
I don't think there's any real value there.
> /* MMIO Base/Limit */
> pci_write_config_dword(dev, PCI_MEMORY_BASE, 0x0000fff0);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e057e8cc63e7..993f9c7dcc00 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -205,6 +205,8 @@ enum pci_dev_flags {
> PCI_DEV_FLAGS_NO_FLR_RESET = (__force pci_dev_flags_t) (1 << 10),
> /* Don't use Relaxed Ordering for TLPs directed at this device */
> PCI_DEV_FLAGS_NO_RELAXED_ORDERING = (__force pci_dev_flags_t) (1 << 11),
> + /* Silent PCI resource realignment messages */
> + PCI_DEV_FLAGS_QUIET_PCI_REALIGN = (__force pci_dev_flags_t) (1 << 12),
> };
>
> enum pci_irq_reroute_variant {
> --
> 2.14.3
>
Hello Andy,
On 03/14/2018 02:41 PM, Andy Shevchenko wrote:
> On Wed, Mar 14, 2018 at 6:34 PM, Desnes A. Nunes do Rosario
> <[email protected]> wrote:
>> Add PCI_DEV_FLAGS_QUIET_PCI_REALIGN to pci_dev_flags and use it to
>> silent PCI realignment messages if the flag is turned on by a driver.
>
> It doesn't explain "Why?"
> Why the driver needs that?
I have written down the reason on the cover letter, but I concur on
creating a second version of the patch's comment. Basically, all PCI
resources on powerpc are printing out expected realignment messages
which are flooding the systems logs.
Perhaps this would be better?
---
"Some architectures such as powerpc has aligned all of its PCI resources
to its PAGE_SIZE during boot, thus the system logs will be flooded by
expected realignment messages, which can be interpreted as a false
positive for total PCI failure on the system.
[root@system user]# dmesg | grep -i disabling
[ 0.692270] pci 0000:00:00.0: Disabling memory decoding and releasing
memory resources
[ 0.692324] pci 0000:00:00.0: disabling bridge mem windows
[ 0.729134] pci 0001:00:00.0: Disabling memory decoding and releasing
memory resources
[ 0.737352] pci 0001:00:00.0: disabling bridge mem windows
[ 0.776295] pci 0002:00:00.0: Disabling memory decoding and releasing
memory resources
[ 0.784509] pci 0002:00:00.0: disabling bridge mem windows
... and goes on for all PCI devices ...
Thus, this patch adds PCI_DEV_FLAGS_NO_REALIGN_MSG to pci_dev_flags and
uses it to silent PCI realignment messages if the flag is turned on by a
driver.
"
---
>
> Another approach is to increase level of the message. Would it be
> accepted by you (in case Bjorn agrees)?
>
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -205,6 +205,8 @@ enum pci_dev_flags {
>> PCI_DEV_FLAGS_NO_FLR_RESET = (__force pci_dev_flags_t) (1 << 10),
>> /* Don't use Relaxed Ordering for TLPs directed at this device */
>> PCI_DEV_FLAGS_NO_RELAXED_ORDERING = (__force pci_dev_flags_t) (1 << 11),
>> + /* Silent PCI resource realignment messages */
>> + PCI_DEV_FLAGS_QUIET_PCI_REALIGN = (__force pci_dev_flags_t) (1 << 12),
>
> I would rather name it _NO_PCI_REALIGN_MSG
I concur on changing it to PCI_DEV_FLAGS_NO_REALIGN_MSG in a second
version of the patchset.
>
>> };
>
Thank you very much for your review,
--
Desnes A. Nunes do Rosário
Hello Bjorn,
On 03/14/2018 03:06 PM, Bjorn Helgaas wrote:
> On Wed, Mar 14, 2018 at 01:34:54PM -0300, Desnes A. Nunes do Rosario wrote:
>> Add PCI_DEV_FLAGS_QUIET_PCI_REALIGN to pci_dev_flags and use it to
>> silent PCI realignment messages if the flag is turned on by a driver.
>>
>> Signed-off-by: Desnes A. Nunes do Rosario <[email protected]>
>> ---
>> drivers/pci/pci.c | 3 ++-
>> drivers/pci/setup-res.c | 3 ++-
>> include/linux/pci.h | 2 ++
>> 3 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 8c71d1a66cdd..be197c944e5f 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -5505,7 +5505,8 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)
>> return;
>> }
>>
>> - pci_info(dev, "Disabling memory decoding and releasing memory resources\n");
>> + if (!(dev->dev_flags & PCI_DEV_FLAGS_QUIET_PCI_REALIGN))
>> + pci_info(dev, "Disabling memory decoding and releasing memory resources\n");
>> pci_read_config_word(dev, PCI_COMMAND, &command);
>> command &= ~PCI_COMMAND_MEMORY;
>> pci_write_config_word(dev, PCI_COMMAND, command);
>> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
>> index 369d48d6c6f1..00a538def763 100644
>> --- a/drivers/pci/setup-res.c
>> +++ b/drivers/pci/setup-res.c
>> @@ -172,7 +172,8 @@ EXPORT_SYMBOL(pci_claim_resource);
>>
>> void pci_disable_bridge_window(struct pci_dev *dev)
>> {
>> - pci_info(dev, "disabling bridge mem windows\n");
>> + if (!(dev->dev_flags & PCI_DEV_FLAGS_QUIET_PCI_REALIGN))
>> + pci_info(dev, "disabling bridge mem windows\n");
>
> As far as I'm concerned, we can just remove these messages completely.
> I don't think there's any real value there.
After I found out that this was happening to all PCI devices on powerpc
due to the __weak
pcibios_default_alignment() interface (necessary for VFIO passthrough
and performance), I confess that this was my first approach to this
matter; however I couldn't vouch the need of these messages on other
architectures.
If there are no further concerns, I definitely prefer sending a second
version of this patch only eliminating these messages and attesting the
reason why.
Thank you very much for your review Bjorn,
--
Desnes A. Nunes do Rosário
On Wed, Mar 14, 2018 at 03:22:44PM -0300, Desnes Augusto Nunes do Ros?rio wrote:
> Hello Bjorn,
>
> On 03/14/2018 03:06 PM, Bjorn Helgaas wrote:
> > On Wed, Mar 14, 2018 at 01:34:54PM -0300, Desnes A. Nunes do Rosario wrote:
> > > Add PCI_DEV_FLAGS_QUIET_PCI_REALIGN to pci_dev_flags and use it to
> > > silent PCI realignment messages if the flag is turned on by a driver.
> > >
> > > Signed-off-by: Desnes A. Nunes do Rosario <[email protected]>
> > > ---
> > > drivers/pci/pci.c | 3 ++-
> > > drivers/pci/setup-res.c | 3 ++-
> > > include/linux/pci.h | 2 ++
> > > 3 files changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index 8c71d1a66cdd..be197c944e5f 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -5505,7 +5505,8 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)
> > > return;
> > > }
> > > - pci_info(dev, "Disabling memory decoding and releasing memory resources\n");
> > > + if (!(dev->dev_flags & PCI_DEV_FLAGS_QUIET_PCI_REALIGN))
> > > + pci_info(dev, "Disabling memory decoding and releasing memory resources\n");
> > > pci_read_config_word(dev, PCI_COMMAND, &command);
> > > command &= ~PCI_COMMAND_MEMORY;
> > > pci_write_config_word(dev, PCI_COMMAND, command);
> > > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> > > index 369d48d6c6f1..00a538def763 100644
> > > --- a/drivers/pci/setup-res.c
> > > +++ b/drivers/pci/setup-res.c
> > > @@ -172,7 +172,8 @@ EXPORT_SYMBOL(pci_claim_resource);
> > > void pci_disable_bridge_window(struct pci_dev *dev)
> > > {
> > > - pci_info(dev, "disabling bridge mem windows\n");
> > > + if (!(dev->dev_flags & PCI_DEV_FLAGS_QUIET_PCI_REALIGN))
> > > + pci_info(dev, "disabling bridge mem windows\n");
> >
> > As far as I'm concerned, we can just remove these messages completely.
> > I don't think there's any real value there.
>
> After I found out that this was happening to all PCI devices on powerpc due
> to the __weak
> pcibios_default_alignment() interface (necessary for VFIO passthrough and
> performance), I confess that this was my first approach to this matter;
> however I couldn't vouch the need of these messages on other architectures.
>
> If there are no further concerns, I definitely prefer sending a second
> version of this patch only eliminating these messages and attesting the
> reason why.
>
> Thank you very much for your review Bjorn,
No problem, welcome to PCI, and I hope we see more of your work!
On 03/14/2018 03:55 PM, Bjorn Helgaas wrote:
>>> As far as I'm concerned, we can just remove these messages completely.
>>> I don't think there's any real value there.
>>
>> After I found out that this was happening to all PCI devices on powerpc due
>> to the __weak
>> pcibios_default_alignment() interface (necessary for VFIO passthrough and
>> performance), I confess that this was my first approach to this matter;
>> however I couldn't vouch the need of these messages on other architectures.
>>
>> If there are no further concerns, I definitely prefer sending a second
>> version of this patch only eliminating these messages and attesting the
>> reason why.
>>
>> Thank you very much for your review Bjorn,
>
> No problem, welcome to PCI, and I hope we see more of your work!
Bjorn,
Awesome! A second version of this fix has been sent with a new title:
"[PATCH, pci, v2] pci: Delete PCI disabling informational messages"
Thanks for the review and warm welcome!
--
Desnes A. Nunes do Rosário
--------------------------
Hi Desnes,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Desnes-A-Nunes-do-Rosario/powerpc-powernv-Tweak-PCI_DEV_FLAGS_QUIET_PCI_REALIGN-on-off-during-boot/20180316-112524
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
arch/powerpc/platforms/powernv/pci.c: In function 'disable_realign_menssages':
>> arch/powerpc/platforms/powernv/pci.c:1114:20: error: 'PCI_DEV_FLAGS_QUIET_PCI_REALIGN' undeclared (first use in this function); did you mean 'PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS'?
dev->dev_flags |= PCI_DEV_FLAGS_QUIET_PCI_REALIGN;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS
arch/powerpc/platforms/powernv/pci.c:1114:20: note: each undeclared identifier is reported only once for each function it appears in
arch/powerpc/platforms/powernv/pci.c: In function 'enable_realign_menssages':
arch/powerpc/platforms/powernv/pci.c:1121:20: error: 'PCI_DEV_FLAGS_QUIET_PCI_REALIGN' undeclared (first use in this function); did you mean 'PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS'?
dev->dev_flags ^= PCI_DEV_FLAGS_QUIET_PCI_REALIGN;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS
vim +1114 arch/powerpc/platforms/powernv/pci.c
1110
1111 /* Fixup that disables PCI realignment menssages for all PCI devices */
1112 static void disable_realign_menssages(struct pci_dev *dev)
1113 {
> 1114 dev->dev_flags |= PCI_DEV_FLAGS_QUIET_PCI_REALIGN;
1115 }
1116 DECLARE_PCI_FIXUP_EARLY(PCI_ANY_ID, PCI_ANY_ID, disable_realign_menssages);
1117
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation