Re: Regression: Screen turns off when booting in EFI mode
On Wed, Mar 27, 2013 at 2:02 PM, Dave Airlie wrote: >> >> That RedHat bugzilla is from kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. >> Does that kernel include Matthew's patch from Mar 19? I'm wondering >> if that report is for the same problem Mantas saw and possibly would >> be *fixed* by the Mar 19 patch, or if it is for a problem with the Mar >> 19 patch itself. >> >> I would check this myself, but I don't know how to figure out what's >> in kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. > > Its 3.9-rc4 + a git snapshot one day after, > > From what I can see 3.9-rc4 contains this patch from Mar 19. Yep, looks like it to me, too. Apparently the Mar 19 patch (547b52463) worked for Mantas but not for Chris. So I'll wait for testing reports from Mantas and Chris before pushing the Mar 26 patches. Linus can take them earlier if he wants, of course :) Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
> > That RedHat bugzilla is from kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. > Does that kernel include Matthew's patch from Mar 19? I'm wondering > if that report is for the same problem Mantas saw and possibly would > be *fixed* by the Mar 19 patch, or if it is for a problem with the Mar > 19 patch itself. > > I would check this myself, but I don't know how to figure out what's > in kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. Its 3.9-rc4 + a git snapshot one day after, >From what I can see 3.9-rc4 contains this patch from Mar 19. Dave. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
[+cc Chris, reporter of Fedora issue] On Mon, Mar 25, 2013 at 5:55 PM, Matthew Garrett wrote: > On Tue, Mar 26, 2013 at 09:52:14AM +1000, Dave Airlie wrote: >> >>> Because it's the only way to get the PCI ROM in some cases, like on >> >>> pretty much all Apples with Radeons. Only using it if we have no other >> >>> options probably makes sense, though. Something like this (entirely >> >>> untested)? >> >> >> >> This looks reasonable. Mantas? >> > >> > It compiles, boots, and even makes the graphics card work again. >> > So it looks good to me. >> >> https://bugzilla.redhat.com/show_bug.cgi?id=927451 >> >> popped up sounds like a regression caused by this. That RedHat bugzilla is from kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. Does that kernel include Matthew's patch from Mar 19? I'm wondering if that report is for the same problem Mantas saw and possibly would be *fixed* by the Mar 19 patch, or if it is for a problem with the Mar 19 patch itself. I would check this myself, but I don't know how to figure out what's in kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. Mantas confirmed that the Mar 19 problem fixed *his* issue, but I haven't seen any reports from either Mantas or Chris (the RedHat bugzilla reporter) for the new Mar 26 patches. > Sigh. I guess we need to figure out where it thinks it's getting that > image from. The alternative is basically to go back to what Linus > suggested, remove this from pci_map_rom() and add an explicit lookup to > the video drivers. > > -- > Matthew Garrett | mj...@srcf.ucam.org -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
[+cc Chris, reporter of Fedora issue] On Mon, Mar 25, 2013 at 5:55 PM, Matthew Garrett mj...@srcf.ucam.org wrote: On Tue, Mar 26, 2013 at 09:52:14AM +1000, Dave Airlie wrote: Because it's the only way to get the PCI ROM in some cases, like on pretty much all Apples with Radeons. Only using it if we have no other options probably makes sense, though. Something like this (entirely untested)? This looks reasonable. Mantas? It compiles, boots, and even makes the graphics card work again. So it looks good to me. https://bugzilla.redhat.com/show_bug.cgi?id=927451 popped up sounds like a regression caused by this. That RedHat bugzilla is from kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. Does that kernel include Matthew's patch from Mar 19? I'm wondering if that report is for the same problem Mantas saw and possibly would be *fixed* by the Mar 19 patch, or if it is for a problem with the Mar 19 patch itself. I would check this myself, but I don't know how to figure out what's in kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. Mantas confirmed that the Mar 19 problem fixed *his* issue, but I haven't seen any reports from either Mantas or Chris (the RedHat bugzilla reporter) for the new Mar 26 patches. Sigh. I guess we need to figure out where it thinks it's getting that image from. The alternative is basically to go back to what Linus suggested, remove this from pci_map_rom() and add an explicit lookup to the video drivers. -- Matthew Garrett | mj...@srcf.ucam.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
That RedHat bugzilla is from kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. Does that kernel include Matthew's patch from Mar 19? I'm wondering if that report is for the same problem Mantas saw and possibly would be *fixed* by the Mar 19 patch, or if it is for a problem with the Mar 19 patch itself. I would check this myself, but I don't know how to figure out what's in kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. Its 3.9-rc4 + a git snapshot one day after, From what I can see 3.9-rc4 contains this patch from Mar 19. Dave. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On Wed, Mar 27, 2013 at 2:02 PM, Dave Airlie airl...@gmail.com wrote: That RedHat bugzilla is from kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. Does that kernel include Matthew's patch from Mar 19? I'm wondering if that report is for the same problem Mantas saw and possibly would be *fixed* by the Mar 19 patch, or if it is for a problem with the Mar 19 patch itself. I would check this myself, but I don't know how to figure out what's in kernel-3.9.0-0.rc4.git0.1.fc19.x86_64. Its 3.9-rc4 + a git snapshot one day after, From what I can see 3.9-rc4 contains this patch from Mar 19. Yep, looks like it to me, too. Apparently the Mar 19 patch (547b52463) worked for Mantas but not for Chris. So I'll wait for testing reports from Mantas and Chris before pushing the Mar 26 patches. Linus can take them earlier if he wants, of course :) Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On Tue, Mar 26, 2013 at 09:52:14AM +1000, Dave Airlie wrote: > >>> Because it's the only way to get the PCI ROM in some cases, like on > >>> pretty much all Apples with Radeons. Only using it if we have no other > >>> options probably makes sense, though. Something like this (entirely > >>> untested)? > >> > >> This looks reasonable. Mantas? > > > > It compiles, boots, and even makes the graphics card work again. > > So it looks good to me. > > https://bugzilla.redhat.com/show_bug.cgi?id=927451 > > popped up sounds like a regression caused by this. Sigh. I guess we need to figure out where it thinks it's getting that image from. The alternative is basically to go back to what Linus suggested, remove this from pci_map_rom() and add an explicit lookup to the video drivers. -- Matthew Garrett | mj...@srcf.ucam.org -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
>>> Because it's the only way to get the PCI ROM in some cases, like on >>> pretty much all Apples with Radeons. Only using it if we have no other >>> options probably makes sense, though. Something like this (entirely >>> untested)? >> >> This looks reasonable. Mantas? > > It compiles, boots, and even makes the graphics card work again. > So it looks good to me. https://bugzilla.redhat.com/show_bug.cgi?id=927451 popped up sounds like a regression caused by this. Dave. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
Because it's the only way to get the PCI ROM in some cases, like on pretty much all Apples with Radeons. Only using it if we have no other options probably makes sense, though. Something like this (entirely untested)? This looks reasonable. Mantas? It compiles, boots, and even makes the graphics card work again. So it looks good to me. https://bugzilla.redhat.com/show_bug.cgi?id=927451 popped up sounds like a regression caused by this. Dave. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On Tue, Mar 26, 2013 at 09:52:14AM +1000, Dave Airlie wrote: Because it's the only way to get the PCI ROM in some cases, like on pretty much all Apples with Radeons. Only using it if we have no other options probably makes sense, though. Something like this (entirely untested)? This looks reasonable. Mantas? It compiles, boots, and even makes the graphics card work again. So it looks good to me. https://bugzilla.redhat.com/show_bug.cgi?id=927451 popped up sounds like a regression caused by this. Sigh. I guess we need to figure out where it thinks it's getting that image from. The alternative is basically to go back to what Linus suggested, remove this from pci_map_rom() and add an explicit lookup to the video drivers. -- Matthew Garrett | mj...@srcf.ucam.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On 2013-03-19 22:20, Linus Torvalds wrote: > On Tue, Mar 19, 2013 at 12:59 PM, Matthew Garrett wrote: >> >> Because it's the only way to get the PCI ROM in some cases, like on >> pretty much all Apples with Radeons. Only using it if we have no other >> options probably makes sense, though. Something like this (entirely >> untested)? > > This looks reasonable. Mantas? It compiles, boots, and even makes the graphics card work again. So it looks good to me. -- Mantas Mikulėnas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On Tue, Mar 19, 2013 at 12:59 PM, Matthew Garrett wrote: > > Because it's the only way to get the PCI ROM in some cases, like on > pretty much all Apples with Radeons. Only using it if we have no other > options probably makes sense, though. Something like this (entirely > untested)? This looks reasonable. Mantas? Trusting the firmware-provided image when we can't find the actual HW image is quite reasonable. It's the "trust firmware unconditionally" part that gets my goat. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On Tue, Mar 19, 2013 at 10:09:22AM -0700, Linus Torvalds wrote:
> Trusting firmware-provided values over the things we can find
> ourselves is known to be fundamentally crap, so what the hell is the
> point of that commit in the first place? The likelihood that firmware
> messes up is pretty damn high. Why would we take idiotic "here's the
> PCI ROM" data from firmware in the first place? What did this fix? We
> know what it broke..
Because it's the only way to get the PCI ROM in some cases, like on
pretty much all Apples with Radeons. Only using it if we have no other
options probably makes sense, though. Something like this (entirely
untested)?
diff --git a/drivers/pci/rom.c b/drivers/pci/rom.c
index ab886b7..a746dd9 100644
--- a/drivers/pci/rom.c
+++ b/drivers/pci/rom.c
@@ -100,6 +100,27 @@ size_t pci_get_rom_size(struct pci_dev *pdev, void __iomem
*rom, size_t size)
return min((size_t)(image - rom), size);
}
+static loff_t pci_find_rom(struct pci_dev *pdev, size_t *size)
+{
+ struct resource *res = >resource[PCI_ROM_RESOURCE];
+ loff_t start;
+
+ /* assign the ROM an address if it doesn't have one */
+ if (res->parent == NULL && pci_assign_resource(pdev,PCI_ROM_RESOURCE))
+ return 0;
+ start = pci_resource_start(pdev, PCI_ROM_RESOURCE);
+ *size = pci_resource_len(pdev, PCI_ROM_RESOURCE);
+
+ if (*size == 0)
+ return 0;
+
+ /* Enable ROM space decodes */
+ if (pci_enable_rom(pdev))
+ return 0;
+
+ return start;
+}
+
/**
* pci_map_rom - map a PCI ROM to kernel space
* @pdev: pointer to pci device struct
@@ -114,21 +135,15 @@ size_t pci_get_rom_size(struct pci_dev *pdev, void
__iomem *rom, size_t size)
void __iomem *pci_map_rom(struct pci_dev *pdev, size_t *size)
{
struct resource *res = >resource[PCI_ROM_RESOURCE];
- loff_t start;
+ loff_t start = 0;
void __iomem *rom;
/*
-* Some devices may provide ROMs via a source other than the BAR
-*/
- if (pdev->rom && pdev->romlen) {
- *size = pdev->romlen;
- return phys_to_virt(pdev->rom);
- /*
* IORESOURCE_ROM_SHADOW set on x86, x86_64 and IA64 supports legacy
* memory map if the VGA enable bit of the Bridge Control register is
* set for embedded VGA.
*/
- } else if (res->flags & IORESOURCE_ROM_SHADOW) {
+ if (res->flags & IORESOURCE_ROM_SHADOW) {
/* primary video rom always starts here */
start = (loff_t)0xC;
*size = 0x2; /* cover C000:0 through E000:0 */
@@ -139,21 +154,21 @@ void __iomem *pci_map_rom(struct pci_dev *pdev, size_t
*size)
return (void __iomem *)(unsigned long)
pci_resource_start(pdev, PCI_ROM_RESOURCE);
} else {
- /* assign the ROM an address if it doesn't have one */
- if (res->parent == NULL &&
- pci_assign_resource(pdev,PCI_ROM_RESOURCE))
- return NULL;
- start = pci_resource_start(pdev, PCI_ROM_RESOURCE);
- *size = pci_resource_len(pdev, PCI_ROM_RESOURCE);
- if (*size == 0)
- return NULL;
-
- /* Enable ROM space decodes */
- if (pci_enable_rom(pdev))
- return NULL;
+ start = pci_find_rom(pdev, size);
}
}
+ /*
+* Some devices may provide ROMs via a source other than the BAR
+*/
+ if (!start && pdev->rom && pdev->romlen) {
+ *size = pdev->romlen;
+ return phys_to_virt(pdev->rom);
+ }
+
+ if (!start)
+ return NULL;
+
rom = ioremap(start, *size);
if (!rom) {
/* restore enable if ioremap fails */
--
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On 2013-03-19 19:09, Linus Torvalds wrote: > This is apparently still outstanding, and Mantas hadn't cc'd the > people involved with the commit itself. I completely forgot about that – sorry. > Mantas, mind changing that "pcibios_add_device()" function so that > instead of setting dev->rom/romlen, it just prints out the values > (including the device address)? Plase also make it print out the > "data->len" field in addition to the rom->xyz fields.. Which variable is the device address stored in? I'm mostly clueless about the kernel beyond adding printk's, unfortunately. But I tried printing out the other values, and this is what I have so far: For 01:00.0 VGA compatible controller [0300]: Advanced Micro Devices [AMD] nee ATI Robson CE [AMD Radeon HD 6300 Series] [1002:68e4] pci :01:00.0: [1002:68e4] type 00 class 0x03 pci :01:00.0: reg 10: [mem 0xc000-0xcfff 64bit pref] pci :01:00.0: reg 18: [mem 0xd002-0xd003 64bit] pci :01:00.0: reg 20: [io 0xd000-0xd0ff] pci :01:00.0: reg 30: [mem 0xd000-0xd001 pref] pci :01:00.0: supports D1 D2 (dbg) - (dbg) old dev->rom: 0x, dev->romlen: 0 (dbg) new dev->rom: 0x029ad058, dev->romlen: 61952 (dbg) pa_data: 0x029ad018 + offset: 64 (dbg) data->type: 3, data->len: 62000 (dbg) rom->segment: 0, rom->bus: 1 (dbg) rom->device: 0, rom->function: 0 (dbg) rom->vendor: 1002, rom->devid: 68e4 <-- radeon (dbg) - And for 05:00.5 Ethernet controller [0200]: JMicron Technology Corp. JMC250 PCI Express Gigabit Ethernet Controller [197b:0250] (rev 03) – haven't checked if it's affected too, but including anyway. pci :05:00.5: [197b:0250] type 00 class 0x02 pci :05:00.5: reg 10: [mem 0xd020-0xd0203fff] pci :05:00.5: reg 18: [io 0x9100-0x917f] pci :05:00.5: reg 1c: [io 0x9000-0x90ff] pci :05:00.5: PME# supported from D0 D3hot D3cold (dbg) - (dbg) old dev->rom: 0x, dev->romlen: 0 (dbg) new dev->rom: 0x029bd058, dev->romlen: 40960 (dbg) pa_data: 0x029bd018 + offset: 64 (dbg) data->type: 3, data->len: 41008 (dbg) rom->segment: 0, rom->bus: 5 (dbg) rom->device: 0, rom->function: 5 (dbg) rom->vendor: 197b, rom->devid: 250 (dbg) - pci :05:00.5: System wakeup disabled by ACPI -- Mantas Mikulėnas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On Tue, Mar 19, 2013 at 10:09 AM, Linus Torvalds wrote: > > Doing things like blindly trusting the firmware data without even > validating it is a really really bad idea. The commit actually looks > seriously broken in other ways too, like blindly doing phys_to_virt() > on that, and then trusting the result Ok, looks like the only thing filling it in is eboot.c, and I guess it relies on the EFI memory allocations having been mapped. Which they hopefully have been. Still, even that seems somewhat debatable. eboot.c does a plain memcpy() on the pci->romimage returned by EfiPciIoAttributeOperationGet. And I can *guarantee* that that doesn't work on some PCI chips that end up sharing the decoder for the ROM and the graphics aperture or other device oddities. Afaik, some Radeons do that, for example. So whoever wrote that eboot thing seems to assume that the world is a lot simpler and saner than it actually is, and that everybody magically got things right. Which they never do. The code was presumably tested on just a couple of machines. The problem (well, at least *one* problem) is that pci_map_rom() actually knows about some of these issues, but if dev->rom and dev->romlen have been set, it trusts them unconditionally. So we'd either need to fix that, or we need to be really *really* sure that we only set dev->rom to guaranteed-correct buffers. At least the radeon code seems to verify that the ROM image starts with 0x55/0xaa, but I'm guessing we can't do that in general, even if it is the traditional PC rom pattern. We only have a few users of "pci_map_rom()", I'm wondering if we can move the "dev->rom/romsize" cases into the callers. Then the callers could decide if they want to trust that "pseudo-shadowed" ROM image (which would test that 55/aa pattern for example), or whether they want to try to map the actual physical ROM. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
This is apparently still outstanding, and Mantas hadn't cc'd the
people involved with the commit itself.
Background: with UEFI, commit f9a37be0f02a ("x86: Use PCI setup data")
apparently results in a black screen for Mantas. The commit reverts
fairly easily (there's been a trivial change to the function since due
to dev->rom now being a proper phys_add_t), and considering that the
commit doesn't explain what the f*ck it is needed for, or what it
would help, I'm inclined to do just that.
Trusting firmware-provided values over the things we can find
ourselves is known to be fundamentally crap, so what the hell is the
point of that commit in the first place? The likelihood that firmware
messes up is pretty damn high. Why would we take idiotic "here's the
PCI ROM" data from firmware in the first place? What did this fix? We
know what it broke..
Doing things like blindly trusting the firmware data without even
validating it is a really really bad idea. The commit actually looks
seriously broken in other ways too, like blindly doing phys_to_virt()
on that, and then trusting the result
Mantas, mind changing that "pcibios_add_device()" function so that
instead of setting dev->rom/romlen, it just prints out the values
(including the device address)? Plase also make it print out the
"data->len" field in addition to the rom->xyz fields..
Linus
On Sat, Mar 9, 2013 at 1:42 PM, Mantas Mikulėnas wrote:
> On 2013-02-22 03:03, Mantas Mikulėnas wrote:
>> On 2013-02-22 01:54, Dave Airlie wrote:
| radeon :01:00.0: No connectors reported connected with modes
| [drm] Cannot find any crtc or sizes - going 1024x768
The connector is definitely connected, since this is a laptop with a
built-in screen...
>>>
>>> Can you get the log with drm.debug=6 from both boots as well?
>>
>> Attached.
>
> The log is also at http://nullroute.eu.org/tmp/2013/dmesg-drm-debug.txt
>
> Not to be annoying, but I hope this can be fixed until 3.9...
>
> (I just tested v3.9-rc1-278-g8343bce, and it still does not detect any
> displays. And if I understood it correctly, "nomodeset" is going to go
> away?)
>
> --
> Mantas Mikulėnas
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
This is apparently still outstanding, and Mantas hadn't cc'd the people involved with the commit itself. Background: with UEFI, commit f9a37be0f02a (x86: Use PCI setup data) apparently results in a black screen for Mantas. The commit reverts fairly easily (there's been a trivial change to the function since due to dev-rom now being a proper phys_add_t), and considering that the commit doesn't explain what the f*ck it is needed for, or what it would help, I'm inclined to do just that. Trusting firmware-provided values over the things we can find ourselves is known to be fundamentally crap, so what the hell is the point of that commit in the first place? The likelihood that firmware messes up is pretty damn high. Why would we take idiotic here's the PCI ROM data from firmware in the first place? What did this fix? We know what it broke.. Doing things like blindly trusting the firmware data without even validating it is a really really bad idea. The commit actually looks seriously broken in other ways too, like blindly doing phys_to_virt() on that, and then trusting the result Mantas, mind changing that pcibios_add_device() function so that instead of setting dev-rom/romlen, it just prints out the values (including the device address)? Plase also make it print out the data-len field in addition to the rom-xyz fields.. Linus On Sat, Mar 9, 2013 at 1:42 PM, Mantas Mikulėnas graw...@gmail.com wrote: On 2013-02-22 03:03, Mantas Mikulėnas wrote: On 2013-02-22 01:54, Dave Airlie wrote: | radeon :01:00.0: No connectors reported connected with modes | [drm] Cannot find any crtc or sizes - going 1024x768 The connector is definitely connected, since this is a laptop with a built-in screen... Can you get the log with drm.debug=6 from both boots as well? Attached. The log is also at http://nullroute.eu.org/tmp/2013/dmesg-drm-debug.txt Not to be annoying, but I hope this can be fixed until 3.9... (I just tested v3.9-rc1-278-g8343bce, and it still does not detect any displays. And if I understood it correctly, nomodeset is going to go away?) -- Mantas Mikulėnas graw...@gmail.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On Tue, Mar 19, 2013 at 10:09 AM, Linus Torvalds torva...@linux-foundation.org wrote: Doing things like blindly trusting the firmware data without even validating it is a really really bad idea. The commit actually looks seriously broken in other ways too, like blindly doing phys_to_virt() on that, and then trusting the result Ok, looks like the only thing filling it in is eboot.c, and I guess it relies on the EFI memory allocations having been mapped. Which they hopefully have been. Still, even that seems somewhat debatable. eboot.c does a plain memcpy() on the pci-romimage returned by EfiPciIoAttributeOperationGet. And I can *guarantee* that that doesn't work on some PCI chips that end up sharing the decoder for the ROM and the graphics aperture or other device oddities. Afaik, some Radeons do that, for example. So whoever wrote that eboot thing seems to assume that the world is a lot simpler and saner than it actually is, and that everybody magically got things right. Which they never do. The code was presumably tested on just a couple of machines. The problem (well, at least *one* problem) is that pci_map_rom() actually knows about some of these issues, but if dev-rom and dev-romlen have been set, it trusts them unconditionally. So we'd either need to fix that, or we need to be really *really* sure that we only set dev-rom to guaranteed-correct buffers. At least the radeon code seems to verify that the ROM image starts with 0x55/0xaa, but I'm guessing we can't do that in general, even if it is the traditional PC rom pattern. We only have a few users of pci_map_rom(), I'm wondering if we can move the dev-rom/romsize cases into the callers. Then the callers could decide if they want to trust that pseudo-shadowed ROM image (which would test that 55/aa pattern for example), or whether they want to try to map the actual physical ROM. Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On 2013-03-19 19:09, Linus Torvalds wrote: This is apparently still outstanding, and Mantas hadn't cc'd the people involved with the commit itself. I completely forgot about that – sorry. Mantas, mind changing that pcibios_add_device() function so that instead of setting dev-rom/romlen, it just prints out the values (including the device address)? Plase also make it print out the data-len field in addition to the rom-xyz fields.. Which variable is the device address stored in? I'm mostly clueless about the kernel beyond adding printk's, unfortunately. But I tried printing out the other values, and this is what I have so far: For 01:00.0 VGA compatible controller [0300]: Advanced Micro Devices [AMD] nee ATI Robson CE [AMD Radeon HD 6300 Series] [1002:68e4] pci :01:00.0: [1002:68e4] type 00 class 0x03 pci :01:00.0: reg 10: [mem 0xc000-0xcfff 64bit pref] pci :01:00.0: reg 18: [mem 0xd002-0xd003 64bit] pci :01:00.0: reg 20: [io 0xd000-0xd0ff] pci :01:00.0: reg 30: [mem 0xd000-0xd001 pref] pci :01:00.0: supports D1 D2 (dbg) - (dbg) old dev-rom: 0x, dev-romlen: 0 (dbg) new dev-rom: 0x029ad058, dev-romlen: 61952 (dbg) pa_data: 0x029ad018 + offset: 64 (dbg) data-type: 3, data-len: 62000 (dbg) rom-segment: 0, rom-bus: 1 (dbg) rom-device: 0, rom-function: 0 (dbg) rom-vendor: 1002, rom-devid: 68e4 -- radeon (dbg) - And for 05:00.5 Ethernet controller [0200]: JMicron Technology Corp. JMC250 PCI Express Gigabit Ethernet Controller [197b:0250] (rev 03) – haven't checked if it's affected too, but including anyway. pci :05:00.5: [197b:0250] type 00 class 0x02 pci :05:00.5: reg 10: [mem 0xd020-0xd0203fff] pci :05:00.5: reg 18: [io 0x9100-0x917f] pci :05:00.5: reg 1c: [io 0x9000-0x90ff] pci :05:00.5: PME# supported from D0 D3hot D3cold (dbg) - (dbg) old dev-rom: 0x, dev-romlen: 0 (dbg) new dev-rom: 0x029bd058, dev-romlen: 40960 (dbg) pa_data: 0x029bd018 + offset: 64 (dbg) data-type: 3, data-len: 41008 (dbg) rom-segment: 0, rom-bus: 5 (dbg) rom-device: 0, rom-function: 5 (dbg) rom-vendor: 197b, rom-devid: 250 (dbg) - pci :05:00.5: System wakeup disabled by ACPI -- Mantas Mikulėnas graw...@gmail.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On Tue, Mar 19, 2013 at 10:09:22AM -0700, Linus Torvalds wrote:
Trusting firmware-provided values over the things we can find
ourselves is known to be fundamentally crap, so what the hell is the
point of that commit in the first place? The likelihood that firmware
messes up is pretty damn high. Why would we take idiotic here's the
PCI ROM data from firmware in the first place? What did this fix? We
know what it broke..
Because it's the only way to get the PCI ROM in some cases, like on
pretty much all Apples with Radeons. Only using it if we have no other
options probably makes sense, though. Something like this (entirely
untested)?
diff --git a/drivers/pci/rom.c b/drivers/pci/rom.c
index ab886b7..a746dd9 100644
--- a/drivers/pci/rom.c
+++ b/drivers/pci/rom.c
@@ -100,6 +100,27 @@ size_t pci_get_rom_size(struct pci_dev *pdev, void __iomem
*rom, size_t size)
return min((size_t)(image - rom), size);
}
+static loff_t pci_find_rom(struct pci_dev *pdev, size_t *size)
+{
+ struct resource *res = pdev-resource[PCI_ROM_RESOURCE];
+ loff_t start;
+
+ /* assign the ROM an address if it doesn't have one */
+ if (res-parent == NULL pci_assign_resource(pdev,PCI_ROM_RESOURCE))
+ return 0;
+ start = pci_resource_start(pdev, PCI_ROM_RESOURCE);
+ *size = pci_resource_len(pdev, PCI_ROM_RESOURCE);
+
+ if (*size == 0)
+ return 0;
+
+ /* Enable ROM space decodes */
+ if (pci_enable_rom(pdev))
+ return 0;
+
+ return start;
+}
+
/**
* pci_map_rom - map a PCI ROM to kernel space
* @pdev: pointer to pci device struct
@@ -114,21 +135,15 @@ size_t pci_get_rom_size(struct pci_dev *pdev, void
__iomem *rom, size_t size)
void __iomem *pci_map_rom(struct pci_dev *pdev, size_t *size)
{
struct resource *res = pdev-resource[PCI_ROM_RESOURCE];
- loff_t start;
+ loff_t start = 0;
void __iomem *rom;
/*
-* Some devices may provide ROMs via a source other than the BAR
-*/
- if (pdev-rom pdev-romlen) {
- *size = pdev-romlen;
- return phys_to_virt(pdev-rom);
- /*
* IORESOURCE_ROM_SHADOW set on x86, x86_64 and IA64 supports legacy
* memory map if the VGA enable bit of the Bridge Control register is
* set for embedded VGA.
*/
- } else if (res-flags IORESOURCE_ROM_SHADOW) {
+ if (res-flags IORESOURCE_ROM_SHADOW) {
/* primary video rom always starts here */
start = (loff_t)0xC;
*size = 0x2; /* cover C000:0 through E000:0 */
@@ -139,21 +154,21 @@ void __iomem *pci_map_rom(struct pci_dev *pdev, size_t
*size)
return (void __iomem *)(unsigned long)
pci_resource_start(pdev, PCI_ROM_RESOURCE);
} else {
- /* assign the ROM an address if it doesn't have one */
- if (res-parent == NULL
- pci_assign_resource(pdev,PCI_ROM_RESOURCE))
- return NULL;
- start = pci_resource_start(pdev, PCI_ROM_RESOURCE);
- *size = pci_resource_len(pdev, PCI_ROM_RESOURCE);
- if (*size == 0)
- return NULL;
-
- /* Enable ROM space decodes */
- if (pci_enable_rom(pdev))
- return NULL;
+ start = pci_find_rom(pdev, size);
}
}
+ /*
+* Some devices may provide ROMs via a source other than the BAR
+*/
+ if (!start pdev-rom pdev-romlen) {
+ *size = pdev-romlen;
+ return phys_to_virt(pdev-rom);
+ }
+
+ if (!start)
+ return NULL;
+
rom = ioremap(start, *size);
if (!rom) {
/* restore enable if ioremap fails */
--
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On Tue, Mar 19, 2013 at 12:59 PM, Matthew Garrett mj...@srcf.ucam.org wrote: Because it's the only way to get the PCI ROM in some cases, like on pretty much all Apples with Radeons. Only using it if we have no other options probably makes sense, though. Something like this (entirely untested)? This looks reasonable. Mantas? Trusting the firmware-provided image when we can't find the actual HW image is quite reasonable. It's the trust firmware unconditionally part that gets my goat. Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On 2013-03-19 22:20, Linus Torvalds wrote: On Tue, Mar 19, 2013 at 12:59 PM, Matthew Garrett mj...@srcf.ucam.org wrote: Because it's the only way to get the PCI ROM in some cases, like on pretty much all Apples with Radeons. Only using it if we have no other options probably makes sense, though. Something like this (entirely untested)? This looks reasonable. Mantas? It compiles, boots, and even makes the graphics card work again. So it looks good to me. -- Mantas Mikulėnas graw...@gmail.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On 2013-02-22 03:03, Mantas Mikulėnas wrote: > On 2013-02-22 01:54, Dave Airlie wrote: >>> >>> | radeon :01:00.0: No connectors reported connected with modes >>> | [drm] Cannot find any crtc or sizes - going 1024x768 >>> >>> The connector is definitely connected, since this is a laptop with a >>> built-in screen... >>> >> >> Can you get the log with drm.debug=6 from both boots as well? > > Attached. The log is also at http://nullroute.eu.org/tmp/2013/dmesg-drm-debug.txt Not to be annoying, but I hope this can be fixed until 3.9... (I just tested v3.9-rc1-278-g8343bce, and it still does not detect any displays. And if I understood it correctly, "nomodeset" is going to go away?) -- Mantas Mikulėnas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
On 2013-02-22 03:03, Mantas Mikulėnas wrote: On 2013-02-22 01:54, Dave Airlie wrote: | radeon :01:00.0: No connectors reported connected with modes | [drm] Cannot find any crtc or sizes - going 1024x768 The connector is definitely connected, since this is a laptop with a built-in screen... Can you get the log with drm.debug=6 from both boots as well? Attached. The log is also at http://nullroute.eu.org/tmp/2013/dmesg-drm-debug.txt Not to be annoying, but I hope this can be fixed until 3.9... (I just tested v3.9-rc1-278-g8343bce, and it still does not detect any displays. And if I understood it correctly, nomodeset is going to go away?) -- Mantas Mikulėnas graw...@gmail.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
> > | radeon :01:00.0: No connectors reported connected with modes > | [drm] Cannot find any crtc or sizes - going 1024x768 > > The connector is definitely connected, since this is a laptop with a > built-in screen... > Can you get the log with drm.debug=6 from both boots as well? Dave. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Regression: Screen turns off when booting in EFI mode
| radeon :01:00.0: No connectors reported connected with modes | [drm] Cannot find any crtc or sizes - going 1024x768 The connector is definitely connected, since this is a laptop with a built-in screen... Can you get the log with drm.debug=6 from both boots as well? Dave. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
