[Qemu-devel] [PATCH 1/1] Make vmport an optional feature at run time.

[Qemu-devel] [PATCH 1/1] Make vmport an optional feature at run time.

[Ian Kirk]

Hi,

This is my first time sending a patch, so apologies for any errors.

Patch makes vmport optionally initiated.

---
 hw/pc.c |    3 ++-
 hw/pc.h |    1 +
 vl.c    |    7 +++++++
 3 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 213ead8..56e1641 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -774,7 +774,8 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
         }
     }
 
-    vmport_init();
+    if (!no_vmport)
+        vmport_init();
 
     /* allocate RAM */
     ram_addr = qemu_ram_alloc(ram_size);
diff --git a/hw/pc.h b/hw/pc.h
index 2862849..b19d626 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -84,6 +84,7 @@ void rtc_set_date(RTCState *s, const struct tm *tm);
 
 /* pc.c */
 extern int fd_bootchk;
+extern int no_vmport;
 
 void ioport_set_a20(int enable);
 int ioport_get_a20(void);
diff --git a/vl.c b/vl.c
index 7ca8420..21012bf 100644
--- a/vl.c
+++ b/vl.c
@@ -217,6 +217,7 @@ const char *vnc_display;
 #endif
 int acpi_enabled = 1;
 int fd_bootchk = 1;
+int no_vmport = 0;
 int no_reboot = 0;
 int no_shutdown = 0;
 int cursor_hide = 1;
@@ -7664,6 +7665,7 @@ static void help(int exitcode)
            "-pflash file    use 'file' as a parallel flash image\n"
            "-boot [a|c|d|n] boot on floppy (a), hard disk (c), CD-ROM (d), or network (n)\n"
            "-snapshot       write to temporary files instead of disk image files\n"
+           "-no-vmport      disable vmport\n"
 #ifdef CONFIG_SDL
            "-no-frame       open SDL window without a frame and window decorations\n"
            "-alt-grab       use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt)\n"
@@ -7825,6 +7827,7 @@ enum {
     QEMU_OPTION_pflash,
     QEMU_OPTION_boot,
     QEMU_OPTION_snapshot,
+    QEMU_OPTION_no_vmport,
 #ifdef TARGET_I386
     QEMU_OPTION_no_fd_bootchk,
 #endif
@@ -7918,6 +7921,7 @@ const QEMUOption qemu_options[] = {
     { "pflash", HAS_ARG, QEMU_OPTION_pflash },
     { "boot", HAS_ARG, QEMU_OPTION_boot },
     { "snapshot", 0, QEMU_OPTION_snapshot },
+    { "no-vmport", 0, QEMU_OPTION_no_vmport },
 #ifdef TARGET_I386
     { "no-fd-bootchk", 0, QEMU_OPTION_no_fd_bootchk },
 #endif
@@ -8411,6 +8415,9 @@ int main(int argc, char **argv)
             case QEMU_OPTION_pflash:
                 drive_add(optarg, PFLASH_ALIAS);
                 break;
+            case QEMU_OPTION_no_vmport:
+                no_vmport = 1;
+                break;
             case QEMU_OPTION_snapshot:
                 snapshot = 1;
                 break;
-- 
1.5.4.1

Re: [Qemu-devel] [PATCH 1/1] Make vmport an optional feature at run time.

[Anthony Liguori]

Ian Kirk wrote:
> Hi,
>
> This is my first time sending a patch, so apologies for any errors.
>
> Patch makes vmport optionally initiated.
>   

Why should it be optional?

Regards,

Anthony Liguori

> ---
>  hw/pc.c |    3 ++-
>  hw/pc.h |    1 +
>  vl.c    |    7 +++++++
>  3 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 213ead8..56e1641 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -774,7 +774,8 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>          }
>      }
>  
> -    vmport_init();
> +    if (!no_vmport)
> +        vmport_init();
>  
>      /* allocate RAM */
>      ram_addr = qemu_ram_alloc(ram_size);
> diff --git a/hw/pc.h b/hw/pc.h
> index 2862849..b19d626 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -84,6 +84,7 @@ void rtc_set_date(RTCState *s, const struct tm *tm);
>  
>  /* pc.c */
>  extern int fd_bootchk;
> +extern int no_vmport;
>  
>  void ioport_set_a20(int enable);
>  int ioport_get_a20(void);
> diff --git a/vl.c b/vl.c
> index 7ca8420..21012bf 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -217,6 +217,7 @@ const char *vnc_display;
>  #endif
>  int acpi_enabled = 1;
>  int fd_bootchk = 1;
> +int no_vmport = 0;
>  int no_reboot = 0;
>  int no_shutdown = 0;
>  int cursor_hide = 1;
> @@ -7664,6 +7665,7 @@ static void help(int exitcode)
>             "-pflash file    use 'file' as a parallel flash image\n"
>             "-boot [a|c|d|n] boot on floppy (a), hard disk (c), CD-ROM (d), or network (n)\n"
>             "-snapshot       write to temporary files instead of disk image files\n"
> +           "-no-vmport      disable vmport\n"
>  #ifdef CONFIG_SDL
>             "-no-frame       open SDL window without a frame and window decorations\n"
>             "-alt-grab       use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt)\n"
> @@ -7825,6 +7827,7 @@ enum {
>      QEMU_OPTION_pflash,
>      QEMU_OPTION_boot,
>      QEMU_OPTION_snapshot,
> +    QEMU_OPTION_no_vmport,
>  #ifdef TARGET_I386
>      QEMU_OPTION_no_fd_bootchk,
>  #endif
> @@ -7918,6 +7921,7 @@ const QEMUOption qemu_options[] = {
>      { "pflash", HAS_ARG, QEMU_OPTION_pflash },
>      { "boot", HAS_ARG, QEMU_OPTION_boot },
>      { "snapshot", 0, QEMU_OPTION_snapshot },
> +    { "no-vmport", 0, QEMU_OPTION_no_vmport },
>  #ifdef TARGET_I386
>      { "no-fd-bootchk", 0, QEMU_OPTION_no_fd_bootchk },
>  #endif
> @@ -8411,6 +8415,9 @@ int main(int argc, char **argv)
>              case QEMU_OPTION_pflash:
>                  drive_add(optarg, PFLASH_ALIAS);
>                  break;
> +            case QEMU_OPTION_no_vmport:
> +                no_vmport = 1;
> +                break;
>              case QEMU_OPTION_snapshot:
>                  snapshot = 1;
>                  break;
>   

Re: [Qemu-devel] [PATCH 1/1] Make vmport an optional feature at run time.

[Ian Kirk]

Anthony Liguori wrote:

> Ian Kirk wrote:
> > Hi,
> >
> > This is my first time sending a patch, so apologies for any errors.
> >
> > Patch makes vmport optionally initiated.
> >
>
> Why should it be optional?

I believe that VMware ESXi (and perhaps other hypervisors/emulators/etc)
doesn't work when it thinks it is running within virtual enviroment, as it
talks to vmport when booting (and fails in the current implemtnation -
perhaps it only works under itself?)

If I comment init_vmport() out, it definately progresses further along the
boot sequence.

Also, I guess, it gives you the option to better emulate a real PC (which
I assume doesn't have vmport).

Re: [Qemu-devel] [PATCH 1/1] Make vmport an optional feature at run time.

[Anthony Liguori]

Ian Kirk wrote:
> Anthony Liguori wrote:
>
>   
>> Ian Kirk wrote:
>>     
>>> Hi,
>>>
>>> This is my first time sending a patch, so apologies for any errors.
>>>
>>> Patch makes vmport optionally initiated.
>>>
>>>       
>> Why should it be optional?
>>     
>
> I believe that VMware ESXi (and perhaps other hypervisors/emulators/etc)
> doesn't work when it thinks it is running within virtual enviroment, as it
> talks to vmport when booting (and fails in the current implemtnation -
> perhaps it only works under itself?)
>   

That's most likely because the vmport emulation isn't complete enough.

> If I comment init_vmport() out, it definately progresses further along the
> boot sequence.
>
> Also, I guess, it gives you the option to better emulate a real PC (which
> I assume doesn't have vmport).
>   

I don't think that's very valuable in and of itself.  Moreover, there 
are probably a lot more issues with respect to getting ESXi to run under 
QEMU.  Adding another command line option to support something that we 
don't know will ever work worries me.  It's just another knob for 
someone to accidentally tweak.

Regards,

Anthony Liguori

[Qemu-devel] [PATCH] 1/1: vmport update

[Todd T. Fries]

Just incase a 'more complete' vmport codeset is useful, here is my
stab at extending it.  I haven't extended it far enough yet that
OpenBSD's vmt(4) interface works, but would be more than happy
if someone else were to carry this flag and run with it..


--- hw/vmport.c.orig	Thu Jul 17 22:45:31 2008
+++ hw/vmport.c	Fri Jul 18 08:23:50 2008
@@ -26,12 +26,29 @@
 #include "pc.h"
 #include "sysemu.h"
 
+/* Port numbers */
+#define VMPORT_CMD		0x5658
+#define VMPORT_RPC		0x5659
+
 #define VMPORT_CMD_GETVERSION 0x0a
 #define VMPORT_CMD_GETRAMSIZE 0x14
+#define VMPORT_CMD_GETTIME    0x17
+#define VMPORT_CMD_RPC        0x1e
 
 #define VMPORT_ENTRIES 0x2c
 #define VMPORT_MAGIC   0x564D5868
 
+#define VMPORTRPC_ENTRIES 0x07
+
+/* RPC sub-commands */
+#define VMPORT_RPC_OPEN		0x00
+#define VMPORT_RPC_SET_LENGTH	0x01
+#define VMPORT_RPC_SET_DATA	0x02
+#define VMPORT_RPC_GET_LENGTH	0x03
+#define VMPORT_RPC_GET_DATA	0x04
+#define VMPORT_RPC_GET_END	0x05
+#define VMPORT_RPC_CLOSE	0x06
+
 typedef struct _VMPortState
 {
     CPUState *env;
@@ -41,6 +58,15 @@ typedef struct _VMPortState
 
 static VMPortState port_state;
 
+typedef struct _VMPortRpc
+{
+    CPUState *env;
+    IOPortReadFunc *func[VMPORTRPC_ENTRIES];
+    void *opaque[VMPORTRPC_ENTRIES];
+} VMPortRpc;
+
+static VMPortState port_rpc;
+
 void vmport_register(unsigned char command, IOPortReadFunc *func, void *opaque)
 {
     if (command >= VMPORT_ENTRIES)
@@ -60,18 +86,36 @@ static uint32_t vmport_ioport_read(void *opaque, uint3
     if (eax != VMPORT_MAGIC)
         return eax;
 
-    command = s->env->regs[R_ECX];
+    command = s->env->regs[R_ECX] & 0xff;
     if (command >= VMPORT_ENTRIES)
         return eax;
     if (!s->func[command])
     {
-        printf("vmport: unknown command %x\n", command);
+        printf("\nqemu vmport: unknown command %x\n", command);
         return eax;
     }
 
     return s->func[command](s->opaque[command], addr);
 }
 
+static uint32_t vmport_rpc_read(void *opaque, uint32_t addr)
+{
+    VMPortRpc *s = opaque;
+    unsigned char command;
+    uint32_t eax;
+    int length;
+
+    eax = s->env->regs[R_EAX];
+    length = s->env->regs[R_ECX];
+    printf("\nqemu vmport_rpc_read: eax=0x%x, length=%d\n",eax,length);
+    if (length > 1) {
+	char *data = (char *) (s->env->regs[R_EDI]);
+	data[0]='1';
+	data[1]=' ';
+    }
+    return eax;
+}
+
 static uint32_t vmport_cmd_get_version(void *opaque, uint32_t addr)
 {
     CPUState *env = opaque;
@@ -79,6 +123,16 @@ static uint32_t vmport_cmd_get_version(void *opaque, u
     return 6;
 }
 
+static uint32_t vmport_cmd_get_time(void *opaque, uint32_t addr)
+{
+    CPUState *env = opaque;
+    struct timeval tv;
+    gettimeofday(&tv, NULL);
+    env->regs[R_EAX] = tv.tv_sec;
+    env->regs[R_EBX] = tv.tv_usec;
+    return tv.tv_sec;
+}
+
 static uint32_t vmport_cmd_ram_size(void *opaque, uint32_t addr)
 {
     CPUState *env = opaque;
@@ -86,13 +140,45 @@ static uint32_t vmport_cmd_ram_size(void *opaque, uint
     return ram_size;
 }
 
+static uint32_t vmport_cmd_rpc(void *opaque, uint32_t addr)
+{
+    CPUState *env = opaque;
+    unsigned char command;
+
+    command = env->regs[R_ECX] >> 16;
+    printf("\nqemu vmport_cmd_rpc: command %d\n",command);
+    switch (command) {
+    case VMPORT_RPC_OPEN:
+	env->regs[R_EAX] = 0x0;
+	env->regs[R_ECX] = 0x10000;
+	env->regs[R_EDX] = 0x10000; /* channel 1 */
+	env->regs[R_ESI] = 0x1234; /* cookie1 */
+	env->regs[R_EDI] = 0x5678; /* cookie2 */
+	break;
+    case VMPORT_RPC_CLOSE:
+    case VMPORT_RPC_SET_LENGTH:
+    case VMPORT_RPC_GET_LENGTH:
+    case VMPORT_RPC_GET_END:
+	env->regs[R_EAX] = 0x0;
+	env->regs[R_ECX] = 0x10000;
+	break;
+    default:
+    	printf("\nqemu vmport_cmd_rpc: got unknown command %d\n",command);
+	break;
+    }
+    return 0;
+}
+
 void vmport_init(CPUState *env)
 {
     port_state.env = env;
 
-    register_ioport_read(0x5658, 1, 4, vmport_ioport_read, &port_state);
+    register_ioport_read(VMPORT_CMD, 1, 4, vmport_ioport_read, &port_state);
+    register_ioport_read(VMPORT_RPC, 1, 4, vmport_rpc_read, &port_rpc);
 
     /* Register some generic port commands */
     vmport_register(VMPORT_CMD_GETVERSION, vmport_cmd_get_version, env);
     vmport_register(VMPORT_CMD_GETRAMSIZE, vmport_cmd_ram_size, env);
+    vmport_register(VMPORT_CMD_GETTIME, vmport_cmd_get_time, env);
+    vmport_register(VMPORT_CMD_RPC, vmport_cmd_rpc, env);
 }

Cheers,
-- 
Todd Fries .. todd@fries.net

 _____________________________________________
|                                             \  1.636.410.0632 (voice)
| Free Daemon Consulting, LLC                 \  1.405.227.9094 (voice)
| http://FreeDaemonConsulting.com             \  1.866.792.3418 (FAX)
| "..in support of free software solutions."  \          250797 (FWD)
|                                             \
 \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
                                                 
              37E7 D3EB 74D0 8D66 A68D  B866 0326 204E 3F42 004A
                        http://todd.fries.net/pgp.txt

Penned by Anthony Liguori on 20080824 19:23.59, we have:
> Ian Kirk wrote:
>> Anthony Liguori wrote:
>>
>>   
>>> Ian Kirk wrote:
>>>     
>>>> Hi,
>>>>
>>>> This is my first time sending a patch, so apologies for any errors.
>>>>
>>>> Patch makes vmport optionally initiated.
>>>>
>>>>       
>>> Why should it be optional?
>>>     
>>
>> I believe that VMware ESXi (and perhaps other hypervisors/emulators/etc)
>> doesn't work when it thinks it is running within virtual enviroment, as it
>> talks to vmport when booting (and fails in the current implemtnation -
>> perhaps it only works under itself?)
>>   
>
> That's most likely because the vmport emulation isn't complete enough.
>
>> If I comment init_vmport() out, it definately progresses further along the
>> boot sequence.
>>
>> Also, I guess, it gives you the option to better emulate a real PC (which
>> I assume doesn't have vmport).
>>   
>
> I don't think that's very valuable in and of itself.  Moreover, there  
> are probably a lot more issues with respect to getting ESXi to run under  
> QEMU.  Adding another command line option to support something that we  
> don't know will ever work worries me.  It's just another knob for  
> someone to accidentally tweak.
>
> Regards,
>
> Anthony Liguori
>

Re: [Qemu-devel] [PATCH 1/1] Make vmport an optional feature at run time.

[Anthony Liguori]

Todd T. Fries wrote:
> Just incase a 'more complete' vmport codeset is useful, here is my
> stab at extending it.  I haven't extended it far enough yet that
> OpenBSD's vmt(4) interface works, but would be more than happy
> if someone else were to carry this flag and run with it..
>
>
>
> +static uint32_t vmport_cmd_get_time(void *opaque, uint32_t addr)
> +{
> +    CPUState *env = opaque;
> +    struct timeval tv;
> +    gettimeofday(&tv, NULL);
> +    env->regs[R_EAX] = tv.tv_sec;
> +    env->regs[R_EBX] = tv.tv_usec;
> +    return tv.tv_sec;
> +}
>
>   

FWIW, this is incorrect.  You are presenting the host's version of wall 
clock time, not the guest's version.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 1/1] Make vmport an optional feature at run time.

[qemu-devel]

Maybe I'm missing the point of this exercise.  I can use the rtc and other
mechanisms to get the guests version of system time.

vmport I understand is to help the guest stay in sync with real time (i.e.
the hosts idea of time).

Am I missing something?
-- 
Todd Fries .. todd@fries.net

 _____________________________________________
|                                             \  1.636.410.0632 (voice)
| Free Daemon Consulting, LLC                 \  1.405.227.9094 (voice)
| http://FreeDaemonConsulting.com             \  1.866.792.3418 (FAX)
| "..in support of free software solutions."  \          250797 (FWD)
|                                             \
 \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
                                                 
              37E7 D3EB 74D0 8D66 A68D  B866 0326 204E 3F42 004A
                        http://todd.fries.net/pgp.txt

Penned by Anthony Liguori on 20080906 21:24.45, we have:
> Todd T. Fries wrote:
>> Just incase a 'more complete' vmport codeset is useful, here is my
>> stab at extending it.  I haven't extended it far enough yet that
>> OpenBSD's vmt(4) interface works, but would be more than happy
>> if someone else were to carry this flag and run with it..
>>
>>
>>
>> +static uint32_t vmport_cmd_get_time(void *opaque, uint32_t addr)
>> +{
>> +    CPUState *env = opaque;
>> +    struct timeval tv;
>> +    gettimeofday(&tv, NULL);
>> +    env->regs[R_EAX] = tv.tv_sec;
>> +    env->regs[R_EBX] = tv.tv_usec;
>> +    return tv.tv_sec;
>> +}
>>
>>   
>
> FWIW, this is incorrect.  You are presenting the host's version of wall  
> clock time, not the guest's version.
>
> Regards,
>
> Anthony Liguori

Re: [Qemu-devel] [PATCH 1/1] Make vmport an optional feature at run time.

[Thiemo Seufer]

Anthony Liguori wrote:
> Ian Kirk wrote:
>> Anthony Liguori wrote:
>>
>>   
>>> Ian Kirk wrote:
>>>     
>>>> Hi,
>>>>
>>>> This is my first time sending a patch, so apologies for any errors.
>>>>
>>>> Patch makes vmport optionally initiated.
>>>>
>>>>       
>>> Why should it be optional?
>>>     
>>
>> I believe that VMware ESXi (and perhaps other hypervisors/emulators/etc)
>> doesn't work when it thinks it is running within virtual enviroment, as it
>> talks to vmport when booting (and fails in the current implemtnation -
>> perhaps it only works under itself?)
>>   
>
> That's most likely because the vmport emulation isn't complete enough.
>
>> If I comment init_vmport() out, it definately progresses further along the
>> boot sequence.
>>
>> Also, I guess, it gives you the option to better emulate a real PC (which
>> I assume doesn't have vmport).
>>   
>
> I don't think that's very valuable in and of itself.  Moreover, there  
> are probably a lot more issues with respect to getting ESXi to run under  
> QEMU.  Adding another command line option to support something that we  
> don't know will ever work worries me.  It's just another knob for  
> someone to accidentally tweak.

At this point I wonder if we should have both a "real PC" machine and
a "virtual x86" machine. We have already seen patches for a xen-specific
variant of the latter.


Thiemo

Re: [Qemu-devel] [PATCH 1/1] Make vmport an optional feature at run time.

[Ian Kirk]

Thiemo Seufer wrote:

> Anthony Liguori wrote:
> > Ian Kirk wrote:
> >> Anthony Liguori wrote:
> >>
> >>
> >>> Ian Kirk wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> This is my first time sending a patch, so apologies for any errors.
> >>>>
> >>>> Patch makes vmport optionally initiated.
> >>>>
> >>>>
> >>> Why should it be optional?
> >>>
> >>
> >> I believe that VMware ESXi (and perhaps other hypervisors/emulators/etc)
> >> doesn't work when it thinks it is running within virtual enviroment, as it
> >> talks to vmport when booting (and fails in the current implemtnation -
> >> perhaps it only works under itself?)
> >>
> >
> > That's most likely because the vmport emulation isn't complete enough.
> >
> >> If I comment init_vmport() out, it definately progresses further along the
> >> boot sequence.
> >>
> >> Also, I guess, it gives you the option to better emulate a real PC (which
> >> I assume doesn't have vmport).
> >>
> >
> > I don't think that's very valuable in and of itself.  Moreover, there
> > are probably a lot more issues with respect to getting ESXi to run under
> > QEMU.  Adding another command line option to support something that we
> > don't know will ever work worries me.  It's just another knob for
> > someone to accidentally tweak.
>
> At this point I wonder if we should have both a "real PC" machine and
> a "virtual x86" machine. We have already seen patches for a xen-specific
> variant of the latter.

I guess this was what I wanted - for QEMU to (more) emulate a real PC
(real PCs don't have vmport?).

If there is a better or more elegant way of doing it ...

Re: [Qemu-devel] [PATCH 1/1] Make vmport an optional feature at run time.

[Anthony Liguori]

Thiemo Seufer wrote:
> Anthony Liguori wrote:
>   
>>
>> I don't think that's very valuable in and of itself.  Moreover, there  
>> are probably a lot more issues with respect to getting ESXi to run under  
>> QEMU.  Adding another command line option to support something that we  
>> don't know will ever work worries me.  It's just another knob for  
>> someone to accidentally tweak.
>>     
>
> At this point I wonder if we should have both a "real PC" machine and
> a "virtual x86" machine. We have already seen patches for a xen-specific
> variant of the latter.
>   

There's no such thing as a standard PC.  There of unique hardware out 
there and it's even worse because a lot of hardware is only meant to be 
accessed by the BIOS.

So I don't think any OS should have a problem with vmport other than 
knowing what it is, and expecting certain things from it that we may not 
be providing.  I don't see this as being any different than any other 
device emulation.  When we find this, we should extend vmport to include 
these features.

However, I don't think we should write new guest code that uses the 
vmport interface, because it's not an interface we have any control over.

Regards,

Anthony Liguori

> Thiemo
>