is_initial_xendomain()

is_initial_xendomain()

[Jan Beulich]

Seeing many cases of uses like

#ifdef CONFIG_XEN_PRIVILEGED_GUEST
	if (is_initial_xendomain()) {
		...
	}
#endif

I'm wondering if it wasn't nice to eliminate the preprocessor
conditionals (which appear to be there only to cut down on
code size) by doing the conditional in a single place instead:

#ifdef CONFIG_XEN_PRIVILEGED_GUEST
#define is_initial_xendomain() (xen_start_info->flags & SIF_INITDOMAIN)
#else
#define is_initial_xendomain() 0
#endif

If that is acceptable, I'm ready to create a respective patch
(but I'd like to avoid spending time on it if there are objections).

Jan

RE: is_initial_xendomain()

[Ian Pratt]

> #ifdef CONFIG_XEN_PRIVILEGED_GUEST
> #define is_initial_xendomain() (xen_start_info->flags &
SIF_INITDOMAIN)
> #else
> #define is_initial_xendomain() 0
> #endif
> 
> If that is acceptable, I'm ready to create a respective patch
> (but I'd like to avoid spending time on it if there are objections).

Personally, I'd like to see CONFIG_XEN_PRIVILEGED_GUEST disappear
altogether. The code size saving is small, and most people use the -xen
kernel rather than  -xen0/-xenU anyway.

Ian

Re: is_initial_xendomain()

[Gerd Hoffmann]

Ian Pratt wrote:
>> #ifdef CONFIG_XEN_PRIVILEGED_GUEST
>> #define is_initial_xendomain() (xen_start_info->flags &
> SIF_INITDOMAIN)
>> #else
>> #define is_initial_xendomain() 0
>> #endif
>>
>> If that is acceptable, I'm ready to create a respective patch
>> (but I'd like to avoid spending time on it if there are objections).
> 
> Personally, I'd like to see CONFIG_XEN_PRIVILEGED_GUEST disappear
> altogether. The code size saving is small, and most people use the -xen
> kernel rather than  -xen0/-xenU anyway.

For development it's handy to have a separate xenU kernel, for both
build time and size.  Most size savings come from dropping all the
drivers though, not CONFIG_XEN_PRIVILEGED_GUEST.  And gcc should be
clever enougth to optimize away "if (0) { code block }", so you get the
size savings even with CONFIG_XEN_PRIVILEGED_GUEST being hidden in some
header file as listed above.

cheers,

  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>
http://www.suse.de/~kraxel/julika-dora.jpeg

Re: is_initial_xendomain()

[Keir Fraser]

On 18/8/06 8:56 am, "Ian Pratt" <m+Ian.Pratt@cl.cam.ac.uk> wrote:

>> #ifdef CONFIG_XEN_PRIVILEGED_GUEST
>> #define is_initial_xendomain() (xen_start_info->flags &
> SIF_INITDOMAIN)
>> #else
>> #define is_initial_xendomain() 0
>> #endif
>> 
>> If that is acceptable, I'm ready to create a respective patch
>> (but I'd like to avoid spending time on it if there are objections).
> 
> Personally, I'd like to see CONFIG_XEN_PRIVILEGED_GUEST disappear
> altogether. The code size saving is small, and most people use the -xen
> kernel rather than  -xen0/-xenU anyway.

A partial cull based on initial_xendomain() would be a good first step.
We'll certainly take the patch if you make one, Jan.

 -- Keir

Re: is_initial_xendomain()

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]

Okay, here's the patch, leaving just a single occurrence of CONFIG_XEN_PRIVILEGED_GUEST.
(Testing was done on 2.6.18-rc4, and I hope there's nothing wrong with the patch as adapted
to 2.6.16.13).

Signed-off-by: Jan Beulich <jbeulich@novell.com>

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 18.08.06 10:57 >>>



On 18/8/06 8:56 am, "Ian Pratt" <m+Ian.Pratt@cl.cam.ac.uk> wrote:

>> #ifdef CONFIG_XEN_PRIVILEGED_GUEST
>> #define is_initial_xendomain() (xen_start_info->flags &
> SIF_INITDOMAIN)
>> #else
>> #define is_initial_xendomain() 0
>> #endif
>> 
>> If that is acceptable, I'm ready to create a respective patch
>> (but I'd like to avoid spending time on it if there are objections).
> 
> Personally, I'd like to see CONFIG_XEN_PRIVILEGED_GUEST disappear
> altogether. The code size saving is small, and most people use the -xen
> kernel rather than  -xen0/-xenU anyway.

A partial cull based on initial_xendomain() would be a good first step.
We'll certainly take the patch if you make one, Jan.

 -- Keir



[-- Attachment #2: xenlinux-cond-is-initdom.patch --]
[-- Type: text/plain, Size: 9364 bytes --]

Index: 2006-08-16/arch/i386/kernel/setup-xen.c
===================================================================
--- 2006-08-16.orig/arch/i386/kernel/setup-xen.c	2006-08-16 08:58:12.000000000 +0200
+++ 2006-08-16/arch/i386/kernel/setup-xen.c	2006-08-18 13:48:40.000000000 +0200
@@ -184,7 +184,6 @@ static struct resource code_resource = {
 	.flags	= IORESOURCE_BUSY | IORESOURCE_MEM
 };
 
-#ifdef CONFIG_XEN_PRIVILEGED_GUEST
 static struct resource system_rom_resource = {
 	.name	= "System ROM",
 	.start	= 0xf0000,
@@ -240,7 +239,6 @@ static struct resource video_rom_resourc
 	.end	= 0xc7fff,
 	.flags	= IORESOURCE_BUSY | IORESOURCE_READONLY | IORESOURCE_MEM
 };
-#endif
 
 static struct resource video_ram_resource = {
 	.name	= "Video RAM area",
@@ -299,7 +297,6 @@ static struct resource standard_io_resou
 #define STANDARD_IO_RESOURCES \
 	(sizeof standard_io_resources / sizeof standard_io_resources[0])
 
-#ifdef CONFIG_XEN_PRIVILEGED_GUEST
 #define romsignature(x) (*(unsigned short *)(x) == 0xaa55)
 
 static int __init romchecksum(unsigned char *rom, unsigned long length)
@@ -317,9 +314,11 @@ static void __init probe_roms(void)
 	unsigned char *rom;
 	int	      i;
 
+#ifdef CONFIG_XEN
 	/* Nothing to do if not running in dom0. */
 	if (!is_initial_xendomain())
 		return;
+#endif
 
 	/* video rom */
 	upper = adapter_rom_resources[0].start;
@@ -379,7 +378,6 @@ static void __init probe_roms(void)
 		start = adapter_rom_resources[i++].end & ~2047UL;
 	}
 }
-#endif
 
 /*
  * Point at the empty zero page to start with. We map the real shared_info
@@ -1359,9 +1357,7 @@ legacy_init_iomem_resources(struct e820e
 {
 	int i;
 
-#if defined(CONFIG_XEN_PRIVILEGED_GUEST) || !defined(CONFIG_XEN)
 	probe_roms();
-#endif
 
 	for (i = 0; i < nr_map; i++) {
 		struct resource *res;
Index: 2006-08-16/arch/i386/pci/irq-xen.c
===================================================================
--- 2006-08-16.orig/arch/i386/pci/irq-xen.c	2006-02-06 09:28:08.000000000 +0100
+++ 2006-08-16/arch/i386/pci/irq-xen.c	2006-08-18 13:48:40.000000000 +0200
@@ -95,7 +95,10 @@ static struct irq_routing_table * __init
 	u8 *addr;
 	struct irq_routing_table *rt;
 
-#ifdef CONFIG_XEN_PRIVILEGED_GUEST
+#ifdef CONFIG_XEN
+	if (!is_initial_xendomain())
+		return NULL;
+#endif
 	if (pirq_table_addr) {
 		rt = pirq_check_routing_table((u8 *) isa_bus_to_virt(pirq_table_addr));
 		if (rt)
@@ -107,7 +110,6 @@ static struct irq_routing_table * __init
 		if (rt)
 			return rt;
 	}
-#endif
 	
 	return NULL;
 }
Index: 2006-08-16/arch/x86_64/kernel/genapic_xen.c
===================================================================
--- 2006-08-16.orig/arch/x86_64/kernel/genapic_xen.c	2006-02-01 18:28:39.000000000 +0100
+++ 2006-08-16/arch/x86_64/kernel/genapic_xen.c	2006-08-18 13:51:08.000000000 +0200
@@ -17,14 +17,8 @@
 #include <linux/kernel.h>
 #include <linux/ctype.h>
 #include <linux/init.h>
-#ifdef CONFIG_XEN_PRIVILEGED_GUEST
 #include <asm/smp.h>
 #include <asm/ipi.h>
-#else
-#include <asm/apic.h>
-#include <asm/apicdef.h>
-#include <asm/genapic.h>
-#endif
 #include <xen/evtchn.h>
 
 DECLARE_PER_CPU(int, ipi_to_irq[NR_IPIS]);
@@ -118,14 +112,12 @@ static void xen_send_IPI_mask(cpumask_t 
 	local_irq_restore(flags);
 }
 
-#ifdef CONFIG_XEN_PRIVILEGED_GUEST
 static int xen_apic_id_registered(void)
 {
 	/* better be set */
 	Dprintk("%s\n", __FUNCTION__);
 	return physid_isset(smp_processor_id(), phys_cpu_present_map);
 }
-#endif
 
 static unsigned int xen_cpu_mask_to_apicid(cpumask_t cpumask)
 {
@@ -144,15 +136,11 @@ static unsigned int phys_pkg_id(int inde
 
 struct genapic apic_xen =  {
 	.name = "xen",
-#ifdef CONFIG_XEN_PRIVILEGED_GUEST
 	.int_delivery_mode = dest_LowestPrio,
-#endif
 	.int_dest_mode = (APIC_DEST_LOGICAL != 0),
 	.int_delivery_dest = APIC_DEST_LOGICAL | APIC_DM_LOWEST,
 	.target_cpus = xen_target_cpus,
-#ifdef CONFIG_XEN_PRIVILEGED_GUEST
 	.apic_id_registered = xen_apic_id_registered,
-#endif
 	.init_apic_ldr = xen_init_apic_ldr,
 	.send_IPI_all = xen_send_IPI_all,
 	.send_IPI_allbutself = xen_send_IPI_allbutself,
Index: 2006-08-16/arch/x86_64/kernel/setup-xen.c
===================================================================
--- 2006-08-16.orig/arch/x86_64/kernel/setup-xen.c	2006-08-16 08:58:12.000000000 +0200
+++ 2006-08-16/arch/x86_64/kernel/setup-xen.c	2006-08-18 13:53:39.000000000 +0200
@@ -189,7 +189,6 @@ struct resource code_resource = {
 
 #define IORESOURCE_ROM (IORESOURCE_BUSY | IORESOURCE_READONLY | IORESOURCE_MEM)
 
-#if defined(CONFIG_XEN_PRIVILEGED_GUEST) || !defined(CONFIG_XEN)
 static struct resource system_rom_resource = {
 	.name = "System ROM",
 	.start = 0xf0000,
@@ -218,19 +217,16 @@ static struct resource adapter_rom_resou
 	{ .name = "Adapter ROM", .start = 0, .end = 0,
 		.flags = IORESOURCE_ROM }
 };
-#endif
 
 #define ADAPTER_ROM_RESOURCES \
 	(sizeof adapter_rom_resources / sizeof adapter_rom_resources[0])
 
-#if defined(CONFIG_XEN_PRIVILEGED_GUEST) || !defined(CONFIG_XEN)
 static struct resource video_rom_resource = {
 	.name = "Video ROM",
 	.start = 0xc0000,
 	.end = 0xc7fff,
 	.flags = IORESOURCE_ROM,
 };
-#endif
 
 static struct resource video_ram_resource = {
 	.name = "Video RAM area",
@@ -239,7 +235,6 @@ static struct resource video_ram_resourc
 	.flags = IORESOURCE_RAM,
 };
 
-#if defined(CONFIG_XEN_PRIVILEGED_GUEST) || !defined(CONFIG_XEN)
 #define romsignature(x) (*(unsigned short *)(x) == 0xaa55)
 
 static int __init romchecksum(unsigned char *rom, unsigned long length)
@@ -257,6 +252,12 @@ static void __init probe_roms(void)
 	unsigned char *rom;
 	int	      i;
 
+#ifdef CONFIG_XEN
+	/* Nothing to do if not running in dom0. */
+	if (!is_initial_xendomain())
+		return;
+#endif
+
 	/* video rom */
 	upper = adapter_rom_resources[0].start;
 	for (start = video_rom_resource.start; start < upper; start += 2048) {
@@ -315,7 +316,6 @@ static void __init probe_roms(void)
 		start = adapter_rom_resources[i++].end & ~2047UL;
 	}
 }
-#endif
 
 static __init void parse_cmdline_early (char ** cmdline_p)
 {
@@ -625,11 +625,8 @@ static void __init reserve_ebda_region(v
 void __init setup_arch(char **cmdline_p)
 {
 	unsigned long kernel_end;
-
-#if defined(CONFIG_XEN_PRIVILEGED_GUEST)
 	struct e820entry *machine_e820;
 	struct xen_memory_map memmap;
-#endif
 
 #ifdef CONFIG_XEN
 	/* Register a call for panic conditions. */
@@ -936,8 +933,8 @@ void __init setup_arch(char **cmdline_p)
 	 * Request address space for all standard RAM and ROM resources
 	 * and also for regions reported as reserved by the e820.
 	 */
-#if defined(CONFIG_XEN_PRIVILEGED_GUEST)
 	probe_roms();
+#ifdef CONFIG_XEN
 	if (is_initial_xendomain()) {
 		machine_e820 = alloc_bootmem_low_pages(PAGE_SIZE);
 
@@ -948,13 +945,8 @@ void __init setup_arch(char **cmdline_p)
 
 		e820_reserve_resources(machine_e820, memmap.nr_entries);
 	} else
-		e820_reserve_resources(e820.map, e820.nr_map);
-#elif defined(CONFIG_XEN)
-	e820_reserve_resources(e820.map, e820.nr_map);
-#else
-	probe_roms();
-	e820_reserve_resources(e820.map, e820.nr_map);
 #endif
+	e820_reserve_resources(e820.map, e820.nr_map);
 
 	request_resource(&iomem_resource, &video_ram_resource);
 
@@ -965,12 +957,12 @@ void __init setup_arch(char **cmdline_p)
 		request_resource(&ioport_resource, &standard_io_resources[i]);
 	}
 
-#if defined(CONFIG_XEN_PRIVILEGED_GUEST)
+#ifdef CONFIG_XEN
 	if (is_initial_xendomain()) {
 		e820_setup_gap(machine_e820, memmap.nr_entries);
 		free_bootmem(__pa(machine_e820), PAGE_SIZE);
 	}
-#elif !defined(CONFIG_XEN)
+#else
 	e820_setup_gap(e820.map, e820.nr_map);
 #endif
 
Index: 2006-08-16/drivers/xen/privcmd/privcmd.c
===================================================================
--- 2006-08-16.orig/drivers/xen/privcmd/privcmd.c	2006-08-16 08:58:12.000000000 +0200
+++ 2006-08-16/drivers/xen/privcmd/privcmd.c	2006-08-18 13:48:41.000000000 +0200
@@ -108,7 +108,6 @@ static int privcmd_ioctl(struct inode *i
 	}
 	break;
 
-#if defined(CONFIG_XEN_PRIVILEGED_GUEST)
 	case IOCTL_PRIVCMD_MMAP: {
 #define PRIVCMD_MMAP_SZ 32
 		privcmd_mmap_t mmapcmd;
@@ -116,6 +115,9 @@ static int privcmd_ioctl(struct inode *i
 		privcmd_mmap_entry_t __user *p;
 		int i, rc;
 
+		if (!is_initial_xendomain())
+			return -EPERM;
+
 		if (copy_from_user(&mmapcmd, udata, sizeof(mmapcmd)))
 			return -EFAULT;
 
@@ -165,6 +167,9 @@ static int privcmd_ioctl(struct inode *i
 		unsigned long addr, mfn; 
 		int i;
 
+		if (!is_initial_xendomain())
+			return -EPERM;
+
 		if (copy_from_user(&m, udata, sizeof(m))) {
 			ret = -EFAULT;
 			goto batch_err;
@@ -215,7 +220,6 @@ static int privcmd_ioctl(struct inode *i
 		break;
 	}
 	break;
-#endif
 
 	default:
 		ret = -EINVAL;
Index: 2006-08-16/include/asm-i386/mach-xen/asm/hypervisor.h
===================================================================
--- 2006-08-16.orig/include/asm-i386/mach-xen/asm/hypervisor.h	2006-08-16 08:58:12.000000000 +0200
+++ 2006-08-16/include/asm-i386/mach-xen/asm/hypervisor.h	2006-08-18 13:48:41.000000000 +0200
@@ -58,7 +58,11 @@ extern shared_info_t *HYPERVISOR_shared_
 
 /* arch/xen/i386/kernel/setup.c */
 extern start_info_t *xen_start_info;
+#ifdef CONFIG_XEN_PRIVILEGED_GUEST
 #define is_initial_xendomain() (xen_start_info->flags & SIF_INITDOMAIN)
+#else
+#define is_initial_xendomain() 0
+#endif
 
 /* arch/xen/kernel/evtchn.c */
 /* Force a proper event-channel callback from Xen. */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: is_initial_xendomain()

[Ian Campbell]

On Fri, 2006-08-18 at 14:15 +0200, Jan Beulich wrote:
> Okay, here's the patch, leaving just a single occurrence of CONFIG_XEN_PRIVILEGED_GUEST.
> (Testing was done on 2.6.18-rc4, and I hope there's nothing wrong with the patch as adapted
> to 2.6.16.13).

Unfortunately the changes to arch/x86_64/kernel/genapic_xen.c broke the
-xenU build. I've reverted just that bit for now until we/I figure out
the correct fix.

Perhaps we want to remove the dependence of CONFIG_X86_LOCAL_APIC on !
XEN_UNPRIVILEGED_GUEST and make sure we correctly gate the relevant
functions with is_initial_xendomain()? The same goes for x86/32 as well
but it was saved from the build breakage by the mach-xen subarch thing.

Ian.