On Wed, Mar 9, 2016 at 8:14 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@xxxxxxx> wrote:
> On Tue, Mar 08, 2016 at 11:33:32PM -0600, Bjorn Helgaas wrote:
>> On Tue, Mar 08, 2016 at 11:27:01PM +0100, Rafael J. Wysocki wrote:
>> > On Tue, Mar 8, 2016 at 11:21 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>> > > On Thu, Feb 11, 2016 at 05:47:07PM +0000, Lorenzo Pieralisi wrote:
>> > >> The [0 - 64k] ACPI PCI IO port resource boundary check in:
>> > >>
[cut]
>> Wait a minute, this doesn't seem right to me.
>>
>> The problem we're trying to fix is that on ia64, we incorrectly
>> discard the PCI host bridge window [io 0x1000000-0x100ffff window].
>>
>> That window is currently discarded by the generic
>> acpi_dev_ioresource_flags() function, where we're removing this code:
>>
>> - if (res->end >= 0x10003)
>> - res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
>>
>> and we're adding the "res->end >= 0x10003" check to
>> arch/x86/pci/acpi.c.
>>
>> But the removal also affects other users of acpi_dev_ioresource_flags(),
>> and that's broader than the scope of this patch. We might want to
>> remove the check, but if we do, it should be in a separate patch by
>> itself so it isn't a hidden side-effect of fixing this ia64 problem.
>>
>> The other users of acpi_dev_ioresource_flags() include:
>>
>> {acpi_lpss_create_device,acpi_create_platform_device,acpi_pci_probe_root_resources,acpi_default_enumeration,bcm_acpi_probe,tpm_tis_acpi_init,acpi_dma_parse_resource_group,acpi_dma_request_slave_chan_by_index,acpi_gpio_resource_lookup,acpi_gpio_count,acpi_i2c_add_device,inv_mpu_process_acpi_config,acpi_spi_add_device}
>> acpi_dev_get_resources
>> acpi_dev_process_resource
>> acpi_dev_resource_io
>> acpi_dev_get_ioresource
>> acpi_dev_ioresource_flags
>>
>> {pnpacpi_add_device,resources_store}
>> pnpacpi_parse_allocated_resource
>> pnpacpi_allocated_resource
>> acpi_dev_resource_io
>> acpi_dev_get_ioresource
>> acpi_dev_ioresource_flags
>>
>> I think the original test in acpi_dev_ioresource_flags() isn't quite
>> correct because it's generic code, but it enforces an arch-specific
>> 64K limit.
>
> Yes, I was about to write to you I noticed the same issue.
>
> That (>=0x10003) check in generic code is an x86ism, it is wrong but
> it is there and given that there are other potential
> users of acpi_dev_ioresource_flags() this patch should
> be dropped I do not want to trigger x86 regressions because
> some IO resources are not filtered.
>
> I am travelling, so can't have a proper look till next week,
> Rafael, please drop this patch.
>
>> Maybe acpi_dev_ioresource_flags() should instead check res->end
>> against ioport_resource.end, as we already do in
>> acpi_pci_root_validate_resources()? Each arch already sets its own
>> ioport_resource.end using IO_SPACE_LIMIT:
>>
>> arch/x86/include/asm/io.h #define IO_SPACE_LIMIT 0xffff
>> arch/ia64/include/asm/io.h #define IO_SPACE_LIMIT 0xffffffffffffffffUL
>
> We can't do that, it may work on IA64 but the ioport_resource is a
> chunk of address space on IA64/ARM64 that has nothing to do with
> the physical address at which the root bridges decode IO space (which
> is what's contained in the resource).
>
> I will have a look next week, please drop this patch.
OK, dropping.
Thanks,
Rafael
--
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
