Skip to contentSkip to conversation

construct_dom0: Initialize variable before use

PATCH In review Christoph Egger 2 participants 5 messages Nov 29, 2007

lore
Share & tools
lore

Christoph Egger

author
#0 lore raw

PATCH 1/1 construct_dom0: Initialize variable before use

Hi!

Attached patch initializes d->arch.physaddr_bitsize properly for a 64bit Dom0/64bit hypervisor.
This bug has been found when booting a NetBSD/Xen Dom0 with more than 4GB RAM.

Without this fix, d->arch.physaddr_bitsize is 0 in domain_clamp_alloc_bitsize(). This causes all attempts to XENMEM_increase_reservation with bits > 0 to fail. More precisely, __alloc_domheap_pages() returns NULL.
This impacts Xen heap allocation in general.
Question: How did that work on Linux Dom0?

I think, initializing physaddr_bitsize to 64 unconditionally is ok because in construct_dom0() it is overridden with the proper value when you boot 64bit hypervisor with a 32bit Dom0. If the hypervisor is not 64bit, physaddr_bitsize isn't used at all anyway.

Signed-off-by Christoph Egger <[email protected]>
Signature · 12 lines
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy
xen/arch/x86/domain_build.c +5 -0
diff -r 6fd17d0dcbcd xen/arch/x86/domain_build.c
@@ -353,6 +353,11 @@ int __init construct_dom0(
     if ( parms.pae == PAEKERN_extended_cr3 )
             set_bit(VMASST_TYPE_pae_extended_cr3, &d->vm_assist);
 
+    /* Unconditionally initialize this member here for 64bit Dom0s.
+     * In the below if (), this is overridden for 32bit Dom0s.
+     */ 
+    d->arch.physaddr_bitsize = 64;
+
     if ( UNSET_ADDR != parms.virt_hv_start_low && elf_32bit(&elf) )
     {
 #if CONFIG_PAGING_LEVELS < 4

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw
Quoted 6 lines · Christoph Egger

Without this fix, d->arch.physaddr_bitsize is 0 in domain_clamp_alloc_bitsize(). This causes all attempts to XENMEM_increase_reservation with bits > 0 to fail. More precisely, __alloc_domheap_pages() returns NULL.
This impacts Xen heap allocation in general.
Question: How did that work on Linux Dom0?

Yes, that's pretty broken. It works for Linux because Linux allocates its lowmem I/o pages (e.g., swiotlb) using the XENMEM_exchange command, and that allocates the new memory anonymously in the first instance. This defeats the bitsize clamp check (which is okay just now because our truncation of the phsyical memory map to 166GB is sufficient to ensure that compat domUs can address all memory).

-- Keir

author
#2 lore raw
Quoted 8 lines · On Thursday 29 November 2007 14:28:00 Keir Fraser wrote:
Quoted 6 lines · Christoph Egger

Without this fix, d->arch.physaddr_bitsize is 0 in domain_clamp_alloc_bitsize(). This causes all attempts to XENMEM_increase_reservation with bits > 0 to fail. More precisely, __alloc_domheap_pages() returns NULL.
This impacts Xen heap allocation in general.
Question: How did that work on Linux Dom0?

Quoted 6 lines

Yes, that's pretty broken. It works for Linux because Linux allocates its lowmem I/o pages (e.g., swiotlb) using the XENMEM_exchange command, and that allocates the new memory anonymously in the first instance. This defeats the bitsize clamp check (which is okay just now because our truncation of the phsyical memory map to 166GB is sufficient to ensure that compat domUs can address all memory).

Thanks for clarification.
NetBSD Dom0 failed when allocating DMA-safe memory above 4GB.

Christoph

Signature · 12 lines
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy
author
#3 lore raw
Quoted 16 lines · On Thursday 29 November 2007 14:37:24 Christoph Egger wrote:
Quoted 8 lines · On Thursday 29 November 2007 14:28:00 Keir Fraser wrote:
Quoted 6 lines · Christoph Egger

Without this fix, d->arch.physaddr_bitsize is 0 in domain_clamp_alloc_bitsize(). This causes all attempts to XENMEM_increase_reservation with bits > 0 to fail. More precisely, __alloc_domheap_pages() returns NULL.
This impacts Xen heap allocation in general.
Question: How did that work on Linux Dom0?

Quoted 6 lines

Yes, that's pretty broken. It works for Linux because Linux allocates its lowmem I/o pages (e.g., swiotlb) using the XENMEM_exchange command, and that allocates the new memory anonymously in the first instance. This defeats the bitsize clamp check (which is okay just now because our truncation of the phsyical memory map to 166GB is sufficient to ensure that compat domUs can address all memory).

Quoted 2 lines

Thanks for clarification.
NetBSD Dom0 failed when allocating DMA-safe memory above 4GB.

Keir: Can you also put this fix onto your patch-queue for Xen 3.1.3, please?

Thanks,
Christoph

Signature · 12 lines
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy
#4 lore raw
Quoted 4 lines · Christoph Egger
Quoted 2 lines

Thanks for clarification.
NetBSD Dom0 failed when allocating DMA-safe memory above 4GB.

Keir: Can you also put this fix onto your patch-queue for Xen 3.1.3, please?

Definitely.

-- Keir