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

Re: [Xen-devel] [PATCH RFC v12 07/21] pvh: Disable unneeded features of HVM containers



On 18/09/13 15:18, Jan Beulich wrote:
On 13.09.13 at 18:25, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
@@ -368,6 +375,7 @@ void hvm_do_resume(struct vcpu *v)
          }
      }
+ check_inject_trap:
Labels are commonly indented by just one space.

Ack -- sorry, emacs its own ideas about how they should be indented. I try to catch these but I don't always notice.


@@ -521,6 +529,7 @@ int hvm_domain_initialise(struct domain *d)
          return -EINVAL;
      }
+ /* PVH: pbut_lock and uc_lock unused, but won't hurt */
      spin_lock_init(&d->arch.hvm_domain.pbuf_lock);
      spin_lock_init(&d->arch.hvm_domain.irq_lock);
      spin_lock_init(&d->arch.hvm_domain.uc_lock);
Typo (pbuf_lock). But the comment is pretty pointless anyway.
And if any future patch starts using either of the mentioned
locks, it'll likely forget to update the comment (and hence it get
stale).

--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -175,6 +175,10 @@ int handle_mmio(void)
      struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
      int rc;
+ /* No MMIO for PVH vcpus */
+    if ( is_pvh_vcpu(curr) )
+        return 0;
+
So why does hvm_pio() not get adjusted similarly?

At the moment I don't think hvm_pio() will be called for PVH guests. IO instructions for PVH domains are handled by the PV io emulation engine. The check here is to make sure that Xen doesn't try to do MMIO for PVH domains on an EPT violation. Mukesh had an "if(!is_pvh_domain)" in the EPT handler before calling hvm_hap_nested_page_fault(), which not only filtered out MMIO, but also any other reason the fault may have happened (e.g., paging or a mem event).

I put the check in handle_mmio() instead of hvm_hap_nested_page_fault() just in case there was another path to that function. I suppose the same logic applies to hvm_pio(). We could put either an ASSERT or just a check that returns failure. (I would lean toward an ASSERT.)

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.