| Message ID | 1294327066-23518-1-git-send-email-jslaby@suse.cz |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Thursday, January 06, 2011 08:17:46 am Jiri Slaby wrote: > There are BIOSes out there where they provide ACPI mapping that > conflicts with fixed one. Then e.g. an IDE controller doesn't work due > to I/O space conflict: > pci 0000:00:1f.1: address space collision: [io 0x0170-0x0177] conflicts with 0000:00:1f.0 [io 0x0100-0x017f] > > 0x0170-0x0177 for IDE controllers is created in pci_setup_device. > 0x0100-0x017f for the ICH4 ISA bridge is created in quirk_ich4_lpc_acpi This might be the right thing to do, but it feels hacky to add more magic numbers (">= 0x180"), so I'd like to understand it better. (Is there a bugzilla for this? Is it a regression? Sounds like an old machine where we should have seen this before.) My guess is that we found this "conflict" and tried to move the controller, but it's hardwired to stay at 0x170 when in compatibility mode. So the hardware is really still at 0x170, but Linux thinks it moved it elsewhere, so it doesn't work. The fixed IDE resources in pci_setup_device() make sense to me; I think section 2.1 of the PCI IDE spec justifies specific OS knowledge of the compatibility mode regions. But I don't know the quirk_ich4_lpc_acpi() history, and this quirk is the sort of information the OS should not have to know about. Theoretically, ACPI tells us about the GPIO/TCO/etc. regions in a generic way via namespace devices or something in the static tables. Is that generic information missing, or is it there and Linux is ignoring it? If we're ignoring it, I'd rather fix that. The main reason for claiming I/O regions is to keep us from placing another device on top of them. But we've had PCIBIOS_MIN_IO = 0x1000 forever, which should keep us from putting anything below that anyway (at least for PCI devices). So there must be some other reason for claiming space in this quirk. Bjorn > The former is fixed, according to specs at that address. > The latter is read from the bridge confspace which contains: > 00000000 86 80 a1 25 0f 00 80 02 02 00 01 06 00 00 80 00 |...%............| > 00000010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| > 00000020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| > 00000030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| > 00000040 01 01 00 00 10 00 00 00 00 00 00 00 00 00 01 00 |................| > 00000050 00 00 00 00 00 00 00 00 01 05 00 00 10 00 00 00 |................| > ... > I.e. 0x00000101 as a base (ORed by 1 meaning I/O space) for ACPI per > ICH4 specs. > > Don't accept bases below 0x180 for this dynamic mapping. > > Signed-off-by: Jiri Slaby <jslaby@suse.cz> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/pci/quirks.c | 9 ++++++++- > 1 files changed, 8 insertions(+), 1 deletions(-) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 53a786f..6d241eb 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -543,7 +543,14 @@ static void __devinit quirk_ich4_lpc_acpi(struct pci_dev *dev) > u32 region; > > pci_read_config_dword(dev, 0x40, ®ion); > - quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, "ICH4 ACPI/GPIO/TCO"); > + /* > + * some BIOSes are bogus and create this dynamic mapping so that it > + * conflicts with fixed. There is no space below 0x180 for > + * ACPI/GPIO/TCO which is 128B long and 128B aligned. > + */ > + if (region >= 0x180) > + quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, > + "ICH4 ACPI/GPIO/TCO"); > > pci_read_config_dword(dev, 0x58, ®ion); > quirk_io_region(dev, region, 64, PCI_BRIDGE_RESOURCES+1, "ICH4 GPIO"); > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/06/2011 08:24 PM, Bjorn Helgaas wrote: > On Thursday, January 06, 2011 08:17:46 am Jiri Slaby wrote: >> There are BIOSes out there where they provide ACPI mapping that >> conflicts with fixed one. Then e.g. an IDE controller doesn't work due >> to I/O space conflict: >> pci 0000:00:1f.1: address space collision: [io 0x0170-0x0177] conflicts with 0000:00:1f.0 [io 0x0100-0x017f] >> >> 0x0170-0x0177 for IDE controllers is created in pci_setup_device. >> 0x0100-0x017f for the ICH4 ISA bridge is created in quirk_ich4_lpc_acpi > > This might be the right thing to do, but it feels hacky to add more > magic numbers (">= 0x180"), so I'd like to understand it better. Yes, definitely, it's a hack, but I haven't found anything better. So I dropped the patch in for discussion. > (Is there a bugzilla for this? Only a novell (opensuse) bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=558740 > Is it a regression? Sounds like an > old machine where we should have seen this before.) Yes, a regression. 2.6.31 and newer doesn't work. 2.6.27 did. I don't know what kernel exactly in between is first defunct. > My guess is that we found this "conflict" and tried to move the > controller, but it's hardwired to stay at 0x170 when in compatibility > mode. So the hardware is really still at 0x170, but Linux thinks it > moved it elsewhere, so it doesn't work. Sorry I didn't get this paragraph. > The fixed IDE resources in pci_setup_device() make sense to me; I > think section 2.1 of the PCI IDE spec justifies specific OS knowledge > of the compatibility mode regions. > > But I don't know the quirk_ich4_lpc_acpi() history, and this quirk > is the sort of information the OS should not have to know about. It's from ICH4 specs. For ICH4, the addresses of where to look for ACPI and GPIO are specified in the PCI BUS conf space. So we look at 0x40 in that space and find 0x101, so we reserve 0x100-0x17f which conflicts with legacy IDE which is 0x170-0x17f. > Theoretically, ACPI tells us about the GPIO/TCO/etc. regions in a > generic way via namespace devices or something in the static tables. > Is that generic information missing, or is it there and Linux is > ignoring it? If we're ignoring it, I'd rather fix that. It works for most boxes I would say. Try to google for "claimed by ICH4 ACPI/GPIO/TCO", it reports sane ranges like 0400-047f or 4000-407f. > The main reason for claiming I/O regions is to keep us from placing > another device on top of them. But we've had PCIBIOS_MIN_IO = 0x1000 > forever, which should keep us from putting anything below that anyway > (at least for PCI devices). So there must be some other reason for > claiming space in this quirk. Anyway, this ACPI space may be below 0x1000 as can be seen above. From the ICH standard, it can be anywhere... thanks,
On Fri, 07 Jan 2011 21:44:35 +0100 Jiri Slaby <jirislaby@gmail.com> wrote: > On 01/06/2011 08:24 PM, Bjorn Helgaas wrote: > > Theoretically, ACPI tells us about the GPIO/TCO/etc. regions in a > > generic way via namespace devices or something in the static tables. > > Is that generic information missing, or is it there and Linux is > > ignoring it? If we're ignoring it, I'd rather fix that. > > It works for most boxes I would say. Try to google for "claimed by ICH4 > ACPI/GPIO/TCO", it reports sane ranges like 0400-047f or 4000-407f. > > > The main reason for claiming I/O regions is to keep us from placing > > another device on top of them. But we've had PCIBIOS_MIN_IO = 0x1000 > > forever, which should keep us from putting anything below that anyway > > (at least for PCI devices). So there must be some other reason for > > claiming space in this quirk. > > Anyway, this ACPI space may be below 0x1000 as can be seen above. From > the ICH standard, it can be anywhere... Is there some other resource we should be considering to avoid this space? And we know it won't work if we try to use the IDE controller in the LPC allocated space? Have you checked the ICH4 specs to see what the decode rules are on this space? I guess LPC has priority and doesn't forward requests through to IDE? Either way, it would be good to understand this issue a little better before we start restricting the LPC space like this. Thanks,
On Friday, January 07, 2011 01:44:35 pm Jiri Slaby wrote: > On 01/06/2011 08:24 PM, Bjorn Helgaas wrote: > > On Thursday, January 06, 2011 08:17:46 am Jiri Slaby wrote: > Only a novell (opensuse) bugzilla: > https://bugzilla.novell.com/show_bug.cgi?id=558740 > > > Is it a regression? Sounds like an > > old machine where we should have seen this before.) > > Yes, a regression. 2.6.31 and newer doesn't work. 2.6.27 did. I don't > know what kernel exactly in between is first defunct. > > > My guess is that we found this "conflict" and tried to move the > > controller, but it's hardwired to stay at 0x170 when in compatibility > > mode. So the hardware is really still at 0x170, but Linux thinks it > > moved it elsewhere, so it doesn't work. > > Sorry I didn't get this paragraph. IDE devices are special and are allowed to respond to the legacy addresses, e.g., 0x170, even if those addresses don't appear in the BARs. That's why we have the special case in pci_setup_device(). But the code that detects the address space conflict and tries to resolve it doesn't know about this hard-wired IDE address decoder. Linux tries to move it by writing to a BAR, and that probably has no effect on a compatibility-mode IDE device. > > Theoretically, ACPI tells us about the GPIO/TCO/etc. regions in a > > generic way via namespace devices or something in the static tables. > > Is that generic information missing, or is it there and Linux is > > ignoring it? If we're ignoring it, I'd rather fix that. > > It works for most boxes I would say. Try to google for "claimed by ICH4 > ACPI/GPIO/TCO", it reports sane ranges like 0400-047f or 4000-407f. My point is that BIOS should be telling the OS about GPIO/TCO/etc. regions via an ACPI mechanism, and, ideally, we would use that rather than reading the address out of chipset-dependent registers. Even though PMBASE says the ACPI registers occupy 128 bytes from 0x100-0x17f, it's likely there's no actual conflict between the last 16 bytes and the IDE device. ACPI probably reports this region via the FADT (the GPE PM register blocks) and possibly a PNP0C02 device. These will probably report something that doesn't conflict with the legacy IDE ports, i.e., a subset of the 0x100-0x17f range. Ooooh, I notice in the bugzilla that something's wrong with SMBIOS (comment 29) and ACPI is disabled because we couldn't find the RSDP (dmesg in comment 27). What sort of machine is this, anyway? We didn't find PNPBIOS, either. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/07/2011 11:37 PM, Jesse Barnes wrote: > On Fri, 07 Jan 2011 21:44:35 +0100 > Jiri Slaby <jirislaby@gmail.com> wrote: > >> On 01/06/2011 08:24 PM, Bjorn Helgaas wrote: >>> Theoretically, ACPI tells us about the GPIO/TCO/etc. regions in a >>> generic way via namespace devices or something in the static tables. >>> Is that generic information missing, or is it there and Linux is >>> ignoring it? If we're ignoring it, I'd rather fix that. >> >> It works for most boxes I would say. Try to google for "claimed by ICH4 >> ACPI/GPIO/TCO", it reports sane ranges like 0400-047f or 4000-407f. >> >>> The main reason for claiming I/O regions is to keep us from placing >>> another device on top of them. But we've had PCIBIOS_MIN_IO = 0x1000 >>> forever, which should keep us from putting anything below that anyway >>> (at least for PCI devices). So there must be some other reason for >>> claiming space in this quirk. >> >> Anyway, this ACPI space may be below 0x1000 as can be seen above. From >> the ICH standard, it can be anywhere... > > Is there some other resource we should be considering to avoid this > space? There are two kinds of address ranges (spaces) in ICH specs. Fixed (address base is fixed) and Variable (base can be changed). And Fixed space should not collide (the behaviour is explicitly unpredictable) with Variable. This ACPI space is defined as Variable. So it cannot collide with any Fixed. Most of the Fixed are below 0x180, there is also 0x376, 0x3f6 for IDE, 0x4D0-0x4D1 for PIC and 0xCF9 for reset generator. > And we know it won't work if we try to use the IDE controller in the > LPC allocated space? Have you checked the ICH4 specs to see what the > decode rules are on this space? I guess LPC has priority and doesn't > forward requests through to IDE? If we read/write to ports 0x170-0x17f, IDE ctrl responds on that machine. I.e. if I remove the quirk, disks are found. (Otherwise they are not due to the conflict.) thanks,
On 01/08/2011 12:03 AM, Bjorn Helgaas wrote: > On Friday, January 07, 2011 01:44:35 pm Jiri Slaby wrote: >> On 01/06/2011 08:24 PM, Bjorn Helgaas wrote: >>> Theoretically, ACPI tells us about the GPIO/TCO/etc. regions in a >>> generic way via namespace devices or something in the static tables. >>> Is that generic information missing, or is it there and Linux is >>> ignoring it? If we're ignoring it, I'd rather fix that. >> >> It works for most boxes I would say. Try to google for "claimed by ICH4 >> ACPI/GPIO/TCO", it reports sane ranges like 0400-047f or 4000-407f. > > My point is that BIOS should be telling the OS about GPIO/TCO/etc. > regions via an ACPI mechanism, and, ideally, we would use that rather > than reading the address out of chipset-dependent registers. > > Even though PMBASE says the ACPI registers occupy 128 bytes from > 0x100-0x17f, it's likely there's no actual conflict between the > last 16 bytes and the IDE device. I wouldn't say so. According to the datasheet 0x60-0x7f of the space (i.e. 0x160-0x17f here) is for TCO registers. There: 0x10 -- Software IRQ Generation Register (i.e. 0x170) 0x11-0x1f -- reserved (0x171-0x17f) So at least 0x170 should be conflicting. Unless TCO is unused/disabled and not mapped there at all. May be that the case? > ACPI probably reports this region via the FADT (the GPE PM register > blocks) and possibly a PNP0C02 device. These will probably report > something that doesn't conflict with the legacy IDE ports, i.e., a > subset of the 0x100-0x17f range. > > Ooooh, I notice in the bugzilla that something's wrong with SMBIOS > (comment 29) and ACPI is disabled because we couldn't find the > RSDP (dmesg in comment 27). What sort of machine is this, anyway? > We didn't find PNPBIOS, either. Hmm, it looks like some old crap. What exact information you would like to know? I've just asked if ACPI is not disabled in BIOS. There should be no machine without ACPI running still in the 21st century, I think. thanks,
On Friday, January 07, 2011 04:29:00 pm Jiri Slaby wrote: > On 01/08/2011 12:03 AM, Bjorn Helgaas wrote: > > On Friday, January 07, 2011 01:44:35 pm Jiri Slaby wrote: > >> On 01/06/2011 08:24 PM, Bjorn Helgaas wrote: > >>> Theoretically, ACPI tells us about the GPIO/TCO/etc. regions in a > >>> generic way via namespace devices or something in the static tables. > >>> Is that generic information missing, or is it there and Linux is > >>> ignoring it? If we're ignoring it, I'd rather fix that. > >> > >> It works for most boxes I would say. Try to google for "claimed by ICH4 > >> ACPI/GPIO/TCO", it reports sane ranges like 0400-047f or 4000-407f. > > > > My point is that BIOS should be telling the OS about GPIO/TCO/etc. > > regions via an ACPI mechanism, and, ideally, we would use that rather > > than reading the address out of chipset-dependent registers. > > > > Even though PMBASE says the ACPI registers occupy 128 bytes from > > 0x100-0x17f, it's likely there's no actual conflict between the > > last 16 bytes and the IDE device. > > I wouldn't say so. According to the datasheet 0x60-0x7f of the space > (i.e. 0x160-0x17f here) is for TCO registers. There: > 0x10 -- Software IRQ Generation Register (i.e. 0x170) > 0x11-0x1f -- reserved (0x171-0x17f) > > So at least 0x170 should be conflicting. Unless TCO is unused/disabled > and not mapped there at all. May be that the case? Maybe. All your patch does is avoid reserving this 0x100-0x1f7 region; it doesn't actually *move* anything. And the IDE device apparently works at the 0x170 compatibility address. So the ICH ACPI stuff is still at 0x100-0x17f, so apparently they don't conflict or maybe the ICH ACPI stuff is disabled. If the box doesn't even have ACPI, I suppose there would be no reason to have the ACPI registers enabled. Is there something in ICH that tells us whether they're enabled? > > ACPI probably reports this region via the FADT (the GPE PM register > > blocks) and possibly a PNP0C02 device. These will probably report > > something that doesn't conflict with the legacy IDE ports, i.e., a > > subset of the 0x100-0x17f range. > > > > Ooooh, I notice in the bugzilla that something's wrong with SMBIOS > > (comment 29) and ACPI is disabled because we couldn't find the > > RSDP (dmesg in comment 27). What sort of machine is this, anyway? > > We didn't find PNPBIOS, either. > > Hmm, it looks like some old crap. What exact information you would like > to know? I've just asked if ACPI is not disabled in BIOS. There should > be no machine without ACPI running still in the 21st century, I think. I'm just wondering if the machine actually does have ACPI, but there's some Linux problem related to finding the tables. If it's really old enough, that wouldn't be so surprising, but I see USB and gigabit NIC hardware, so it's not truly ancient. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/08/2011 01:16 AM, Bjorn Helgaas wrote: > On Friday, January 07, 2011 04:29:00 pm Jiri Slaby wrote: >> On 01/08/2011 12:03 AM, Bjorn Helgaas wrote: >>> On Friday, January 07, 2011 01:44:35 pm Jiri Slaby wrote: >>>> On 01/06/2011 08:24 PM, Bjorn Helgaas wrote: >>>>> Theoretically, ACPI tells us about the GPIO/TCO/etc. regions in a >>>>> generic way via namespace devices or something in the static tables. >>>>> Is that generic information missing, or is it there and Linux is >>>>> ignoring it? If we're ignoring it, I'd rather fix that. >>>> >>>> It works for most boxes I would say. Try to google for "claimed by ICH4 >>>> ACPI/GPIO/TCO", it reports sane ranges like 0400-047f or 4000-407f. >>> >>> My point is that BIOS should be telling the OS about GPIO/TCO/etc. >>> regions via an ACPI mechanism, and, ideally, we would use that rather >>> than reading the address out of chipset-dependent registers. >>> >>> Even though PMBASE says the ACPI registers occupy 128 bytes from >>> 0x100-0x17f, it's likely there's no actual conflict between the >>> last 16 bytes and the IDE device. >> >> I wouldn't say so. According to the datasheet 0x60-0x7f of the space >> (i.e. 0x160-0x17f here) is for TCO registers. There: >> 0x10 -- Software IRQ Generation Register (i.e. 0x170) >> 0x11-0x1f -- reserved (0x171-0x17f) >> >> So at least 0x170 should be conflicting. Unless TCO is unused/disabled >> and not mapped there at all. May be that the case? > > Maybe. All your patch does is avoid reserving this 0x100-0x1f7 > region; it doesn't actually *move* anything. And the IDE device > apparently works at the 0x170 compatibility address. So the > ICH ACPI stuff is still at 0x100-0x17f, so apparently they don't > conflict or maybe the ICH ACPI stuff is disabled. If the box > doesn't even have ACPI, I suppose there would be no reason to > have the ACPI registers enabled. Is there something in ICH > that tells us whether they're enabled? Hmm, there is: bit 4: ACPI Enable (ACPI_EN) — R/W. 0 = Disable. 1 = Decode of the I/O range pointed to by the ACPI Base register is enabled, and the ACPI power management function is enabled. Note that the APM power management ranges (B2/B3h) are always enabled and are not affected by this bit. at 0x44 in the bridge conf space. So we should definitely check the value. I don't have the actual value in that register when ACPI is disabled in BIOS. From the run where acpi=off was passed to the kernel, there is 0x10 (i.e. ACPI_EN=1). However I don't know whether ACPI was disabled in BIOS at that time. >>> ACPI probably reports this region via the FADT (the GPE PM register >>> blocks) and possibly a PNP0C02 device. These will probably report >>> something that doesn't conflict with the legacy IDE ports, i.e., a >>> subset of the 0x100-0x17f range. >>> >>> Ooooh, I notice in the bugzilla that something's wrong with SMBIOS >>> (comment 29) and ACPI is disabled because we couldn't find the >>> RSDP (dmesg in comment 27). What sort of machine is this, anyway? >>> We didn't find PNPBIOS, either. >> >> Hmm, it looks like some old crap. What exact information you would like >> to know? I've just asked if ACPI is not disabled in BIOS. There should >> be no machine without ACPI running still in the 21st century, I think. > > I'm just wondering if the machine actually does have ACPI, but > there's some Linux problem related to finding the tables. If it's > really old enough, that wouldn't be so surprising, but I see USB > and gigabit NIC hardware, so it's not truly ancient. The box is: http://www.xembedded.com/content/vme/processors/xvme-690.php and has ACPI, but the user disabled ACPI (I don't know why yet). thanks,
On Saturday, January 08, 2011 02:58:01 am Jiri Slaby wrote: > On 01/08/2011 01:16 AM, Bjorn Helgaas wrote: > > On Friday, January 07, 2011 04:29:00 pm Jiri Slaby wrote: > >> On 01/08/2011 12:03 AM, Bjorn Helgaas wrote: > >>> On Friday, January 07, 2011 01:44:35 pm Jiri Slaby wrote: > >>>> On 01/06/2011 08:24 PM, Bjorn Helgaas wrote: > >>>>> Theoretically, ACPI tells us about the GPIO/TCO/etc. regions in a > >>>>> generic way via namespace devices or something in the static tables. > >>>>> Is that generic information missing, or is it there and Linux is > >>>>> ignoring it? If we're ignoring it, I'd rather fix that. > >>>> > >>>> It works for most boxes I would say. Try to google for "claimed by ICH4 > >>>> ACPI/GPIO/TCO", it reports sane ranges like 0400-047f or 4000-407f. > >>> > >>> My point is that BIOS should be telling the OS about GPIO/TCO/etc. > >>> regions via an ACPI mechanism, and, ideally, we would use that rather > >>> than reading the address out of chipset-dependent registers. > >>> > >>> Even though PMBASE says the ACPI registers occupy 128 bytes from > >>> 0x100-0x17f, it's likely there's no actual conflict between the > >>> last 16 bytes and the IDE device. > >> > >> I wouldn't say so. According to the datasheet 0x60-0x7f of the space > >> (i.e. 0x160-0x17f here) is for TCO registers. There: > >> 0x10 -- Software IRQ Generation Register (i.e. 0x170) > >> 0x11-0x1f -- reserved (0x171-0x17f) > >> > >> So at least 0x170 should be conflicting. Unless TCO is unused/disabled > >> and not mapped there at all. May be that the case? > > > > Maybe. All your patch does is avoid reserving this 0x100-0x1f7 > > region; it doesn't actually *move* anything. And the IDE device > > apparently works at the 0x170 compatibility address. So the > > ICH ACPI stuff is still at 0x100-0x17f, so apparently they don't > > conflict or maybe the ICH ACPI stuff is disabled. If the box > > doesn't even have ACPI, I suppose there would be no reason to > > have the ACPI registers enabled. Is there something in ICH > > that tells us whether they're enabled? > > Hmm, there is: > bit 4: ACPI Enable (ACPI_EN) — R/W. > 0 = Disable. > 1 = Decode of the I/O range pointed to by the ACPI Base register is > enabled, and the ACPI power management function is enabled. Note that > the APM power management ranges (B2/B3h) are always enabled and are not > affected by this bit. > > at 0x44 in the bridge conf space. So we should definitely check the value. > > I don't have the actual value in that register when ACPI is disabled in > BIOS. From the run where acpi=off was passed to the kernel, there is > 0x10 (i.e. ACPI_EN=1). However I don't know whether ACPI was disabled in > BIOS at that time. Checking ACPI_EN before doing anything in the quirk looks like the simplest thing (if the BIOS actually sets ACPI_EN=0 when it disables ACPI). Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/10/2011 07:40 PM, Bjorn Helgaas wrote: > On Saturday, January 08, 2011 02:58:01 am Jiri Slaby wrote: >> On 01/08/2011 01:16 AM, Bjorn Helgaas wrote: >>> On Friday, January 07, 2011 04:29:00 pm Jiri Slaby wrote: >>>> On 01/08/2011 12:03 AM, Bjorn Helgaas wrote: >>>>> On Friday, January 07, 2011 01:44:35 pm Jiri Slaby wrote: >>>>>> On 01/06/2011 08:24 PM, Bjorn Helgaas wrote: >>>>>>> Theoretically, ACPI tells us about the GPIO/TCO/etc. regions in a >>>>>>> generic way via namespace devices or something in the static tables. >>>>>>> Is that generic information missing, or is it there and Linux is >>>>>>> ignoring it? If we're ignoring it, I'd rather fix that. >>>>>> >>>>>> It works for most boxes I would say. Try to google for "claimed by ICH4 >>>>>> ACPI/GPIO/TCO", it reports sane ranges like 0400-047f or 4000-407f. >>>>> >>>>> My point is that BIOS should be telling the OS about GPIO/TCO/etc. >>>>> regions via an ACPI mechanism, and, ideally, we would use that rather >>>>> than reading the address out of chipset-dependent registers. >>>>> >>>>> Even though PMBASE says the ACPI registers occupy 128 bytes from >>>>> 0x100-0x17f, it's likely there's no actual conflict between the >>>>> last 16 bytes and the IDE device. >>>> >>>> I wouldn't say so. According to the datasheet 0x60-0x7f of the space >>>> (i.e. 0x160-0x17f here) is for TCO registers. There: >>>> 0x10 -- Software IRQ Generation Register (i.e. 0x170) >>>> 0x11-0x1f -- reserved (0x171-0x17f) >>>> >>>> So at least 0x170 should be conflicting. Unless TCO is unused/disabled >>>> and not mapped there at all. May be that the case? >>> >>> Maybe. All your patch does is avoid reserving this 0x100-0x1f7 >>> region; it doesn't actually *move* anything. And the IDE device >>> apparently works at the 0x170 compatibility address. So the >>> ICH ACPI stuff is still at 0x100-0x17f, so apparently they don't >>> conflict or maybe the ICH ACPI stuff is disabled. If the box >>> doesn't even have ACPI, I suppose there would be no reason to >>> have the ACPI registers enabled. Is there something in ICH >>> that tells us whether they're enabled? >> >> Hmm, there is: >> bit 4: ACPI Enable (ACPI_EN) — R/W. >> 0 = Disable. >> 1 = Decode of the I/O range pointed to by the ACPI Base register is >> enabled, and the ACPI power management function is enabled. Note that >> the APM power management ranges (B2/B3h) are always enabled and are not >> affected by this bit. >> >> at 0x44 in the bridge conf space. So we should definitely check the value. >> >> I don't have the actual value in that register when ACPI is disabled in >> BIOS. From the run where acpi=off was passed to the kernel, there is >> 0x10 (i.e. ACPI_EN=1). However I don't know whether ACPI was disabled in >> BIOS at that time. > > Checking ACPI_EN before doing anything in the quirk looks like > the simplest thing (if the BIOS actually sets ACPI_EN=0 when > it disables ACPI). Unfortunately, they double checked and the BIOS leaves ACPI_EN=1 even when ACPI is disabled. From hexdump -Cv /sys/bus/pci/devices/0000\:00\:1f.0/config: 00000040 01 01 00 00 10 00 00 00 00 00 00 00 00 00 01 00 ^base addr^|^-- 4th bit ~ ACPI_EN But I think we still should add the check in any case, the same for GPIO (there is GPIO_EN) and maybe for newer ICHs. What do you think? The problem is we can't add a quirk based on DMI for this one, since there is no DMI table. I'm out of ideas now. thanks,
On Thursday, January 13, 2011 03:07:24 am Jiri Slaby wrote: > On 01/10/2011 07:40 PM, Bjorn Helgaas wrote: > > On Saturday, January 08, 2011 02:58:01 am Jiri Slaby wrote: > >> On 01/08/2011 01:16 AM, Bjorn Helgaas wrote: > >>> On Friday, January 07, 2011 04:29:00 pm Jiri Slaby wrote: > >>>> On 01/08/2011 12:03 AM, Bjorn Helgaas wrote: > >>>>> On Friday, January 07, 2011 01:44:35 pm Jiri Slaby wrote: > >>>>>> On 01/06/2011 08:24 PM, Bjorn Helgaas wrote: > >>>>>>> Theoretically, ACPI tells us about the GPIO/TCO/etc. regions in a > >>>>>>> generic way via namespace devices or something in the static tables. > >>>>>>> Is that generic information missing, or is it there and Linux is > >>>>>>> ignoring it? If we're ignoring it, I'd rather fix that. > >>>>>> > >>>>>> It works for most boxes I would say. Try to google for "claimed by ICH4 > >>>>>> ACPI/GPIO/TCO", it reports sane ranges like 0400-047f or 4000-407f. > >>>>> > >>>>> My point is that BIOS should be telling the OS about GPIO/TCO/etc. > >>>>> regions via an ACPI mechanism, and, ideally, we would use that rather > >>>>> than reading the address out of chipset-dependent registers. > >>>>> > >>>>> Even though PMBASE says the ACPI registers occupy 128 bytes from > >>>>> 0x100-0x17f, it's likely there's no actual conflict between the > >>>>> last 16 bytes and the IDE device. > >>>> > >>>> I wouldn't say so. According to the datasheet 0x60-0x7f of the space > >>>> (i.e. 0x160-0x17f here) is for TCO registers. There: > >>>> 0x10 -- Software IRQ Generation Register (i.e. 0x170) > >>>> 0x11-0x1f -- reserved (0x171-0x17f) > >>>> > >>>> So at least 0x170 should be conflicting. Unless TCO is unused/disabled > >>>> and not mapped there at all. May be that the case? > >>> > >>> Maybe. All your patch does is avoid reserving this 0x100-0x1f7 > >>> region; it doesn't actually *move* anything. And the IDE device > >>> apparently works at the 0x170 compatibility address. So the > >>> ICH ACPI stuff is still at 0x100-0x17f, so apparently they don't > >>> conflict or maybe the ICH ACPI stuff is disabled. If the box > >>> doesn't even have ACPI, I suppose there would be no reason to > >>> have the ACPI registers enabled. Is there something in ICH > >>> that tells us whether they're enabled? > >> > >> Hmm, there is: > >> bit 4: ACPI Enable (ACPI_EN) — R/W. > >> 0 = Disable. > >> 1 = Decode of the I/O range pointed to by the ACPI Base register is > >> enabled, and the ACPI power management function is enabled. Note that > >> the APM power management ranges (B2/B3h) are always enabled and are not > >> affected by this bit. > >> > >> at 0x44 in the bridge conf space. So we should definitely check the value. > >> > >> I don't have the actual value in that register when ACPI is disabled in > >> BIOS. From the run where acpi=off was passed to the kernel, there is > >> 0x10 (i.e. ACPI_EN=1). However I don't know whether ACPI was disabled in > >> BIOS at that time. > > > > Checking ACPI_EN before doing anything in the quirk looks like > > the simplest thing (if the BIOS actually sets ACPI_EN=0 when > > it disables ACPI). > > Unfortunately, they double checked and the BIOS leaves ACPI_EN=1 even > when ACPI is disabled. > From hexdump -Cv /sys/bus/pci/devices/0000\:00\:1f.0/config: > 00000040 01 01 00 00 10 00 00 00 00 00 00 00 00 00 01 00 > ^base addr^|^-- 4th bit ~ ACPI_EN > > But I think we still should add the check in any case, the same for GPIO > (there is GPIO_EN) and maybe for newer ICHs. What do you think? > > The problem is we can't add a quirk based on DMI for this one, since > there is no DMI table. > > I'm out of ideas now. I think we're back to the question of why we have the ICH4 quirk in the first place, and I don't know the answer to that. If we didn't have the quirk at all, this machine would work. Other machines *should* work without, but maybe there's a BIOS or old Linux bug that the quirk covers up. I added Linus because he added some similar "should be unnecessary" code for ICH7+ (894886e5d). Maybe you could make the ICH4 quirk only claim the region if it's above PCIBIOS_MIN_IO, i.e., if it's in the area where we could put another PCI device on top of it? Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 13, 2011 at 3:19 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > > I think we're back to the question of why we have the ICH4 quirk in > the first place, and I don't know the answer to that. Iirc, there were several laptops that didn't have the ACPI region mentioned in any of the regular places, and we'd allocate the PCMCIA IO region on top of them. The machine would boot, but if anybody ever inserted a PCCard into the machine, the first access to the IO region would generally just halt it (because it was trying to read the PCCard, but the APCI region decodes first, and then the read from that usually put the CPU in a sleep state that it would never wake up from for obvious reasons). So we do want the ICH4 quirk. > Maybe you could make the ICH4 quirk only claim the region if it's > above PCIBIOS_MIN_IO, i.e., if it's in the area where we could put > another PCI device on top of it? That may well be the best solution - we'd only mark it reserved if it might conflict with a dynamic allocation, and if it conflicts with other motherboard resources (like the IDE ports), then we don't know what the decode priority is anyway, so.. In this case, it looks like the legacy IDE ports get decoded before the dynamic BAR's. Which is not too much of a surprise (fixed addresses tend to be the first to get decoded). Linus -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/14/2011 01:15 AM, Linus Torvalds wrote: > On Thu, Jan 13, 2011 at 3:19 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: >> >> I think we're back to the question of why we have the ICH4 quirk in >> the first place, and I don't know the answer to that. > > Iirc, there were several laptops that didn't have the ACPI region > mentioned in any of the regular places, and we'd allocate the PCMCIA > IO region on top of them. The machine would boot, but if anybody ever > inserted a PCCard into the machine, the first access to the IO region > would generally just halt it (because it was trying to read the > PCCard, but the APCI region decodes first, and then the read from that > usually put the CPU in a sleep state that it would never wake up from > for obvious reasons). > > So we do want the ICH4 quirk. Yes, this is an "official" way how ICH4 (and later) advertises the region. The original commit says: commit df4ba87cfc2239aa85e21178fca35cf6644a6dd1 Author: Linus Torvalds <torvalds@home.osdl.org> Date: Thu Oct 23 17:52:25 2003 -0700 Add a quirk for the Intel ICH-[45] to add special ACPI regions. This fixes resource conflicts due to IO decode that doesn't show up with a normal PCI probe (we do similar quirks for most other chipsets). Without it, the kernel doesn't know about some magic IO decodes for the chips. >> Maybe you could make the ICH4 quirk only claim the region if it's >> above PCIBIOS_MIN_IO, i.e., if it's in the area where we could put >> another PCI device on top of it? > > That may well be the best solution - we'd only mark it reserved if it > might conflict with a dynamic allocation, and if it conflicts with > other motherboard resources (like the IDE ports), then we don't know > what the decode priority is anyway, so.. > > In this case, it looks like the legacy IDE ports get decoded before > the dynamic BAR's. Which is not too much of a surprise (fixed > addresses tend to be the first to get decoded). I will attach 2 patches for this as a reply. I created 2 variants: A is for all quirk B is only for ICH4 and ICH6 thanks,
On Friday, January 14, 2011 03:31:16 am Jiri Slaby wrote: > On 01/14/2011 01:15 AM, Linus Torvalds wrote: > > On Thu, Jan 13, 2011 at 3:19 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > >> > >> I think we're back to the question of why we have the ICH4 quirk in > >> the first place, and I don't know the answer to that. > > > > Iirc, there were several laptops that didn't have the ACPI region > > mentioned in any of the regular places, and we'd allocate the PCMCIA > > IO region on top of them. The machine would boot, but if anybody ever > > inserted a PCCard into the machine, the first access to the IO region > > would generally just halt it (because it was trying to read the > > PCCard, but the APCI region decodes first, and then the read from that > > usually put the CPU in a sleep state that it would never wake up from > > for obvious reasons). > > > > So we do want the ICH4 quirk. > > Yes, this is an "official" way how ICH4 (and later) advertises the region. The quirk is a bug workaround, *not* the "official, planned" way to deal with these regions. The official way is to use ACPI, because that's a generic way that doesn't require changes for new versions of ICH. The bug might be that BIOS didn't mention the region in the expected places. Or it might be that the BIOS mentioned it, but Linux didn't deal with it correctly. For example, the Linux ACPI/PNP core mostly ignores the resources mentioned in ACPI _CRS methods. We only look at those in individual drivers. The PNP system.c driver reserves PNP0C01/02 resources, but the BIOS could use other PNP IDs, and Linux would ignore them (PNPACPI parses them, but doesn't reserve them in any way). Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/14/2011 10:10 AM, Bjorn Helgaas wrote: > On Friday, January 14, 2011 03:31:16 am Jiri Slaby wrote: >> On 01/14/2011 01:15 AM, Linus Torvalds wrote: >>> On Thu, Jan 13, 2011 at 3:19 PM, Bjorn Helgaas<bjorn.helgaas@hp.com> wrote: >>>> >>>> I think we're back to the question of why we have the ICH4 quirk in >>>> the first place, and I don't know the answer to that. >>> >>> Iirc, there were several laptops that didn't have the ACPI region >>> mentioned in any of the regular places, and we'd allocate the PCMCIA >>> IO region on top of them. The machine would boot, but if anybody ever >>> inserted a PCCard into the machine, the first access to the IO region >>> would generally just halt it (because it was trying to read the >>> PCCard, but the APCI region decodes first, and then the read from that >>> usually put the CPU in a sleep state that it would never wake up from >>> for obvious reasons). >>> >>> So we do want the ICH4 quirk. >> >> Yes, this is an "official" way how ICH4 (and later) advertises the region. > > The quirk is a bug workaround, *not* the "official, planned" way to > deal with these regions. The official way is to use ACPI, because > that's a generic way that doesn't require changes for new versions > of ICH. > > The bug might be that BIOS didn't mention the region in the expected > places. Or it might be that the BIOS mentioned it, but Linux didn't > deal with it correctly. For example, the Linux ACPI/PNP core mostly > ignores the resources mentioned in ACPI _CRS methods. We only look > at those in individual drivers. The PNP system.c driver reserves > PNP0C01/02 resources, but the BIOS could use other PNP IDs, and Linux > would ignore them (PNPACPI parses them, but doesn't reserve them in > any way). The patch to add the quirk was added back in 2003, and I believe using ACPI in Linux (other than for HT sibling discovery) was an uncommon configuration back then. It's quite possible the resources were reserved in ACPI on such systems and the quirk was only needed for systems with ACPI disabled in either the BIOS or the kernel. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/14/2011 05:10 PM, Bjorn Helgaas wrote: > On Friday, January 14, 2011 03:31:16 am Jiri Slaby wrote: >> On 01/14/2011 01:15 AM, Linus Torvalds wrote: >>> On Thu, Jan 13, 2011 at 3:19 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: >>>> >>>> I think we're back to the question of why we have the ICH4 quirk in >>>> the first place, and I don't know the answer to that. >>> >>> Iirc, there were several laptops that didn't have the ACPI region >>> mentioned in any of the regular places, and we'd allocate the PCMCIA >>> IO region on top of them. The machine would boot, but if anybody ever >>> inserted a PCCard into the machine, the first access to the IO region >>> would generally just halt it (because it was trying to read the >>> PCCard, but the APCI region decodes first, and then the read from that >>> usually put the CPU in a sleep state that it would never wake up from >>> for obvious reasons). >>> >>> So we do want the ICH4 quirk. >> >> Yes, this is an "official" way how ICH4 (and later) advertises the region. > > The quirk is a bug workaround, *not* the "official, planned" way to > deal with these regions. The official way is to use ACPI, because > that's a generic way that doesn't require changes for new versions > of ICH. Ok, I understand that. For non-ACPI setups this is probably the only place to look at. Anyway, has anybody had a chance to look at the patches? Any comments, nacks/acks? https://lkml.org/lkml/2011/1/14/115 https://lkml.org/lkml/2011/1/14/113 https://lkml.org/lkml/2011/1/14/114 thanks,
On Tue, 08 Feb 2011 10:55:01 +0100 Jiri Slaby <jirislaby@gmail.com> wrote: > On 01/14/2011 05:10 PM, Bjorn Helgaas wrote: > > On Friday, January 14, 2011 03:31:16 am Jiri Slaby wrote: > >> On 01/14/2011 01:15 AM, Linus Torvalds wrote: > >>> On Thu, Jan 13, 2011 at 3:19 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > >>>> > >>>> I think we're back to the question of why we have the ICH4 quirk in > >>>> the first place, and I don't know the answer to that. > >>> > >>> Iirc, there were several laptops that didn't have the ACPI region > >>> mentioned in any of the regular places, and we'd allocate the PCMCIA > >>> IO region on top of them. The machine would boot, but if anybody ever > >>> inserted a PCCard into the machine, the first access to the IO region > >>> would generally just halt it (because it was trying to read the > >>> PCCard, but the APCI region decodes first, and then the read from that > >>> usually put the CPU in a sleep state that it would never wake up from > >>> for obvious reasons). > >>> > >>> So we do want the ICH4 quirk. > >> > >> Yes, this is an "official" way how ICH4 (and later) advertises the region. > > > > The quirk is a bug workaround, *not* the "official, planned" way to > > deal with these regions. The official way is to use ACPI, because > > that's a generic way that doesn't require changes for new versions > > of ICH. > > Ok, I understand that. For non-ACPI setups this is probably the only > place to look at. > > Anyway, has anybody had a chance to look at the patches? Any comments, > nacks/acks? > > https://lkml.org/lkml/2011/1/14/115 > https://lkml.org/lkml/2011/1/14/113 > https://lkml.org/lkml/2011/1/14/114 I don't have a problem making the quirk quirkier, but it would be nice to get rid of the need for it entirely (though we can leave that to Bjorn :). Can you re-submit these three against my linux-next branch? Thanks,
On 02/08/2011 10:20 PM, Jesse Barnes wrote: > On Tue, 08 Feb 2011 10:55:01 +0100 > Jiri Slaby <jirislaby@gmail.com> wrote: > >> On 01/14/2011 05:10 PM, Bjorn Helgaas wrote: >>> On Friday, January 14, 2011 03:31:16 am Jiri Slaby wrote: >>>> On 01/14/2011 01:15 AM, Linus Torvalds wrote: >>>>> On Thu, Jan 13, 2011 at 3:19 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: >>>>>> >>>>>> I think we're back to the question of why we have the ICH4 quirk in >>>>>> the first place, and I don't know the answer to that. >>>>> >>>>> Iirc, there were several laptops that didn't have the ACPI region >>>>> mentioned in any of the regular places, and we'd allocate the PCMCIA >>>>> IO region on top of them. The machine would boot, but if anybody ever >>>>> inserted a PCCard into the machine, the first access to the IO region >>>>> would generally just halt it (because it was trying to read the >>>>> PCCard, but the APCI region decodes first, and then the read from that >>>>> usually put the CPU in a sleep state that it would never wake up from >>>>> for obvious reasons). >>>>> >>>>> So we do want the ICH4 quirk. >>>> >>>> Yes, this is an "official" way how ICH4 (and later) advertises the region. >>> >>> The quirk is a bug workaround, *not* the "official, planned" way to >>> deal with these regions. The official way is to use ACPI, because >>> that's a generic way that doesn't require changes for new versions >>> of ICH. >> >> Ok, I understand that. For non-ACPI setups this is probably the only >> place to look at. >> >> Anyway, has anybody had a chance to look at the patches? Any comments, >> nacks/acks? >> >> https://lkml.org/lkml/2011/1/14/115 >> https://lkml.org/lkml/2011/1/14/113 >> https://lkml.org/lkml/2011/1/14/114 > > I don't have a problem making the quirk quirkier, but it would be nice > to get rid of the need for it entirely (though we can leave that to > Bjorn :). Can you re-submit these three against my linux-next branch? Ok, I can. But do you want solution 113 or 114 -- they solve the same, but in a different manner? thanks,
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 53a786f..6d241eb 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -543,7 +543,14 @@ static void __devinit quirk_ich4_lpc_acpi(struct pci_dev *dev) u32 region; pci_read_config_dword(dev, 0x40, ®ion); - quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, "ICH4 ACPI/GPIO/TCO"); + /* + * some BIOSes are bogus and create this dynamic mapping so that it + * conflicts with fixed. There is no space below 0x180 for + * ACPI/GPIO/TCO which is 128B long and 128B aligned. + */ + if (region >= 0x180) + quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, + "ICH4 ACPI/GPIO/TCO"); pci_read_config_dword(dev, 0x58, ®ion); quirk_io_region(dev, region, 64, PCI_BRIDGE_RESOURCES+1, "ICH4 GPIO");
There are BIOSes out there where they provide ACPI mapping that conflicts with fixed one. Then e.g. an IDE controller doesn't work due to I/O space conflict: pci 0000:00:1f.1: address space collision: [io 0x0170-0x0177] conflicts with 0000:00:1f.0 [io 0x0100-0x017f] 0x0170-0x0177 for IDE controllers is created in pci_setup_device. 0x0100-0x017f for the ICH4 ISA bridge is created in quirk_ich4_lpc_acpi The former is fixed, according to specs at that address. The latter is read from the bridge confspace which contains: 00000000 86 80 a1 25 0f 00 80 02 02 00 01 06 00 00 80 00 |...%............| 00000010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| 00000020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| 00000030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| 00000040 01 01 00 00 10 00 00 00 00 00 00 00 00 00 01 00 |................| 00000050 00 00 00 00 00 00 00 00 01 05 00 00 10 00 00 00 |................| ... I.e. 0x00000101 as a base (ORed by 1 meaning I/O space) for ACPI per ICH4 specs. Don't accept bases below 0x180 for this dynamic mapping. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Cc: "David S. Miller" <davem@davemloft.net> Cc: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/pci/quirks.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)