[PATCH] vmm: Add MSRs to readregs / writeregs

2017-04-29 Thread Pratik Vyas

Hello tech@,

This is a patch that extends the readregs and writeregs vmm(4) ioctl to
read and write MSRs as well.

It also sets the IA32_VMX_IA32E_MODE_GUEST entry control in
vcpu_reset_regs based on the value of EFER_LMA.

There are changes to vmmvar.h and would require a `make includes` step
to build vmd(8). This should also not alter any behaviour of vmd(8).

Context: These are the kernel changes required to implement vmctl send
and vmctl receive. vmctl send / receive are two new options that will
support snapshotting VMs and migrating VMs from one host to another.
This project was undertaken at San Jose State University along with my
three teammates CCed with mlarkin@ as our advisor.

Patches to vmd(8) that implement vmctl send and vmctl receive are
forthcoming.


Thanks,
Pratik



Index: sys/arch/amd64/amd64/vmm.c
===
RCS file: /home/pdvyas/cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.137
diff -u -p -a -u -r1.137 vmm.c
--- sys/arch/amd64/amd64/vmm.c  28 Apr 2017 10:09:37 -  1.137
+++ sys/arch/amd64/amd64/vmm.c  30 Apr 2017 00:01:21 -
@@ -1334,7 +1334,9 @@ vcpu_readregs_vmx(struct vcpu *vcpu, uin
uint64_t sel, limit, ar;
uint64_t *gprs = vrs->vrs_gprs;
uint64_t *crs = vrs->vrs_crs;
+   uint64_t *msrs = vrs->vrs_msrs;
struct vcpu_segment_info *sregs = vrs->vrs_sregs;
+   struct vmx_msr_store *msr_store;

if (vcpu_reload_vmcs_vmx(>vc_control_pa))
return (EINVAL);
@@ -1402,6 +1404,14 @@ vcpu_readregs_vmx(struct vcpu *vcpu, uin
goto errout;
}

+   msr_store = (struct vmx_msr_store *)vcpu->vc_vmx_msr_exit_save_va;
+
+   if (regmask & VM_RWREGS_MSRS) {
+   for (i = 0; i < VCPU_REGS_NMSRS; i++) {
+   msrs[i] = msr_store[i].vms_data;
+   }
+   }
+
goto out;

errout:
@@ -1448,7 +1458,9 @@ vcpu_writeregs_vmx(struct vcpu *vcpu, ui
uint64_t limit, ar;
uint64_t *gprs = vrs->vrs_gprs;
uint64_t *crs = vrs->vrs_crs;
+   uint64_t *msrs = vrs->vrs_msrs;
struct vcpu_segment_info *sregs = vrs->vrs_sregs;
+   struct vmx_msr_store *msr_store;

if (loadvmcs) {
if (vcpu_reload_vmcs_vmx(>vc_control_pa))
@@ -1518,6 +1530,14 @@ vcpu_writeregs_vmx(struct vcpu *vcpu, ui
goto errout;
}

+   msr_store = (struct vmx_msr_store *)vcpu->vc_vmx_msr_exit_save_va;
+
+   if (regmask & VM_RWREGS_MSRS) {
+   for (i = 0; i < VCPU_REGS_NMSRS; i++) {
+   msr_store[i].vms_data = msrs[i];
+   }
+   }
+
goto out;

errout:
@@ -2128,7 +2148,7 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s
 * IA32_VMX_LOAD_DEBUG_CONTROLS
 * IA32_VMX_LOAD_IA32_PERF_GLOBAL_CTRL_ON_ENTRY
 */
-   if (ug == 1)
+   if (ug == 1 && !(vrs->vrs_msrs[VCPU_REGS_EFER] & EFER_LMA))
want1 = 0;
else
want1 = IA32_VMX_IA32E_MODE_GUEST;
@@ -2277,26 +2297,12 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s
 */
msr_store = (struct vmx_msr_store *)vcpu->vc_vmx_msr_exit_save_va;

-   /*
-* Make sure LME is enabled in EFER if restricted guest mode is
-* needed.
-*/
-   msr_store[0].vms_index = MSR_EFER;
-   if (ug == 1)
-   msr_store[0].vms_data = 0ULL;   /* Initial value */
-   else
-   msr_store[0].vms_data = EFER_LME;
-
-   msr_store[1].vms_index = MSR_STAR;
-   msr_store[1].vms_data = 0ULL;   /* Initial value */
-   msr_store[2].vms_index = MSR_LSTAR;
-   msr_store[2].vms_data = 0ULL;   /* Initial value */
-   msr_store[3].vms_index = MSR_CSTAR;
-   msr_store[3].vms_data = 0ULL;   /* Initial value */
-   msr_store[4].vms_index = MSR_SFMASK;
-   msr_store[4].vms_data = 0ULL;   /* Initial value */
-   msr_store[5].vms_index = MSR_KERNELGSBASE;
-   msr_store[5].vms_data = 0ULL;   /* Initial value */
+   msr_store[VCPU_REGS_EFER].vms_index = MSR_EFER;
+   msr_store[VCPU_REGS_STAR].vms_index = MSR_STAR;
+   msr_store[VCPU_REGS_LSTAR].vms_index = MSR_LSTAR;
+   msr_store[VCPU_REGS_CSTAR].vms_index = MSR_CSTAR;
+   msr_store[VCPU_REGS_SFMASK].vms_index = MSR_SFMASK;
+   msr_store[VCPU_REGS_KGSBASE].vms_index = MSR_KERNELGSBASE;

/*
 * Currently we have the same count of entry/exit MSRs loads/stores
@@ -2359,6 +2365,13 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s
ret = vcpu_writeregs_vmx(vcpu, VM_RWREGS_ALL, 0, vrs);

/*
+* Make sure LME is enabled in EFER if restricted guest mode is
+* needed.
+*/
+   if (ug == 0)
+   msr_store[VCPU_REGS_EFER].vms_data |= EFER_LME;
+
+   /*
 * Set up the MSR bitmap
 */
memset((uint8_t *)vcpu->vc_msr_bitmap_va, 0xFF, 

Re: arm64 gic-v3 driver

2017-04-29 Thread Patrick Wildt
On Sat, Apr 29, 2017 at 06:45:15PM +0200, Mark Kettenis wrote:
> The firefly rk3399 board has a gic-v3, so I'd like to get Dale's
> driver into the tree.  I did some further cleanup, renamed the driver
> to agintc(4) (as jsg@ felt the "new" in angintc(4) did't make sense)
> and fixed two bugs:
> 
> * prival in setipl() was used uninitialized
> 
> * apparently a dsb instruction is needed after reading the ICC_IAR1
>   register
> 
> I deliberately tried to keep the differences to ampintc(4) as small as
> possible such that it easier to compare the code.  I have a small
> followup diff that will tweak ampintc(4) in the cases where the code
> in agintc(4) was cleaner to start with.
> 
> There may be more bugs lurking in this code as I'm still seeing plenty
> of weird behaviour.  But at least this will get me going.
> 
> ok?

Yes, please.  I have seen some whitespace issues which I commented
inline, feel free to fix those before committing.  Need to do another
read later, but please just go ahead.

> 
> 
> Index: arch/arm64/dev/agintc.c
> ===
> RCS file: arch/arm64/dev/agintc.c
> diff -N arch/arm64/dev/agintc.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ arch/arm64/dev/agintc.c   29 Apr 2017 16:33:50 -
> @@ -0,0 +1,785 @@
> +/* $OpenBSD$ */
> +/*
> + * Copyright (c) 2007, 2009, 2011, 2017 Dale Rahn 
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +/*
> + * This is a device driver for the GICv3/GICv4 IP from ARM as specified
> + * in IHI0069C, an example of this hardware is the GIC 500.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#define ICC_PMR  s3_0_c4_c6_0
> +#define ICC_IAR0 s3_0_c12_c8_0
> +#define ICC_EOIR0s3_0_c12_c8_1
> +#define ICC_HPPIR0   s3_0_c12_c8_2
> +#define ICC_BPR0 s3_0_c12_c8_3
> +
> +#define ICC_DIR  s3_0_c12_c11_1
> +#define ICC_SGI1Rs3_0_c12_c11_5
> +#define ICC_SGI0Rs3_0_c12_c11_7
> +
> +#define ICC_IAR1 s3_0_c12_c12_0
> +#define ICC_EOIR1s3_0_c12_c12_1
> +#define ICC_HPPIR1   s3_0_c12_c12_2
> +#define ICC_BPR1 s3_0_c12_c12_3
> +#define ICC_CTLR s3_0_c12_c12_4
> +#define ICC_SRE_EL1  s3_0_c12_c12_5
> +#define  ICC_SRE_EL1_EN  0x7
> +#define ICC_IGRPEN0  s3_0_c12_c12_6
> +#define ICC_IGRPEN1  s3_0_c12_c12_7
> +
> +#define _STR(x) #x
> +#define STR(x) _STR(x)
> +
> +/* distributor registers */
> +#define  GICD_CTLR   0x
> +// non-secure
> +#define  GICD_CTLR_RWP   (1U << 31)

Replace the first space with a tab.

> +#define   GICD_CTRL_EnableGrp1   (1 << 0)
> +#define   GICD_CTRL_EnableGrp1A  (1 << 1)
> +#define   GICD_CTRL_ARE_NS   (1 << 4)
> +#define  GICD_TYPER  0x0004
> +#define   GICD_TYPER_ITLINE_M0xf
> +#define  GICD_IIDR   0x0008
> +#define  GICD_ISENABLER(i)   (0x0100 + (IRQ_TO_REG32(i) * 4))
> +#define  GICD_ICENABLER(i)   (0x0180 + (IRQ_TO_REG32(i) * 4))
> +#define  GICD_ISPENDR(i) (0x0200 + (IRQ_TO_REG32(i) * 4))
> +#define  GICD_ICPENDR(i) (0x0280 + (IRQ_TO_REG32(i) * 4))
> +#define  GICD_ISACTIVER(i)   (0x0300 + (IRQ_TO_REG32(i) * 4))
> +#define  GICD_ICACTIVER(i)   (0x0380 + (IRQ_TO_REG32(i) * 4))
> +#define  GICD_IPRIORITYR(i)  (0x0400 + (i))
> +#define  GICD_ICFGR(i)   (0x0c00 + (IRQ_TO_REG16(i) * 4))
> +#define  GICD_IROUTER(i) (0x6000 + ((i) * 8))
> +
> +/* redistributor registers */
> +#define GICR_CTLR0x0
> +#define  GICR_CTLR_RWP   ((1U << 31) | (1 << 3))
> +#define GICR_IIDR0x4
> +#define GICR_TYPER   0x8
> +#define  GICR_TYPER_LAST (1 << 4)
> +#define  GICR_TYPER_VLPIS(1 << 1)
> +#define GICR_WAKER   0x00014
> +#define  GICR_WAKER_X31  (1U << 31)
> +#define  GICR_WAKER_CHILDRENASLEEP   (1 << 2)
> +#define  GICR_WAKER_PROCESSORSLEEP   (1 << 1)
> +#define  GICR_WAKER_X0   (1 << 0)
> +#define GICR_ISENABLE0   0x10100
> +#define 

arm64 gic-v3 driver

2017-04-29 Thread Mark Kettenis
The firefly rk3399 board has a gic-v3, so I'd like to get Dale's
driver into the tree.  I did some further cleanup, renamed the driver
to agintc(4) (as jsg@ felt the "new" in angintc(4) did't make sense)
and fixed two bugs:

* prival in setipl() was used uninitialized

* apparently a dsb instruction is needed after reading the ICC_IAR1
  register

I deliberately tried to keep the differences to ampintc(4) as small as
possible such that it easier to compare the code.  I have a small
followup diff that will tweak ampintc(4) in the cases where the code
in agintc(4) was cleaner to start with.

There may be more bugs lurking in this code as I'm still seeing plenty
of weird behaviour.  But at least this will get me going.

ok?


Index: arch/arm64/dev/agintc.c
===
RCS file: arch/arm64/dev/agintc.c
diff -N arch/arm64/dev/agintc.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ arch/arm64/dev/agintc.c 29 Apr 2017 16:33:50 -
@@ -0,0 +1,785 @@
+/* $OpenBSD$ */
+/*
+ * Copyright (c) 2007, 2009, 2011, 2017 Dale Rahn 
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/*
+ * This is a device driver for the GICv3/GICv4 IP from ARM as specified
+ * in IHI0069C, an example of this hardware is the GIC 500.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+#include 
+
+#define ICC_PMRs3_0_c4_c6_0
+#define ICC_IAR0   s3_0_c12_c8_0
+#define ICC_EOIR0  s3_0_c12_c8_1
+#define ICC_HPPIR0 s3_0_c12_c8_2
+#define ICC_BPR0   s3_0_c12_c8_3
+
+#define ICC_DIRs3_0_c12_c11_1
+#define ICC_SGI1R  s3_0_c12_c11_5
+#define ICC_SGI0R  s3_0_c12_c11_7
+
+#define ICC_IAR1   s3_0_c12_c12_0
+#define ICC_EOIR1  s3_0_c12_c12_1
+#define ICC_HPPIR1 s3_0_c12_c12_2
+#define ICC_BPR1   s3_0_c12_c12_3
+#define ICC_CTLR   s3_0_c12_c12_4
+#define ICC_SRE_EL1s3_0_c12_c12_5
+#define  ICC_SRE_EL1_EN0x7
+#define ICC_IGRPEN0s3_0_c12_c12_6
+#define ICC_IGRPEN1s3_0_c12_c12_7
+
+#define _STR(x) #x
+#define STR(x) _STR(x)
+
+/* distributor registers */
+#defineGICD_CTLR   0x
+// non-secure
+#define  GICD_CTLR_RWP (1U << 31)
+#define GICD_CTRL_EnableGrp1   (1 << 0)
+#define GICD_CTRL_EnableGrp1A  (1 << 1)
+#define GICD_CTRL_ARE_NS   (1 << 4)
+#defineGICD_TYPER  0x0004
+#define GICD_TYPER_ITLINE_M0xf
+#defineGICD_IIDR   0x0008
+#defineGICD_ISENABLER(i)   (0x0100 + (IRQ_TO_REG32(i) * 4))
+#defineGICD_ICENABLER(i)   (0x0180 + (IRQ_TO_REG32(i) * 4))
+#defineGICD_ISPENDR(i) (0x0200 + (IRQ_TO_REG32(i) * 4))
+#defineGICD_ICPENDR(i) (0x0280 + (IRQ_TO_REG32(i) * 4))
+#defineGICD_ISACTIVER(i)   (0x0300 + (IRQ_TO_REG32(i) * 4))
+#defineGICD_ICACTIVER(i)   (0x0380 + (IRQ_TO_REG32(i) * 4))
+#defineGICD_IPRIORITYR(i)  (0x0400 + (i))
+#defineGICD_ICFGR(i)   (0x0c00 + (IRQ_TO_REG16(i) * 4))
+#defineGICD_IROUTER(i) (0x6000 + ((i) * 8))
+
+/* redistributor registers */
+#define GICR_CTLR  0x0
+#define  GICR_CTLR_RWP ((1U << 31) | (1 << 3))
+#define GICR_IIDR  0x4
+#define GICR_TYPER 0x8
+#define  GICR_TYPER_LAST   (1 << 4)
+#define  GICR_TYPER_VLPIS  (1 << 1)
+#define GICR_WAKER 0x00014
+#define  GICR_WAKER_X31(1U << 31)
+#define  GICR_WAKER_CHILDRENASLEEP (1 << 2)
+#define  GICR_WAKER_PROCESSORSLEEP (1 << 1)
+#define  GICR_WAKER_X0 (1 << 0)
+#define GICR_ISENABLE0 0x10100
+#define GICR_ICENABLE0 0x10180
+#define GICR_ISPENDR0  0x10200
+#define GICR_ICPENDR0  0x10280
+#define GICR_ISACTIVE0 0x10300
+#define GICR_ICACTIVE0 0x10380
+#define GICR_IPRIORITYR(i) (0x10400 + (i))
+#define GICR_ICFGR00x10c00
+#define GICR_ICFGR10x10c04
+
+#define IRQ_TO_REG32(i)(((i) >> 5) & 0x7)
+#define IRQ_TO_REG32BIT(i) ((i) & 0x1f)
+
+#define IRQ_TO_REG16(i)   

[PATCH] Clean up obsolete information in gettimeofday.2

2017-04-29 Thread Bryan Linton
Is this worth deleting now that struct timezone *tzp is no longer
used?

I was originally going to send a small diff changing only the line
that read, "If tp or tzp is NULL, the associated time information
will not be returned or set" since gettimeofday(, NULL) works
exactly as one would expect.

I thought that it might be worth pruning this now, but if it's
still worth keeping the information around, I don't have strong
objection against keeping it either.

-- 
Bryan

Index: gettimeofday.2
===
RCS file: /cvs/src/lib/libc/sys/gettimeofday.2,v
retrieving revision 1.29
diff -u -r1.29 gettimeofday.2
--- gettimeofday.2  10 Sep 2015 17:55:21 -  1.29
+++ gettimeofday.2  29 Apr 2017 14:00:07 -
@@ -61,18 +61,14 @@
 .Dq ticks .
 If
 .Fa tp
-or
-.Fa tzp
 is
 .Dv NULL ,
 the associated time
 information will not be returned or set.
 .Pp
-The structures pointed to by
+The structure pointed to by
 .Fa tp
-and
-.Fa tzp
-are defined in
+is defined in
 .In sys/time.h
 as:
 .Bd -literal
@@ -80,20 +76,7 @@
time_t  tv_sec; /* seconds since Jan. 1, 1970 */
suseconds_t tv_usec;/* and microseconds */
 };
-
-struct timezone {
-   int tz_minuteswest; /* of Greenwich */
-   int tz_dsttime; /* type of dst correction to apply */
-};
 .Ed
-.Pp
-The
-.Fa timezone
-structure indicates the local time zone
-(measured in minutes of time westward from Greenwich),
-and a flag that, if nonzero, indicates that
-Daylight Saving time applies locally during
-the appropriate part of the year.
 .Pp
 Only the superuser may set the time of day or time zone.
 If the system securelevel is greater than 1 (see


Re: uaudio_drain() not needed?

2017-04-29 Thread Michael W. Bombardieri
On Mon, Apr 24, 2017 at 08:29:17AM +0200, Alexandre Ratchov wrote:
> On Mon, Apr 24, 2017 at 01:04:12AM +0800, Michael W. Bombardieri wrote:
> > Hi,
> > 
> > When I remove uaudio_drain() on my laptop the attach/detach still
> > seems to work as expected.
> > I did a test with two usb soundcards. Audio files were played &
> > recorded using aucat.
> > 
> > * boot system (no audio device because I disabled azalia)
> > * attach device1 (Creative card)
> > * play wav file1 (reference file)
> > * detach device1
> > * attach device2 (Yamaha card)
> > * play wav file1
> > * record wav file2
> > * detach device2
> > * attach device2
> > * play wav file2
> > * detach device2
> > * attach device1
> > * play wav file2
> > * detach device1
> > 
> > So far this has only been tested on amd64. Maybe it produces
> > issues for your uaudio device though.
> > 
> 
> AFAICS this is correct.  uaudio_drain() used to be the "drain"
> method, but the audio(4) layer doesn't use it anymore.  IMO, it
> shouldn't be called by uaudio_detach(), the device is gone and
> there are no outstanding requests.

FWIW I have now tested this diff on i386. Everything worked ok.
One test I neglected earlier was to physically disconnect usb
audio device while playing or recording with aucat--that also
worked.