Index: sys/amd64/vmm/amd/vmcb.c
===================================================================
--- sys/amd64/vmm/amd/vmcb.c	(revision 272213)
+++ sys/amd64/vmm/amd/vmcb.c	(working copy)
@@ -111,6 +111,45 @@
 	return (seg);
 }
 
+static int
+vmcb_access(struct svm_softc *softc, int vcpu, int write, int ident,
+	uint64_t *val)
+{
+	struct vmcb *vmcb;
+	int off, bytes;
+	char *ptr;
+
+	vmcb 	= svm_get_vmcb(softc, vcpu);
+	off 	= VMCB_ACCESS_OFFSET(ident);
+	bytes 	= VMCB_ACCESS_BYTES(ident);
+	
+	if (off + bytes >= sizeof (struct vmcb))
+		return (EINVAL);
+
+	ptr = (char *)vmcb;
+
+	if (!write)
+		*val = 0;
+	
+	switch (bytes) {
+	case 8:
+	case 4:
+	case 2:
+		if (write)
+			memcpy(ptr + off, val, bytes);
+		else
+			memcpy(val, ptr + off, bytes);
+		break;
+	default:
+		printf("Invalid size %d, offset 0x%x\n", bytes, off);

neel:
- replace the printf with a KTR
- return (EINVAL)

+	}
+
+	/* If SVM support caching, force to re-read from memory. */

neel: "Invalidate the entire VMCB state cache" perhaps?

+	if (write)
+		svm_set_dirty(softc, vcpu, 0xFFFFFFFF);
+	
+	return (0);
+}
 /*
  * Read from segment selector, control and general purpose register of VMCB.
  */
@@ -126,6 +165,9 @@
 	state = &vmcb->state;
 	err = 0;
 
+	if (VMCB_ACCESS_OK(ident))
+		return (vmcb_access(sc, vcpu, 0, ident, retval));
+
 	switch (ident) {
 	case VM_REG_GUEST_CR0:
 		*retval = state->cr0;
@@ -209,6 +251,9 @@
 	state = &vmcb->state;
 	dirtyseg = 0;
 	err = 0;
+	
+	if (VMCB_ACCESS_OK(ident))
+		return (vmcb_access(sc, vcpu, 1, ident, &val));
 
 	switch (ident) {
 	case VM_REG_GUEST_CR0:
Index: sys/amd64/vmm/amd/vmcb.h
===================================================================
--- sys/amd64/vmm/amd/vmcb.h	(revision 272213)
+++ sys/amd64/vmm/amd/vmcb.h	(working copy)
@@ -164,6 +164,43 @@
 #define VMCB_EXITINTINFO_VALID(x)	(((x) & BIT(31)) ? 1 : 0)
 #define VMCB_EXITINTINFO_EC(x)		(((x) >> 32) & 0xFFFFFFFF)
 
+/* Offset of various VMCB fields. */
+#define VMCB_OFF_CTRL(x)	(x)
+#define VMCB_OFF_STATE(x)	((x) + 0x400)
+
+#define VMCB_OFF_INTERCEPT_CR		VMCB_OFF_CTRL(0x0)
+#define VMCB_OFF_EXCEPTION_BITMAP	VMCB_OFF_CTRL(0x8)
+#define VMCB_OFF_INTERCEPT_BITMAP	VMCB_OFF_CTRL(0xC)
+#define VMCB_OFF_IO_PERM		VMCB_OFF_CTRL(0x40)
+#define VMCB_OFF_MSR_PERM		VMCB_OFF_CTRL(0x48)
+#define VMCB_OFF_TSC_OFFSET		VMCB_OFF_CTRL(0x50)
+#define VMCB_OFF_ASID			VMCB_OFF_CTRL(0x58)
+#define VMCB_OFF_TLB_CTRL		VMCB_OFF_CTRL(0x5C)
+#define VMCB_OFF_VIRQ			VMCB_OFF_CTRL(0x60)
+#define VMCB_OFF_EXIT_REASON		VMCB_OFF_CTRL(0x70)
+#define VMCB_OFF_EXITINFO1		VMCB_OFF_CTRL(0x78)
+#define VMCB_OFF_EXITINFO2		VMCB_OFF_CTRL(0x80)
+#define VMCB_OFF_EXITINTINFO		VMCB_OFF_CTRL(0x88)
+#define VMCB_OFF_AVIC_BAR		VMCB_OFF_CTRL(0x98)
+#define VMCB_OFF_NPT_BASE		VMCB_OFF_CTRL(0xB0)
+#define VMCB_OFF_AVIC_PAGE		VMCB_OFF_CTRL(0xE0)
+#define VMCB_OFF_AVIC_LT		VMCB_OFF_CTRL(0xF0)
+#define VMCB_OFF_AVIC_PT		VMCB_OFF_CTRL(0xF8)
+#define VMCB_OFF_SYSENTER_CS		VMCB_OFF_STATE(0x228)
+#define VMCB_OFF_SYSENTER_ESP		VMCB_OFF_STATE(0x230)
+#define VMCB_OFF_SYSENTER_EIP		VMCB_OFF_STATE(0x238)
+#define VMCB_OFF_GUEST_PAT		VMCB_OFF_STATE(0x268)
+
+/*
+ * Encode the VMCB offset and bytes that we want to read from VMCB.
+ */
+#define VMCB_ACCESS(o, w)               (0x80000000 | (((w) & 0xF) << 16) | \
+						((o) & 0xFFF))
+#define VMCB_ACCESS_OK(v)               ((v) & 0x80000000 )
+#define VMCB_ACCESS_BYTES(v)            (((v) >> 16) & 0xF)
+#define VMCB_ACCESS_OFFSET(v)           ((v) & 0xFFF)

neel:
- style: Put a single tab character between the #define and the macro name.

+
+#ifdef _KERNEL
 /* VMCB save state area segment format */
 struct vmcb_segment {
 	uint16_t	selector;
@@ -287,4 +324,5 @@
 int	vmcb_getdesc(void *arg, int vcpu, int ident, struct seg_desc *desc);
 int	vmcb_seg(struct vmcb *vmcb, int ident, struct vmcb_segment *seg);
 
+#endif /* _KERNEL */
 #endif /* _VMCB_H_ */

neel: (bhyvectl.c)
- Just delete everything regarding PLE window and PLE gap. When we add support
  for this we'll add it back in the right way.
  This should also get rid of the 'error1' local variable in main().

- Remove bogus declaration of 'cpu_vendor()'.

- When setting the exception bitmap for AMD/SVM you should be calling
  'vm_set_vmcb_field()' rather than 'vm_get_vmcb_field()'.

- In 'get_io_bitmap' processing can you change all the format strings to "%#lx"?

- In 'get_guest_sysenter' the alignment of VMCS_GUEST_IA32_SYSENTER_CS and
  VMCS_GUEST_IA32_SYSENTER_ESP is off.

- The flag field for "get-exit-reason" should no longer be called
  'get_vmcs_exit_reason' since it is no longer VMCS-specific.
  How about renaming it to 'get_exit_reason' instead?

- The usage field for AMD/SVM does not match all the options in amd_options[].

- There is a typo in the name of the option "--get-vmcb--all-intercepts".
  Currently it is called "--get-vmcb--all_intercept".

- get_misc_vmcb(): the names of the registers and VMCS fields in the rest of
  the file are not captilized so I would recommend making them lowercase in
  this function too. Also, you don't need to prefix the field names with
  "VMCB" since it should be obvious from the context.

  There are also a number of lines in this function which have a single tab
  on them. You should delete the tab and make the lines truly empty.

  Also you should change the printf format strings to either "%#lx" or "%#x"
  depending on whether you are printing a 8-byte long or a 4-byte integer.

  Currently there are a number of places where the format string used is
  "0x%08lx" which looks a bit bogus.

- In dump_vm_run_exitcode() the field names should be lowercase to match the
  rest of the function.

- msr_name(): Instead of using numeric constants to identify the MSRs you
  should use the macro names. For e.g.,
  case MSR_TSC:
  	return ("MSR_TSC");

- "Refernce" -> "Reference" above dump_amd_msr_pm()

- "Referene" -> "Reference" above dump_intel_msr_pm()

- dump_msr_bitmap(): you should change the following conditional back to
  what it was originally:
  done:
  	if (bitmap != MAP_FAILED)	<------
		munmap((void *)bitmap, map_size);

- Recommend changing the names of the functions:
  get_misc_vmcs() -> get_vmcs_fields()
  get_misc_vmcb() -> get_vmcb_fields()

- setup_options()
  - all_opts[] -> common_opts[]
  - Is there any reason why 'all_opts[]' is declared 'static' but 'amd_opts[]'
    and 'intel_opts[]' are not? I don't have a preference either way but would
    like all three option structures to get the same treatment.
  - Rather than updating the 'all_opts[]' array in place you can just malloc
    an appropriately sized array and copy both 'common_opts[]' and processor
    specific opts[] into it. It also removes the arbitrary restriction of
    MAX_OPTS.