qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v8 8/8] vmport: Add VMware all ring hack


From: Don Slutz
Subject: Re: [Qemu-devel] [PATCH v8 8/8] vmport: Add VMware all ring hack
Date: Fri, 03 Jul 2015 09:08:12 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0

On 07/01/15 02:50, Michael S. Tsirkin wrote:
On Tue, Jun 23, 2015 at 07:39:33PM -0400, Don Slutz wrote:
From: Don Slutz <address@hidden>

This is done by adding a new machine property vmware-port-ring3 that
needs to be enabled to have any effect.  It only effects accel=tcg
mode.  It is needed if you want to use VMware tools in accel=tcg
mode.

Signed-off-by: Don Slutz <address@hidden>
CC: Don Slutz <address@hidden>
IMO this patch isn't ready yet.

---
  hw/i386/pc.c             | 26 +++++++++++++++++++++++++-
  hw/i386/pc_piix.c        |  2 +-
  hw/i386/pc_q35.c         |  2 +-
  include/hw/i386/pc.h     |  6 +++++-
  target-i386/cpu-qom.h    |  3 +++
  target-i386/seg_helper.c |  9 +++++++++
  6 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7072930..3ae4476 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1042,7 +1042,9 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
      object_unref(OBJECT(cpu));
  }
-void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
+/* vmware_port_ring3 true says enable VMware port access in ring3. */
+void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge,
+                  bool vmware_port_ring3)
  {
      int i;
      X86CPU *cpu = NULL;
@@ -1073,6 +1075,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState 
*icc_bridge)
              error_report_err(error);
              exit(1);
          }
+        cpu->allow_vmport_ring3 = vmware_port_ring3;
          object_unref(OBJECT(cpu));
      }
@@ -1835,6 +1838,21 @@ static bool pc_machine_get_aligned_dimm(Object *obj, Error **errp)
      return pcms->enforce_aligned_dimm;
  }
+static bool pc_machine_get_vmware_port_ring3(Object *obj, Error **errp)
+{
+    PCMachineState *pcms = PC_MACHINE(obj);
+
+    return pcms->vmware_port_ring3;
+}
+
+static void pc_machine_set_vmware_port_ring3(Object *obj, bool value,
+                                             Error **errp)
+{
+    PCMachineState *pcms = PC_MACHINE(obj);
+
+    pcms->vmware_port_ring3 = value;
+}
+
  static void pc_machine_initfn(Object *obj)
  {
      PCMachineState *pcms = PC_MACHINE(obj);
@@ -1865,6 +1883,12 @@ static void pc_machine_initfn(Object *obj)
      object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALIGNED_DIMM,
                               pc_machine_get_aligned_dimm,
                               NULL, NULL);
+
+    pcms->vmware_port_ring3 = false;
+    object_property_add_bool(obj, PC_MACHINE_VMWARE_PORT_RING3,
+                             pc_machine_get_vmware_port_ring3,
+                             pc_machine_set_vmware_port_ring3,
+                             NULL);
New properties should have a description.

Ok.



  }
static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e142f75..890c45e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -146,7 +146,7 @@ static void pc_init1(MachineState *machine)
      object_property_add_child(qdev_get_machine(), "icc-bridge",
                                OBJECT(icc_bridge), NULL);
- pc_cpus_init(machine->cpu_model, icc_bridge);
+    pc_cpus_init(machine->cpu_model, icc_bridge, 
pc_machine->vmware_port_ring3);
if (kvm_enabled() && kvmclock_enabled) {
          kvmclock_create();
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 082cd93..8288a5f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -137,7 +137,7 @@ static void pc_q35_init(MachineState *machine)
      object_property_add_child(qdev_get_machine(), "icc-bridge",
                                OBJECT(icc_bridge), NULL);
- pc_cpus_init(machine->cpu_model, icc_bridge);
+    pc_cpus_init(machine->cpu_model, icc_bridge, 
pc_machine->vmware_port_ring3);
      pc_acpi_init("q35-acpi-dsdt.aml");
kvmclock_create();
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 3270a97..f9e74c7 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -40,6 +40,7 @@ struct PCMachineState {
uint64_t max_ram_below_4g;
      OnOffAuto vmport;
+    bool vmware_port_ring3;
      bool enforce_aligned_dimm;
  };
@@ -48,6 +49,7 @@ struct PCMachineState {
  #define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
  #define PC_MACHINE_VMPORT           "vmport"
  #define PC_MACHINE_ENFORCE_ALIGNED_DIMM "enforce-aligned-dimm"
+#define PC_MACHINE_VMWARE_PORT_RING3 "vmware-port-ring3"
/**
   * PCMachineClass:
Creating it at the pc level and propagating makes code
messy but I'd go along with it if it made sense from
user's point of view, but it does not seem to make sense:
to me this looks more like a CPU feature than a machine property.
Or the property of the vmport rpc device that you created.

Well this is not a clear area. It does look more like a CPU feature. I went with a machine feature because that is where "vmport" currently is.



If it's required for the rpc device to be used -
why not set this unconditionally if the rpc device
is created? Not saying it's a good idea - just asking.

This is a complex question.

The VMware provided tools and open-vm-tools (https://github.com/vmware/open-vm-tools) currently require it. However arranging IOPL(3) in linux for these is not that hard if the user has the correct privilege.

However the other vmport available actions do not require this to be enabled, but can be accessed if enabled.

Xen does not need or use this patch. KVM (which I do not know a lot about) also needs a change similar to this. So this is only for TCG case.

Paolo Bonzini said:

-------- Forwarded Message --------
Subject:        Re: [PATCH v4 6/7] vmport: Add VMware all ring hack
Date:   Thu, 30 Apr 2015 11:25:26 -0400
From:   Paolo Bonzini <address@hidden>
To: Slutz, Donald Christopher <address@hidden>, address@hidden <address@hidden> CC: Michael S. Tsirkin <address@hidden>, Markus Armbruster <address@hidden>, Luiz Capitulino <address@hidden>, Anthony Liguori <address@hidden>, Andreas Färber <address@hidden>, Richard Henderson <address@hidden>



On 30/04/2015 17:15, Don Slutz wrote:
This is done by adding a new machine property vmware-port-ring3 that
needs to be enabled to have any effect.  It only effects accel=tcg
mode.  It is needed if you want to use VMware tools in accel=tcg
mode.

How does it work on KVM or Xen?

Xen change is in progress (patch posted).  I have not done any work on
HVM yet.

Ok, no problem.  Just wanted to be sure this is simply the TCG case.
For KVM we can piggyback on the same X86CPU variable, so it's better if
you do not use hflags2.

Paolo


So it may make more sense to switch to the CPU feature.

I do not think it makes sense to set this by default if the vmware_rpc device exists (not at all sure how to get something like this coded).

   -Don Slutz


@@ -161,7 +163,9 @@ extern int fd_bootchk;
  void pc_register_ferr_irq(qemu_irq irq);
  void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
-void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
+/* vmware_port_ring3 true says enable VMware port access in ring3. */
+void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge,
+                  bool vmware_port_ring3);
  void pc_hot_add_cpu(const int64_t id, Error **errp);
  void pc_acpi_init(const char *default_dsdt);
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 7a4fddd..ccbc5bb 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -109,6 +109,9 @@ typedef struct X86CPU {
       */
      bool enable_pmu;
+ /* allow_vmport_ring3 true says enable VMware port access in ring3 */
+    bool allow_vmport_ring3;
+
      /* in order to simplify APIC support, we leave this pointer to the
         user */
      struct DeviceState *apic_state;
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 8a4271e..2ddb84f 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -2566,6 +2566,15 @@ static inline void check_io(CPUX86State *env, int addr, 
int size)
  {
      int io_offset, val, mask;
+ /* vmport hack: skip iopl checking for VMware port 0x5658 (see
+     * vmport_realizefn()) */
+    if (addr == 0x5658) {
+        X86CPU *cpu = x86_env_get_cpu(env);
+        if (cpu->allow_vmport_ring3) {
+            return;
+        }
+    }
+
      /* TSS must be a valid 32 bit one */
      if (!(env->tr.flags & DESC_P_MASK) ||
          ((env->tr.flags >> DESC_TYPE_SHIFT) & 0xf) != 9 ||
--
1.8.3.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]