Re: [Xen-devel] [PATCH] x86: Don't crash when mapping a page using EFI runtime page tables

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.05.15 at 19:41, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 15/05/15 17:08, Ross Lagerwall wrote:
>> When an interrupt is received during an EFI runtime service call, Xen
>> may call map_domain_page() while using the EFI runtime page tables.
>> This fails because, although the EFI runtime page tables are a
>> copy of the idle domain's page tables, current points at a different
>> domain's vCPU.
>>
>> To fix this, return NULL from mapcache_current_vcpu() when using the EFI
>> runtime page tables which is treated equivalently to running in an idle
>> vCPU.
>>
>> This issue can be reproduced by repeatedly calling GetVariable() from
>> dom0 while using VT-d, since VT-d frequently maps a page from interrupt
>> context.
>>
>> Example call trace:
>> [<ffff82d0801615dc>] __find_next_zero_bit+0x28/0x60
>> [<ffff82d08016a10e>] map_domain_page+0x4c6/0x4eb
>> [<ffff82d080156ae6>] map_vtd_domain_page+0xd/0xf
>> [<ffff82d08015533a>] msi_msg_read_remap_rte+0xe3/0x1d8
>> [<ffff82d08014e516>] iommu_read_msi_from_ire+0x31/0x34
>> [<ffff82d08016ff6c>] set_msi_affinity+0x134/0x17a
>> [<ffff82d0801737b5>] move_masked_irq+0x5c/0x98
>> [<ffff82d080173816>] move_native_irq+0x25/0x36
>> [<ffff82d08016ffcb>] ack_nonmaskable_msi_irq+0x19/0x20
>> [<ffff82d08016ffdb>] ack_maskable_msi_irq+0x9/0x37
>> [<ffff82d080173e8b>] do_IRQ+0x251/0x635
>> [<ffff82d080234502>] common_interrupt+0x62/0x70
>> [<00000000df7ed2be>] 00000000df7ed2be
>>
>> Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> 
> This is actually a VT-d issue which I have fallen over before, and
> didn't have time to fix back then.  I had a cascade crash from having to
> use map_domain_page() to disable the IOMMU following a crash in the
> middle of a context switch.
> 
> The VT-d code should use map_domain_page_global() for various of the
> root structures, which will simply the code and reduce the runtime cost
> of a lot of the codepaths.

Yes.

> However, the overall crash proves that map_domain_page() is not safe for
> use in IRQ or exception context.  In the case that v == current,
> map_domain_page() will degrade to mfn_to_virt() which is only safe if
> one can guarantee that the frame being mapped is present in the direct
> mapped region.  The map_domain_page() used by the VT-d code very
> certainly isn't.

I'm not following: There's certainly no special casing of v == current
in map_domain_page(). And hence I can't see the IRQ/exception
context issue you try to describe here.

I'm wondering whether we shouldn't instead simply disable interrupts
while doing EFI runtime services calls.

> We probably want to avoid using map_domain_page() when printing a
> hypervisor page walk to avoid deadlocks in situations like this.

That wouldn't be very helpful - such printing happens when we're
about to halt the system due to a crash, and hence would hamper
analysis of the cause. Or did you mean to replace the use of
map_domain_page() there by e.g. a fixmap based mechanism, to
avoid the lock state dependency?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel