On Mon, Apr 08, 2019 at 05:35:51PM +0100, John Garry wrote: > On 08/04/2019 14:47, Guenter Roeck wrote: > > > > > > FC patch 1/4 ("resource: Request IO port regions from children > > > > > > of ioport_resource"). > > > > > > > > > > Maybe I'm missing something, but on x86, drivers like f71882fg do not > > > > > crash the system because inb() *never* causes a crash. > > > > > > > > > > If you want to build that driver for ARM, I think you need to make > > > > > sure that inb() on ARM also *never* causes a crash. I don't think > > > > > changing f71882fg and all the similar drivers is the right answer. > > > > > > > > > > > > > Agreed. As I had mentioned earlier, the driver changes are orthogonal: > > > > the drivers should request the IO region before accessing it, primarily > > > > to avoid conflicting accesses by multiple drivers in parallel. For > > > > example, the f71882fg driver supports chips which implement hardware > > > > monitoring as well as watchdog functionality, and both the hwmon > > > > and the watchdog driver may try to access the io space. > > > > > > > > If and how the system ensures that the IO region exists and/or that > > > > inb() always succeeds is a different question. I would prefer a less > > > > complex solution than the one suggested here, but that is my personal > > > > opionion. > > > > > > Hi Guenter, > > > > > > I have a question about these super-IO accesses: > > > > > > To me, it's not good that these hwmon, watchdog, gpio, etc drivers > > > make unconstrained accesses to 0x2e and 0x4e ports (ignoring the > > > request_muxed_region() call). > > > > > > The issue I see is that on an arm, IO space for some other device may > > > be mapped in this region, so it would not be right for these drivers > > > to access those same regions. > > > > > Yes, but then there _could_ be some arm or arm64 device supporting one > > of those chips, > > so we can not just add something like "depends on !(ARM || ARM64)". > > This looks like what has been added for PPC in commmit 746cdfbf01c0. > > However, agreed, it's not a good approach. > > > > > > Is there any other platform check which can be made to ensure that > > > accesses these super-IO ports is appropriate? > > > > > > > Not that I know of. It would make some sense to provide API functions > > for Super-IO accesses, but that would be a lot of work, and I guess > > it isn't really valuable enough for anyone to pick up and do. > > > > Normally, if you have such a system, the respective drivers should not be > > built. After all, this isn't the only instance where drivers > > unconditionally > > access some io region, no matter if the underlying hardware exists or not. > > The only real defense against that is to not build those drivers into > > a given kernel. > > If we're going to support a multi-plaform kernel for a given arch, then we > can't always avoid it. > > It seems that the only solution on the table now is to discard these IO port > accesses on arm64 when the IO port are not mapped. Hmm, how are you going to achieve that? I'm not sure we can guarantee a synchronous abort, so I'd be nervous about anything that tries to handle the exception after making the unmapped access. Will