From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 532BCC433EF for ; Wed, 22 Sep 2021 18:07:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 31F8F6103C for ; Wed, 22 Sep 2021 18:07:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237004AbhIVSJV (ORCPT ); Wed, 22 Sep 2021 14:09:21 -0400 Received: from mout.kundenserver.de ([212.227.17.24]:43567 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236943AbhIVSJT (ORCPT ); Wed, 22 Sep 2021 14:09:19 -0400 Received: from mail-wr1-f52.google.com ([209.85.221.52]) by mrelayeu.kundenserver.de (mreue109 [213.165.67.113]) with ESMTPSA (Nemesis) id 1MO9r5-1mDM7h1MCX-00OVpH; Wed, 22 Sep 2021 20:07:48 +0200 Received: by mail-wr1-f52.google.com with SMTP id t8so9609785wri.1; Wed, 22 Sep 2021 11:07:48 -0700 (PDT) X-Gm-Message-State: AOAM531EvrZsZ/MJwvsnNHroTDltQ1C5RFYgcWJV8Zm+xMTB55KASgPe POCO+huxo8wz/7di8HxSbCmSXilanw+3MIeGNqk= X-Google-Smtp-Source: ABdhPJxefGoFSv5mbWEbjSxVO7l7TV7dRfGNx9j+Tk9BFQvOVsDSTR9Sbgfrbt8kzfTLzbvI3ZJIrLmP2e+7OX9g2CQ= X-Received: by 2002:a5d:4b50:: with SMTP id w16mr351144wrs.71.1632334067884; Wed, 22 Sep 2021 11:07:47 -0700 (PDT) MIME-Version: 1.0 References: <20210922042041.16326-1-sergio.paracuellos@gmail.com> In-Reply-To: From: Arnd Bergmann Date: Wed, 22 Sep 2021 20:07:31 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3] PCI: of: Avoid pci_remap_iospace() when PCI_IOBASE not defined To: Sergio Paracuellos Cc: Arnd Bergmann , linux-pci , Bjorn Helgaas , Linux Kernel Mailing List , linux-staging@lists.linux.dev, gregkh , Liviu Dudau , Rob Herring , Catalin Marinas , Thomas Bogendoerfer Content-Type: text/plain; charset="UTF-8" X-Provags-ID: V03:K1:3uUo8NMwM2S2a/uvloOvVOnQq0MPX/jY5hR7RsDtM5WPQzA/qo0 NdxezEg4/W6nPzo37ydklPX1d2p5O3yhJOspvzEwNOea/Y9epQuF8pEo8HJA35jxkfWQr60 BtwATr9trm4m4eyjtt+eld1bze0EK3tV6VI7XwEiBnTns+OAV8k44tY3JzEgjTMf04KZ8W9 uS6nfahotLdp39xGumlYw== X-UI-Out-Filterresults: notjunk:1;V03:K0:LcwMq24g7nY=:/BLgL2kznKy7LZE5Q/uZj/ HRC9D/pnW7ctRvDooRpFHPx/dTvCv549ng688FSwACOGeJjc6oAhmEKy4YlJC/T0Nz9ahI3Ce 8QD4FGioJ4i6HtnL8Y5hm3PbYWFCrYsl0bZSQ5KYgA3/c1+7d9wVl48ERp5bYZeBa/dPxp5kC LzMhNUokxEW61Qb9fYm9pZjVz1jsX6gSzZChqN04F3A4sBEprjy3Axq2emTr8zGhUnpv8tcF3 0QGIAc+I8Yt9jTyG20H2yu3NpOw/rL1EeavWdziVWGd47hnUPf1dKHOXXyIaJj5x75z3J6gL+ /6GA7U2AaJEsK9QLbDaDWNAdqdnXMyYlMh1oINQ0NNXrujQK3ijMhKwiR7NbdJt/3cFd0pirg 2t18kiKfqAX8hEDOYZZkm7gzpdPXuWz603MerSrEXmzEKzc3OUup5yXTkXB3P+Wz0jmh/ZH71 EfKAMjEgYBgYzC4yQRJasfCPE9/sRZj3p8hM7PYWvc0/P9A7xDgWud6P1JV5vFViR+RkgH5Qc 5v9hc1JeN9DFCJyiNiCbmCRCPtQRWumFZ6+4aPruSPHYJtR78Jff6z3sQ+a318hyMOhzALMYV muX0yU1LsK/JGFu4JkwOKK/yyvS8SSeyKAmTT/qE4Rp25+2XsgJKM5LYyf2SrEAIubXAOYwSf /OTR375d7pLBA/s+uY6/ofLUEAVePMWSfuDgTBRPbnbP/Cu96WPTspMIBinsGWRuCE1kAMuIV wsMaxZd8ZeQWb8Ydl/06nrHSbInCSh3Q7Bw2FZJG70jZW1lS0dqNGSgaS0SR7frqYay+tTqX6 nazcbzLsDngs2It77K7jxSYmLPFUZuGL6KdYVWL0q2dS86LKW5POeie8cjpdTtOxivru5rcB5 JRZx8e9kYw87ULdun+Iw== Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 22, 2021 at 7:42 PM Sergio Paracuellos wrote: > On Wed, Sep 22, 2021 at 5:47 PM Arnd Bergmann wrote: > > > > I'm not completely sure where your platform fits in here, it sounds like you > > address them using a machine specific physical address as the base in > > inb() plus the port number as an offset, is that correct? > > I guess none of the above options? I will try to explain this as per > my understanding. > > [+cc Thomas Bogendoerfer as mips maintainer and with better knowledge > of mips platforms than me] > > On MIPS I/O ports are memory mapped, so we access them using normal > load/store instructions. > Mips 'plat_mem_setup()' function does a 'set_io_port_base(KSEG1)'. > There, variable 'mips_io_port_base' > is set then using this address which is a virtual address to which all > ports are being mapped. > KSEG1 addresses are uncached and are not translated by the MMU. This > KSEG1 range is directly mapped in physical space starting with address > 0x0. > Because of this reason, defining PCI_IOBASE as KSEG1 won't work since, > at the end 'pci_parse_request_of_pci_ranges' tries to remap to a fixed > virtual address (PCI_IOBASE). This can't work for KSEG1 addresses. > What happens if I try to do that is that I get bad addresses at pci > enumeration for IO resources. Mips ralink mt7621 SoC (which is the one > I am using and trying to mainline the driver from staging) have I/O at > address 0x1e160000. So instead of getting this address for pcie IO > BARS I get a range from 0x0000 to 0xffff since 'pci_adress_to_pio' in > that case got that range 0x0-0xffff which is wrong. To have this > working this way we would need to put PCI_IOBASE somewhere into KSEG2 > which will result in creating TLB entries for IO addresses, which most > of the time isn't needed on MIPS because of access via KSEG1. Instead > of that, what happens when I avoid defining PCI_IOBASE and set > IO_SPACE_LIMIT (See [0] and [1] commits already added to staging tree > which was part of this patch series for context of what works with > this patch together) all works properly. There have also been some > patches accepted in the past which avoid this > 'pci_parse_request_of_pci_ranges' call since it is not working for > most pci legacy drivers of arch/mips for ralinks platform [2]. > > So I am not sure what should be the correct approach to properly make > this work (this one works for me and I cannot see others better) but I > will be happy to try whatever you propose for me to do. Ok, thank you for the detailed explanation. I suppose you can use the generic infrastructure in asm-generic/io.h if you "#define PCI_IOBASE mips_io_port_base". In this case, you should have an architecture specific implementation of pci_remap_iospace() that assigns mips_io_port_base. pci_remap_iospace() was originally meant as an architecture specific helper, but it moved into generic code after all architectures had the same requirements. If MIPS has different requirements, then it should not be shared. I don't yet understand how you deal with having multiple PCIe host bridge devices if they have distinct I/O port ranges. Without remapping to dynamic virtual addresses, does that mean that every MMIO register between the first and last PCIe bridge also shows up in /dev/ioport? Or do you only support port I/O on the first PCIe host bridge? Note that you could also decide to completely sidestep the problem by just defining port I/O to be unavailable on MIPS when probing a generic host bridge driver. Most likely this is never going to be used anyway, and it will be rather hard to test if you don't already have the need ;-) Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.17.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A08333FCB for ; Wed, 22 Sep 2021 18:07:50 +0000 (UTC) Received: from mail-wr1-f54.google.com ([209.85.221.54]) by mrelayeu.kundenserver.de (mreue108 [213.165.67.113]) with ESMTPSA (Nemesis) id 1Mq2rM-1nF2aE1Uws-00n9mk for ; Wed, 22 Sep 2021 20:07:48 +0200 Received: by mail-wr1-f54.google.com with SMTP id u18so9495827wrg.5 for ; Wed, 22 Sep 2021 11:07:48 -0700 (PDT) X-Gm-Message-State: AOAM531zWv1n2l0ObJ9ctbJ3w34rSBro5bR7jPfm7FSpKFHDE6cAS/MZ fOIIN0FS0LtPYrhw8r4hwtLQpUYqJHorcTr2ymQ= X-Google-Smtp-Source: ABdhPJxefGoFSv5mbWEbjSxVO7l7TV7dRfGNx9j+Tk9BFQvOVsDSTR9Sbgfrbt8kzfTLzbvI3ZJIrLmP2e+7OX9g2CQ= X-Received: by 2002:a5d:4b50:: with SMTP id w16mr351144wrs.71.1632334067884; Wed, 22 Sep 2021 11:07:47 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20210922042041.16326-1-sergio.paracuellos@gmail.com> In-Reply-To: From: Arnd Bergmann Date: Wed, 22 Sep 2021 20:07:31 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3] PCI: of: Avoid pci_remap_iospace() when PCI_IOBASE not defined To: Sergio Paracuellos Cc: Arnd Bergmann , linux-pci , Bjorn Helgaas , Linux Kernel Mailing List , linux-staging@lists.linux.dev, gregkh , Liviu Dudau , Rob Herring , Catalin Marinas , Thomas Bogendoerfer Content-Type: text/plain; charset="UTF-8" X-Provags-ID: V03:K1:Y069/beaVg4X7z5pmI26ObmGFDQ9zDeLavKRe48fVNRXtz8NX9B kyb5g2ZJcAiGT9t6nV8djzB73vdQw7UmTMn0OTRjgnI9FcJrF3PfS/HuRQ6DuRzgul6Ykpw QWS6RgAzSdwqFh7tQ+jA45tJEcd73uKMucMMZ1+1N181hqb3pExCSyUlyaTu+gsGxY+fANm PXVBVPqrktq40fwxRQ/5A== X-Spam-Flag: NO X-UI-Out-Filterresults: notjunk:1;V03:K0:NzCh8GrJTpQ=:o/pnWkKclsTj1feO5JlbC1 mml9QjVX4N9Pi9aEVL47qc6PsV/GphIn/ZHGL2wZ1ZQo26WbFfQt86s13Wo2VPJBzHxCqzyAM nV++RrHMhCAPPBYqrrwk6gOwMY+bfiHEtIb27qjlLKYHjaGYKvKeUqQ53XT1xhznFaB0weLQM LIL8a+vf65LlKKumW1jr4IlPmZHlbL4Yb8vk6t+Cu8GkQpRc6+rtNBiMvJwNAitm3Rzj4dQNS PukR+1xfFolUQDRN6pY1PBMewavLzeMRQ7qZvtAPYbo/qSVX3FsetUuPa/12NvehUpaliPfWm QwAG4i8G1lqygZqIt7jVvbOi3IPsyv0gPe5EACGSjfdq3xK25G0vxb0TjB//HED9MCYrlVR1+ pgR+QdZJwwjmqNQ9HgTvc9NvaG+7WLa/9BCtjlnX+4+/iFDx5Rob5TXX+lLT5Eqq1FbPVNfop Ogl3EOToWgvM5YzJE8X3UG5bHqx17SgeANmyEMlC40uVwAbGxmRQD2VsO0PHk/9xWwOl+6MZU pz1N997Sk4AkVpUY2f0GYM0v3G5iT4ydJbcmEzTeIDnschSWfkn/1ATURU6zTAI8enSDr0km7 TDKonPFSfS9sul3J0GSNK3z9TYkU2vHLuqfjrTe/QJ1AIIM8cH88ENeD27Qwa3hK/3iEqg3mw wuCQM9kXmTsxf43Uw541ynvNSXssX4NLwwOCDqGzNRa77VlFMpFk2/nU2fzDOhhnQEQrwWcd6 Sqta5QwLWI68yjWqiLdWIXojjCAaQZXCvlSDtqL8BU+y//ezPQhy+FqRH4lZXXO+wrM+xjI1m MagKl9MtQiDhqlxmMPYdw4st3xNvARpHg8YAKPnQnM5K+XIx/IjIklAP+L/oybG9+kU4tGeRo bunYL96G4YDgmYinZbrA== On Wed, Sep 22, 2021 at 7:42 PM Sergio Paracuellos wrote: > On Wed, Sep 22, 2021 at 5:47 PM Arnd Bergmann wrote: > > > > I'm not completely sure where your platform fits in here, it sounds like you > > address them using a machine specific physical address as the base in > > inb() plus the port number as an offset, is that correct? > > I guess none of the above options? I will try to explain this as per > my understanding. > > [+cc Thomas Bogendoerfer as mips maintainer and with better knowledge > of mips platforms than me] > > On MIPS I/O ports are memory mapped, so we access them using normal > load/store instructions. > Mips 'plat_mem_setup()' function does a 'set_io_port_base(KSEG1)'. > There, variable 'mips_io_port_base' > is set then using this address which is a virtual address to which all > ports are being mapped. > KSEG1 addresses are uncached and are not translated by the MMU. This > KSEG1 range is directly mapped in physical space starting with address > 0x0. > Because of this reason, defining PCI_IOBASE as KSEG1 won't work since, > at the end 'pci_parse_request_of_pci_ranges' tries to remap to a fixed > virtual address (PCI_IOBASE). This can't work for KSEG1 addresses. > What happens if I try to do that is that I get bad addresses at pci > enumeration for IO resources. Mips ralink mt7621 SoC (which is the one > I am using and trying to mainline the driver from staging) have I/O at > address 0x1e160000. So instead of getting this address for pcie IO > BARS I get a range from 0x0000 to 0xffff since 'pci_adress_to_pio' in > that case got that range 0x0-0xffff which is wrong. To have this > working this way we would need to put PCI_IOBASE somewhere into KSEG2 > which will result in creating TLB entries for IO addresses, which most > of the time isn't needed on MIPS because of access via KSEG1. Instead > of that, what happens when I avoid defining PCI_IOBASE and set > IO_SPACE_LIMIT (See [0] and [1] commits already added to staging tree > which was part of this patch series for context of what works with > this patch together) all works properly. There have also been some > patches accepted in the past which avoid this > 'pci_parse_request_of_pci_ranges' call since it is not working for > most pci legacy drivers of arch/mips for ralinks platform [2]. > > So I am not sure what should be the correct approach to properly make > this work (this one works for me and I cannot see others better) but I > will be happy to try whatever you propose for me to do. Ok, thank you for the detailed explanation. I suppose you can use the generic infrastructure in asm-generic/io.h if you "#define PCI_IOBASE mips_io_port_base". In this case, you should have an architecture specific implementation of pci_remap_iospace() that assigns mips_io_port_base. pci_remap_iospace() was originally meant as an architecture specific helper, but it moved into generic code after all architectures had the same requirements. If MIPS has different requirements, then it should not be shared. I don't yet understand how you deal with having multiple PCIe host bridge devices if they have distinct I/O port ranges. Without remapping to dynamic virtual addresses, does that mean that every MMIO register between the first and last PCIe bridge also shows up in /dev/ioport? Or do you only support port I/O on the first PCIe host bridge? Note that you could also decide to completely sidestep the problem by just defining port I/O to be unavailable on MIPS when probing a generic host bridge driver. Most likely this is never going to be used anyway, and it will be rather hard to test if you don't already have the need ;-) Arnd