Hi Greg,

sorry to bother you but I missed that the smp_cpu_get_cpu_address()
address used here was only added with the
commit 42d211a1ae3b77 ("s390/cpuinfo: show processor physical address")
which landed in v5.7-rc1. This would therefore break if ever called
(luckily it would not be called on any shipped hardware) and
also causes a missing declaration warning as reported by
Naresh Kamboju thanks!
Since this is as of now just a spec fix, as on all known hardware
the Linux CPU Id always matches the CPU Address, I would
recommend to simply revert the commit.
Thanks in advance!

Best regards,
Niklas Schnelle

On 12/10/20 3:26 PM, Greg Kroah-Hartman wrote:
> From: Alexander Gordeev <agord...@linux.ibm.com>
> 
> commit a2bd4097b3ec242f4de4924db463a9c94530e03a upstream.
> 
> The directed MSIs are delivered to CPUs whose address is
> written to the MSI message address. The current code assumes
> that a CPU logical number (as it is seen by the kernel)
> is also the CPU address.
> 
> The above assumption is not correct, as the CPU address
> is rather the value returned by STAP instruction. That
> value does not necessarily match the kernel logical CPU
> number.
> 
> Fixes: e979ce7bced2 ("s390/pci: provide support for CPU directed interrupts")
> Cc: <sta...@vger.kernel.org> # v5.2+
> Signed-off-by: Alexander Gordeev <agord...@linux.ibm.com>
> Reviewed-by: Halil Pasic <pa...@linux.ibm.com>
> Reviewed-by: Niklas Schnelle <schne...@linux.ibm.com>
> Signed-off-by: Niklas Schnelle <schne...@linux.ibm.com>
> Signed-off-by: Heiko Carstens <h...@linux.ibm.com>
> Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> 
> ---
>  arch/s390/pci/pci_irq.c |   14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
> @@ -103,9 +103,10 @@ static int zpci_set_irq_affinity(struct
>  {
>       struct msi_desc *entry = irq_get_msi_desc(data->irq);
>       struct msi_msg msg = entry->msg;
> +     int cpu_addr = smp_cpu_get_cpu_address(cpumask_first(dest));
>  
>       msg.address_lo &= 0xff0000ff;
> -     msg.address_lo |= (cpumask_first(dest) << 8);
> +     msg.address_lo |= (cpu_addr << 8);
>       pci_write_msi_msg(data->irq, &msg);
>  
>       return IRQ_SET_MASK_OK;
> @@ -238,6 +239,7 @@ int arch_setup_msi_irqs(struct pci_dev *
>       unsigned long bit;
>       struct msi_desc *msi;
>       struct msi_msg msg;
> +     int cpu_addr;
>       int rc, irq;
>  
>       zdev->aisb = -1UL;
> @@ -287,9 +289,15 @@ int arch_setup_msi_irqs(struct pci_dev *
>                                        handle_percpu_irq);
>               msg.data = hwirq - bit;
>               if (irq_delivery == DIRECTED) {
> +                     if (msi->affinity)
> +                             cpu = cpumask_first(&msi->affinity->mask);
> +                     else
> +                             cpu = 0;
> +                     cpu_addr = smp_cpu_get_cpu_address(cpu);
> +
>                       msg.address_lo = zdev->msi_addr & 0xff0000ff;
> -                     msg.address_lo |= msi->affinity ?
> -                             (cpumask_first(&msi->affinity->mask) << 8) : 0;
> +                     msg.address_lo |= (cpu_addr << 8);
> +
>                       for_each_possible_cpu(cpu) {
>                               airq_iv_set_data(zpci_ibv[cpu], hwirq, irq);
>                       }
> 
> 

Reply via email to