From: John Garry <john.garry@huawei.com> To: Bjorn Helgaas <helgaas@kernel.org> Cc: <rjw@rjwysocki.net>, <wangkefeng.wang@huawei.com>, <lorenzo.pieralisi@arm.com>, <arnd@arndb.de>, <linux-pci@vger.kernel.org>, <will.deacon@arm.com>, <linuxarm@huawei.com>, <linux-kernel@vger.kernel.org>, <catalin.marinas@arm.com>, <andriy.shevchenko@linux.intel.com>, <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v4 1/3] lib: logic_pio: Use logical PIO low-level accessors for !CONFIG_INDIRECT_PIO Date: Fri, 14 Jun 2019 13:22:31 +0100 [thread overview] Message-ID: <4c49bf7d-f68e-0b79-f03d-03958dac640e@huawei.com> (raw) In-Reply-To: <20190614115056.GP13533@google.com> On 14/06/2019 12:50, Bjorn Helgaas wrote: > On Fri, Jun 14, 2019 at 10:02:52AM +0100, John Garry wrote: >> On 13/06/2019 21:09, Bjorn Helgaas wrote: >>> On Thu, Jun 13, 2019 at 10:39:12AM +0100, John Garry wrote: >>>> On 13/06/2019 03:39, Bjorn Helgaas wrote: >>>>> I'm not sure it's even safe, because CONFIG_INDIRECT_PIO depends on >>>>> ARM64, but PCI_IOBASE is defined on most arches via asm-generic/io.h, >>>>> so this potentially affects arches other than ARM64. >>>> >>>> It would do. It would affect any arch which defines PCI_IOBASE and >>>> does not have arch-specific definition of inb et all. >> >>> What's the reason for testing PCI_IOBASE instead of >>> CONFIG_INDIRECT_PIO? If there's a reason it's needed, that's fine, >>> but it does make this much more complicated to review. >> >> For ARM64, we have PCI_IOBASE defined but may not have >> CONFIG_INDIRECT_PIO defined. Currently CONFIG_INDIRECT_PIO is only >> selected by CONFIG_HISILICON_LPC. >> >> As such, we should make this change also for when >> CONFIG_INDIRECT_PIO is not defined. Hi Bjorn, > > OK. This is all very important for the commit log -- we need to > understand what arches are affected and the reason they need it. Right, and to repeat, this would affect other archs which define PCI_IOBASE and don't have custom IO port accessors definitions. There are a few remaining even after the recent arch clear out . I have it at arm64, microblaze, and unicore32. Arch m68k defines PCI_IOBASE but seems to have its own IO port accessors. Same again for some arm machines. At least I should cc those arch maintainers. > > Since the goal of this series is to fix an ARM64-specific issue, "ARM64" was in the headline banner, but it would apply to other archs, as mentioned above. I should have made that clearer. and > the typical port I/O model is for each arch to #define its own inb(), > maybe it would make sense to move the "#define inb logic_inb" from > linux/logic_pio.h to arm64/include/asm/io.h? > CONFIG_INDIRECT_PIO has been indirectly enabled in ARM64 defconfig for some time, so I think that it's ok for ARM64 arch Kconfig to select it at this stage. From that, we could make the change suggested. And, in addition to that, we can make the change in this series just for CONFIG_INDIRECT_PIO. But I think that the other archs, above, could benefit from the changes in this series, so it would be shame to omit them. > The "#ifndef inb" arrangement gets pretty complicated when it occurs > more than one place (asm-generic/io.h and logic_pio.h) and we have to > start worrying about the ordering of #includes. I agree on that. Cheers, John > > Bjorn > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > >
WARNING: multiple messages have this Message-ID (diff)
From: John Garry <john.garry@huawei.com> To: Bjorn Helgaas <helgaas@kernel.org> Cc: rjw@rjwysocki.net, wangkefeng.wang@huawei.com, lorenzo.pieralisi@arm.com, arnd@arndb.de, linux-pci@vger.kernel.org, will.deacon@arm.com, linuxarm@huawei.com, linux-kernel@vger.kernel.org, catalin.marinas@arm.com, andriy.shevchenko@linux.intel.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v4 1/3] lib: logic_pio: Use logical PIO low-level accessors for !CONFIG_INDIRECT_PIO Date: Fri, 14 Jun 2019 13:22:31 +0100 [thread overview] Message-ID: <4c49bf7d-f68e-0b79-f03d-03958dac640e@huawei.com> (raw) In-Reply-To: <20190614115056.GP13533@google.com> On 14/06/2019 12:50, Bjorn Helgaas wrote: > On Fri, Jun 14, 2019 at 10:02:52AM +0100, John Garry wrote: >> On 13/06/2019 21:09, Bjorn Helgaas wrote: >>> On Thu, Jun 13, 2019 at 10:39:12AM +0100, John Garry wrote: >>>> On 13/06/2019 03:39, Bjorn Helgaas wrote: >>>>> I'm not sure it's even safe, because CONFIG_INDIRECT_PIO depends on >>>>> ARM64, but PCI_IOBASE is defined on most arches via asm-generic/io.h, >>>>> so this potentially affects arches other than ARM64. >>>> >>>> It would do. It would affect any arch which defines PCI_IOBASE and >>>> does not have arch-specific definition of inb et all. >> >>> What's the reason for testing PCI_IOBASE instead of >>> CONFIG_INDIRECT_PIO? If there's a reason it's needed, that's fine, >>> but it does make this much more complicated to review. >> >> For ARM64, we have PCI_IOBASE defined but may not have >> CONFIG_INDIRECT_PIO defined. Currently CONFIG_INDIRECT_PIO is only >> selected by CONFIG_HISILICON_LPC. >> >> As such, we should make this change also for when >> CONFIG_INDIRECT_PIO is not defined. Hi Bjorn, > > OK. This is all very important for the commit log -- we need to > understand what arches are affected and the reason they need it. Right, and to repeat, this would affect other archs which define PCI_IOBASE and don't have custom IO port accessors definitions. There are a few remaining even after the recent arch clear out . I have it at arm64, microblaze, and unicore32. Arch m68k defines PCI_IOBASE but seems to have its own IO port accessors. Same again for some arm machines. At least I should cc those arch maintainers. > > Since the goal of this series is to fix an ARM64-specific issue, "ARM64" was in the headline banner, but it would apply to other archs, as mentioned above. I should have made that clearer. and > the typical port I/O model is for each arch to #define its own inb(), > maybe it would make sense to move the "#define inb logic_inb" from > linux/logic_pio.h to arm64/include/asm/io.h? > CONFIG_INDIRECT_PIO has been indirectly enabled in ARM64 defconfig for some time, so I think that it's ok for ARM64 arch Kconfig to select it at this stage. From that, we could make the change suggested. And, in addition to that, we can make the change in this series just for CONFIG_INDIRECT_PIO. But I think that the other archs, above, could benefit from the changes in this series, so it would be shame to omit them. > The "#ifndef inb" arrangement gets pretty complicated when it occurs > more than one place (asm-generic/io.h and logic_pio.h) and we have to > start worrying about the ordering of #includes. I agree on that. Cheers, John > > Bjorn > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-06-14 12:22 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-11 14:12 [PATCH v4 0/3] Fix ARM64 crash for accessing unmapped IO port regions John Garry 2019-06-11 14:12 ` John Garry 2019-06-11 14:12 ` [PATCH v4 1/3] lib: logic_pio: Use logical PIO low-level accessors for !CONFIG_INDIRECT_PIO John Garry 2019-06-11 14:12 ` John Garry 2019-06-13 2:39 ` Bjorn Helgaas 2019-06-13 2:39 ` Bjorn Helgaas 2019-06-13 9:39 ` John Garry 2019-06-13 9:39 ` John Garry 2019-06-13 20:09 ` Bjorn Helgaas 2019-06-13 20:09 ` Bjorn Helgaas 2019-06-14 9:02 ` John Garry 2019-06-14 9:02 ` John Garry 2019-06-14 11:50 ` Bjorn Helgaas 2019-06-14 11:50 ` Bjorn Helgaas 2019-06-14 12:22 ` John Garry [this message] 2019-06-14 12:22 ` John Garry 2019-06-13 13:58 ` Bjorn Helgaas 2019-06-13 13:58 ` Bjorn Helgaas 2019-06-13 15:21 ` John Garry 2019-06-13 15:21 ` John Garry 2019-06-11 14:12 ` [PATCH v4 2/3] lib: logic_pio: Reject accesses to unregistered CPU MMIO regions John Garry 2019-06-11 14:12 ` John Garry 2019-06-13 3:20 ` Bjorn Helgaas 2019-06-13 3:20 ` Bjorn Helgaas 2019-06-13 7:47 ` Arnd Bergmann 2019-06-13 7:47 ` Arnd Bergmann 2019-06-13 10:17 ` John Garry 2019-06-13 10:17 ` John Garry 2019-06-13 13:46 ` Bjorn Helgaas 2019-06-13 13:46 ` Bjorn Helgaas 2019-06-13 14:09 ` John Garry 2019-06-13 14:09 ` John Garry 2019-06-11 14:12 ` [PATCH v4 3/3] lib: logic_pio: Fix up a print John Garry 2019-06-11 14:12 ` John Garry
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4c49bf7d-f68e-0b79-f03d-03958dac640e@huawei.com \ --to=john.garry@huawei.com \ --cc=andriy.shevchenko@linux.intel.com \ --cc=arnd@arndb.de \ --cc=catalin.marinas@arm.com \ --cc=helgaas@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linuxarm@huawei.com \ --cc=lorenzo.pieralisi@arm.com \ --cc=rjw@rjwysocki.net \ --cc=wangkefeng.wang@huawei.com \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.