All,
considering the non-negligible number of systems where firmware
has ACPI methods that reference IO-APIC pages (causing
method execution to fail on non-pvops Dom0, and crashing pvops),
I''m wondering whether we shouldn''t relax treatment of IO-APIC
MMIO space by moving it from the completely access denied state
that it''s currently in (due to construct_dom0() having
/* I/O APICs. */
for ( i = 0; i < nr_ioapics; i++ )
{
mfn = paddr_to_pfn(mp_ioapics[i].mpc_apicaddr);
if ( smp_found_config )
rc |= iomem_deny_access(dom0, mfn, mfn);
}
) to allowing read access, and dropping writes (through the MMIO
R/O emulation added for specific PCI devices during 4.2
development).
Afaict this ought to be safe, as no reads of currently defined
IO-APIC registers have side effects. But of course we don''t know
what extensions there might be to come.
If we do so (and perhaps even independently of this) we probably
ought to enforce consistent cachability attributes on the secondary
mappings Dom0 then is able to establish - either by further
modifying the requested flags, or by outright denying mapping
requests that aren''t specifying UC. While the latter would be my
preference and would work for the MMCFG case, the ACPI case
described here wouldn''t be covered - Linux''es ACPICA creates
cachable mappings regardless of whether a SystemMemory is
RAM or MMIO (thus risking problems even on native).
Opinions appreciated,
Jan
>>> On 12.03.13 at 11:00, "Jan Beulich" <JBeulich@suse.com> wrote: > If we do so (and perhaps even independently of this) we probably > ought to enforce consistent cachability attributes on the secondary > mappings Dom0 then is able to establish - either by further > modifying the requested flags, or by outright denying mapping > requests that aren''t specifying UC. While the latter would be my > preference and would work for the MMCFG case, the ACPI case > described here wouldn''t be covered - Linux''es ACPICA creates > cachable mappings regardless of whether a SystemMemory is > RAM or MMIO (thus risking problems even on native).Perhaps, if the mapping request didn''t specify UC, we should look up the MTRR covering the range, and permit the mapping if the effective cachability would nevertheless be UC. That would cover the ACPICA case as well (as long as the firmware at least gets the MTRRs set up correctly). Jan
On 12/03/13 10:00, Jan Beulich wrote:> All, > > considering the non-negligible number of systems where firmware > has ACPI methods that reference IO-APIC pages (causing > method execution to fail on non-pvops Dom0, and crashing pvops), > I''m wondering whether we shouldn''t relax treatment of IO-APIC > MMIO space by moving it from the completely access denied state > that it''s currently in (due to construct_dom0() having > > /* I/O APICs. */ > for ( i = 0; i < nr_ioapics; i++ ) > { > mfn = paddr_to_pfn(mp_ioapics[i].mpc_apicaddr); > if ( smp_found_config ) > rc |= iomem_deny_access(dom0, mfn, mfn); > } > > ) to allowing read access, and dropping writes (through the MMIO > R/O emulation added for specific PCI devices during 4.2 > development). > > Afaict this ought to be safe, as no reads of currently defined > IO-APIC registers have side effects. But of course we don''t know > what extensions there might be to come. > > If we do so (and perhaps even independently of this) we probably > ought to enforce consistent cachability attributes on the secondary > mappings Dom0 then is able to establish - either by further > modifying the requested flags, or by outright denying mapping > requests that aren''t specifying UC. While the latter would be my > preference and would work for the MMCFG case, the ACPI case > described here wouldn''t be covered - Linux''es ACPICA creates > cachable mappings regardless of whether a SystemMemory is > RAM or MMIO (thus risking problems even on native). > > Opinions appreciated, > JanHow likely is it for the IO-APIC to change? It is needed for legacy reasons but I cant see it changing much in the future. We have even been at the same "latest version" for a long time now. If no registers currently have read side-effects (I have just checked the latest reference I can find and cant spot any side-effects), and it solves failure/crash situations on some hardware, I would vote in favor of such a system. If however we do find a system in the future which has read side-effects, we could possibly degrade a read mapping to trap-and-emulate for safety. Purely a nit - the name "iomem_deny_access" is confusing for a function which adds a RO mapping to a domain. I realize it got this name based on its first use (changing RW PCI mappings to RO), but it might be prudent to rename it to something such as "iomem_ro_mapping" or so. ~Andrew
>>> On 12.03.13 at 12:42, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > Purely a nit - the name "iomem_deny_access" is confusing for a function > which adds a RO mapping to a domain. I realize it got this name based > on its first use (changing RW PCI mappings to RO), but it might be > prudent to rename it to something such as "iomem_ro_mapping" or so.The code I copied where this function was used is code that we currently have in place. My plan would be to change that (including the place where the setup happens), such that the function you ask about can retain both its name and its effect. Jan
Konrad Rzeszutek Wilk
2013-Mar-12 12:51 UTC
Re: IO-APIC pages being accessed by ACPI methods
On Tue, Mar 12, 2013 at 10:00:08AM +0000, Jan Beulich wrote:> All, > > considering the non-negligible number of systems where firmware > has ACPI methods that reference IO-APIC pages (causing > method execution to fail on non-pvops Dom0, and crashing pvops),Hmm. I recall only seeing one issue on xen-devel with an IBM box. Were there more?> I''m wondering whether we shouldn''t relax treatment of IO-APIC > MMIO space by moving it from the completely access denied state > that it''s currently in (due to construct_dom0() having > > /* I/O APICs. */ > for ( i = 0; i < nr_ioapics; i++ ) > { > mfn = paddr_to_pfn(mp_ioapics[i].mpc_apicaddr); > if ( smp_found_config ) > rc |= iomem_deny_access(dom0, mfn, mfn); > } > > ) to allowing read access, and dropping writes (through the MMIO > R/O emulation added for specific PCI devices during 4.2 > development). > > Afaict this ought to be safe, as no reads of currently defined > IO-APIC registers have side effects. But of course we don''t know > what extensions there might be to come. > > If we do so (and perhaps even independently of this) we probably > ought to enforce consistent cachability attributes on the secondary > mappings Dom0 then is able to establish - either by further > modifying the requested flags, or by outright denying mapping > requests that aren''t specifying UC. While the latter would be my > preference and would work for the MMCFG case, the ACPI case > described here wouldn''t be covered - Linux''es ACPICA creates > cachable mappings regardless of whether a SystemMemory is > RAM or MMIO (thus risking problems even on native).Eww. That looks like a bug in the ACPICA? Perhaps my recollection is incorrect but shouldn''t MMIO regions mostly be UC. Only drivers that know what they are doing (say graphic cards) can choose to set that to WC? Or by cachable mappings you mean WB?> > Opinions appreciated, > Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >
>>> On 12.03.13 at 13:51, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > On Tue, Mar 12, 2013 at 10:00:08AM +0000, Jan Beulich wrote: >> considering the non-negligible number of systems where firmware >> has ACPI methods that reference IO-APIC pages (causing >> method execution to fail on non-pvops Dom0, and crashing pvops), > > Hmm. I recall only seeing one issue on xen-devel with an IBM box. > Were there more?I recall a couple, but I can''t tell whether they weren''t all with the same or similar hardware. Some may pre-date your working on Xen though.>> If we do so (and perhaps even independently of this) we probably >> ought to enforce consistent cachability attributes on the secondary >> mappings Dom0 then is able to establish - either by further >> modifying the requested flags, or by outright denying mapping >> requests that aren''t specifying UC. While the latter would be my >> preference and would work for the MMCFG case, the ACPI case >> described here wouldn''t be covered - Linux''es ACPICA creates >> cachable mappings regardless of whether a SystemMemory is >> RAM or MMIO (thus risking problems even on native). > > Eww. That looks like a bug in the ACPICA? Perhaps my recollection > is incorrect but shouldn''t MMIO regions mostly be UC. Only drivers > that know what they are doing (say graphic cards) can choose to > set that to WC?As said - by virtue of the MTRRs being set up right, this works in general. But I do indeed think that this is a latent bug in the ACPICA glue code (the more considering that it''s supposed to be arch invariant, yet non-x86 platforms don''t necessarily have concepts similar to x86''s MTRRs).> Or by cachable mappings you mean WB?Yes, that''s the main kind of caching done these days (who cares about WT)? And to avoid misunderstanding - WC to me is not really a cachable memory type, and ACPICA doesn''t use it. Jan