Re: [Xen-devel] [PATCH v2 3/4] adjust special domain creation (and call it earlier on x86)

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.06.19 at 15:35, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 04/06/2019 13:43, Jan Beulich wrote:
>> Split out this mostly arch-independent code into a common-code helper
>> function. (This does away with Arm's arch_init_memory() altogether.)
>>
>> On x86 this needs to happen before acpi_boot_init(): Commit 9fa94e1058
>> ("x86/ACPI: also parse AMD IOMMU tables early") only appeared to work
>> fine - it's really broken, and doesn't crash (on non-EFI AMD systems)
>> only because of there being a mapping of linear address 0 during early
>> boot. On EFI there is:
>>
>>  Early fatal page fault at e008:ffff82d08024d58e (cr2=0000000000000220, 
>> ec=0000)
>>  ----[ Xen-4.13-unstable  x86_64  debug=y   Not tainted ]----
>>  CPU:    0
>>  RIP:    e008:[<ffff82d08024d58e>] pci.c#_pci_hide_device+0x17/0x3a
>>  RFLAGS: 0000000000010046   CONTEXT: hypervisor
>>  rax: 0000000000000000   rbx: 0000000000006000   rcx: 0000000000000000
>>  rdx: ffff83104f2ee9b0   rsi: ffff82e0209e5d48   rdi: ffff83104f2ee9a0
>>  rbp: ffff82d08081fce0   rsp: ffff82d08081fcb8   r8:  0000000000000000
>>  r9:  8000000000000000   r10: 0180000000000000   r11: 7fffffffffffffff
>>  r12: ffff83104f2ee9a0   r13: 0000000000000002   r14: ffff83104f2ee4b0
>>  r15: 0000000000000064   cr0: 0000000080050033   cr4: 00000000000000a0
>>  cr3: 000000009f614000   cr2: 0000000000000220
>>  fsb: 0000000000000000   gsb: 0000000000000000   gss: 0000000000000000
>>  ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: 0000   cs: e008
>>  Xen code around <ffff82d08024d58e> (pci.c#_pci_hide_device+0x17/0x3a):
>>   48 89 47 38 48 8d 57 10 <48> 8b 88 20 02 00 00 48 89 51 08 48 89 4f 10 48
>>  Xen stack trace from rsp=ffff82d08081fcb8:
>> [...]
>>  Xen call trace:
>>     [<ffff82d08024d58e>] pci.c#_pci_hide_device+0x17/0x3a
>> [   [<                >] pci_ro_device+...]
> 
> What is this in the stack trace?

The entry missing here, to make the whole thing sensible. See
the other patch I did send ("x86/traps: widen condition for
logging top-of-stack").

>>     [<ffff82d080617fe1>] amd_iommu_detect_one_acpi+0x161/0x249
>>     [<ffff82d0806186ac>] iommu_acpi.c#detect_iommu_acpi+0xb5/0xe7
>>     [<ffff82d08061cde0>] acpi_table_parse+0x61/0x90
>>     [<ffff82d080619e7d>] amd_iommu_detect_acpi+0x17/0x19
>>     [<ffff82d08061790b>] acpi_ivrs_init+0x20/0x5b
>>     [<ffff82d08062e838>] acpi_boot_init+0x301/0x30f
>>     [<ffff82d080628b10>] __start_xen+0x1daf/0x28a2
>>  
>>  Pagetable walk from 0000000000000220:
>>   L4[0x000] = 000000009f44f063 ffffffffffffffff
>>   L3[0x000] = 000000009f44b063 ffffffffffffffff
>>   L2[0x000] = 0000000000000000 ffffffffffffffff
>>  
>>  ****************************************
>>  Panic on CPU 0:
>>  FATAL TRAP: vector = 14 (page fault)
>>  [error_code=0000] , IN INTERRUPT CONTEXT
>>  ****************************************
>>
>> Of course the bug would nevertheless have lead to post-boot crashes as
>> soon as the list would actually get traversed.
>>
>> Take the opportunity and
>> - convert BUG_ON()s being moved to panic(),
>> - add __read_mostly annotations to the dom_* definitions.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Sorry for not noticing this before, but s/special/system/ to match up
> with the existing terminology in is_system_domain()

Easily done.

>> --- a/xen/include/xen/mm.h
>> +++ b/xen/include/xen/mm.h
>> @@ -642,6 +642,9 @@ static inline void filtered_flush_tlb_ma
>>      }
>>  }
>>  
>> +/* Private domain structs for DOMID_XEN, DOMID_IO, etc. */
>> +extern struct domain *dom_xen, *dom_io, *dom_cow;
>> +
> 
> Any chance you can move these higher up, to before the include of
> <asm/mm.h>, or you're going to break Julien's M2P series.

Hmm, I could, albeit I did intentionally place them there. In fact I
had them elsewhere first, until the build broke because of the use
of dom_xen in share_xen_page_with_privileged_guests(). That's
what made me decide to put it where it now is (and I'd prefer to
keep it there for now).

> With at least the name adjusted, Reviewed-by: Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>

Thanks.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel