On Mon, Mar 14, 2016 at 02:27:08PM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 14, 2016 at 02:53:22PM +0000, Lorenzo Pieralisi wrote:
[...]
> > Even if we do that, call to acpi_pci_root_validate_resources() for
> > IO space is wrong on IA64, we are comparing an IO resource against
> > ioport_resource there, but the Linux IO port space for the host
> > bridge is created in IA64 pci_acpi_root_prepare_resources() with
> > the call to add_io_space() which happens *after* the sequence above
> > is executed.
> >
> > On IA64:
> >
> > pci_acpi_root_prepare_resources()
> > -> acpi_pci_probe_root_resources()
> > -> acpi_dev_get_resources()
> > -> acpi_pci_root_validate_resources()
> > -> add_io_space() # this is where the Linux IO port space for the bridge is
> > created and that's when we can validate the IO
> > resource against ioport_resource
> >
> > I have no experience/knowledge of IA64 systems so I may be totally wrong,
> > I just want to understand.
> >
> > Comments welcome, Hanjun proved my understanding by testing on IA64 and
> > current mainline just does not work (see commit log for failure messages),
> > feedback from IA64 people is really needed here, we have to get this fixed
> > please (and through that fix, getting it to work on ARM64 too).
>
> I/O port translations makes my head hurt, but I think you're right.
>
> The raw I/O resources from ACPI _CRS are definitely different from the
> Linux ioport spaces constructed by add_io_space(), so we shouldn't
> compare the two.
>
> If we need to validate the raw I/O ports from _CRS, I suppose in
> theory we should be able to check that they're contained in the raw
> I/O port range of an upstream window.
>
> I think the problem is that 3772aea7d6f3 started applying the
> x86-specific "[0-0xffff]" restriction to all arches, so if you want to
> back that out so it applies only on x86 (but to all devices, not just
> PNP0A03), I think that would make sense.
I noticed there is already an X86 specific check in:
drivers/acpi/resource.c (ie valid_IRQ)
I can add something like code below there and be done with it:
#ifdef CONFIG_X86
static inline bool io_space_valid(struct resource *res)
{
return res->end < 0x10003;
}
#else
static inline bool io_space_valid(struct resource *res) { return true; }
#endif
if Rafael is ok with that. Whatever I do requires an arch specific hook
(empty/always-true on !CONFIG_X86) to be called from
acpi_dev_ioresource_flags(), otherwise removing that check really becomes
a minefield.
Other solution is reverting back to not using acpi_dev_get_resources()
on IA64 and ARM64 which defeats the whole purpose of Jiang's consolidation,
so I won't go there.
Next step is removing (or refactoring) acpi_pci_root_validate_resources()
for IORESOURCE_IO from acpi_pci_probe_root_resources(), it is a bogus check
as our discussion above highlighted and does not work on ARM64 (it
probably does on IA64 since IO_SPACE_LIMIT == 0xffffffffffffffffUL, but
that's conceptually wrong regardless).
Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
