cdio read_track() malloc/free

2017-06-18 Thread Michael W. Bombardieri
Hello,

In cdio's read_track() the error condition around sio_write() doesn't free 
local storage "sec"
before it returns. The other error cases appear to free it.

- Michael


Index: rip.c
===
RCS file: /cvs/src/usr.bin/cdio/rip.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 rip.c
--- rip.c   20 Aug 2015 22:32:41 -  1.16
+++ rip.c   19 Jun 2017 03:27:14 -
@@ -398,6 +398,7 @@ read_track(struct track *ti)
}
if (ti->hdl != NULL &&
(sio_write(ti->hdl, sec, blksize) == 0)) {
+   free(sec);
sio_close(ti->hdl);
ti->hdl = NULL;
warnx("\nerror while writing to audio output");



Re: inteldrm: add a handler for the WSDISPLAYIO_GINFO ioctl

2017-06-18 Thread Ted Unangst
Frederic Cambus wrote:
> On Sat, Jun 17, 2017 at 05:27:38PM -0400, Ted Unangst wrote:
> 
> > > Add a handler for the WSDISPLAYIO_GINFO ioctl in inteldrm, allowing
> > > to retrieve basic information about a framebuffer display.
> 
> [...]
> 
> > why? what program will use this information?
> 
> The reason I would like to add this handler is to be able to get the
> frame buffer resolution in wsfontload(8). The default width/height is
> currently hardcoded and ought to be computed from the frame buffer
> resolution, there is a TODO in the code.

ok, this makes sense.



radeondrm: add a handler for the WSDISPLAYIO_GINFO ioctl

2017-06-18 Thread Frederic Cambus
Hi tech@,

Add a handler for the WSDISPLAYIO_GINFO ioctl in radeondrm, allowing
to retrieve basic information about a framebuffer display.

Same rationale as the diff I sent for inteldrm, and it can also be
tested with wsconsctl which will return new display values for width,
height and depth.

Comments? OK?

Index: sys/dev/pci/drm/radeon/radeon_kms.c
===
RCS file: /cvs/src/sys/dev/pci/drm/radeon/radeon_kms.c,v
retrieving revision 1.49
diff -u -p -r1.49 radeon_kms.c
--- sys/dev/pci/drm/radeon/radeon_kms.c 8 Jan 2017 12:11:54 -   1.49
+++ sys/dev/pci/drm/radeon/radeon_kms.c 18 Jun 2017 11:09:53 -
@@ -354,9 +354,19 @@ struct wsdisplay_accessops radeondrm_acc
 int
 radeondrm_wsioctl(void *v, u_long cmd, caddr_t data, int flag, struct proc *p)
 {
+   struct rasops_info *ri = v;
+   struct wsdisplay_fbinfo *wdf;
+
switch (cmd) {
case WSDISPLAYIO_GTYPE:
*(int *)data = WSDISPLAY_TYPE_RADEONDRM;
+   return 0;
+   case WSDISPLAYIO_GINFO:
+   wdf = (struct wsdisplay_fbinfo *)data;
+   wdf->width = ri->ri_width;
+   wdf->height = ri->ri_height;
+   wdf->depth = ri->ri_depth;
+   wdf->cmsize = 0;
return 0;
default:
return -1;



[patch] mg: fix overflow on vteeol() (resend/bump)

2017-06-18 Thread Hiltjo Posthuma
Hey,

This is a resend/bump of a patch about a month ago, can it get applied?

Original message below:


mg crashes with certain (unicode) characters and moving the cursor to the
end of the line.

The characters are printed to the screen as \nnn in vtpute() and vtcol is
updated, however vteeol() will write beyond the buffer.

A test file contains the data:

——

It is printed to the screen as: \342\200\224\342... etc.

It is safer to use vtpute() here because it checks if vtcol >= 0.

Below is a patch:


diff --git a/usr.bin/mg/display.c b/usr.bin/mg/display.c
index 7af723ce268..d7c22554753 100644
--- a/usr.bin/mg/display.c
+++ b/usr.bin/mg/display.c
@@ -381,11 +381,8 @@ vtpute(int c)
 void
 vteeol(void)
 {
-   struct video *vp;
-
-   vp = vscreen[vtrow];
while (vtcol < ncol)
-   vp->v_text[vtcol++] = ' ';
+   vtpute(' ');
 }
 
 /*

-- 
Kind regards,
Hiltjo



wsfontload(8): close file descriptors on all exit paths

2017-06-18 Thread Frederic Cambus
Hi tech@,

Close file descriptors on all exit paths in wsfontload(8).

Comments? OK?

Index: usr.sbin/wsfontload/wsfontload.c
===
RCS file: /cvs/src/usr.sbin/wsfontload/wsfontload.c,v
retrieving revision 1.18
diff -u -p -r1.18 wsfontload.c
--- usr.sbin/wsfontload/wsfontload.c17 Jun 2017 19:27:54 -  1.18
+++ usr.sbin/wsfontload/wsfontload.c18 Jun 2017 12:56:22 -
@@ -166,6 +166,7 @@ main(int argc, char *argv[])
i++;
} while(res == 0);
 
+   close(wsfd);
return (0);
}
 
@@ -245,6 +246,8 @@ main(int argc, char *argv[])
if (res < 0)
err(3, "WSDISPLAYIO_LDFONT");
 
+   close(ffd);
+   close(wsfd);
return (0);
 }
 



Re: faster timecounters for kvm/xen

2017-06-18 Thread Mike Belopuhov
On Sun, Jun 18, 2017 at 21:20 +1000, Jonathan Matthew wrote:
> On Fri, Jun 16, 2017 at 10:25:29AM +0200, Mike Belopuhov wrote:
> > Now regarding the diff.  pvbus_init_vcpu.  Ah yes, please.
> > It was a chicken and the egg problem for me: I didn't have
> > Xen, but wanted a callback from cpu_hatch to setup shared
> > info pages and events (interrupt delivery) for all CPUs.
> > So please factor it out and let's get that committed.
> 
> Updated version of this is below.  The init_cpu function pointer is now in
> the pvbus_hv so it's easier to decide what it does at runtime.
>
[...]
> oks on this bit?
>

OK mikeb

> Index: arch/amd64/amd64/cpu.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
> retrieving revision 1.105
> diff -u -p -r1.105 cpu.c
> --- arch/amd64/amd64/cpu.c30 May 2017 15:11:32 -  1.105
> +++ arch/amd64/amd64/cpu.c18 Jun 2017 09:16:12 -
> @@ -67,6 +67,7 @@
>  #include "lapic.h"
>  #include "ioapic.h"
>  #include "vmm.h"
> +#include "pvbus.h"
>  
>  #include 
>  #include 
> @@ -103,6 +104,10 @@
>  #include 
>  #endif
>  
> +#if NPVBUS > 0
> +#include 
> +#endif
> +
>  #include 
>  #include 
>  #include 
> @@ -728,6 +733,9 @@ cpu_hatch(void *v)
>   lldt(0);
>  
>   cpu_init(ci);
> +#if NPVBUS > 0
> + pvbus_init_cpu();
> +#endif
>  
>   /* Re-initialise memory range handling on AP */
>   if (mem_range_softc.mr_op != NULL)
> Index: arch/i386/i386/cpu.c
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/cpu.c,v
> retrieving revision 1.84
> diff -u -p -r1.84 cpu.c
> --- arch/i386/i386/cpu.c  30 May 2017 15:11:32 -  1.84
> +++ arch/i386/i386/cpu.c  18 Jun 2017 09:16:13 -
> @@ -67,6 +67,7 @@
>  #include "lapic.h"
>  #include "ioapic.h"
>  #include "vmm.h"
> +#include "pvbus.h"
>  
>  #include 
>  #include 
> @@ -104,6 +105,10 @@
>  #include 
>  #endif
>  
> +#if NPVBUS > 0
> +#include 
> +#endif
> +
>  #include 
>  #include 
>  #include 
> @@ -626,6 +631,9 @@ cpu_hatch(void *v)
>  
>   ci->ci_curpmap = pmap_kernel();
>   cpu_init(ci);
> +#if NPVBUS > 0
> + pvbus_init_cpu();
> +#endif
>  
>   /* Re-initialise memory range handling on AP */
>   if (mem_range_softc.mr_op != NULL)
> Index: dev/pv/pvbus.c
> ===
> RCS file: /cvs/src/sys/dev/pv/pvbus.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 pvbus.c
> --- dev/pv/pvbus.c10 Jan 2017 17:16:39 -  1.16
> +++ dev/pv/pvbus.c18 Jun 2017 09:16:17 -
> @@ -210,6 +210,19 @@ pvbus_identify(void)
>   has_hv_cpuid = 1;
>  }
>  
> +void
> +pvbus_init_cpu(void)
> +{
> + int i;
> +
> + for (i = 0; i < PVBUS_MAX; i++) {
> + if (pvbus_hv[i].hv_base == 0)
> + continue;
> + if (pvbus_hv[i].hv_init_cpu != NULL)
> + (pvbus_hv[i].hv_init_cpu)(_hv[i]);
> + }
> +}
> +
>  int
>  pvbus_activate(struct device *self, int act)
>  {
> Index: dev/pv/pvvar.h
> ===
> RCS file: /cvs/src/sys/dev/pv/pvvar.h,v
> retrieving revision 1.9
> diff -u -p -r1.9 pvvar.h
> --- dev/pv/pvvar.h10 Jan 2017 17:16:39 -  1.9
> +++ dev/pv/pvvar.h18 Jun 2017 09:16:17 -
> @@ -56,6 +56,7 @@ struct pvbus_hv {
>  
>   void*hv_arg;
>   int (*hv_kvop)(void *, int, char *, char *, size_t);
> + void(*hv_init_cpu)(struct pvbus_hv *);
>  };
>  
>  struct pvbus_softc {
> @@ -77,6 +78,7 @@ struct pv_attach_args {
>  
>  void  pvbus_identify(void);
>  int   pvbus_probe(void);
> +void  pvbus_init_cpu(void);
>  void  pvbus_reboot(struct device *);
>  void  pvbus_shutdown(struct device *);
>  



Re: faster timecounters for kvm/xen

2017-06-18 Thread Jonathan Matthew
On Fri, Jun 16, 2017 at 10:25:29AM +0200, Mike Belopuhov wrote:
> On Fri, Jun 16, 2017 at 16:31 +1000, Jonathan Matthew wrote:
> > Recently I updated the kernel lock profiling stuff I've been working on, 
> > since
> > it  had been rotting a bit since witness was introduced.  Running my diff 
> > on a
> > KVM VM, I found there was a pretty huge performance impact (10 minutes to
> > build a kernel instead of 4), which turned out to be because reading the
> > emulated HPET in KVM is slow, and lock profiling involves a lot of extra
> > clock reads.  The diff below adds a new TSC-based timecounter implementation
> > for KVM and Xen to remedy this.
> > 
> > KVM and Xen provide frequently-updated views of system time from the host to
> > each vcpu in a way that lets the VM get accurate high resolution time 
> > without
> > much work.  Linux calls this mechanism 'pvclock' so I'm doing the same.
> > 
> > The pvclock structure gives you a system time (in nanoseconds), the TSC
> > reading from when the time was updated, and scaling factors for converting 
> > TSC
> > values to nanoseconds.  Usually you subtract the TSC reading in the pvclock
> > structure from a current reading, convert that to nanoseconds, and add it to
> > the system time.  I decided to go the other way in order to keep all the
> > available resolution.
> > 
> > Using pvclock as the timecounter reduces the overhead of lock profiling to
> > almost nothing.  Even without the extra clock reads for lock profiling,
> > it cuts a few seconds off kernel compile time on a 2 vcpu vm.  I've run it
> > for ~12 hours without ntpd and the clock keeps time accurately.
> > 
> > One wrinkle here is that the KVM pvclock mechanism requires setup on each 
> > vcpu,
> > so I added a new pvbus function that gets called from cpu_hatch, allowing 
> > any
> > hypervisor-specific setup to happen there.
> > 
> > I still need to try this on xen, but comments at this stage are welcome.
> >
> 
> Cool!  You've beaten both of us to it :)
> 
> Last time I've tried uebayashi's pvclock on Xen, it didn't
> work for me.  I didn't have time to investigate why but
> probably because we need per-cpu readings.  Which you do
> for KVM.  I'll test this on Xen as soon as I get to the
> office.
> 
> Now regarding the diff.  pvbus_init_vcpu.  Ah yes, please.
> It was a chicken and the egg problem for me: I didn't have
> Xen, but wanted a callback from cpu_hatch to setup shared
> info pages and events (interrupt delivery) for all CPUs.
> So please factor it out and let's get that committed.

Updated version of this is below.  The init_cpu function pointer is now in
the pvbus_hv so it's easier to decide what it does at runtime.

> 
> I don't know if it's a good idea to depend on Xen's
> definition of vcpu_time_info.  I think I have factored
> it out into the pvclock_time_info and put it into the
> pvclockvar.h or something like that.  And then made Xen
> use those definitions instead of its own.  Dunno what's
> the best course of action here.
> 
> But this brings another point: where and how to perform
> the pvclock initialization and attachment.  In your diff
> pvclock_xen_init comes a bit too early: none of the Xen
> things are initialized at that point, shared info page
> isn't allocated.

I'm dropping the xen bits for now, since they need more work in a few different
ways.

> 
> I told Stefan in Munich that perhaps having a kvm.c shim
> that would prepare and attach pvclock (and maybe provide
> some flags and other bells and whistles).
> 
> I think we need to call pvclock attachment from Xen code
> where it's appropriate, not from pvbus code.  Or do a
> config_attach on it.  Why didn't you want to put it in
> its own device driver?

I was hoping it'd remain as simple as the first diff, but since things aren't
going that way I'm happy to reconsider.

> 
> It's nice that this version avoids using assembly. Any idea
> what was the reason for Linux/FreeBSD code to use it?  Were
> they afraid to lose precision maybe?

I think so.  We probably want to try harder to keep the high bits of the system
time since the low 32 bits wrap every 4s or so.


oks on this bit?

Index: arch/amd64/amd64/cpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
retrieving revision 1.105
diff -u -p -r1.105 cpu.c
--- arch/amd64/amd64/cpu.c  30 May 2017 15:11:32 -  1.105
+++ arch/amd64/amd64/cpu.c  18 Jun 2017 09:16:12 -
@@ -67,6 +67,7 @@
 #include "lapic.h"
 #include "ioapic.h"
 #include "vmm.h"
+#include "pvbus.h"
 
 #include 
 #include 
@@ -103,6 +104,10 @@
 #include 
 #endif
 
+#if NPVBUS > 0
+#include 
+#endif
+
 #include 
 #include 
 #include 
@@ -728,6 +733,9 @@ cpu_hatch(void *v)
lldt(0);
 
cpu_init(ci);
+#if NPVBUS > 0
+   pvbus_init_cpu();
+#endif
 
/* Re-initialise memory range handling on AP */
if (mem_range_softc.mr_op != NULL)
Index: arch/i386/i386/cpu.c

patch: tighten make parser and fix bug in makefile

2017-06-18 Thread Marc Espie
While reading thru gnu/usr.bin/cc, I couldn't figure out where DEPENDFILE
comes from.

Turns out it comes from FreeBSD, which currently defines it in bsd.own.mk.

The variable is empty on OpenBSD, thus yielding an always-false condition,
which was definitely not the intent.

This does put the right condition (probably not for long) and also fixes
make's parser to reject this kind of problem directly.

(currently running a src/xenocara make build/release to make sure this
doesn't affect other directories).

Once this is done, okay ?


Index: usr.bin/make/cond.c
===
RCS file: /cvs/src/usr.bin/make/cond.c,v
retrieving revision 1.51
diff -u -p -r1.51 cond.c
--- usr.bin/make/cond.c 21 Oct 2016 16:12:38 -  1.51
+++ usr.bin/make/cond.c 18 Jun 2017 10:13:32 -
@@ -292,6 +292,9 @@ CondDoExists(struct Name *arg)
bool result;
char *path;
 
+   if (arg->s == arg->e)
+   Parse_Error(PARSE_FATAL, "Empty file name in .if exists()");
+
path = Dir_FindFilei(arg->s, arg->e, defaultPath);
if (path != NULL) {
result = true;
Index: gnu/usr.bin/cc/cc_tools/Makefile
===
RCS file: /cvs/src/gnu/usr.bin/cc/cc_tools/Makefile,v
retrieving revision 1.14
diff -u -p -r1.14 Makefile
--- gnu/usr.bin/cc/cc_tools/Makefile1 Sep 2016 11:03:09 -   1.14
+++ gnu/usr.bin/cc/cc_tools/Makefile18 Jun 2017 10:13:32 -
@@ -533,7 +533,7 @@ CLEANFILES+=${GENSRCS} ${GENONLY} ${GEN
 
 #---
 # Manual dependencies.
-.if !exists(${DEPENDFILE})
+.if !exists(.depend)
 .include  "Makefile.dep"
 .endif
 



Re: inteldrm: add a handler for the WSDISPLAYIO_GINFO ioctl

2017-06-18 Thread Frederic Cambus
On Sat, Jun 17, 2017 at 05:27:38PM -0400, Ted Unangst wrote:

> > Add a handler for the WSDISPLAYIO_GINFO ioctl in inteldrm, allowing
> > to retrieve basic information about a framebuffer display.

[...]

> why? what program will use this information?

The reason I would like to add this handler is to be able to get the
frame buffer resolution in wsfontload(8). The default width/height is
currently hardcoded and ought to be computed from the frame buffer
resolution, there is a TODO in the code.

Most framebuffer drivers have handlers already, it's only missing in KMS
drivers.