Re: [PATCH] efi: Fix the size not consistent issue when unmapping memory map
On Mon, Apr 16, 2018 at 06:35:22PM -0600, Randy Wright wrote: > ... I will plan to run the same test tomorrow on > a build of the SuSE 4.4.120-94.17 kernel, on which I had also reported > the original bug. I carried out the test on the older kernel today. I found the version of arch/x86/kernel/kexec-bzimage64.c in the SuSE kernel version 4.4.120-94.17 was sufficiently different from the newer version that one chunk of the patch under discussion was rejected. Specifically, the 'struct kexec_buf kbuf' is not found in that older SuSE version of kexec-bzimage64.c. But I made the same modification to the calculation of variable params_misc_sz in the older source, ran the same test, and there were no longer any warnings from early_ioremap.c. -- Randy WrightHewlett Packard Enterprise Phone: (970) 898-0998 Mail: rwri...@hpe.com
Re: [PATCH] efi: Fix the size not consistent issue when unmapping memory map
On Mon, Apr 16, 2018 at 06:35:22PM -0600, Randy Wright wrote: > ... I will plan to run the same test tomorrow on > a build of the SuSE 4.4.120-94.17 kernel, on which I had also reported > the original bug. I carried out the test on the older kernel today. I found the version of arch/x86/kernel/kexec-bzimage64.c in the SuSE kernel version 4.4.120-94.17 was sufficiently different from the newer version that one chunk of the patch under discussion was rejected. Specifically, the 'struct kexec_buf kbuf' is not found in that older SuSE version of kexec-bzimage64.c. But I made the same modification to the calculation of variable params_misc_sz in the older source, ran the same test, and there were no longer any warnings from early_ioremap.c. -- Randy WrightHewlett Packard Enterprise Phone: (970) 898-0998 Mail: rwri...@hpe.com
Re: [PATCH] efi: Fix the size not consistent issue when unmapping memory map
On Mon, Apr 16, 2018 at 02:37:38PM +0800, joeyli wrote: > Hi Randy, > ... > Randy, do you want to try Dave's kexec patch on your environment? Please > remove > my patch first. > > Thanks a lot! > Joey Lee Hi Joey, I tried Dave's patch to kexec-bzimage64.c on my build of the SuSE 4.12.14-15 kernel. I ran the same test as I did with your patch: I verified the early_ioremap.c warnings occurred with a crash triggered from a kexec boot of the unmodified kernel. Then I applied the patch to kexec-bzimage64.c, rebuilt, re-ran the test to crash from the kexec'ed kernel, and verified the warnings are no longer seen. I'm out of time today, but I will plan to run the same test tomorrow on a build of the SuSE 4.4.120-94.17 kernel, on which I had also reported the original bug. -- Randy WrightHewlett Packard Enterprise Phone: (970) 898-0998 Mail: rwri...@hpe.com
Re: [PATCH] efi: Fix the size not consistent issue when unmapping memory map
On Mon, Apr 16, 2018 at 02:37:38PM +0800, joeyli wrote: > Hi Randy, > ... > Randy, do you want to try Dave's kexec patch on your environment? Please > remove > my patch first. > > Thanks a lot! > Joey Lee Hi Joey, I tried Dave's patch to kexec-bzimage64.c on my build of the SuSE 4.12.14-15 kernel. I ran the same test as I did with your patch: I verified the early_ioremap.c warnings occurred with a crash triggered from a kexec boot of the unmodified kernel. Then I applied the patch to kexec-bzimage64.c, rebuilt, re-ran the test to crash from the kexec'ed kernel, and verified the warnings are no longer seen. I'm out of time today, but I will plan to run the same test tomorrow on a build of the SuSE 4.4.120-94.17 kernel, on which I had also reported the original bug. -- Randy WrightHewlett Packard Enterprise Phone: (970) 898-0998 Mail: rwri...@hpe.com
[PATCH] Documentation/vm/pagemap.txt: correct location of page-types tool
The page-types tool was relocated to tools/vm in the 3.4 kernel timeframe. Signed-off-by: Randy Wright --- Documentation/vm/pagemap.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt index 6fbd55e..6bfbc17 100644 --- a/Documentation/vm/pagemap.txt +++ b/Documentation/vm/pagemap.txt @@ -131,7 +131,8 @@ Short descriptions to the page flags: 13. SWAPCACHE page is mapped to swap space, ie. has an associated swap entry 14. SWAPBACKED page is backed by swap/RAM -The page-types tool in this directory can be used to query the above flags. +The page-types tool in the tools/vm directory can be used to query the +above flags. Using pagemap to do something useful: -- 1.7.11.3 -- 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/
[PATCH] Documentation/vm/pagemap.txt: correct location of page-types tool
The page-types tool was relocated to tools/vm in the 3.4 kernel timeframe. Signed-off-by: Randy Wright rwri...@hp.com --- Documentation/vm/pagemap.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt index 6fbd55e..6bfbc17 100644 --- a/Documentation/vm/pagemap.txt +++ b/Documentation/vm/pagemap.txt @@ -131,7 +131,8 @@ Short descriptions to the page flags: 13. SWAPCACHE page is mapped to swap space, ie. has an associated swap entry 14. SWAPBACKED page is backed by swap/RAM -The page-types tool in this directory can be used to query the above flags. +The page-types tool in the tools/vm directory can be used to query the +above flags. Using pagemap to do something useful: -- 1.7.11.3 -- 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: [PATCH RFC] function probe_roms accessing improper addresses
Hi Matthew, You made some interesting suggestions: > 1) Declare that any x86 hardware supported by Linux *must* support > probing in this address region On the first: one way to be compliant with such a requirement would be to design systems that implement softfail in this particular region. What about a system that hardfails, but on which the resulting machine check can be handled by the kernel machine check handler? Would appropriate re-ordering of the kernel initialization code to support such systems be acceptable? Also, let me mention a possible amendment to your first idea: what if the mandate that probing be supported were qualified by some attribute that could be indicated in the UEFI environment? For example: instead of just a hole in the UEFI memory map, what if this range was specifically present and typed as EfiUnusableMemory? Another idea for UEFI systems - but one requiring a UEFI specification change - might be adding a UEFI variable that if present, indicates any area not explicitly included and typed in the UEFI memory map (including the legacy adapter region) must be explicitly avoided by an OS. > 2) Don't call probe_roms() by default, but leave it up to the graphics > drivers. If they can get the rom by any other means then don't call it. One the second idea: there are a quite a lot of video drivers in the kernel source tree. Do you have a suggestion for how to evaluate which ones rely on the setup performed by probe_roms? -- Randy WrightHewlett-Packard Company Phone: (970) 898-0998 Mail: rwri...@hp.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: [PATCH RFC] function probe_roms accessing improper addresses
Hi Matthew, You made some interesting suggestions: 1) Declare that any x86 hardware supported by Linux *must* support probing in this address region On the first: one way to be compliant with such a requirement would be to design systems that implement softfail in this particular region. What about a system that hardfails, but on which the resulting machine check can be handled by the kernel machine check handler? Would appropriate re-ordering of the kernel initialization code to support such systems be acceptable? Also, let me mention a possible amendment to your first idea: what if the mandate that probing be supported were qualified by some attribute that could be indicated in the UEFI environment? For example: instead of just a hole in the UEFI memory map, what if this range was specifically present and typed as EfiUnusableMemory? Another idea for UEFI systems - but one requiring a UEFI specification change - might be adding a UEFI variable that if present, indicates any area not explicitly included and typed in the UEFI memory map (including the legacy adapter region) must be explicitly avoided by an OS. 2) Don't call probe_roms() by default, but leave it up to the graphics drivers. If they can get the rom by any other means then don't call it. One the second idea: there are a quite a lot of video drivers in the kernel source tree. Do you have a suggestion for how to evaluate which ones rely on the setup performed by probe_roms? -- Randy WrightHewlett-Packard Company Phone: (970) 898-0998 Mail: rwri...@hp.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: [PATCH RFC] function probe_roms accessing improper addresses
I would like to summarize progress I have made on this issue in the past few days, and then appeal for help in evaluating the options. Matthew and H. Peter Anvin, I would appreciate your additional input especially. The general idea under consideration here is to avoid access to the legacy adapter space. My original rfc patch applied this restriction to all efi_enabled systems. I am modifying this to implement this as a quirk, and the question is how best to activate such a quirk. Option 1: Activating via Machine Check I investigated the idea of activating based on encountering a machine check. As an experiment, to avoid boot-time code sequencing issues, I moved the probe_roms call to after the MC handler initialization. However, I ran into prototype firmware issues. As I had suspected might be the case, the access was still treated as fatal by platform firmware and the kernel machine check handler does not get a chance to run. Since I could not resolve this with resources available to me, I had to set this approach aside. Option 2: Activating via DMI properties I have written and tested code implementing the following approach. With just a bit of clean-up I could send this as a new patch RFC if it sounds workable. First, I added a new member to the x86_platform_ops structure: int (*legacy_range_accessible)(phys_addr_t); The default function provided for this operation always returns an indication that access is permitted. If appropriate to the platform, the default function pointer can be replaced at runtime with a function that disallows access. This provides a test by which post-boot code - such as devmem_is_allowed - can query whether access to a particular address should be permitted. Because this check must occur in a relatively small window between dmi initialization and the call to probe_roms, I added what I think could be a generally useful early quirk check at the end of dmi_scan_machine: if (dmi_available) dmi_check_system(dmi_early_quirks); Here, dmi_early_quirks is a typical dmi_system_id array, initially containing an entry for my prototype system. When matched for the properties of interest, it calls out via the .callback entry to a new function I named x86_mark_legacy_adapter_range_inaccessible. This function does the following: a. it replaces the x86_init_resources.probe_roms function pointer with x86_init_noop; and b. it replaces the above-mentioned legacy_range_accessible callout in the x86_platform_ops structure with a function that disapproves of access to the legacy adapter region. So in this approach, I don't patch the function probe_roms itself; rather, I entirely avoid calling it. A specific point on which I'd like to get advice is regarding the policy for accepting a submission based on DMI properties of an unreleased prototype platform. This focuses on the weak point of any "opt-in" type of quirk: if the DMI properties of the product change over the product lifecyle (there's always a version n+1!) then the patch must be amended. But at that point, the modification required is simply altering text string content in a dmi_system_id array. Option 3: Boot command line activation | 3. Can it be activated on demand for testing on other platforms? A | kernel boot command line parameter could be added, for example. How does | the community feel about adding more of those? I am considering this option in addition to - not instead of - the automatic activation by the dmi properties, but it remains an area on which I want to get further input before posting a new patch rfc: is it appropriate to include a boot command interface to activate this feature on demand for platforms that don't match via the DMI_MATCH criteria? If new boot command parameters are discouraged, is there a better paradigm for forced activation that I should investigate? Any other feedback? -- Randy WrightHewlett-Packard Company Phone: (970) 898-0998 Mail: rwri...@hp.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: [PATCH RFC] function probe_roms accessing improper addresses
I would like to summarize progress I have made on this issue in the past few days, and then appeal for help in evaluating the options. Matthew and H. Peter Anvin, I would appreciate your additional input especially. The general idea under consideration here is to avoid access to the legacy adapter space. My original rfc patch applied this restriction to all efi_enabled systems. I am modifying this to implement this as a quirk, and the question is how best to activate such a quirk. Option 1: Activating via Machine Check I investigated the idea of activating based on encountering a machine check. As an experiment, to avoid boot-time code sequencing issues, I moved the probe_roms call to after the MC handler initialization. However, I ran into prototype firmware issues. As I had suspected might be the case, the access was still treated as fatal by platform firmware and the kernel machine check handler does not get a chance to run. Since I could not resolve this with resources available to me, I had to set this approach aside. Option 2: Activating via DMI properties I have written and tested code implementing the following approach. With just a bit of clean-up I could send this as a new patch RFC if it sounds workable. First, I added a new member to the x86_platform_ops structure: int (*legacy_range_accessible)(phys_addr_t); The default function provided for this operation always returns an indication that access is permitted. If appropriate to the platform, the default function pointer can be replaced at runtime with a function that disallows access. This provides a test by which post-boot code - such as devmem_is_allowed - can query whether access to a particular address should be permitted. Because this check must occur in a relatively small window between dmi initialization and the call to probe_roms, I added what I think could be a generally useful early quirk check at the end of dmi_scan_machine: if (dmi_available) dmi_check_system(dmi_early_quirks); Here, dmi_early_quirks is a typical dmi_system_id array, initially containing an entry for my prototype system. When matched for the properties of interest, it calls out via the .callback entry to a new function I named x86_mark_legacy_adapter_range_inaccessible. This function does the following: a. it replaces the x86_init_resources.probe_roms function pointer with x86_init_noop; and b. it replaces the above-mentioned legacy_range_accessible callout in the x86_platform_ops structure with a function that disapproves of access to the legacy adapter region. So in this approach, I don't patch the function probe_roms itself; rather, I entirely avoid calling it. A specific point on which I'd like to get advice is regarding the policy for accepting a submission based on DMI properties of an unreleased prototype platform. This focuses on the weak point of any opt-in type of quirk: if the DMI properties of the product change over the product lifecyle (there's always a version n+1!) then the patch must be amended. But at that point, the modification required is simply altering text string content in a dmi_system_id array. Option 3: Boot command line activation | 3. Can it be activated on demand for testing on other platforms? A | kernel boot command line parameter could be added, for example. How does | the community feel about adding more of those? I am considering this option in addition to - not instead of - the automatic activation by the dmi properties, but it remains an area on which I want to get further input before posting a new patch rfc: is it appropriate to include a boot command interface to activate this feature on demand for platforms that don't match via the DMI_MATCH criteria? If new boot command parameters are discouraged, is there a better paradigm for forced activation that I should investigate? Any other feedback? -- Randy WrightHewlett-Packard Company Phone: (970) 898-0998 Mail: rwri...@hp.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: [PATCH RFC] function probe_roms accessing improper addresses
> Date: Thu, 4 Oct 2012 20:22:56 +0100 > From: Matthew Garrett > To: rwri...@hp.com > Cc: linux-kernel@vger.kernel.org > Subject: Re: [PATCH RFC] function probe_roms accessing improper addresses > on UEFI systems > Message-ID: <20121004192256.ga6...@srcf.ucam.org> > References: <201210032353.q93nrkni018...@filesys1.fc.hp.com> > > On Wed, Oct 03, 2012 at 05:53:46PM -0600, Randy Wright wrote: > > > The following proposed patch takes advantage of the fact that on EFI > > systems, the memory map provides a better description of the physical > > space than on pre-EFI legacy systems. If the efi_enabled state variable > > indicates the kernel is running on an UEFI system, the patch will use > > information from the UEFI memory map so as not to access addresses that > > should avoided according to the UEFI specification. > > This turns out to be awkward. Some (mostly older) EFI platforms still > only provide the video ROM through the 0xc window, and that's > sometimes needed even if the platform isn't using int10 for anything > (for instance, some Intel graphics machines only provide the VBT through > the video ROM and don't provide that via the PCI BAR). And, of course, > they have an EFI memory map that just shows a hole there. > > So we can't distinguish between the two cases easily. The only thing I > can think of would be to push that policy out to the graphics drivers > and have them trigger a scan only if they can't get the required > information from any other source. I suspect that this patch as is would > break graphics on a reasonable number of EFI platforms. > -- > Matthew Garrett | mj...@srcf.ucam.org Hi Matthew, I appreciate your description of the problems with my approach, as well as the reply from h...@zytor.com (H. Peter Anvin) in response to my mention of this patch in another thread. His reply contained a couple of suggestions that initially appeal to me more than an approach requiring a change to a set of video drivers, the size of which I don't quite know how to scope. In that other thread, hpa said: | One option would be to quirk it; obviously there is some piece of | hardware which does cause this #MC and hopefully we could use that to | detect that specific regions should be excluded; another option would be | to trap the #MC during ROM probing. I definitely see the appeal of trapping the #MC and triggering a solution from that, if it can be made to work. I've spent some time evaluating that, and I see these issues: 1. I don't believe the kernel's MC handler is initialized early enough to handle a machine check occurring as early as probe_roms. Probe_roms is called very early in boot. I see this as the call stack: start_kernel->setup_arch->probe_roms Whereas the machine check initialization for x86 appears to come later: start_kernel->check_bugs->identify_boot_cpu->identify_cpu->mcheck_cpu_init At present, I do not want to tackle such a major reordering of intialization as would be required to change this. 2. For all platforms, is the setup of chipset and cpu address decoding robust enough to allow the OS to handle the resulting machine check and recover? I've worked with some platforms in the past where this was not always the case, the result being that for some unpopulated address ranges, the resulting machine check would not be recoverable. Because of the above difficulties with the MC handler idea, I have focused my thoughts more on the quirk idea that hpa mentioned. I've been investigating some existing examples in the kernel, and trying to understand some of the issues involved with designing a new one. 1. Can the interface be chosen to present the needed interface to all callers? I recognize this as a challenge if a single interface is to be used both in early boot (e.g. probe_roms) and later runtime (e.g. devmem_is_allowed). Something like a new member added to the x86_platform_ops structure? 2. How can it automatically be activated for platforms that need it? I see quite a few quirks selected by cpu id, but that's probably not appropriate here. Again, activating it by hitting the #MC in probe_roms would be cool, but I see it as involving a major reordering of initialization code. So I'm left thinking about something in keying off the dmi platform strings, which fortunately are initialized thusly: start_kernel->setup_arch->dmi_scan_machine convenient, as it's just before probe_roms is called. 3. Can it be activated on demand for testing on other platforms? A kernel boot command line parameter could be added, for example. How does the community feel about adding more of those? What are other design issues I'm overlooking? Are there are existing quirks that strike you as particularly good models for this case? -- Randy WrightHewlett-Packard Company Phone: (970) 898-0998
Re: [PATCH RFC] function probe_roms accessing improper addresses
Date: Thu, 4 Oct 2012 20:22:56 +0100 From: Matthew Garrett mj...@srcf.ucam.org To: rwri...@hp.com Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH RFC] function probe_roms accessing improper addresses on UEFI systems Message-ID: 20121004192256.ga6...@srcf.ucam.org References: 201210032353.q93nrkni018...@filesys1.fc.hp.com On Wed, Oct 03, 2012 at 05:53:46PM -0600, Randy Wright wrote: The following proposed patch takes advantage of the fact that on EFI systems, the memory map provides a better description of the physical space than on pre-EFI legacy systems. If the efi_enabled state variable indicates the kernel is running on an UEFI system, the patch will use information from the UEFI memory map so as not to access addresses that should avoided according to the UEFI specification. This turns out to be awkward. Some (mostly older) EFI platforms still only provide the video ROM through the 0xc window, and that's sometimes needed even if the platform isn't using int10 for anything (for instance, some Intel graphics machines only provide the VBT through the video ROM and don't provide that via the PCI BAR). And, of course, they have an EFI memory map that just shows a hole there. So we can't distinguish between the two cases easily. The only thing I can think of would be to push that policy out to the graphics drivers and have them trigger a scan only if they can't get the required information from any other source. I suspect that this patch as is would break graphics on a reasonable number of EFI platforms. -- Matthew Garrett | mj...@srcf.ucam.org Hi Matthew, I appreciate your description of the problems with my approach, as well as the reply from h...@zytor.com (H. Peter Anvin) in response to my mention of this patch in another thread. His reply contained a couple of suggestions that initially appeal to me more than an approach requiring a change to a set of video drivers, the size of which I don't quite know how to scope. In that other thread, hpa said: | One option would be to quirk it; obviously there is some piece of | hardware which does cause this #MC and hopefully we could use that to | detect that specific regions should be excluded; another option would be | to trap the #MC during ROM probing. I definitely see the appeal of trapping the #MC and triggering a solution from that, if it can be made to work. I've spent some time evaluating that, and I see these issues: 1. I don't believe the kernel's MC handler is initialized early enough to handle a machine check occurring as early as probe_roms. Probe_roms is called very early in boot. I see this as the call stack: start_kernel-setup_arch-probe_roms Whereas the machine check initialization for x86 appears to come later: start_kernel-check_bugs-identify_boot_cpu-identify_cpu-mcheck_cpu_init At present, I do not want to tackle such a major reordering of intialization as would be required to change this. 2. For all platforms, is the setup of chipset and cpu address decoding robust enough to allow the OS to handle the resulting machine check and recover? I've worked with some platforms in the past where this was not always the case, the result being that for some unpopulated address ranges, the resulting machine check would not be recoverable. Because of the above difficulties with the MC handler idea, I have focused my thoughts more on the quirk idea that hpa mentioned. I've been investigating some existing examples in the kernel, and trying to understand some of the issues involved with designing a new one. 1. Can the interface be chosen to present the needed interface to all callers? I recognize this as a challenge if a single interface is to be used both in early boot (e.g. probe_roms) and later runtime (e.g. devmem_is_allowed). Something like a new member added to the x86_platform_ops structure? 2. How can it automatically be activated for platforms that need it? I see quite a few quirks selected by cpu id, but that's probably not appropriate here. Again, activating it by hitting the #MC in probe_roms would be cool, but I see it as involving a major reordering of initialization code. So I'm left thinking about something in keying off the dmi platform strings, which fortunately are initialized thusly: start_kernel-setup_arch-dmi_scan_machine convenient, as it's just before probe_roms is called. 3. Can it be activated on demand for testing on other platforms? A kernel boot command line parameter could be added, for example. How does the community feel about adding more of those? What are other design issues I'm overlooking? Are there are existing quirks that strike you as particularly good models for this case? -- Randy WrightHewlett-Packard Company Phone: (970) 898-0998 Mail: rwri...@hp.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
Re: [PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine
On 10/02/2012 11:20 PM, Matthew Garrett wrote: > Oh, right, got you. In that case I think we potentially need a > finer-grained check on EFI platforms - the EFI memory map is kind enough > to tell us the difference between unusable regions and io regions, and > we could avoid access to the unusable ones. I wanted to mention in this context a patch RFC I posted yesterday as a distinct thread which is visible as https://lkml.org/lkml/2012/10/3/589 with the subject: [PATCH RFC] function probe_roms accessing improper addresses on UEFI systems. While the mechanism is different from Tmac's patch (because probe_roms runs early in boot, prior to resource structure initialization) the intent of the change is the same: to prevent a machine check resulting from access to inappropriate physical addresses. I will monitor both this thread as well as my own recent RFC posting since the underlying issues are common. -- Randy Wright+1-970-898-0998 -- 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: [PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine
On 10/02/2012 11:20 PM, Matthew Garrett wrote: Oh, right, got you. In that case I think we potentially need a finer-grained check on EFI platforms - the EFI memory map is kind enough to tell us the difference between unusable regions and io regions, and we could avoid access to the unusable ones. I wanted to mention in this context a patch RFC I posted yesterday as a distinct thread which is visible as https://lkml.org/lkml/2012/10/3/589 with the subject: [PATCH RFC] function probe_roms accessing improper addresses on UEFI systems. While the mechanism is different from Tmac's patch (because probe_roms runs early in boot, prior to resource structure initialization) the intent of the change is the same: to prevent a machine check resulting from access to inappropriate physical addresses. I will monitor both this thread as well as my own recent RFC posting since the underlying issues are common. -- Randy Wrightrwri...@hp.com+1-970-898-0998 -- 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/
[PATCH RFC] function probe_roms accessing improper addresses on UEFI systems
The code in function probe_roms accesses the legacy adapter space (in
the physical address range 640k-1mb) without regard to how this range is
described in the memory map. The assumption in the code is that the
result of an access to unpopulated space is a soft fail, returning -1.
This assumption is not universally true; some systems may cause a hard
error (MCE) when accessing an unpopulated physical address.
The following proposed patch takes advantage of the fact that on EFI
systems, the memory map provides a better description of the physical
space than on pre-EFI legacy systems. If the efi_enabled state variable
indicates the kernel is running on an UEFI system, the patch will use
information from the UEFI memory map so as not to access addresses that
should avoided according to the UEFI specification.
See also a discussion underway in the context of a patch to devmem_is_allowed
https://lkml.org/lkml/2012/10/2/458
which has the same overall goal of restricting access to
inappropriate ranges of the EFI memory map.
Following is my proposed patch based on v3.6. I would appreciate input
on the following points.
1. Is the implementation properly interpreting the UEFI specification by
disallowing probes of addresses for which the type is EFI_RESERVED_TYPE,
EFI_UNUSABLE_MEMORY, or otherwise unknown?
2. The patch includes some simple test cases in function test_probes
that can be customized to the testing needs of a particular target
system. This test code is conditionally enabled based on the
pre-processor symbol DO_TEST_PROBES; if that is not defined, the body of
function test_probes() is empty. This code can be enabled and modified
as needed to test specific addresses of interest. Is this code useful
to include in an upstream patch? If not, is there a more generally
useful testing strategy?
Please CC me directly with comments or questions.
Signed-off-by: Randy Wright
---
arch/x86/kernel/probe_roms.c | 71 ++
1 file changed, 71 insertions(+)
diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
index d5f15c3..76c20c9 100644
--- a/arch/x86/kernel/probe_roms.c
+++ b/arch/x86/kernel/probe_roms.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include /* efi_enabled, efi_mem_type */
#include
#include
@@ -177,11 +178,46 @@ EXPORT_SYMBOL(pci_biosrom_size);
#define ROMSIGNATURE 0xaa55
+/*
+ * efi_paddr_valid returns 1 if the input physical address
+ * is valid in the efi memory map, 0 otherwise.
+ */
+static int __init efi_paddr_valid(phys_addr_t paddr)
+{
+ u32 type = efi_mem_type((unsigned long)paddr);
+ printk(KERN_DEBUG "efi_paddr_valid(0x%lx): type 0x%lx\n",
+ (unsigned long)paddr, (unsigned long)type);
+ if (type == EFI_RESERVED_TYPE
+ || type == EFI_UNUSABLE_MEMORY
+ || type >= EFI_MAX_MEMORY_TYPE)
+ return 0;
+ return 1;
+}
+
+/*
+ * efi_vaddr_valid translates itsinput va to a pa, then
+ * returns the result of efi_paddr_valid applied to that pa.
+ */
+
+static int __init efi_vaddr_valid(const unsigned char *vaddr)
+{
+ phys_addr_t paddr = virt_to_phys((volatile void *)vaddr);
+ return efi_paddr_valid(paddr);
+}
+
static int __init romsignature(const unsigned char *rom)
{
const unsigned short * const ptr = (const unsigned short *)rom;
unsigned short sig;
+ if (efi_enabled) {
+ int rv = efi_vaddr_valid(rom);
+ printk(KERN_DEBUG
+ "romsignature: efi_vaddr_valid(0x%llx) returned %d\n",
+(unsigned long long)rom, rv);
+ if (0 == rv)
+ return 0;
+ }
return probe_kernel_address(ptr, sig) == 0 && sig == ROMSIGNATURE;
}
@@ -194,6 +230,39 @@ static int __init romchecksum(const unsigned char *rom,
unsigned long length)
return !length && !sum;
}
+/* #define DO_TEST_PROBES "include customizable test code" */
+static void __init test_probes(void)
+{
+#ifdef DO_TEST_PROBES
+ static struct {
+ phys_addr_t paddr;
+ int ptype;
+ } plist[] = {
+ {0x3e76ULL, 0}, /* RESERVED */
+ {0x009ffda9f000ULL, 1}, /* LOADER CODE */
+ {0x0005e000ULL, 2}, /* LOADER DATA */
+ {0x0006ULL, 3}, /* BOOT CODE (why after EBS?) */
+ {0x3640ULL, 4}, /* BOOT DATA (why after EBS?) */
+ {0x009fffc96000ULL, 5}, /* RUNTIME CODE */
+ {0x009fffcd3000ULL, 6}, /* RUNTIME DATA */
+ {0x1000ULL, 7}, /* CONVENTIONAL MEM */
+ {0x3e75f000ULL, 9}, /* ACPI RECLAIM */
+
[PATCH RFC] function probe_roms accessing improper addresses on UEFI systems
The code in function probe_roms accesses the legacy adapter space (in
the physical address range 640k-1mb) without regard to how this range is
described in the memory map. The assumption in the code is that the
result of an access to unpopulated space is a soft fail, returning -1.
This assumption is not universally true; some systems may cause a hard
error (MCE) when accessing an unpopulated physical address.
The following proposed patch takes advantage of the fact that on EFI
systems, the memory map provides a better description of the physical
space than on pre-EFI legacy systems. If the efi_enabled state variable
indicates the kernel is running on an UEFI system, the patch will use
information from the UEFI memory map so as not to access addresses that
should avoided according to the UEFI specification.
See also a discussion underway in the context of a patch to devmem_is_allowed
https://lkml.org/lkml/2012/10/2/458
which has the same overall goal of restricting access to
inappropriate ranges of the EFI memory map.
Following is my proposed patch based on v3.6. I would appreciate input
on the following points.
1. Is the implementation properly interpreting the UEFI specification by
disallowing probes of addresses for which the type is EFI_RESERVED_TYPE,
EFI_UNUSABLE_MEMORY, or otherwise unknown?
2. The patch includes some simple test cases in function test_probes
that can be customized to the testing needs of a particular target
system. This test code is conditionally enabled based on the
pre-processor symbol DO_TEST_PROBES; if that is not defined, the body of
function test_probes() is empty. This code can be enabled and modified
as needed to test specific addresses of interest. Is this code useful
to include in an upstream patch? If not, is there a more generally
useful testing strategy?
Please CC me directly with comments or questions.
Signed-off-by: Randy Wright rwri...@hp.com
---
arch/x86/kernel/probe_roms.c | 71 ++
1 file changed, 71 insertions(+)
diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
index d5f15c3..76c20c9 100644
--- a/arch/x86/kernel/probe_roms.c
+++ b/arch/x86/kernel/probe_roms.c
@@ -11,6 +11,7 @@
#include linux/pfn.h
#include linux/pci.h
#include linux/export.h
+#include linux/efi.h /* efi_enabled, efi_mem_type */
#include asm/probe_roms.h
#include asm/pci-direct.h
@@ -177,11 +178,46 @@ EXPORT_SYMBOL(pci_biosrom_size);
#define ROMSIGNATURE 0xaa55
+/*
+ * efi_paddr_valid returns 1 if the input physical address
+ * is valid in the efi memory map, 0 otherwise.
+ */
+static int __init efi_paddr_valid(phys_addr_t paddr)
+{
+ u32 type = efi_mem_type((unsigned long)paddr);
+ printk(KERN_DEBUG efi_paddr_valid(0x%lx): type 0x%lx\n,
+ (unsigned long)paddr, (unsigned long)type);
+ if (type == EFI_RESERVED_TYPE
+ || type == EFI_UNUSABLE_MEMORY
+ || type = EFI_MAX_MEMORY_TYPE)
+ return 0;
+ return 1;
+}
+
+/*
+ * efi_vaddr_valid translates itsinput va to a pa, then
+ * returns the result of efi_paddr_valid applied to that pa.
+ */
+
+static int __init efi_vaddr_valid(const unsigned char *vaddr)
+{
+ phys_addr_t paddr = virt_to_phys((volatile void *)vaddr);
+ return efi_paddr_valid(paddr);
+}
+
static int __init romsignature(const unsigned char *rom)
{
const unsigned short * const ptr = (const unsigned short *)rom;
unsigned short sig;
+ if (efi_enabled) {
+ int rv = efi_vaddr_valid(rom);
+ printk(KERN_DEBUG
+ romsignature: efi_vaddr_valid(0x%llx) returned %d\n,
+(unsigned long long)rom, rv);
+ if (0 == rv)
+ return 0;
+ }
return probe_kernel_address(ptr, sig) == 0 sig == ROMSIGNATURE;
}
@@ -194,6 +230,39 @@ static int __init romchecksum(const unsigned char *rom,
unsigned long length)
return !length !sum;
}
+/* #define DO_TEST_PROBES include customizable test code */
+static void __init test_probes(void)
+{
+#ifdef DO_TEST_PROBES
+ static struct {
+ phys_addr_t paddr;
+ int ptype;
+ } plist[] = {
+ {0x3e76ULL, 0}, /* RESERVED */
+ {0x009ffda9f000ULL, 1}, /* LOADER CODE */
+ {0x0005e000ULL, 2}, /* LOADER DATA */
+ {0x0006ULL, 3}, /* BOOT CODE (why after EBS?) */
+ {0x3640ULL, 4}, /* BOOT DATA (why after EBS?) */
+ {0x009fffc96000ULL, 5}, /* RUNTIME CODE */
+ {0x009fffcd3000ULL, 6}, /* RUNTIME DATA */
+ {0x1000ULL, 7}, /* CONVENTIONAL MEM */
+ {0x3e75f000ULL, 9}, /* ACPI RECLAIM
