Messages in this thread | | | Date | Fri, 19 Oct 2012 09:20:41 -0600 | From | Randy Wright <> | Subject | 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 Wright Hewlett-Packard Company Phone: (970) 898-0998 Mail: rwright@hp.com
| |