[PATCH 1/2] Remove memory size from linker script

[PATCH 1/2] Remove memory size from linker script

[Anthony Liguori]

The memory size is currently hardcoded into the linker script (end_of_memory).
This prevents the memory size from being specified dynamically in kvmctl.
This patch adds a PIO port that can be used to query the memory size in the
tests.

Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

diff --git a/user/flat.lds b/user/flat.lds
index 7dd922c..61f1057 100644
--- a/user/flat.lds
+++ b/user/flat.lds
@@ -13,6 +13,5 @@ SECTIONS
     .bss : { *(.bss) }
     . = ALIGN(4K);
     edata = .;
-    end_of_memory = 128M;
 }
 
diff --git a/user/main.c b/user/main.c
index 3eb8dea..9a57a24 100644
--- a/user/main.c
+++ b/user/main.c
@@ -144,7 +144,15 @@ static int test_inl(void *opaque, uint16_t addr, uint32_t *value)
 {
 	if (apic_io(addr, 0, value))
 		return 0;
-	printf("inl 0x%x\n", addr);
+
+	switch (addr) {
+	case 0xd1:
+		*value = 128 * 1024 * 1024;
+		break;
+	default:
+		printf("inl 0x%x\n", addr);
+		break;
+	}
 	return 0;
 }
 
diff --git a/user/test/vm.c b/user/test/vm.c
index 2eb8d96..e0e78d7 100644
--- a/user/test/vm.c
+++ b/user/test/vm.c
@@ -58,7 +58,8 @@ void free_page(void *page)
     free = page;
 }
 
-extern char edata, end_of_memory;
+extern char edata;
+static unsigned long end_of_memory;
 
 #define PTE_PRESENT (1ull << 0)
 #define PTE_PSE     (1ull << 7)
@@ -202,10 +203,18 @@ static void setup_mmu(unsigned long len)
     print("paging enabled\n");
 }
 
+static unsigned int inl(unsigned short port)
+{
+    unsigned int val;
+    asm volatile("inl %w1, %0" : "=a"(val) : "Nd"(port));
+    return val;
+}
+
 void setup_vm()
 {
-    free_memory(&edata, &end_of_memory - &edata);
-    setup_mmu((long)&end_of_memory);
+    end_of_memory = inl(0xd1);
+    free_memory(&edata, end_of_memory - (unsigned long)&edata);
+    setup_mmu(end_of_memory);
 }
 
 void *vmalloc(unsigned long size)

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

[PATCH 2/2] Allow memory to be specified in kvmctl

[Anthony Liguori]

This patch adds a --memory option to kvmctl to allow the memory size of the
guest to be specified.

Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

diff --git a/user/main.c b/user/main.c
index 9a57a24..7fc0924 100644
--- a/user/main.c
+++ b/user/main.c
@@ -57,6 +57,7 @@ static __thread int vcpu;
 static int apic_ipi_vector = 0xff;
 static sigset_t kernel_sigmask;
 static sigset_t ipi_sigmask;
+static uint64_t memory_size = 128 * 1024 * 1024;
 
 struct vcpu_info {
 	pid_t tid;
@@ -147,7 +148,7 @@ static int test_inl(void *opaque, uint16_t addr, uint32_t *value)
 
 	switch (addr) {
 	case 0xd1:
-		*value = 128 * 1024 * 1024;
+		*value = memory_size;
 		break;
 	default:
 		printf("inl 0x%x\n", addr);
@@ -328,14 +329,16 @@ static void start_vcpu(int n)
 static void usage(const char *progname)
 {
 	fprintf(stderr,
-		"Usage: %s [OPTIONS] [bootstrap] flatfile\n"
-		"KVM test harness.\n"
-		"\n"
-		"  -s, --smp=NUM          create a VM with NUM virtual CPUs\n"
-		"  -p, --protected-mode   start VM in protected mode\n"
-		"  -h, --help             display this help screen and exit\n"
-		"\n"
-		"Report bugs to <kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>.\n"
+"Usage: %s [OPTIONS] [bootstrap] flatfile\n"
+"KVM test harness.\n"
+"\n"
+"  -s, --smp=NUM          create a VM with NUM virtual CPUs\n"
+"  -p, --protected-mode   start VM in protected mode\n"
+"  -m, --memory=NUM[GMKB] allocate NUM memory for virtual machine.  A suffix\n"
+"                         can be used to change the unit (default: `M')\n"
+"  -h, --help             display this help screen and exit\n"
+"\n"
+"Report bugs to <kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>.\n"
 		, progname);
 }
 
@@ -348,16 +351,18 @@ int main(int argc, char **argv)
 {
 	void *vm_mem;
 	int i;
-	const char *sopts = "s:ph";
+	const char *sopts = "s:phm:";
 	struct option lopts[] = {
 		{ "smp", 1, 0, 's' },
 		{ "protected-mode", 0, 0, 'p' },
+		{ "memory", 1, 0, 'm' },
 		{ "help", 0, 0, 'h' },
 		{ 0 },
 	};
 	int opt_ind, ch;
 	bool enter_protected_mode = false;
 	int nb_args;
+	char *endptr;
 
 	while ((ch = getopt_long(argc, argv, sopts, lopts, &opt_ind)) != -1) {
 		switch (ch) {
@@ -367,6 +372,24 @@ int main(int argc, char **argv)
 		case 'p':
 			enter_protected_mode = true;
 			break;
+		case 'm':
+			memory_size = strtoull(optarg, &endptr, 0);
+			switch (*endptr) {
+			case 'G': case 'g':
+				memory_size <<= 10;
+			case '\0':
+			case 'M': case 'm':
+				memory_size <<= 10;
+			case 'K': case 'k':
+				memory_size <<= 10;
+				break;
+			default:
+				fprintf(stderr,
+					"Unrecongized memory suffix: %c\n",
+					*endptr);
+				exit(1);
+			}
+			break;
 		case 'h':
 			usage(argv[0]);
 			exit(0);
@@ -401,7 +424,7 @@ int main(int argc, char **argv)
 		fprintf(stderr, "kvm_init failed\n");
 		return 1;
 	}
-	if (kvm_create(kvm, 128 * 1024 * 1024, &vm_mem) < 0) {
+	if (kvm_create(kvm, memory_size, &vm_mem) < 0) {
 		kvm_finalize(kvm);
 		fprintf(stderr, "kvm_create failed\n");
 		return 1;

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH 2/2] Allow memory to be specified in kvmctl

[Muli Ben-Yehuda]

On Tue, Oct 09, 2007 at 04:31:54PM -0500, Anthony Liguori wrote:

> This patch adds a --memory option to kvmctl to allow the memory size of the
> guest to be specified.
> 
> Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> 
> diff --git a/user/main.c b/user/main.c
> index 9a57a24..7fc0924 100644
> --- a/user/main.c
> +++ b/user/main.c
> @@ -57,6 +57,7 @@ static __thread int vcpu;
>  static int apic_ipi_vector = 0xff;
>  static sigset_t kernel_sigmask;
>  static sigset_t ipi_sigmask;
> +static uint64_t memory_size = 128 * 1024 * 1024;
>  
>  struct vcpu_info {
>  	pid_t tid;
> @@ -147,7 +148,7 @@ static int test_inl(void *opaque, uint16_t addr, uint32_t *value)
>  
>  	switch (addr) {
>  	case 0xd1:
> -		*value = 128 * 1024 * 1024;
> +		*value = memory_size;
>  		break;
>  	default:
>  		printf("inl 0x%x\n", addr);
> @@ -328,14 +329,16 @@ static void start_vcpu(int n)
>  static void usage(const char *progname)
>  {
>  	fprintf(stderr,
> -		"Usage: %s [OPTIONS] [bootstrap] flatfile\n"
> -		"KVM test harness.\n"
> -		"\n"
> -		"  -s, --smp=NUM          create a VM with NUM virtual CPUs\n"
> -		"  -p, --protected-mode   start VM in protected mode\n"
> -		"  -h, --help             display this help screen and exit\n"
> -		"\n"
> -		"Report bugs to <kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>.\n"
> +"Usage: %s [OPTIONS] [bootstrap] flatfile\n"
> +"KVM test harness.\n"
> +"\n"
> +"  -s, --smp=NUM          create a VM with NUM virtual CPUs\n"
> +"  -p, --protected-mode   start VM in protected mode\n"
> +"  -m, --memory=NUM[GMKB] allocate NUM memory for virtual machine.  A suffix\n"
> +"                         can be used to change the unit (default: `M')\n"
> +"  -h, --help             display this help screen and exit\n"
> +"\n"
> +"Report bugs to <kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>.\n"
>  		, progname);
>  }
>  
> @@ -348,16 +351,18 @@ int main(int argc, char **argv)
>  {
>  	void *vm_mem;
>  	int i;
> -	const char *sopts = "s:ph";
> +	const char *sopts = "s:phm:";
>  	struct option lopts[] = {
>  		{ "smp", 1, 0, 's' },
>  		{ "protected-mode", 0, 0, 'p' },
> +		{ "memory", 1, 0, 'm' },
>  		{ "help", 0, 0, 'h' },
>  		{ 0 },
>  	};
>  	int opt_ind, ch;
>  	bool enter_protected_mode = false;
>  	int nb_args;
> +	char *endptr;
>  
>  	while ((ch = getopt_long(argc, argv, sopts, lopts, &opt_ind)) != -1) {
>  		switch (ch) {
> @@ -367,6 +372,24 @@ int main(int argc, char **argv)
>  		case 'p':
>  			enter_protected_mode = true;
>  			break;
> +		case 'm':
> +			memory_size = strtoull(optarg, &endptr, 0);
> +			switch (*endptr) {
> +			case 'G': case 'g':
> +				memory_size <<= 10;
> +			case '\0':
> +			case 'M': case 'm':
> +				memory_size <<= 10;
> +			case 'K': case 'k':
> +				memory_size <<= 10;
> +				break;

Cute trick with the fall-through and shifts... not quite Duff's
device, but cute. Please consider adding a /* fallthrough */ comment
to make it obvious.

> +			default:
> +				fprintf(stderr,
> +					"Unrecongized memory suffix: %c\n",
> +					*endptr);
> +				exit(1);
> +			}
> +			break;

How about adding a sanity check that memory_size makes sense here
rather than having kvm_create() fail obscurely? For example if the
user got the memory size wrong for some reason we'll end up with
memory_size = 0 here.

Cheers,
Muli
-- 
SYSTOR 2007 --- 1st Annual Haifa Systems and Storage Conference 2007
http://www.haifa.il.ibm.com/Workshops/systor2007/

Virtualization workshop: Oct 29th, 2007 | Storage workshop: Oct 30th, 2007

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH 2/2] Allow memory to be specified in kvmctl

[Anthony Liguori]

Muli Ben-Yehuda wrote:
>>  	while ((ch = getopt_long(argc, argv, sopts, lopts, &opt_ind)) != -1) {
>>  		switch (ch) {
>> @@ -367,6 +372,24 @@ int main(int argc, char **argv)
>>  		case 'p':
>>  			enter_protected_mode = true;
>>  			break;
>> +		case 'm':
>> +			memory_size = strtoull(optarg, &endptr, 0);
>> +			switch (*endptr) {
>> +			case 'G': case 'g':
>> +				memory_size <<= 10;
>> +			case '\0':
>> +			case 'M': case 'm':
>> +				memory_size <<= 10;
>> +			case 'K': case 'k':
>> +				memory_size <<= 10;
>> +				break;
>>     
>
> Cute trick with the fall-through and shifts... not quite Duff's
> device, but cute. Please consider adding a /* fallthrough */ comment
> to make it obvious.
>   

I'll make it more clear and send out the series again tomorrow when 
others have gotten a chance to review.

>> +			default:
>> +				fprintf(stderr,
>> +					"Unrecongized memory suffix: %c\n",
>> +					*endptr);
>> +				exit(1);
>> +			}
>> +			break;
>>     
>
> How about adding a sanity check that memory_size makes sense here
> rather than having kvm_create() fail obscurely? For example if the
> user got the memory size wrong for some reason we'll end up with
> memory_size = 0 here.
>   

There's an exit(1) and it's using stroull() so the only way that 
memory_size could equal 0 is if the user specified --memory=0.  I'm not 
sure I agree it's worth checking for that sort of circumstance, perhaps 
the user had a reason for doing it?

Regards,

Anthony Liguori

> Cheers,
> Muli
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH 2/2] Allow memory to be specified in kvmctl

[Muli Ben-Yehuda]

On Tue, Oct 09, 2007 at 05:09:19PM -0500, Anthony Liguori wrote:

> >>+			default:
> >>+				fprintf(stderr,
> >>+					"Unrecongized memory suffix: %c\n",
> >>+					*endptr);
> >>+				exit(1);
> >>+			}
> >>+			break;
> >>    
> >
> >How about adding a sanity check that memory_size makes sense here
> >rather than having kvm_create() fail obscurely? For example if the
> >user got the memory size wrong for some reason we'll end up with
> >memory_size = 0 here.
> 
> There's an exit(1) and it's using stroull() so the only way that
> memory_size could equal 0 is if the user specified --memory=0.  I'm

Or made a mistake and specified memory=G, or memory=100G when he meant
100M, etc.

> not sure I agree it's worth checking for that sort of circumstance,
> perhaps the user had a reason for doing it?

I'm a fan of the principle of least surprise.

Cheers,
Muli
-- 
SYSTOR 2007 --- 1st Annual Haifa Systems and Storage Conference 2007
http://www.haifa.il.ibm.com/Workshops/systor2007/

Virtualization workshop: Oct 29th, 2007 | Storage workshop: Oct 30th, 2007

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH 2/2] Allow memory to be specified in kvmctl

[Anthony Liguori]

Muli Ben-Yehuda wrote:
> On Tue, Oct 09, 2007 at 05:09:19PM -0500, Anthony Liguori wrote:
>
>   
>>>> +			default:
>>>> +				fprintf(stderr,
>>>> +					"Unrecongized memory suffix: %c\n",
>>>> +					*endptr);
>>>> +				exit(1);
>>>> +			}
>>>> +			break;
>>>>    
>>>>         
>>> How about adding a sanity check that memory_size makes sense here
>>> rather than having kvm_create() fail obscurely? For example if the
>>> user got the memory size wrong for some reason we'll end up with
>>> memory_size = 0 here.
>>>       
>> There's an exit(1) and it's using stroull() so the only way that
>> memory_size could equal 0 is if the user specified --memory=0.  I'm
>>     
>
> Or made a mistake and specified memory=G, or memory=100G when he meant
> 100M, etc.
>   

Okay, in that case, I agree.  Have updated my patch.

Regards,

Anthony Liguori

>> not sure I agree it's worth checking for that sort of circumstance,
>> perhaps the user had a reason for doing it?
>>     
>
> I'm a fan of the principle of least surprise.
>
> Cheers,
> Muli
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/