[permalink] [raw]

Subject: Re: [PATCH V1 6/9] PCI/TPH: Retrieve steering tag from ACPI _DSM

2024-05-10 05:26:19

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH V1 6/9] PCI/TPH: Retrieve steering tag from ACPI _DSM

Hi Wei,

kernel test robot noticed the following build errors:

[auto build test ERROR on pci/for-linus]
[also build test ERROR on awilliam-vfio/next linus/master awilliam-vfio/for-linus v6.9-rc7 next-20240509]
[cannot apply to pci/next horms-ipvs/master]
[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/Wei-Huang/PCI-Introduce-PCIe-TPH-support-framework/20240510-003504
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git for-linus
patch link: https://lore.kernel.org/r/20240509162741.1937586-7-wei.huang2%40amd.com
patch subject: [PATCH V1 6/9] PCI/TPH: Retrieve steering tag from ACPI _DSM
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20240510/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project b910bebc300dafb30569cecc3017b446ea8eafa0)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240510/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

In file included from drivers/pci/pcie/tph.c:13:
In file included from include/linux/acpi.h:14:
In file included from include/linux/device.h:32:
In file included from include/linux/device/driver.h:21:
In file included from include/linux/module.h:19:
In file included from include/linux/elf.h:6:
In file included from arch/s390/include/asm/elf.h:173:
In file included from arch/s390/include/asm/mmu_context.h:11:
In file included from arch/s390/include/asm/pgalloc.h:18:
In file included from include/linux/mm.h:2208:
include/linux/vmstat.h:508:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
508 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
509 | item];
| ~~~~
include/linux/vmstat.h:515:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
515 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
516 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:527:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
527 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
528 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:536:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
536 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
537 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/pci/pcie/tph.c:17:
In file included from include/linux/msi.h:27:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:78:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
| ^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
| ^
In file included from drivers/pci/pcie/tph.c:17:
In file included from include/linux/msi.h:27:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:78:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
| ^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
| ^
In file included from drivers/pci/pcie/tph.c:17:
In file included from include/linux/msi.h:27:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:78:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
692 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
700 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
708 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
717 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
726 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
735 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
>> drivers/pci/pcie/tph.c:221:39: error: use of undeclared identifier 'pci_acpi_dsm_guid'
221 | out_obj = acpi_evaluate_dsm(handle, &pci_acpi_dsm_guid, MIN_ST_DSM_REV,
| ^
17 warnings and 1 error generated.

vim +/pci_acpi_dsm_guid +221 drivers/pci/pcie/tph.c

196
197 #define MIN_ST_DSM_REV 7
198 #define ST_DSM_FUNC_INDEX 0xf
199 static bool invoke_dsm(acpi_handle handle, u32 cpu_uid, u8 ph,
200 u8 target_type, bool cache_ref_valid,
201 u64 cache_ref, union st_info *st_out)
202 {
203 union acpi_object in_obj, in_buf[3], *out_obj;
204
205 in_buf[0].integer.type = ACPI_TYPE_INTEGER;
206 in_buf[0].integer.value = 0; /* 0 => processor cache steering tags */
207
208 in_buf[1].integer.type = ACPI_TYPE_INTEGER;
209 in_buf[1].integer.value = cpu_uid;
210
211 in_buf[2].integer.type = ACPI_TYPE_INTEGER;
212 in_buf[2].integer.value = ph & 3;
213 in_buf[2].integer.value |= (target_type & 1) << 2;
214 in_buf[2].integer.value |= (cache_ref_valid & 1) << 3;
215 in_buf[2].integer.value |= (cache_ref << 32);
216
217 in_obj.type = ACPI_TYPE_PACKAGE;
218 in_obj.package.count = ARRAY_SIZE(in_buf);
219 in_obj.package.elements = in_buf;
220
> 221 out_obj = acpi_evaluate_dsm(handle, &pci_acpi_dsm_guid, MIN_ST_DSM_REV,
222 ST_DSM_FUNC_INDEX, &in_obj);
223
224 if (!out_obj)
225 return false;
226
227 if (out_obj->type != ACPI_TYPE_BUFFER) {
228 pr_err("invalid return type %d from TPH _DSM\n",
229 out_obj->type);
230 ACPI_FREE(out_obj);
231 return false;
232 }
233
234 st_out->value = *((u64 *)(out_obj->buffer.pointer));
235
236 ACPI_FREE(out_obj);
237
238 return true;
239 }
240

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-05-10 07:04:46

by Ajit Khaparde

[permalink] [raw]

Subject: Re: [PATCH V1 8/9] bnxt_en: Add TPH support in BNXT driver

On Thu, May 9, 2024 at 2:50 PM Vadim Fedorenko
<[email protected]> wrote:
>
> On 09/05/2024 17:27, Wei Huang wrote:
> > From: Manoj Panicker <[email protected]>
> >
> > As a usage example, this patch implements TPH support in Broadcom BNXT
> > device driver by invoking pcie_tph_set_st() function when interrupt
> > affinity is changed.
> >
> > Reviewed-by: Ajit Khaparde <[email protected]>
> > Reviewed-by: Andy Gospodarek <[email protected]>
> > Reviewed-by: Wei Huang <[email protected]>
> > Signed-off-by: Manoj Panicker <[email protected]>
> > ---
> > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 51 +++++++++++++++++++++++
> > drivers/net/ethernet/broadcom/bnxt/bnxt.h | 4 ++
> > 2 files changed, 55 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > index 2c2ee79c4d77..be9c17566fb4 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > @@ -55,6 +55,7 @@
> > #include <net/page_pool/helpers.h>
> > #include <linux/align.h>
> > #include <net/netdev_queues.h>
> > +#include <linux/pci-tph.h>
> >
> > #include "bnxt_hsi.h"
> > #include "bnxt.h"
> > @@ -10491,6 +10492,7 @@ static void bnxt_free_irq(struct bnxt *bp)
> > free_cpumask_var(irq->cpu_mask);
> > irq->have_cpumask = 0;
> > }
> > + irq_set_affinity_notifier(irq->vector, NULL);
> > free_irq(irq->vector, bp->bnapi[i]);
> > }
> >
> > @@ -10498,6 +10500,45 @@ static void bnxt_free_irq(struct bnxt *bp)
> > }
> > }
> >
> > +static void bnxt_rtnl_lock_sp(struct bnxt *bp);
> > +static void bnxt_rtnl_unlock_sp(struct bnxt *bp);
> > +static void bnxt_irq_affinity_notify(struct irq_affinity_notify *notify,
> > + const cpumask_t *mask)
> > +{
> > + struct bnxt_irq *irq;
> > +
> > + irq = container_of(notify, struct bnxt_irq, affinity_notify);
> > + cpumask_copy(irq->cpu_mask, mask);
> > +
> > + if (!pcie_tph_set_st(irq->bp->pdev, irq->msix_nr,
> > + cpumask_first(irq->cpu_mask),
> > + TPH_MEM_TYPE_VM, PCI_TPH_REQ_TPH_ONLY))
> > + pr_err("error in configuring steering tag\n");
> > +
> > + if (netif_running(irq->bp->dev)) {
> > + rtnl_lock();
> > + bnxt_close_nic(irq->bp, false, false);
> > + bnxt_open_nic(irq->bp, false, false);
> > + rtnl_unlock();
> > + }
>
> Is it really needed? It will cause link flap and pause in the traffic
> service for the device. Why the device needs full restart in this case?

In that sequence only the rings are recreated for the hardware to sync
up the tags.

Actually its not a full restart. There is no link reinit or other
heavy lifting in this sequence.
The pause in traffic may be momentary. Do IRQ/CPU affinities change frequently?
Probably not?

>
>
> > +}
> > +
> > +static void bnxt_irq_affinity_release(struct kref __always_unused *ref)
> > +{
> > +}
> > +
> > +static inline void __bnxt_register_notify_irqchanges(struct bnxt_irq *irq)
>
> No inlines in .c files, please. Let compiler decide what to inline.
>
> > +{
> > + struct irq_affinity_notify *notify;
> > +
> > + notify = &irq->affinity_notify;
> > + notify->irq = irq->vector;
> > + notify->notify = bnxt_irq_affinity_notify;
> > + notify->release = bnxt_irq_affinity_release;
> > +
> > + irq_set_affinity_notifier(irq->vector, notify);
> > +}
> > +
> > static int bnxt_request_irq(struct bnxt *bp)
> > {
> > int i, j, rc = 0;
> > @@ -10543,6 +10584,7 @@ static int bnxt_request_irq(struct bnxt *bp)
> > int numa_node = dev_to_node(&bp->pdev->dev);
> >
> > irq->have_cpumask = 1;
> > + irq->msix_nr = map_idx;
> > cpumask_set_cpu(cpumask_local_spread(i, numa_node),
> > irq->cpu_mask);
> > rc = irq_set_affinity_hint(irq->vector, irq->cpu_mask);
> > @@ -10552,6 +10594,15 @@ static int bnxt_request_irq(struct bnxt *bp)
> > irq->vector);
> > break;
> > }
> > +
> > + if (!pcie_tph_set_st(bp->pdev, i,
> > + cpumask_first(irq->cpu_mask),
> > + TPH_MEM_TYPE_VM, PCI_TPH_REQ_TPH_ONLY)) {
> > + netdev_err(bp->dev, "error in setting steering tag\n");
> > + } else {
> > + irq->bp = bp;
> > + __bnxt_register_notify_irqchanges(irq);
> > + }
> > }
> > }
> > return rc;
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > index dd849e715c9b..0d3442590bb4 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > @@ -1195,6 +1195,10 @@ struct bnxt_irq {
> > u8 have_cpumask:1;
> > char name[IFNAMSIZ + 2];
> > cpumask_var_t cpu_mask;
> > +
> > + int msix_nr;
> > + struct bnxt *bp;
> > + struct irq_affinity_notify affinity_notify;
> > };
> >
> > #define HWRM_RING_ALLOC_TX 0x1
>

Attachments:

smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2024-05-10 10:22:30

On Thu, May 09, 2024 at 11:27:39AM -0500, Wei Huang wrote:
> +:Copyright: |copy| 2024 Advanced Micro Devices, Inc.
> +:Authors: - Eric van Tassell <[email protected]>
> + - Wei Huang <[email protected]>

You can directly embed copyright symbol without having to pull in <isonum.txt>:

---- >8 ----
diff --git a/Documentation/PCI/tph.rst b/Documentation/PCI/tph.rst
index ea9c8313f3e4f8..d7043fb0b71b3a 100644
--- a/Documentation/PCI/tph.rst
+++ b/Documentation/PCI/tph.rst
@@ -5,7 +5,7 @@ TPH Support
===========

-:Copyright: |copy| 2024 Advanced Micro Devices, Inc.
+:Copyright: © 2024 Advanced Micro Devices, Inc.
:Authors: - Eric van Tassell <[email protected]>
- Wei Huang <[email protected]>

Thanks.

--
An old man doll... just what I always wanted! - Clara

Attachments:

(No filename) (832.00 B)
signature.asc (235.00 B)
Download all attachments