Skip to contentSkip to conversation

libxc: Refactor x86 specific code into x86 specific files

PATCH In review Christoph Egger 3 participants 4 messages Nov 9, 2011 – Dec 2, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 libxc: Refactor x86 specific code into x86 specific files

Move x86 specific code into x86 specific files. Eliminate arch specific PAGE constants by using common XC_PAGE_* contants.

Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
tools/libxc/Makefile +1 -0
diff -r c2a8bde9ad6e tools/libxc/Makefile
@@ -13,6 +13,7 @@ CTRL_SRCS-y       += xc_domain.c
 CTRL_SRCS-y       += xc_evtchn.c
 CTRL_SRCS-y       += xc_gnttab.c
 CTRL_SRCS-y       += xc_misc.c
+CTRL_SRCS-$(CONFIG_X86) += xc_misc_x86.c
 CTRL_SRCS-y       += xc_flask.c
 CTRL_SRCS-y       += xc_physdev.c
 CTRL_SRCS-y       += xc_private.c
tools/libxc/xc_dom_ia64.c +6 -6
diff -r c2a8bde9ad6e tools/libxc/xc_dom_ia64.c
@@ -83,7 +83,7 @@ int start_info_ia64(struct xc_dom_image 
         bp->initrd_start = start_info->mod_start;
         bp->initrd_size = start_info->mod_len;
     }
-    bp->command_line = (dom->start_info_pfn << PAGE_SHIFT_IA64)
+    bp->command_line = (dom->start_info_pfn << XC_PAGE_SHIFT)
         + offsetof(start_info_t, cmd_line);
     if ( dom->cmdline )
     {
@@ -128,7 +128,7 @@ static int vcpu_ia64(struct xc_dom_image
 #ifdef __ia64__			/* FIXME */
     ctxt->regs.ar.fpsr = xc_ia64_fpsr_default();
 #endif
-    ctxt->regs.r[28] = (dom->start_info_pfn << PAGE_SHIFT_IA64)
+    ctxt->regs.r[28] = (dom->start_info_pfn << XC_PAGE_SHIFT)
         + sizeof(start_info_ia64_t);
     return 0;
 }
@@ -138,7 +138,7 @@ static int vcpu_ia64(struct xc_dom_image
 static struct xc_dom_arch xc_dom_arch = {
     .guest_type = "xen-3.0-ia64",
     .native_protocol = XEN_IO_PROTO_ABI_IA64,
-    .page_shift = PAGE_SHIFT_IA64,
+    .page_shift = XC_PAGE_SHIFT,
     .alloc_magic_pages = alloc_magic_pages,
     .start_info = start_info_ia64,
     .shared_info = shared_info_ia64,
@@ -148,7 +148,7 @@ static struct xc_dom_arch xc_dom_arch = 
 static struct xc_dom_arch xc_dom_arch_ia64be = {
     .guest_type = "xen-3.0-ia64be",
     .native_protocol = XEN_IO_PROTO_ABI_IA64,
-    .page_shift = PAGE_SHIFT_IA64,
+    .page_shift = XC_PAGE_SHIFT,
     .alloc_magic_pages = alloc_magic_pages,
     .start_info = start_info_ia64,
     .shared_info = shared_info_ia64,
@@ -173,8 +173,8 @@ int arch_setup_meminit(struct xc_dom_ima
     /* setup initial p2m */
     if (dom->guest_type && strcmp(dom->guest_type,
                                   "hvm-3.0-ia64-sioemu") == 0) {
-        start = FW_MEM_BASE >> PAGE_SHIFT_IA64;
-        nbr = FW_MEM_SIZE >> PAGE_SHIFT_IA64;
+        start = FW_MEM_BASE >> XC_PAGE_SHIFT;
+        nbr = FW_MEM_SIZE >> XC_PAGE_SHIFT;
     } else {
         start = 0;
         nbr = dom->total_pages;
tools/libxc/xc_dom_x86.c +20 -20
diff -r c2a8bde9ad6e tools/libxc/xc_dom_x86.c
@@ -89,7 +89,7 @@ static int count_pgtables(struct xc_dom_
     pages = extra_pages;
     for ( ; ; )
     {
-        try_virt_end = round_up(dom->virt_alloc_end + pages * PAGE_SIZE_X86,
+        try_virt_end = round_up(dom->virt_alloc_end + pages * XC_PAGE_SIZE,
                                 bits_to_mask(22)); /* 4MB alignment */
         dom->pg_l4 =
             nr_page_tables(dom, dom->parms.virt_base, try_virt_end, l4_bits);
@@ -107,7 +107,7 @@ static int count_pgtables(struct xc_dom_
         }
         dom->pgtables = dom->pg_l4 + dom->pg_l3 + dom->pg_l2 + dom->pg_l1;
         pages = dom->pgtables + extra_pages;
-        if ( dom->virt_alloc_end + pages * PAGE_SIZE_X86 <= try_virt_end + 1 )
+        if ( dom->virt_alloc_end + pages * XC_PAGE_SIZE <= try_virt_end + 1 )
             break;
     }
     dom->virt_pgtab_end = try_virt_end + 1;
@@ -132,7 +132,7 @@ static int count_pgtables_x86_32_pae(str
                           L3_PAGETABLE_SHIFT_PAE, L2_PAGETABLE_SHIFT_PAE);
 }
 
-#define pfn_to_paddr(pfn) ((xen_paddr_t)(pfn) << PAGE_SHIFT_X86)
+#define pfn_to_paddr(pfn) ((xen_paddr_t)(pfn) << XC_PAGE_SHIFT)
 
 static int setup_pgtables_x86_32(struct xc_dom_image *dom)
 {
@@ -145,7 +145,7 @@ static int setup_pgtables_x86_32(struct 
     xen_pfn_t pgpfn;
 
     for ( addr = dom->parms.virt_base; addr < dom->virt_pgtab_end;
-          addr += PAGE_SIZE_X86 )
+          addr += XC_PAGE_SIZE )
     {
         if ( l1tab == NULL )
         {
@@ -159,7 +159,7 @@ static int setup_pgtables_x86_32(struct 
 
         /* make L1 entry */
         l1off = l1_table_offset_i386(addr);
-        pgpfn = (addr - dom->parms.virt_base) >> PAGE_SHIFT_X86;
+        pgpfn = (addr - dom->parms.virt_base) >> XC_PAGE_SHIFT;
         l1tab[l1off] =
             pfn_to_paddr(xc_dom_p2m_guest(dom, pgpfn)) | L1_PROT;
         if ( (addr >= dom->pgtables_seg.vstart) && 
@@ -264,7 +264,7 @@ static int setup_pgtables_x86_32_pae(str
     l3tab = xc_dom_pfn_to_ptr(dom, l3pfn, 1);
 
     for ( addr = dom->parms.virt_base; addr < dom->virt_pgtab_end;
-          addr += PAGE_SIZE_X86 )
+          addr += XC_PAGE_SIZE )
     {
         if ( l2tab == NULL )
         {
@@ -290,7 +290,7 @@ static int setup_pgtables_x86_32_pae(str
 
         /* make L1 entry */
         l1off = l1_table_offset_pae(addr);
-        pgpfn = (addr - dom->parms.virt_base) >> PAGE_SHIFT_X86;
+        pgpfn = (addr - dom->parms.virt_base) >> XC_PAGE_SHIFT;
         l1tab[l1off] =
             pfn_to_paddr(xc_dom_p2m_guest(dom, pgpfn)) | L1_PROT;
         if ( (addr >= dom->pgtables_seg.vstart) &&
@@ -345,7 +345,7 @@ static int setup_pgtables_x86_64(struct 
     xen_pfn_t pgpfn;
 
     for ( addr = dom->parms.virt_base; addr < dom->virt_pgtab_end;
-          addr += PAGE_SIZE_X86 )
+          addr += XC_PAGE_SIZE )
     {
         if ( l3tab == NULL )
         {
@@ -383,7 +383,7 @@ static int setup_pgtables_x86_64(struct 
 
         /* make L1 entry */
         l1off = l1_table_offset_x86_64(addr);
-        pgpfn = (addr - dom->parms.virt_base) >> PAGE_SHIFT_X86;
+        pgpfn = (addr - dom->parms.virt_base) >> XC_PAGE_SHIFT;
         l1tab[l1off] =
             pfn_to_paddr(xc_dom_p2m_guest(dom, pgpfn)) | L1_PROT;
         if ( (addr >= dom->pgtables_seg.vstart) && 
@@ -438,7 +438,7 @@ static int start_info_x86_32(struct xc_d
     strncpy(start_info->magic, dom->guest_type, sizeof(start_info->magic));
     start_info->magic[sizeof(start_info->magic) - 1] = '\0';
     start_info->nr_pages = dom->total_pages;
-    start_info->shared_info = shinfo << PAGE_SHIFT_X86;
+    start_info->shared_info = shinfo << XC_PAGE_SHIFT;
     start_info->pt_base = dom->pgtables_seg.vstart;
     start_info->nr_pt_frames = dom->pgtables;
     start_info->mfn_list = dom->p2m_seg.vstart;
@@ -478,7 +478,7 @@ static int start_info_x86_64(struct xc_d
     strncpy(start_info->magic, dom->guest_type, sizeof(start_info->magic));
     start_info->magic[sizeof(start_info->magic) - 1] = '\0';
     start_info->nr_pages = dom->total_pages;
-    start_info->shared_info = shinfo << PAGE_SHIFT_X86;
+    start_info->shared_info = shinfo << XC_PAGE_SHIFT;
     start_info->pt_base = dom->pgtables_seg.vstart;
     start_info->nr_pt_frames = dom->pgtables;
     start_info->mfn_list = dom->p2m_seg.vstart;
@@ -550,9 +550,9 @@ static int vcpu_x86_32(struct xc_dom_ima
     ctxt->user_regs.cs = FLAT_KERNEL_CS_X86_32;
     ctxt->user_regs.eip = dom->parms.virt_entry;
     ctxt->user_regs.esp =
-        dom->parms.virt_base + (dom->bootstack_pfn + 1) * PAGE_SIZE_X86;
+        dom->parms.virt_base + (dom->bootstack_pfn + 1) * XC_PAGE_SIZE;
     ctxt->user_regs.esi =
-        dom->parms.virt_base + (dom->start_info_pfn) * PAGE_SIZE_X86;
+        dom->parms.virt_base + (dom->start_info_pfn) * XC_PAGE_SIZE;
     ctxt->user_regs.eflags = 1 << 9; /* Interrupt Enable */
 
     ctxt->kernel_ss = ctxt->user_regs.ss;
@@ -589,9 +589,9 @@ static int vcpu_x86_64(struct xc_dom_ima
     ctxt->user_regs.cs = FLAT_KERNEL_CS_X86_64;
     ctxt->user_regs.rip = dom->parms.virt_entry;
     ctxt->user_regs.rsp =
-        dom->parms.virt_base + (dom->bootstack_pfn + 1) * PAGE_SIZE_X86;
+        dom->parms.virt_base + (dom->bootstack_pfn + 1) * XC_PAGE_SIZE;
     ctxt->user_regs.rsi =
-        dom->parms.virt_base + (dom->start_info_pfn) * PAGE_SIZE_X86;
+        dom->parms.virt_base + (dom->start_info_pfn) * XC_PAGE_SIZE;
     ctxt->user_regs.rflags = 1 << 9; /* Interrupt Enable */
 
     ctxt->kernel_ss = ctxt->user_regs.ss;
@@ -611,7 +611,7 @@ static int vcpu_x86_64(struct xc_dom_ima
 static struct xc_dom_arch xc_dom_32 = {
     .guest_type = "xen-3.0-x86_32",
     .native_protocol = XEN_IO_PROTO_ABI_X86_32,
-    .page_shift = PAGE_SHIFT_X86,
+    .page_shift = XC_PAGE_SHIFT,
     .sizeof_pfn = 4,
     .alloc_magic_pages = alloc_magic_pages,
     .count_pgtables = count_pgtables_x86_32,
@@ -623,7 +623,7 @@ static struct xc_dom_arch xc_dom_32 = {
 static struct xc_dom_arch xc_dom_32_pae = {
     .guest_type = "xen-3.0-x86_32p",
     .native_protocol = XEN_IO_PROTO_ABI_X86_32,
-    .page_shift = PAGE_SHIFT_X86,
+    .page_shift = XC_PAGE_SHIFT,
     .sizeof_pfn = 4,
     .alloc_magic_pages = alloc_magic_pages,
     .count_pgtables = count_pgtables_x86_32_pae,
@@ -636,7 +636,7 @@ static struct xc_dom_arch xc_dom_32_pae 
 static struct xc_dom_arch xc_dom_64 = {
     .guest_type = "xen-3.0-x86_64",
     .native_protocol = XEN_IO_PROTO_ABI_X86_64,
-    .page_shift = PAGE_SHIFT_X86,
+    .page_shift = XC_PAGE_SHIFT,
     .sizeof_pfn = 8,
     .alloc_magic_pages = alloc_magic_pages,
     .count_pgtables = count_pgtables_x86_64,
@@ -855,13 +855,13 @@ int arch_setup_bootlate(struct xc_dom_im
     DOMPRINTF("%s: shared_info: pfn 0x%" PRIpfn ", mfn 0x%" PRIpfn "",
               __FUNCTION__, dom->shared_info_pfn, dom->shared_info_mfn);
     shared_info = xc_map_foreign_range(dom->xch, dom->guest_domid,
-                                       PAGE_SIZE_X86,
+                                       XC_PAGE_SIZE,
                                        PROT_READ | PROT_WRITE,
                                        shinfo);
     if ( shared_info == NULL )
         return -1;
     dom->arch_hooks->shared_info(dom, shared_info);
-    munmap(shared_info, PAGE_SIZE_X86);
+    munmap(shared_info, XC_PAGE_SIZE);
 
     return 0;
 }
tools/libxc/xc_misc.c +0 -22
diff -r c2a8bde9ad6e tools/libxc/xc_misc.c
@@ -177,28 +177,6 @@ int xc_sched_id(xc_interface *xch,
     return 0;
 }
 
-#if defined(__i386__) || defined(__x86_64__)
-int xc_mca_op(xc_interface *xch, struct xen_mc *mc)
-{
-    int ret = 0;
-    DECLARE_HYPERCALL;
-    DECLARE_HYPERCALL_BOUNCE(mc, sizeof(*mc), XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
-
-    if ( xc_hypercall_bounce_pre(xch, mc) )
-    {
-        PERROR("Could not bounce xen_mc memory buffer");
-        return -1;
-    }
-    mc->interface_version = XEN_MCA_INTERFACE_VERSION;
-
-    hypercall.op = __HYPERVISOR_mca;
-    hypercall.arg[0] = HYPERCALL_BUFFER_AS_ARG(mc);
-    ret = do_xen_hypercall(xch, &hypercall);
-    xc_hypercall_bounce_post(xch, mc);
-    return ret;
-}
-#endif
-
 int xc_perfc_reset(xc_interface *xch)
 {
     DECLARE_SYSCTL;
tools/libxc/xc_misc_x86.c new file +51 -0

51 lines not on this page · raw message

tools/libxc/xc_offline_page.c +2 -2
diff -r c2a8bde9ad6e tools/libxc/xc_offline_page.c
@@ -369,7 +369,7 @@ static int __clear_pte(xc_interface *xch
 
     /* XXX Check for PSE bit here */
     /* Hit one entry */
-    if ( ((pte >> PAGE_SHIFT_X86) & MFN_MASK_X86) == mfn)
+    if ( ((pte >> XC_PAGE_SHIFT) & MFN_MASK_X86) == mfn)
     {
         *new_pte = pte & ~_PAGE_PRESENT;
         if (!backup_ptes(table_mfn, table_offset, backup))
@@ -400,7 +400,7 @@ static int __update_pte(xc_interface *xc
         if (pte & _PAGE_PRESENT)
             ERROR("Page present while in backup ptes\n");
         pte &= ~MFN_MASK_X86;
-        pte |= (new_mfn << PAGE_SHIFT_X86) | _PAGE_PRESENT;
+        pte |= (new_mfn << XC_PAGE_SHIFT) | _PAGE_PRESENT;
         *new_pte = pte;
         return 1;
     }
tools/libxc/xenctrl.h +39 -102

185 lines not on this page · raw message

tools/libxc/xenctrl_x86.h new file +111 -0

111 lines not on this page · raw message

tools/libxc/xg_private.h +6 -111

142 lines not on this page · raw message

tools/libxc/xg_x86_private.h new file +121 -0

121 lines not on this page · raw message

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

#1 lore raw

Christoph Egger writes ("[Xen-devel] [PATCH] libxc: Refactor x86 specific code into x86 specific files"):

Quoted 3 lines

Move x86 specific code into x86 specific files. Eliminate arch specific PAGE constants by using common XC_PAGE_* contants.

I'm not sure I understand the motivation for this.

Ian.

#2 lore raw
Quoted 6 lines · Ian Jackson

Christoph Egger writes ("[Xen-devel] [PATCH] libxc: Refactor x86 specific code into x86 specific files"):

Quoted 3 lines

Move x86 specific code into x86 specific files. Eliminate arch specific PAGE constants by using common XC_PAGE_* contants.

I'm not sure I understand the motivation for this.

Code cleanup, better readability, better code organization.

Christoph

Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
#3 lore raw
Quoted 9 lines · Christoph Egger
Quoted 6 lines · Ian Jackson

Christoph Egger writes ("[Xen-devel] [PATCH] libxc: Refactor x86 specific code into x86 specific files"):

Quoted 3 lines

Move x86 specific code into x86 specific files. Eliminate arch specific PAGE constants by using common XC_PAGE_* contants.

I'm not sure I understand the motivation for this.

Code cleanup, better readability, better code organization.

At the very least each cleanup should be presented individually so we can judge them on their merits.

The switch to a consistent set of XC_PAGE_* constants doesn't seem like such a bad idea.

Likewise moving the x86 PT constants out of xg_private.h doesn't seem unreasonable.

I'm less convinced by the more general movement of interface definitions/implementations out of #ifdef in the .c and .h files into foo_x86.[ch]. Seems to me that this just obfuscates the interfaces by requiring one to go and look in multiple places.

I think what you've done to union cpu_guest_context_any_t and friends is not an improvement.

The first two hunks of change in xenctrl.h seem like pointless code motion within the same header.

why have you moved the x86 stuff into x86.h but left the ia64 stuff ifdef'd in the main headers. For any given construct it should be all or nothing IMHO.

You haven't made any arrangements to install the new headers.

Ian.