Re: armv7/arm64: just a bit || some tc_counter_mask fixes

2018-03-12 Thread Artturi Alm
On Mon, Mar 12, 2018 at 11:25:09PM +0100, Mark Kettenis wrote:
> > Date: Fri, 9 Mar 2018 19:43:42 +0200
> > From: Artturi Alm 
> > 
> > Hi,
> > 
> > lazy retry[0],
> > to give these timers their (likely) copy-paste-robbed MSB back.
> > 
> > -Artturi
> > 
> > [0] https://marc.info/?l=openbsd-tech=150421823512699=1
> 
> This doesn't actually fix anything; 31 bits are plenty.  Leaving off
> the top bit protects against signed/unsigned mistakes although there
> shouldn't be any.  So I'm not sure this is worth it.
> 

forgot about the 'protects against..':
iirc. that mask is only ever used in sys/kern/kern_tc.c, so within
these timers it does nothing, so i see no protection from it.
i think it would be known by now if kern_tc.c was unable to get
along w/the 32bit rollover.

-Artturi

> > diff --git sys/arch/arm/cortex/agtimer.c sys/arch/arm/cortex/agtimer.c
> > index 8d622c058a4..160e22e6949 100644
> > --- sys/arch/arm/cortex/agtimer.c
> > +++ sys/arch/arm/cortex/agtimer.c
> > @@ -46,7 +46,7 @@ int32_t agtimer_frequency = TIMER_FREQUENCY;
> >  u_int agtimer_get_timecount(struct timecounter *);
> >  
> >  static struct timecounter agtimer_timecounter = {
> > -   agtimer_get_timecount, NULL, 0x7fff, 0, "agtimer", 0, NULL
> > +   agtimer_get_timecount, NULL, 0x, 0, "agtimer", 0, NULL
> >  };
> >  
> >  #define MAX_ARM_CPUS   8
> > diff --git sys/arch/arm/cortex/amptimer.c sys/arch/arm/cortex/amptimer.c
> > index 81880c1574a..66f0ccfed64 100644
> > --- sys/arch/arm/cortex/amptimer.c
> > +++ sys/arch/arm/cortex/amptimer.c
> > @@ -67,7 +67,7 @@ int32_t amptimer_frequency = TIMER_FREQUENCY;
> >  u_int amptimer_get_timecount(struct timecounter *);
> >  
> >  static struct timecounter amptimer_timecounter = {
> > -   amptimer_get_timecount, NULL, 0x7fff, 0, "amptimer", 0, NULL
> > +   amptimer_get_timecount, NULL, 0x, 0, "amptimer", 0, NULL
> >  };
> >  
> >  #define MAX_ARM_CPUS   8
> > diff --git sys/arch/arm64/dev/agtimer.c sys/arch/arm64/dev/agtimer.c
> > index 0e6e6a3bc6e..7d9557e706f 100644
> > --- sys/arch/arm64/dev/agtimer.c
> > +++ sys/arch/arm64/dev/agtimer.c
> > @@ -43,7 +43,7 @@ int32_t agtimer_frequency = TIMER_FREQUENCY;
> >  u_int agtimer_get_timecount(struct timecounter *);
> >  
> >  static struct timecounter agtimer_timecounter = {
> > -   agtimer_get_timecount, NULL, 0x7fff, 0, "agtimer", 0, NULL
> > +   agtimer_get_timecount, NULL, 0x, 0, "agtimer", 0, NULL
> >  };
> >  
> >  #define MAX_ARM_CPUS   8
> > diff --git sys/arch/armv7/omap/gptimer.c sys/arch/armv7/omap/gptimer.c
> > index e87db41106e..2e6c8cf8d42 100644
> > --- sys/arch/armv7/omap/gptimer.c
> > +++ sys/arch/armv7/omap/gptimer.c
> > @@ -117,7 +117,7 @@ int gptimer_irq = 0;
> >  u_int gptimer_get_timecount(struct timecounter *);
> >  
> >  static struct timecounter gptimer_timecounter = {
> > -   gptimer_get_timecount, NULL, 0x7fff, 0, "gptimer", 0, NULL
> > +   gptimer_get_timecount, NULL, 0x, 0, "gptimer", 0, NULL
> >  };
> >  
> >  volatile u_int32_t nexttickevent;
> > 
> > 



Re: armv7/arm64: just a bit || some tc_counter_mask fixes

2018-03-12 Thread Artturi Alm
On Mon, Mar 12, 2018 at 11:25:09PM +0100, Mark Kettenis wrote:
> > Date: Fri, 9 Mar 2018 19:43:42 +0200
> > From: Artturi Alm 
> > 
> > Hi,
> > 
> > lazy retry[0],
> > to give these timers their (likely) copy-paste-robbed MSB back.
> > 
> > -Artturi
> > 
> > [0] https://marc.info/?l=openbsd-tech=150421823512699=1
> 
> This doesn't actually fix anything; 31 bits are plenty.  Leaving off
> the top bit protects against signed/unsigned mistakes although there
> shouldn't be any.  So I'm not sure this is worth it.
> 

I still think the current masks are only brought over(by accident) from
some macppc timer actually having had only 31bits in the early 90s.
also, fwiw. all these timers have atleast 32bits in use on other BSDs.

now if focusing only the documentation issue i'm having with these masks
in use, is that either this(comment) should be changed(sys/timetc.h):

 47 timecounter_get_t   *tc_get_timecount;
 48 /*
 49  * This function reads the counter.  It is not required to
 50  * mask any unimplemented bits out, as long as they are
 51  * constant.
 52  */

or the *tc_get_timecount functions of affected timers be changed to mask
out the non-constant bit, which i think would be silly.

tbh., i'm only worried about amptimer w/high default freq(396MHz),
and only because i'm still optimistic we would see the HZ bump[0] in near
future, making the lack of MSB possibly more of an possible issue?

-Artturi

[0] https://marc.info/?l=openbsd-tech=150274123011009=1



Re: armv7/arm64: just a bit || some tc_counter_mask fixes

2018-03-12 Thread Mark Kettenis
> Date: Fri, 9 Mar 2018 19:43:42 +0200
> From: Artturi Alm 
> 
> Hi,
> 
> lazy retry[0],
> to give these timers their (likely) copy-paste-robbed MSB back.
> 
> -Artturi
> 
> [0] https://marc.info/?l=openbsd-tech=150421823512699=1

This doesn't actually fix anything; 31 bits are plenty.  Leaving off
the top bit protects against signed/unsigned mistakes although there
shouldn't be any.  So I'm not sure this is worth it.

> diff --git sys/arch/arm/cortex/agtimer.c sys/arch/arm/cortex/agtimer.c
> index 8d622c058a4..160e22e6949 100644
> --- sys/arch/arm/cortex/agtimer.c
> +++ sys/arch/arm/cortex/agtimer.c
> @@ -46,7 +46,7 @@ int32_t agtimer_frequency = TIMER_FREQUENCY;
>  u_int agtimer_get_timecount(struct timecounter *);
>  
>  static struct timecounter agtimer_timecounter = {
> - agtimer_get_timecount, NULL, 0x7fff, 0, "agtimer", 0, NULL
> + agtimer_get_timecount, NULL, 0x, 0, "agtimer", 0, NULL
>  };
>  
>  #define MAX_ARM_CPUS 8
> diff --git sys/arch/arm/cortex/amptimer.c sys/arch/arm/cortex/amptimer.c
> index 81880c1574a..66f0ccfed64 100644
> --- sys/arch/arm/cortex/amptimer.c
> +++ sys/arch/arm/cortex/amptimer.c
> @@ -67,7 +67,7 @@ int32_t amptimer_frequency = TIMER_FREQUENCY;
>  u_int amptimer_get_timecount(struct timecounter *);
>  
>  static struct timecounter amptimer_timecounter = {
> - amptimer_get_timecount, NULL, 0x7fff, 0, "amptimer", 0, NULL
> + amptimer_get_timecount, NULL, 0x, 0, "amptimer", 0, NULL
>  };
>  
>  #define MAX_ARM_CPUS 8
> diff --git sys/arch/arm64/dev/agtimer.c sys/arch/arm64/dev/agtimer.c
> index 0e6e6a3bc6e..7d9557e706f 100644
> --- sys/arch/arm64/dev/agtimer.c
> +++ sys/arch/arm64/dev/agtimer.c
> @@ -43,7 +43,7 @@ int32_t agtimer_frequency = TIMER_FREQUENCY;
>  u_int agtimer_get_timecount(struct timecounter *);
>  
>  static struct timecounter agtimer_timecounter = {
> - agtimer_get_timecount, NULL, 0x7fff, 0, "agtimer", 0, NULL
> + agtimer_get_timecount, NULL, 0x, 0, "agtimer", 0, NULL
>  };
>  
>  #define MAX_ARM_CPUS 8
> diff --git sys/arch/armv7/omap/gptimer.c sys/arch/armv7/omap/gptimer.c
> index e87db41106e..2e6c8cf8d42 100644
> --- sys/arch/armv7/omap/gptimer.c
> +++ sys/arch/armv7/omap/gptimer.c
> @@ -117,7 +117,7 @@ int gptimer_irq = 0;
>  u_int gptimer_get_timecount(struct timecounter *);
>  
>  static struct timecounter gptimer_timecounter = {
> - gptimer_get_timecount, NULL, 0x7fff, 0, "gptimer", 0, NULL
> + gptimer_get_timecount, NULL, 0x, 0, "gptimer", 0, NULL
>  };
>  
>  volatile u_int32_t nexttickevent;
> 
> 



Re: arm64: Makefile.arm64 av7 content

2018-03-12 Thread Mark Kettenis
> Date: Fri, 9 Mar 2018 14:10:41 +0200
> From: Artturi Alm 
> 
> On Tue, Mar 06, 2018 at 11:34:23PM +0200, Artturi Alm wrote:
> > Hi,
> > 
> > was 'grepping' for something, and these came up.
> > 
> > i guess atleast trampoline.S would work w/o including assym.h at all,
> > but here is what i'm sure enough of w/o actually compiling yet. :D
> > 
> > -Artturi
> > 
> > 
> > diff --git sys/arch/arm64/conf/Makefile.arm64 
> > sys/arch/arm64/conf/Makefile.arm64
> > index bd56f7a1427..f98d82ff0f7 100644
> > --- sys/arch/arm64/conf/Makefile.arm64
> > +++ sys/arch/arm64/conf/Makefile.arm64
> > @@ -143,9 +143,9 @@ cleandir: clean
> >  depend obj:
> >  
> >  locore0.o: ${_archdir}/${_arch}/locore0.S assym.h
> > -in_cksum_arm.o fiq_subr.o bcopyinout.o copystr.o sigcode.o copy.o: assym.h
> > -cpuswitch.o exception.o bcopy_page.o irq_dispatch.o support.o: assym.h
> > -locore.o vectors.o trampoline.o: assym.h
> > +copy.o copystr.o.o: assym.h
> > +cpuswitch.o exception.o support.o: assym.h
> > +locore.o trampoline.o: assym.h
> >  
> >  hardlink-obsd:
> > [[ ! -f /bsd ]] || cmp -s bsd /bsd || ln -f /bsd /obsd
> 
> diff below is what i ended up w/after fixing the extra '.o'-mistake in
> above diff. built on pine64.
> 
> -Artturi

I'm inclined to leave the assym.h includes even if those files
currently don't use any symbols from there.  So I committed
essentially your first diff.

Thanks,

Mark

> diff --git sys/arch/arm64/arm64/support.S sys/arch/arm64/arm64/support.S
> index ea0212b59ae..cccfd2f51ae 100644
> --- sys/arch/arm64/arm64/support.S
> +++ sys/arch/arm64/arm64/support.S
> @@ -39,8 +39,6 @@ __FBSDID("$FreeBSD: head/sys/arm64/arm64/support.S 297615 
> 2016-04-06 14:08:10Z a
>  #include 
>  #include 
>  
> -#include "assym.h"
> -
>  #ifdef DDB
>  ENTRY(setjmp)
>   /* Store the stack pointer */
> diff --git sys/arch/arm64/arm64/trampoline.S sys/arch/arm64/arm64/trampoline.S
> index 4de835ddb5c..2f633918c5c 100644
> --- sys/arch/arm64/arm64/trampoline.S
> +++ sys/arch/arm64/arm64/trampoline.S
> @@ -18,7 +18,6 @@
>  
>  #include 
>  #include 
> -#include "assym.h"
>  
>   .text
>  
> diff --git sys/arch/arm64/conf/Makefile.arm64 
> sys/arch/arm64/conf/Makefile.arm64
> index bd56f7a1427..c56b597ba3c 100644
> --- sys/arch/arm64/conf/Makefile.arm64
> +++ sys/arch/arm64/conf/Makefile.arm64
> @@ -143,9 +143,9 @@ cleandir: clean
>  depend obj:
>  
>  locore0.o: ${_archdir}/${_arch}/locore0.S assym.h
> -in_cksum_arm.o fiq_subr.o bcopyinout.o copystr.o sigcode.o copy.o: assym.h
> -cpuswitch.o exception.o bcopy_page.o irq_dispatch.o support.o: assym.h
> -locore.o vectors.o trampoline.o: assym.h
> +copy.o copystr.o: assym.h
> +cpuswitch.o exception.o: assym.h
> +locore.o: assym.h
>  
>  hardlink-obsd:
>   [[ ! -f /bsd ]] || cmp -s bsd /bsd || ln -f /bsd /obsd
> 
> 



Re: Kernel size beyond 16 MB on amd64

2018-03-12 Thread Mike Larkin
On Mon, Mar 12, 2018 at 05:50:42PM +0100, Franco Fichtner wrote:
> Hi,
> 
> With regard to a commit[1] by Theo in 2013, several questions
> in the years before and a partial lift of the limitation on
> i386 a while back (2015?) I'd like to ask what the future plans
> are for OpenBSD.
> 
> Peeking at NetBSD, where the amd64 was bootstrapped, they are at
> 48 MB kernel size at the moment.
> 
> Will a future OpenBSD release increase it?
> 
> What can we do to help?
> 
> 
> Cheers,
> Franco
> 
> --
> [1] https://github.com/openbsd/src/commit/453010f2034
> 

That commit was only to move the kernel phys load address to 16MB.

If the kernel should grow to a point where we run past some limit, we'll fix
it. Right now, bsd.gdb kernels are > 50MB and load fine.

-ml



unhandled firmware response in iwm0

2018-03-12 Thread Jan Schreiber
Hi,

when connecting to an open wifi I get following message:

iwm0: unhandled firmware response 0xce/0xb82c rx ring 0[48]

I added another case which ignores the IWM_BT_PROFILE_NOTIFICATION.
I didn't notice any change when connecting, except that the message is
gone.

diff --git sys/dev/pci/if_iwm.c sys/dev/pci/if_iwm.c
index fa3c1c82cc8..5b7b116dba4 100644
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -7320,6 +7320,9 @@ iwm_notif_intr(struct iwm_softc *sc)
IWM_FSEQ_VER_MISMATCH_NOTIFICATION):
break;
 
+   case IWM_BT_PROFILE_NOTIFICATION:
+   break;
+
/*
 * Firmware versions 21 and 22 generate some DEBUG_LOG_MSG
 * messages. Just ignore them for now.



Kernel size beyond 16 MB on amd64

2018-03-12 Thread Franco Fichtner
Hi,

With regard to a commit[1] by Theo in 2013, several questions
in the years before and a partial lift of the limitation on
i386 a while back (2015?) I'd like to ask what the future plans
are for OpenBSD.

Peeking at NetBSD, where the amd64 was bootstrapped, they are at
48 MB kernel size at the moment.

Will a future OpenBSD release increase it?

What can we do to help?


Cheers,
Franco

--
[1] https://github.com/openbsd/src/commit/453010f2034



Re: sparc64 softraid boot: workaround for memory leak

2018-03-12 Thread Stefan Sperling
On Sun, Mar 04, 2018 at 05:02:45PM +0100, Stefan Sperling wrote:
> On my T5220 LDOM guests cannot boot from softraid because ofwboot
> crashes with a "Fast Data Access MMU Miss" while loading the kernel:
> 
> >> OpenBSD BOOT 1.9   
> ERROR: /iscsi-hba: No iscsi-network-bootpath property  
> sr0*
> Passphrase:   
> Booting sr0:a/bsd   
> 8480888@0x100
> ERROR: Last Trap: Fast Data Access MMU Miss 
> 
> I've tracked down the failure to a crash in OF_open() called in sr_strategy().
> There is no missing OF_close() call. So it seems a memory/resource leak
> of some kind is happening in firmware during OF_open()/OF_close().
> 
> Affected firmware version info:
> 
>   SP firmware 3.0.12.8.a
>   SP firmware build number: 108523
>   SP firmware date: Fri Mar 11 07:19:16 PST 2016
>   SP filesystem version: 0.1.22
> 
> hypervisor_version = Hypervisor 1.10.7.h 2016/03/11 07:13
> obp_version = OpenBoot 4.33.6.g 2016/03/11 06:05
> post_version = POST 4.33.6.g 2016/03/11 06:15
> status = OpenBSD running
> sysfw_version = Sun System Firmware 7.4.10.a 2016/03/11 07:45
> 
> The diff below allows a guest to boot from softraid on this machine.

I haven't had any feedback on the previous diff. However, I felt it
was a bit of a hack, so I tried to come up with a cleaner solution.

The diff below opens the softraid boot chunk early on and keeps the
handle open until a kernel has been loaded from the softraid volume.
This means sr_strategy() does not have to worry about obtaining a
handle from the firmware anymore.

Also consolidate some repeated lines of code in sr_strategy().

Tested on a T5220 ldom guest (CRYPTO) and a v240 machine (RAID1).

Index: boot.c
===
RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/boot.c,v
retrieving revision 1.27
diff -u -p -r1.27 boot.c
--- boot.c  11 Sep 2016 17:53:26 -  1.27
+++ boot.c  12 Mar 2018 10:26:23 -
@@ -248,6 +248,8 @@ loadfile(int fd, char *args)
close(fd);
 
 #ifdef SOFTRAID
+   if (bootdev_dip)
+   OF_close(bootdev_dip->sr_handle);
sr_clear_keys();
 #endif
chain(entry, args, ssym, esym);
@@ -283,12 +285,11 @@ fail:
 }
 
 #ifdef SOFTRAID
-/* Set bootdev_dip to the software boot volume, if specified. */
+/* Set bootdev_dip to the softraid boot volume, if specified. */
 static int
 srbootdev(const char *bootline)
 {
struct sr_boot_volume *bv;
-   struct diskinfo *dip;
int unit;
 
bootdev_dip = NULL;
@@ -320,9 +321,23 @@ srbootdev(const char *bootline)
return EPERM;
 
if (bv->sbv_diskinfo == NULL) {
+   struct sr_boot_chunk *bc;
+   struct diskinfo *dip, *bc_dip;
+   int sr_handle;
+
+   /* All reads will come from the boot chunk. */
+   bc = sr_vol_boot_chunk(bv);
+   if (bc == NULL)
+   return ENXIO;
+   bc_dip = (struct diskinfo *)bc->sbc_diskinfo;
+   sr_handle = OF_open(bc_dip->path);
+   if (sr_handle == -1)
+   return EIO;
+
dip = alloc(sizeof(struct diskinfo));
bzero(dip, sizeof(*dip));
dip->sr_vol = bv;
+   dip->sr_handle = sr_handle;
bv->sbv_diskinfo = dip;
}
 
@@ -331,7 +346,8 @@ srbootdev(const char *bootline)
 
/* Attempt to read disklabel. */
bv->sbv_part = 'c';
-   if (sr_getdisklabel(bv, >disklabel)) {
+   if (sr_getdisklabel(bv, _dip->disklabel)) {
+   OF_close(bootdev_dip->sr_handle);
free(bv->sbv_diskinfo, sizeof(struct diskinfo));
bv->sbv_diskinfo = NULL;
bootdev_dip = NULL;
Index: disk.h
===
RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/disk.h,v
retrieving revision 1.1
diff -u -p -r1.1 disk.h
--- disk.h  26 Nov 2014 20:30:41 -  1.1
+++ disk.h  12 Mar 2018 10:26:23 -
@@ -36,7 +36,10 @@
 struct diskinfo {
char path[256];
struct disklabel disklabel;
+
+   /* Used during softraid boot. */
struct sr_boot_volume *sr_vol;
+   int sr_handle; /* open handle for reading from boot chunk */
 
TAILQ_ENTRY(diskinfo) list;
 };
Index: ofdev.c
===
RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/ofdev.c,v
retrieving revision 1.25
diff -u -p -r1.25 ofdev.c
--- ofdev.c 1 Oct 2015 16:08:20 -  

Re: [PATCH] Fix font rendering error in FreeType 2.8.1 update, memory allocation related?

2018-03-12 Thread Bryan Linton
On 2018-03-11 18:03:27, Matthieu Herrb  wrote:
> On Sun, Mar 11, 2018 at 11:58:50AM +0900, Bryan Linton wrote:
> > [I sent a copy of this email to tech@ 24 hours ago and it hasn't
> > shown up yet.  Apologies if this ends up being a double-post.]
> > 
> > Hello tech@
> > 
> > I'm not sure if it's bad form to crosspost this from bugs@ to
> > tech@ or not, but I have a patch that fixes a font rendering issue
> > with the new FreeType 2.8.1 update that went in last December.
> > 
> > The beginning of the thread can be found at the following link.
> > It also includes a screenshot (linked at the very bottom of the
> > page) of how it manifests in an xterm.  FWIW, I also see the same
> > rendering issue in Firefox, so it's not just limited to the console:
> > https://marc.info/?l=openbsd-bugs=151835980313045=2
> > 
> > I managed to bisect things, and found that this one-line diff
> > fixes the issue for me.  It's a reversion back to the line of code
> > that was present in FreeType 2.8.0.
> 
> Thanks for bisecting the problem which apparently only happens for
> some specific fonts since I've not been able to reproduce it with any
> other font nor have I seen other reports of similar issues.
> 
> Did you try to report it upstreams ? (in the mean time Freetype 2.9
> has been released, but without change to this code.
> 

I have not reported this to upstream yet.  I see that you have
already done so in my stead though.  Thank you for doing so.

> Reading the commit bd28952e23bcd268a623ea5202e1cde4a92defe4 from
> upstreams they seem to assume that the memory allocated by
> memory->alloc() is already zeroed. The original but report is
> https://savannah.nongnu.org/bugs/?51816
> 
> Does an explicit zeroing like in the patch below also fix the issue
> for you ?
> 

Yes, I can confirm that your patch fixes this issue for me.  Thank
you very much for looking into this for me.

-- 
Bryan