linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Silent PCI realignment messages during boot
@ 2018-03-14 16:34 Desnes A. Nunes do Rosario
  2018-03-14 16:34 ` [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem Desnes A. Nunes do Rosario
  2018-03-14 16:34 ` [PATCH 2/2, powerpc/powernv] powerpc/powernv: Tweak PCI_DEV_FLAGS_QUIET_PCI_REALIGN on/off during boot Desnes A. Nunes do Rosario
  0 siblings, 2 replies; 11+ messages in thread
From: Desnes A. Nunes do Rosario @ 2018-03-14 16:34 UTC (permalink / raw)
  To: linux-kernel, linux-pci, linuxppc-dev
  Cc: bhelgaas, benh, paulus, mpe, ruscur, aik, david, fbarrat, brking

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 <desnesn@linux.vnet.ibm.com>

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem
  2018-03-14 16:34 [PATCH 0/2] Silent PCI realignment messages during boot Desnes A. Nunes do Rosario
@ 2018-03-14 16:34 ` Desnes A. Nunes do Rosario
  2018-03-14 17:41   ` Andy Shevchenko
  2018-03-14 18:06   ` Bjorn Helgaas
  2018-03-14 16:34 ` [PATCH 2/2, powerpc/powernv] powerpc/powernv: Tweak PCI_DEV_FLAGS_QUIET_PCI_REALIGN on/off during boot Desnes A. Nunes do Rosario
  1 sibling, 2 replies; 11+ messages in thread
From: Desnes A. Nunes do Rosario @ 2018-03-14 16:34 UTC (permalink / raw)
  To: linux-kernel, linux-pci, linuxppc-dev
  Cc: bhelgaas, benh, paulus, mpe, ruscur, aik, david, fbarrat, brking

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 <desnesn@linux.vnet.ibm.com>
---
 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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2, powerpc/powernv] powerpc/powernv: Tweak PCI_DEV_FLAGS_QUIET_PCI_REALIGN on/off during boot
  2018-03-14 16:34 [PATCH 0/2] Silent PCI realignment messages during boot Desnes A. Nunes do Rosario
  2018-03-14 16:34 ` [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem Desnes A. Nunes do Rosario
@ 2018-03-14 16:34 ` Desnes A. Nunes do Rosario
  2018-03-16  8:41   ` kbuild test robot
  1 sibling, 1 reply; 11+ messages in thread
From: Desnes A. Nunes do Rosario @ 2018-03-14 16:34 UTC (permalink / raw)
  To: linux-kernel, linux-pci, linuxppc-dev
  Cc: bhelgaas, benh, paulus, mpe, ruscur, aik, david, fbarrat, brking

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 <desnesn@linux.vnet.ibm.com>
---
 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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem
  2018-03-14 16:34 ` [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem Desnes A. Nunes do Rosario
@ 2018-03-14 17:41   ` Andy Shevchenko
  2018-03-14 17:42     ` Andy Shevchenko
  2018-03-14 18:09     ` Desnes Augusto Nunes do Rosário
  2018-03-14 18:06   ` Bjorn Helgaas
  1 sibling, 2 replies; 11+ messages in thread
From: Andy Shevchenko @ 2018-03-14 17:41 UTC (permalink / raw)
  To: Desnes A. Nunes do Rosario
  Cc: Linux Kernel Mailing List, linux-pci,
	open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Bjorn Helgaas,
	Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, ruscur,
	aik, david, fbarrat, brking

On Wed, Mar 14, 2018 at 6:34 PM, Desnes A. Nunes do Rosario
<desnesn@linux.vnet.ibm.com> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem
  2018-03-14 17:41   ` Andy Shevchenko
@ 2018-03-14 17:42     ` Andy Shevchenko
  2018-03-14 18:09     ` Desnes Augusto Nunes do Rosário
  1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2018-03-14 17:42 UTC (permalink / raw)
  To: Desnes A. Nunes do Rosario
  Cc: Linux Kernel Mailing List, linux-pci,
	open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Bjorn Helgaas,
	Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, ruscur,
	aik, david, fbarrat, brking

On Wed, Mar 14, 2018 at 7:41 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem
  2018-03-14 16:34 ` [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem Desnes A. Nunes do Rosario
  2018-03-14 17:41   ` Andy Shevchenko
@ 2018-03-14 18:06   ` Bjorn Helgaas
  2018-03-14 18:22     ` Desnes Augusto Nunes do Rosário
  1 sibling, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2018-03-14 18:06 UTC (permalink / raw)
  To: Desnes A. Nunes do Rosario
  Cc: linux-kernel, linux-pci, linuxppc-dev, bhelgaas, benh, paulus,
	mpe, ruscur, aik, david, fbarrat, brking

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 <desnesn@linux.vnet.ibm.com>
> ---
>  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
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem
  2018-03-14 17:41   ` Andy Shevchenko
  2018-03-14 17:42     ` Andy Shevchenko
@ 2018-03-14 18:09     ` Desnes Augusto Nunes do Rosário
  1 sibling, 0 replies; 11+ messages in thread
From: Desnes Augusto Nunes do Rosário @ 2018-03-14 18:09 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linux Kernel Mailing List, linux-pci,
	open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Bjorn Helgaas,
	Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, ruscur,
	aik, david, fbarrat, brking

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
> <desnesn@linux.vnet.ibm.com> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem
  2018-03-14 18:06   ` Bjorn Helgaas
@ 2018-03-14 18:22     ` Desnes Augusto Nunes do Rosário
  2018-03-14 18:55       ` Bjorn Helgaas
  0 siblings, 1 reply; 11+ messages in thread
From: Desnes Augusto Nunes do Rosário @ 2018-03-14 18:22 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, linux-pci, linuxppc-dev, bhelgaas, benh, paulus,
	mpe, ruscur, aik, david, fbarrat, brking

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 <desnesn@linux.vnet.ibm.com>
>> ---
>>   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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem
  2018-03-14 18:22     ` Desnes Augusto Nunes do Rosário
@ 2018-03-14 18:55       ` Bjorn Helgaas
  2018-03-14 21:12         ` Desnes Augusto Nunes do Rosário
  0 siblings, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2018-03-14 18:55 UTC (permalink / raw)
  To: Desnes Augusto Nunes do Rosário
  Cc: linux-kernel, linux-pci, linuxppc-dev, bhelgaas, benh, paulus,
	mpe, ruscur, aik, david, fbarrat, brking

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 <desnesn@linux.vnet.ibm.com>
> > > ---
> > >   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!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem
  2018-03-14 18:55       ` Bjorn Helgaas
@ 2018-03-14 21:12         ` Desnes Augusto Nunes do Rosário
  0 siblings, 0 replies; 11+ messages in thread
From: Desnes Augusto Nunes do Rosário @ 2018-03-14 21:12 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, linux-pci, linuxppc-dev, bhelgaas, benh, paulus,
	mpe, ruscur, aik, david, fbarrat, brking

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
--------------------------

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2, powerpc/powernv] powerpc/powernv: Tweak PCI_DEV_FLAGS_QUIET_PCI_REALIGN on/off during boot
  2018-03-14 16:34 ` [PATCH 2/2, powerpc/powernv] powerpc/powernv: Tweak PCI_DEV_FLAGS_QUIET_PCI_REALIGN on/off during boot Desnes A. Nunes do Rosario
@ 2018-03-16  8:41   ` kbuild test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-03-16  8:41 UTC (permalink / raw)
  To: Desnes A. Nunes do Rosario
  Cc: kbuild-all, linux-kernel, linux-pci, linuxppc-dev, bhelgaas,
	benh, paulus, mpe, ruscur, aik, david, fbarrat, brking

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

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24001 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-03-16  8:42 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14 16:34 [PATCH 0/2] Silent PCI realignment messages during boot Desnes A. Nunes do Rosario
2018-03-14 16:34 ` [PATCH 1/2, pci] pci: Addition of PCI_DEV_FLAGS_QUIET_PCI_REALIGN attribute to the PCI subsystem Desnes A. Nunes do Rosario
2018-03-14 17:41   ` Andy Shevchenko
2018-03-14 17:42     ` Andy Shevchenko
2018-03-14 18:09     ` Desnes Augusto Nunes do Rosário
2018-03-14 18:06   ` Bjorn Helgaas
2018-03-14 18:22     ` Desnes Augusto Nunes do Rosário
2018-03-14 18:55       ` Bjorn Helgaas
2018-03-14 21:12         ` Desnes Augusto Nunes do Rosário
2018-03-14 16:34 ` [PATCH 2/2, powerpc/powernv] powerpc/powernv: Tweak PCI_DEV_FLAGS_QUIET_PCI_REALIGN on/off during boot Desnes A. Nunes do Rosario
2018-03-16  8:41   ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).