Re: Add T490 Thunderbolt 3 controller to pcidevs
On Wed, Sep 14, 2022 at 08:51:46PM -0400, Kurt Mosiejczuk wrote: > On Thu, Sep 15, 2022 at 10:36:40AM +1000, Jonathan Gray wrote: > > On Wed, Sep 14, 2022 at 12:49:48PM -0400, Kurt Mosiejczuk wrote: > > > This adds the IDs for the thunderbolt 3 controller parts in my T490 > > > > ok? > > > Shouldn't this be JHL6240 not JHL6420? > > > https://downloadmirror.intel.com/28658/eng/tbt_release_notes_17.4.77.400.txt > > JHL6240 Series - "Alpine Ridge LP" > > > https://ark.intel.com/content/www/us/en/ark/products/94030/intel-jhl6240-thunderbolt-3-controller.html > > I mean, sure, if you want to be a hidebound "put the digits in the correct > order" kind of person. :D > > Corrected diff below. ok jsg@ > > --Kurt > > Index: pcidevs > === > RCS file: /cvs/src/sys/dev/pci/pcidevs,v > retrieving revision 1.2004 > diff -u -p -r1.2004 pcidevs > --- pcidevs 2 Sep 2022 10:34:07 - 1.2004 > +++ pcidevs 15 Sep 2022 00:50:17 - > @@ -4008,6 +4008,9 @@ product INTEL I219_LM7 0x15bb I219-LM > product INTEL I219_V70x15bc I219-V > product INTEL I219_LM6 0x15bd I219-LM > product INTEL I219_V60x15be I219-V > +product INTEL JHL62400x15bf JHL6240 Thunderbolt 3 > +product INTEL JHL6240_PCIE 0x15c0 JHL6240 Thunderbolt 3 > +product INTEL JHL6240_XHCI 0x15c1 JHL6240 Thunderbolt 3 > product INTEL X550EM_A_KR0x15c2 X553 Backplane > product INTEL X550EM_A_KR_L 0x15c3 X553 Backplane > product INTEL X550EM_A_SFP_N 0x15c4 X553 SFP+ > >
Re: Add T490 Thunderbolt 3 controller to pcidevs
On Thu, Sep 15, 2022 at 10:36:40AM +1000, Jonathan Gray wrote: > On Wed, Sep 14, 2022 at 12:49:48PM -0400, Kurt Mosiejczuk wrote: > > This adds the IDs for the thunderbolt 3 controller parts in my T490 > > ok? > Shouldn't this be JHL6240 not JHL6420? > https://downloadmirror.intel.com/28658/eng/tbt_release_notes_17.4.77.400.txt > JHL6240 Series - "Alpine Ridge LP" > https://ark.intel.com/content/www/us/en/ark/products/94030/intel-jhl6240-thunderbolt-3-controller.html I mean, sure, if you want to be a hidebound "put the digits in the correct order" kind of person. :D Corrected diff below. --Kurt Index: pcidevs === RCS file: /cvs/src/sys/dev/pci/pcidevs,v retrieving revision 1.2004 diff -u -p -r1.2004 pcidevs --- pcidevs 2 Sep 2022 10:34:07 - 1.2004 +++ pcidevs 15 Sep 2022 00:50:17 - @@ -4008,6 +4008,9 @@ product INTEL I219_LM70x15bb I219-LM product INTEL I219_V7 0x15bc I219-V product INTEL I219_LM6 0x15bd I219-LM product INTEL I219_V6 0x15be I219-V +product INTEL JHL6240 0x15bf JHL6240 Thunderbolt 3 +product INTEL JHL6240_PCIE 0x15c0 JHL6240 Thunderbolt 3 +product INTEL JHL6240_XHCI 0x15c1 JHL6240 Thunderbolt 3 product INTEL X550EM_A_KR 0x15c2 X553 Backplane product INTEL X550EM_A_KR_L0x15c3 X553 Backplane product INTEL X550EM_A_SFP_N 0x15c4 X553 SFP+
Re: Add T490 Thunderbolt 3 controller to pcidevs
On Wed, Sep 14, 2022 at 12:49:48PM -0400, Kurt Mosiejczuk wrote: > This adds the IDs for the thunderbolt 3 controller parts in my T490 > > ok? Shouldn't this be JHL6240 not JHL6420? https://downloadmirror.intel.com/28658/eng/tbt_release_notes_17.4.77.400.txt JHL6240 Series - "Alpine Ridge LP" https://ark.intel.com/content/www/us/en/ark/products/94030/intel-jhl6240-thunderbolt-3-controller.html > > --Kurt > > Index: pcidevs > === > RCS file: /cvs/src/sys/dev/pci/pcidevs,v > retrieving revision 1.2004 > diff -u -p -r1.2004 pcidevs > --- pcidevs 2 Sep 2022 10:34:07 - 1.2004 > +++ pcidevs 14 Sep 2022 16:47:36 - > @@ -4008,6 +4008,9 @@ product INTEL I219_LM7 0x15bb I219-LM > product INTEL I219_V70x15bc I219-V > product INTEL I219_LM6 0x15bd I219-LM > product INTEL I219_V60x15be I219-V > +product INTEL JHL64200x15bf JHL6420 Thunderbolt 3 > +product INTEL JHL6420_PCIE 0x15c0 JHL6420 Thunderbolt 3 > +product INTEL JHL6420_XHCI 0x15c1 JHL6420 Thunderbolt 3 > product INTEL X550EM_A_KR0x15c2 X553 Backplane > product INTEL X550EM_A_KR_L 0x15c3 X553 Backplane > product INTEL X550EM_A_SFP_N 0x15c4 X553 SFP+ > >
Re: Towards unlocking mmap(2) & munmap(2)
On 14/09/22(Wed) 15:47, Klemens Nanni wrote: > On 14.09.22 18:55, Mike Larkin wrote: > > On Sun, Sep 11, 2022 at 12:26:31PM +0200, Martin Pieuchot wrote: > > > Diff below adds a minimalist set of assertions to ensure proper locks > > > are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of > > > mmap(2) for anons and munmap(2). > > > > > > Please test it with WITNESS enabled and report back. > > > > > > > Do you want this tested in conjunction with the aiodoned diff or by itself? > > This diff looks like a subset of the previous uvm lock assertion diff > that came out of the previous "unlock mmap(2) for anonymous mappings" > thread[0]. > > https://marc.info/?l=openbsd-tech&m=164423248318212&w=2 > > It didn't land eventually, I **think** syzcaller was a blocker which we > only realised once it was committed and picked up by syzcaller. > > Now it's been some time and more UVM changes landed, but the majority > (if not all) lock assertions and comments from the above linked diff > should still hold true. > > mpi, I can dust off and resend that diff, If you want. > Nothing for release, but perhaps it helps testing your current efforts. Please hold on, this diff is known to trigger a KASSERT() with witness. I'll send an update version soon. Thank you for disregarding this diff for the moment.
Re: bgpd fix connected route removal
On Wed, Sep 14, 2022 at 07:24:56PM +0200, Claudio Jeker wrote:
> The kroute_remove() code will fail hard when a connected route is removed.
> Most commonly this happens when an interface is deconfigured.
> The problem is that there is no logic to match against connected routes.
> Connected routes have no real nexthop and just use the ifindex as
> identifier, so adjust kroute_matchgw() to select the right route.
>
> This diff solves the problem I have seen while testing.
ok tb
> --
> :wq Claudio
>
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.298
> diff -u -p -r1.298 kroute.c
> --- kroute.c 30 Aug 2022 16:00:21 - 1.298
> +++ kroute.c 14 Sep 2022 17:00:37 -
> @@ -136,14 +136,14 @@ int kif_compare(struct kif *, struct kif
>
> struct kroute*kroute_find(struct ktable *, const struct bgpd_addr *,
> uint8_t, uint8_t);
> -struct kroute*kroute_matchgw(struct kroute *, struct bgpd_addr *);
> +struct kroute*kroute_matchgw(struct kroute *, struct kroute_full *);
> int kroute_insert(struct ktable *, struct kroute_full *);
> int kroute_remove(struct ktable *, struct kroute_full *, int);
> void kroute_clear(struct ktable *);
>
> struct kroute6 *kroute6_find(struct ktable *, const struct bgpd_addr *,
> uint8_t, uint8_t);
> -struct kroute6 *kroute6_matchgw(struct kroute6 *, struct bgpd_addr *);
> +struct kroute6 *kroute6_matchgw(struct kroute6 *, struct kroute_full
> *);
> void kroute6_clear(struct ktable *);
>
> struct knexthop *knexthop_find(struct ktable *, struct bgpd_addr *);
> @@ -1583,16 +1583,20 @@ kroute_find(struct ktable *kt, const str
> }
>
> struct kroute *
> -kroute_matchgw(struct kroute *kr, struct bgpd_addr *gw)
> +kroute_matchgw(struct kroute *kr, struct kroute_full *kf)
> {
> in_addr_t nexthop;
>
> - if (gw->aid != AID_INET) {
> - log_warnx("%s: no nexthop defined", __func__);
> + if (kf->flags & F_CONNECTED) {
> + do {
> + if (kr->ifindex == kf->ifindex)
> + return (kr);
> + kr = kr->next;
> + } while (kr);
> return (NULL);
> }
> - nexthop = gw->v4.s_addr;
>
> + nexthop = kf->nexthop.v4.s_addr;
> do {
> if (kr->nexthop.s_addr == nexthop)
> return (kr);
> @@ -1735,7 +1739,7 @@ kroute4_remove(struct ktable *kt, struct
> /* get the correct route to remove */
> krm = kr;
> if (!any) {
> - if ((krm = kroute_matchgw(kr, &kf->nexthop)) == NULL) {
> + if ((krm = kroute_matchgw(kr, kf)) == NULL) {
> log_warnx("delete %s/%u: route not found",
> log_addr(&kf->prefix), kf->prefixlen);
> return (-2);
> @@ -1805,7 +1809,7 @@ kroute6_remove(struct ktable *kt, struct
> /* get the correct route to remove */
> krm = kr;
> if (!any) {
> - if ((krm = kroute6_matchgw(kr, &kf->nexthop)) == NULL) {
> + if ((krm = kroute6_matchgw(kr, kf)) == NULL) {
> log_warnx("delete %s/%u: route not found",
> log_addr(&kf->prefix), kf->prefixlen);
> return (-2);
> @@ -1919,19 +1923,23 @@ kroute6_find(struct ktable *kt, const st
> }
>
> struct kroute6 *
> -kroute6_matchgw(struct kroute6 *kr, struct bgpd_addr *gw)
> +kroute6_matchgw(struct kroute6 *kr, struct kroute_full *kf)
> {
> struct in6_addr nexthop;
>
> - if (gw->aid != AID_INET6) {
> - log_warnx("%s: no nexthop defined", __func__);
> + if (kf->flags & F_CONNECTED) {
> + do {
> + if (kr->ifindex == kf->ifindex)
> + return (kr);
> + kr = kr->next;
> + } while (kr);
> return (NULL);
> }
> - nexthop = gw->v6;
>
> + nexthop = kf->nexthop.v6;
> do {
> if (memcmp(&kr->nexthop, &nexthop, sizeof(nexthop)) == 0 &&
> - kr->nexthop_scope_id == gw->scope_id)
> + kr->nexthop_scope_id == kf->nexthop.scope_id)
> return (kr);
> kr = kr->next;
> } while (kr);
> @@ -3117,8 +3125,7 @@ kr_fib_change(struct ktable *kt, struct
> if (!(kf->flags & F_BGPD)) {
> /* get the correct route */
> if (mpath && type == RTM_CHANGE &&
> - (kr = kroute_matchgw(kr, &kf->nexthop)) ==
> - NULL) {
> + (kr = kroute_matchgw(kr, kf)) == NULL) {
> log_warnx("%s[change]: "
>
bgpd fix connected route removal
The kroute_remove() code will fail hard when a connected route is removed.
Most commonly this happens when an interface is deconfigured.
The problem is that there is no logic to match against connected routes.
Connected routes have no real nexthop and just use the ifindex as
identifier, so adjust kroute_matchgw() to select the right route.
This diff solves the problem I have seen while testing.
--
:wq Claudio
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.298
diff -u -p -r1.298 kroute.c
--- kroute.c30 Aug 2022 16:00:21 - 1.298
+++ kroute.c14 Sep 2022 17:00:37 -
@@ -136,14 +136,14 @@ int kif_compare(struct kif *, struct kif
struct kroute *kroute_find(struct ktable *, const struct bgpd_addr *,
uint8_t, uint8_t);
-struct kroute *kroute_matchgw(struct kroute *, struct bgpd_addr *);
+struct kroute *kroute_matchgw(struct kroute *, struct kroute_full *);
int kroute_insert(struct ktable *, struct kroute_full *);
int kroute_remove(struct ktable *, struct kroute_full *, int);
voidkroute_clear(struct ktable *);
struct kroute6 *kroute6_find(struct ktable *, const struct bgpd_addr *,
uint8_t, uint8_t);
-struct kroute6 *kroute6_matchgw(struct kroute6 *, struct bgpd_addr *);
+struct kroute6 *kroute6_matchgw(struct kroute6 *, struct kroute_full *);
voidkroute6_clear(struct ktable *);
struct knexthop*knexthop_find(struct ktable *, struct bgpd_addr *);
@@ -1583,16 +1583,20 @@ kroute_find(struct ktable *kt, const str
}
struct kroute *
-kroute_matchgw(struct kroute *kr, struct bgpd_addr *gw)
+kroute_matchgw(struct kroute *kr, struct kroute_full *kf)
{
in_addr_t nexthop;
- if (gw->aid != AID_INET) {
- log_warnx("%s: no nexthop defined", __func__);
+ if (kf->flags & F_CONNECTED) {
+ do {
+ if (kr->ifindex == kf->ifindex)
+ return (kr);
+ kr = kr->next;
+ } while (kr);
return (NULL);
}
- nexthop = gw->v4.s_addr;
+ nexthop = kf->nexthop.v4.s_addr;
do {
if (kr->nexthop.s_addr == nexthop)
return (kr);
@@ -1735,7 +1739,7 @@ kroute4_remove(struct ktable *kt, struct
/* get the correct route to remove */
krm = kr;
if (!any) {
- if ((krm = kroute_matchgw(kr, &kf->nexthop)) == NULL) {
+ if ((krm = kroute_matchgw(kr, kf)) == NULL) {
log_warnx("delete %s/%u: route not found",
log_addr(&kf->prefix), kf->prefixlen);
return (-2);
@@ -1805,7 +1809,7 @@ kroute6_remove(struct ktable *kt, struct
/* get the correct route to remove */
krm = kr;
if (!any) {
- if ((krm = kroute6_matchgw(kr, &kf->nexthop)) == NULL) {
+ if ((krm = kroute6_matchgw(kr, kf)) == NULL) {
log_warnx("delete %s/%u: route not found",
log_addr(&kf->prefix), kf->prefixlen);
return (-2);
@@ -1919,19 +1923,23 @@ kroute6_find(struct ktable *kt, const st
}
struct kroute6 *
-kroute6_matchgw(struct kroute6 *kr, struct bgpd_addr *gw)
+kroute6_matchgw(struct kroute6 *kr, struct kroute_full *kf)
{
struct in6_addr nexthop;
- if (gw->aid != AID_INET6) {
- log_warnx("%s: no nexthop defined", __func__);
+ if (kf->flags & F_CONNECTED) {
+ do {
+ if (kr->ifindex == kf->ifindex)
+ return (kr);
+ kr = kr->next;
+ } while (kr);
return (NULL);
}
- nexthop = gw->v6;
+ nexthop = kf->nexthop.v6;
do {
if (memcmp(&kr->nexthop, &nexthop, sizeof(nexthop)) == 0 &&
- kr->nexthop_scope_id == gw->scope_id)
+ kr->nexthop_scope_id == kf->nexthop.scope_id)
return (kr);
kr = kr->next;
} while (kr);
@@ -3117,8 +3125,7 @@ kr_fib_change(struct ktable *kt, struct
if (!(kf->flags & F_BGPD)) {
/* get the correct route */
if (mpath && type == RTM_CHANGE &&
- (kr = kroute_matchgw(kr, &kf->nexthop)) ==
- NULL) {
+ (kr = kroute_matchgw(kr, kf)) == NULL) {
log_warnx("%s[change]: "
"mpath route not found", __func__);
goto add4;
@@ -3183,8 +3190,7 @@ add4:
if (!(kf->flags & F_BGPD)) {
Add T490 Thunderbolt 3 controller to pcidevs
This adds the IDs for the thunderbolt 3 controller parts in my T490 ok? --Kurt Index: pcidevs === RCS file: /cvs/src/sys/dev/pci/pcidevs,v retrieving revision 1.2004 diff -u -p -r1.2004 pcidevs --- pcidevs 2 Sep 2022 10:34:07 - 1.2004 +++ pcidevs 14 Sep 2022 16:47:36 - @@ -4008,6 +4008,9 @@ product INTEL I219_LM70x15bb I219-LM product INTEL I219_V7 0x15bc I219-V product INTEL I219_LM6 0x15bd I219-LM product INTEL I219_V6 0x15be I219-V +product INTEL JHL6420 0x15bf JHL6420 Thunderbolt 3 +product INTEL JHL6420_PCIE 0x15c0 JHL6420 Thunderbolt 3 +product INTEL JHL6420_XHCI 0x15c1 JHL6420 Thunderbolt 3 product INTEL X550EM_A_KR 0x15c2 X553 Backplane product INTEL X550EM_A_KR_L0x15c3 X553 Backplane product INTEL X550EM_A_SFP_N 0x15c4 X553 SFP+
Re: Towards unlocking mmap(2) & munmap(2)
On 14.09.22 18:55, Mike Larkin wrote: On Sun, Sep 11, 2022 at 12:26:31PM +0200, Martin Pieuchot wrote: Diff below adds a minimalist set of assertions to ensure proper locks are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of mmap(2) for anons and munmap(2). Please test it with WITNESS enabled and report back. Do you want this tested in conjunction with the aiodoned diff or by itself? This diff looks like a subset of the previous uvm lock assertion diff that came out of the previous "unlock mmap(2) for anonymous mappings" thread[0]. https://marc.info/?l=openbsd-tech&m=164423248318212&w=2 It didn't land eventually, I **think** syzcaller was a blocker which we only realised once it was committed and picked up by syzcaller. Now it's been some time and more UVM changes landed, but the majority (if not all) lock assertions and comments from the above linked diff should still hold true. mpi, I can dust off and resend that diff, If you want. Nothing for release, but perhaps it helps testing your current efforts.
Re: Towards unlocking mmap(2) & munmap(2)
On Sun, Sep 11, 2022 at 12:26:31PM +0200, Martin Pieuchot wrote:
> Diff below adds a minimalist set of assertions to ensure proper locks
> are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of
> mmap(2) for anons and munmap(2).
>
> Please test it with WITNESS enabled and report back.
>
Do you want this tested in conjunction with the aiodoned diff or by itself?
> Index: uvm/uvm_addr.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_addr.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 uvm_addr.c
> --- uvm/uvm_addr.c21 Feb 2022 10:26:20 - 1.31
> +++ uvm/uvm_addr.c11 Sep 2022 09:08:10 -
> @@ -416,6 +416,8 @@ uvm_addr_invoke(struct vm_map *map, stru
> !(hint >= uaddr->uaddr_minaddr && hint < uaddr->uaddr_maxaddr))
> return ENOMEM;
>
> + vm_map_assert_anylock(map);
> +
> error = (*uaddr->uaddr_functions->uaddr_select)(map, uaddr,
> entry_out, addr_out, sz, align, offset, prot, hint);
>
> Index: uvm/uvm_fault.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
> retrieving revision 1.132
> diff -u -p -r1.132 uvm_fault.c
> --- uvm/uvm_fault.c 31 Aug 2022 01:27:04 - 1.132
> +++ uvm/uvm_fault.c 11 Sep 2022 08:57:35 -
> @@ -1626,6 +1626,7 @@ uvm_fault_unwire_locked(vm_map_t map, va
> struct vm_page *pg;
>
> KASSERT((map->flags & VM_MAP_INTRSAFE) == 0);
> + vm_map_assert_anylock(map);
>
> /*
>* we assume that the area we are unwiring has actually been wired
> Index: uvm/uvm_map.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_map.c,v
> retrieving revision 1.294
> diff -u -p -r1.294 uvm_map.c
> --- uvm/uvm_map.c 15 Aug 2022 15:53:45 - 1.294
> +++ uvm/uvm_map.c 11 Sep 2022 09:37:44 -
> @@ -162,6 +162,8 @@ int
> uvm_map_inentry_recheck(u_long, v
>struct p_inentry *);
> boolean_t uvm_map_inentry_fix(struct proc *, struct p_inentry *,
>vaddr_t, int (*)(vm_map_entry_t), u_long);
> +boolean_t uvm_map_is_stack_remappable(struct vm_map *,
> + vaddr_t, vsize_t);
> /*
> * Tree management functions.
> */
> @@ -491,6 +493,8 @@ uvmspace_dused(struct vm_map *map, vaddr
> vaddr_t stack_begin, stack_end; /* Position of stack. */
>
> KASSERT(map->flags & VM_MAP_ISVMSPACE);
> + vm_map_assert_anylock(map);
> +
> vm = (struct vmspace *)map;
> stack_begin = MIN((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr);
> stack_end = MAX((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr);
> @@ -570,6 +574,8 @@ uvm_map_isavail(struct vm_map *map, stru
> if (addr + sz < addr)
> return 0;
>
> + vm_map_assert_anylock(map);
> +
> /*
>* Kernel memory above uvm_maxkaddr is considered unavailable.
>*/
> @@ -1446,6 +1452,8 @@ uvm_map_mkentry(struct vm_map *map, stru
> entry->guard = 0;
> entry->fspace = 0;
>
> + vm_map_assert_wrlock(map);
> +
> /* Reset free space in first. */
> free = uvm_map_uaddr_e(map, first);
> uvm_mapent_free_remove(map, free, first);
> @@ -1573,6 +1581,8 @@ boolean_t
> uvm_map_lookup_entry(struct vm_map *map, vaddr_t address,
> struct vm_map_entry **entry)
> {
> + vm_map_assert_anylock(map);
> +
> *entry = uvm_map_entrybyaddr(&map->addr, address);
> return *entry != NULL && !UVM_ET_ISHOLE(*entry) &&
> (*entry)->start <= address && (*entry)->end > address;
> @@ -1692,6 +1702,8 @@ uvm_map_is_stack_remappable(struct vm_ma
> vaddr_t end = addr + sz;
> struct vm_map_entry *first, *iter, *prev = NULL;
>
> + vm_map_assert_anylock(map);
> +
> if (!uvm_map_lookup_entry(map, addr, &first)) {
> printf("map stack 0x%lx-0x%lx of map %p failed: no mapping\n",
> addr, end, map);
> @@ -1843,6 +1855,8 @@ uvm_mapent_mkfree(struct vm_map *map, st
> vaddr_t addr; /* Start of freed range. */
> vaddr_t end; /* End of freed range. */
>
> + UVM_MAP_REQ_WRITE(map);
> +
> prev = *prev_ptr;
> if (prev == entry)
> *prev_ptr = prev = NULL;
> @@ -1971,10 +1985,7 @@ uvm_unmap_remove(struct vm_map *map, vad
> if (start >= end)
> return;
>
> - if ((map->flags & VM_MAP_INTRSAFE) == 0)
> - splassert(IPL_NONE);
> - else
> - splassert(IPL_VM);
> + vm_map_assert_wrlock(map);
>
> /* Find first affected entry. */
> entry = uvm_map_entrybyaddr(&map->addr, start);
> @@ -4027,6 +4038,8 @@ uvm_map_checkprot(struct vm_map *map, va
> {
> struct vm_map_entry *entry;
>
> + vm_map_assert_anylock(map);
> +
> if (start < map->min_offset || end > map->max_offset || sta
Re: apldckbd(4): add fn key combose for Page Up/Down
On Wed, Sep 14, 2022 at 11:56:48AM +, Miod Vallat wrote:
> > Hey,
> >
> > the diff below adds FN key combos for Page Up, Page Down and some more
> > on the M2 keyboard. Most of the logic was copied from ukbd.
>
> This means most of the munging logic should move from ukbd into hidkbd,
> but that can be done later.
Right, that was also my idea. For now I'd be happy to have them working
at all.
>
> If you don't want to do this yet, you need to address two points:
> - the result of the hid_locate() call should be checked.
> - because of this, invocation of apldckbd_munge() should only happen if
> that call had succeeded.
>
Updated diff below.
diff --git a/sys/arch/arm64/dev/apldc.c b/sys/arch/arm64/dev/apldc.c
index 1db551988e6..0ffac192b3f 100644
--- a/sys/arch/arm64/dev/apldc.c
+++ b/sys/arch/arm64/dev/apldc.c
@@ -1066,6 +1066,22 @@ struct apldckbd_softc {
struct apldchidev_softc *sc_hidev;
struct hidkbd sc_kbd;
int sc_spl;
+ struct hid_location sc_fn;
+ int sc_has_fn;
+};
+
+struct apldckbd_translation {
+ uint8_t original;
+ uint8_t translation;
+};
+
+static const struct apldckbd_translation apldckbd_trans_table[] = {
+ { 40, 73 }, /* return -> insert */
+ { 42, 76 }, /* backspace -> delete */
+ { 79, 77 }, /* right -> end */
+ { 80, 74 }, /* left -> home */
+ { 81, 78 }, /* down -> page down */
+ { 82, 75 } /* up -> page up */
};
void apldckbd_cngetc(void *, u_int *, int *);
@@ -1122,6 +1138,9 @@ apldckbd_attach(struct device *parent, struct device
*self, void *aux)
printf("\n");
+ sc->sc_has_fn = hid_locate(aa->aa_desc, aa->aa_desclen,
+ HID_USAGE2(HUP_APPLE, HUG_FN_KEY), 1, hid_input, &sc->sc_fn, NULL);
+
if (kbd->sc_console_keyboard) {
extern struct wskbd_mapdata ukbd_keymapdata;
@@ -1133,14 +1152,41 @@ apldckbd_attach(struct device *parent, struct device
*self, void *aux)
hidkbd_attach_wskbd(kbd, KB_US | KB_DEFAULT, &apldckbd_accessops);
}
+void
+apldckbd_munge(void *v, uint8_t *ibuf, size_t ilen)
+{
+ struct apldckbd_softc *sc = v;
+ struct hidkbd *kbd = &sc->sc_kbd;
+ uint8_t *pos, *spos, *epos;
+ const struct apldckbd_translation *tbl;
+ size_t tsize;
+
+ if (!hid_get_data(ibuf, ilen, &sc->sc_fn))
+ return;
+
+ spos = ibuf + kbd->sc_keycodeloc.pos / 8;
+ epos = spos + kbd->sc_nkeycode;
+
+ for (pos = spos; pos != epos; pos++) {
+ tbl = apldckbd_trans_table;
+ tsize = nitems(apldckbd_trans_table);
+ for (; tsize != 0; tbl++, tsize--)
+ if (tbl->original == *pos)
+ *pos = tbl->translation;
+ }
+}
+
void
apldckbd_intr(struct device *self, uint8_t *packet, size_t packetlen)
{
struct apldckbd_softc *sc = (struct apldckbd_softc *)self;
struct hidkbd *kbd = &sc->sc_kbd;
- if (kbd->sc_enabled)
+ if (kbd->sc_enabled) {
+ if (sc->sc_has_fn)
+ apldckbd_munge(sc, &packet[1], packetlen - 1);
hidkbd_input(kbd, &packet[1], packetlen - 1);
+ }
}
int
--
2.37.2
Re: apldckbd(4): add fn key combose for Page Up/Down
> Hey, > > the diff below adds FN key combos for Page Up, Page Down and some more > on the M2 keyboard. Most of the logic was copied from ukbd. This means most of the munging logic should move from ukbd into hidkbd, but that can be done later. If you don't want to do this yet, you need to address two points: - the result of the hid_locate() call should be checked. - because of this, invocation of apldckbd_munge() should only happen if that call had succeeded.
man sed(1): instructions wrt whitespace in functions
section "DESCRIPTION" of sed man(1) page, says >The form of a sed command is as follows: >[address[,address]]function[arguments] >Whitespace may be inserted before the first address and the function >portions of the command. https://github.com/openbsd/src/blob/16748403c325ab5b4e9457bf8f0879a6698daba9/usr.bin/sed/sed.1#L138-L139 I don't understand where this whitespace would go, perhaps it should say: >Whitespace may be inserted between the address and the function >portions of the command. if I use a `-f command_file` I tested that I can indeed insert a whitespace between address & function portions, e.g. `1,3p` as well as `1,3 p` work. For this and the following mentions of whitespace, I have not managed to make whitespace work when command was given as an argument (either alone or part of -e flag). In my tests, I could insert whitespaces in sed commands only in `command_file`. This behavior is not mentioned in the man page. section "SED FUNCTIONS": >The r and w functions, as well as the w flag to the s function, take a >file parameter, which should be separated from the function or flag by >whitespace. The r & w & s (with w flag) functions work also if file parameter is NOT separated, while if it IS separated by whitespace, they only work as part of `command_file`. perhaps, the instruction should read something like >The r and w functions, as well as the w flag to the s function, take a >file parameter, which can be separated from the function or flag by >whitespace in `command_file`. I haven't looked into IEEE specification, perhaps the word "should" applies to that, and the sentence should be left as is. then we have >The b, r, s, t, w, y, and : functions all accept additional arguments. >The synopses below indicate which arguments have to be separated from the >function letters by whitespace characters. > >[2addr]b [label] >[1addr]r file >[2addr]s/RE/replacement/flags >[2addr]t [label] >[2addr]w file >[2addr]y/string1/string2/ >[0addr]:label again, (b,r,s,t,w) functions work without whitespace separation of arguments, while with whitespace only as part of `command_file` so why does it say `have to` (again, maybe because of IEEE spec?). maybe the wording should be >The b, r, s, t, w, y, and : functions all accept additional arguments. >The synopses below indicate which arguments can be separated from the >function letters by whitespace character in `command_file` In short, it is frustrating for someone learning sed, testing the sed functions using command argument (alone or part of -e), to be told to insert whitespace when this only works as part of `command_file`. Best, Luka P.S. I appreciate my suggestions getting replies, unfortunately I'm struggling with mail. I don't receive any replies in my inbox, and have to check https://marc.info/?l=openbsd-tech to see them.
apldckbd(4): add fn key combose for Page Up/Down
Hey,
the diff below adds FN key combos for Page Up, Page Down and some more
on the M2 keyboard. Most of the logic was copied from ukbd.
This makes scrolling tmux a lot more fun.
ok?
diff --git a/sys/arch/arm64/dev/apldc.c b/sys/arch/arm64/dev/apldc.c
index 82a17df59b5..a4db46d8a92 100644
--- a/sys/arch/arm64/dev/apldc.c
+++ b/sys/arch/arm64/dev/apldc.c
@@ -1066,6 +1066,21 @@ struct apldckbd_softc {
struct apldchidev_softc *sc_hidev;
struct hidkbd sc_kbd;
int sc_spl;
+ struct hid_location sc_fn;
+};
+
+struct apldckbd_translation {
+ uint8_t original;
+ uint8_t translation;
+};
+
+static const struct apldckbd_translation apldckbd_trans_table[] = {
+ { 40, 73 }, /* return -> insert */
+ { 42, 76 }, /* backspace -> delete */
+ { 79, 77 }, /* right -> end */
+ { 80, 74 }, /* left -> home */
+ { 81, 78 }, /* down -> page down */
+ { 82, 75 } /* up -> page up */
};
void apldckbd_cngetc(void *, u_int *, int *);
@@ -1122,6 +1137,9 @@ apldckbd_attach(struct device *parent, struct device
*self, void *aux)
printf("\n");
+ hid_locate(aa->aa_desc, aa->aa_desclen, HID_USAGE2(HUP_APPLE,
HUG_FN_KEY),
+ 1, hid_input, &sc->sc_fn, NULL);
+
if (kbd->sc_console_keyboard) {
extern struct wskbd_mapdata ukbd_keymapdata;
@@ -1133,14 +1151,40 @@ apldckbd_attach(struct device *parent, struct device
*self, void *aux)
hidkbd_attach_wskbd(kbd, KB_US | KB_DEFAULT, &apldckbd_accessops);
}
+void
+apldckbd_munge(void *v, uint8_t *ibuf, size_t ilen)
+{
+ struct apldckbd_softc *sc = v;
+ struct hidkbd *kbd = &sc->sc_kbd;
+ uint8_t *pos, *spos, *epos;
+ const struct apldckbd_translation *tbl;
+ size_t tsize;
+
+ if (!hid_get_data(ibuf, ilen, &sc->sc_fn))
+ return;
+
+ spos = ibuf + kbd->sc_keycodeloc.pos / 8;
+ epos = spos + kbd->sc_nkeycode;
+
+ for (pos = spos; pos != epos; pos++) {
+ tbl = apldckbd_trans_table;
+ tsize = nitems(apldckbd_trans_table);
+ for (; tsize != 0; tbl++, tsize--)
+ if (tbl->original == *pos)
+ *pos = tbl->translation;
+ }
+}
+
void
apldckbd_intr(struct device *self, uint8_t *packet, size_t packetlen)
{
struct apldckbd_softc *sc = (struct apldckbd_softc *)self;
struct hidkbd *kbd = &sc->sc_kbd;
- if (kbd->sc_enabled)
+ if (kbd->sc_enabled) {
+ apldckbd_munge(sc, &packet[1], packetlen - 1);
hidkbd_input(kbd, &packet[1], packetlen - 1);
+ }
}
int
--
2.37.3
Re: [RFC] Adding ESRT and EFI variables for fwupd
> Date: Fri, 9 Sep 2022 15:17:55 +0300
> From: Sergii Dmytruk
>
> Hi Mark,
>
> Any news? I since setup gdb debugging of OVMF and figured out my EFI RT
> issues and it returns data fine now (was wrong calling ABI), but I'm not
> making /dev/efi as it sounds like you've done it already. Where are you
> at with this? Can I help to move this forward?
Hi Sergii,
Sorry, not much. I've been a bit busy; some evil person put a lenovo
x13s on my desk ;).
I dug out the patch I was working on. The goal of that patch was to
support rebooting machines using EFI runtime services, which makes the
Microsoft Surface Go 3 reboot correctly. Unfortunately this breaks
other machines, which is why I parked the diff.
The diff is basically a port of the arm64 code, and I want to make an
effort to keep things in sync. My plan is to get this bit in soon
after OpenBSD 7.2 is released. That'll be a few weeks from now. I
may use some of the time in between to make this a bit more robust
such that if we crash during an EFI runtime services call, the kernel
can recover. This diff doesn't create dev/efi yet, but that will
follow soon after.
I did discuss the approach with Theo and some other OpenBSD developers
really. I think this is very useful functionality, but there are some
concerns about making it too easy to set EFI variables from within
OpenBSD as this can have serious security implications. Traditionally
we only allow this kind of thing when securelevel is -1 or 0. See
https://man.openbsd.org/securelevel.7 for more information about
securelevel. That would mean you can only run fwupd from single-user
mode.
Cheers, and thank you for your patience,
Mark
Index: arch/amd64/amd64/acpi_machdep.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/acpi_machdep.c,v
retrieving revision 1.104
diff -u -p -r1.104 acpi_machdep.c
--- arch/amd64/amd64/acpi_machdep.c 7 Aug 2022 23:56:06 - 1.104
+++ arch/amd64/amd64/acpi_machdep.c 14 Sep 2022 07:58:01 -
@@ -330,10 +330,14 @@ void
acpi_attach_machdep(struct acpi_softc *sc)
{
extern void (*cpuresetfn)(void);
+ extern void (*powerdownfn)(void);
sc->sc_interrupt = isa_intr_establish(NULL, sc->sc_fadt->sci_int,
IST_LEVEL, IPL_BIO, acpi_interrupt, sc, sc->sc_dev.dv_xname);
- cpuresetfn = acpi_reset;
+ if (!cpuresetfn)
+ cpuresetfn = acpi_reset;
+ if (!powerdownfn)
+ powerdownfn = acpi_powerdown;
#ifndef SMALL_KERNEL
/*
Index: arch/amd64/amd64/bios.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/bios.c,v
retrieving revision 1.45
diff -u -p -r1.45 bios.c
--- arch/amd64/amd64/bios.c 21 Feb 2022 11:03:39 - 1.45
+++ arch/amd64/amd64/bios.c 14 Sep 2022 07:58:01 -
@@ -30,6 +30,7 @@
#include
#include "acpi.h"
+#include "efi.h"
#include "mpbios.h"
#include "pci.h"
@@ -189,6 +190,18 @@ out:
break;
}
}
+
+#if NEFI > 0
+ if (bios_efiinfo != NULL) {
+ struct bios_attach_args ba;
+
+ memset(&ba, 0, sizeof(ba));
+ ba.ba_name = "efi";
+ ba.ba_memt = X86_BUS_SPACE_MEM;
+
+ config_found(self, &ba, bios_print);
+ }
+#endif
#if NACPI > 0
{
Index: arch/amd64/amd64/efi.c
===
RCS file: arch/amd64/amd64/efi.c
diff -N arch/amd64/amd64/efi.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ arch/amd64/amd64/efi.c 14 Sep 2022 07:58:01 -
@@ -0,0 +1,315 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2022 Mark Kettenis
+ *
+ * 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.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include
+extern paddr_t cr3_reuse_pcid;
+extern void (*cpuresetfn)(void);
+extern void (*powerdownfn)(void);
+
+#include
+
+#include
+
+extern todr_chip_handle_t todr_handle;
+
+extern EFI_MEMORY_DESCRIPTOR *mmap;
+
+struct efi_softc {
+ struct device sc_dev;
+ struct pmap *sc_pm;
+ EFI_RUNTIME_SERVICES *sc_rs;
+ u_long sc_psw;
+ uint64_tsc_cr3;
Re: man sed(1), fix comment about two addresses
On Wed, Sep 14, 2022 at 07:00:52AM +, Klemens Nanni wrote: > On Wed, Sep 14, 2022 at 06:48:48AM +0100, Jason McIntyre wrote: > > On Wed, Sep 14, 2022 at 12:40:55AM +0200, Luka Krmpoti?? wrote: > > > sed's man(1) page, section "sed addresses", has a note: > > > > > > https://github.com/openbsd/src/blob/16748403c325ab5b4e9457bf8f0879a6698daba9/usr.bin/sed/sed.1#L175-L176 > > > > (If the second address is a number less than or equal to the line > > > > number > > > first selected, only that line is selected.) > > > > > > to me, this makes it sound as if the second address would be selected, > > > so as if in the range `3,1` line 1 would be selected, when in actuality it > > > is line 3 > > > ```ksh > > > $ sed -n 3,1p < > > > one > > > > two > > > > three > > > > EOF > > > three > > > ``` > > > I recommend the note to be made clearer, by being explicit about "that > > > line" > > > > (If the second address is a number less than or equal to the line number > > > first selected, only the first address is selected.) > > > > > > Best, > > > Luka > > > > > > P.S. > > > it is after midnight here > > > > hi. > > > > i rather think i agree. diff below to address that. in addition, i don;t > > think it helpes having that entire sentence in brackets, so i removed > > them. > > > > will let this sit a little in case of feedback. > > jmc > > > > Index: sed.1 > > === > > RCS file: /cvs/src/usr.bin/sed/sed.1,v > > retrieving revision 1.61 > > diff -u -p -r1.61 sed.1 > > --- sed.1 3 Aug 2022 08:16:50 - 1.61 > > +++ sed.1 14 Sep 2022 05:47:07 - > > @@ -172,8 +172,8 @@ that match the address. > > A command line with two addresses selects the inclusive range from > > the first pattern space that matches the first address through the next > > pattern space that matches the second. > > -(If the second address is a number less than or equal to the line number > > -first selected, only that line is selected.) > > +If the second address is a number less than or equal to the line number > > +first selected, only the first line is selected. > > I'd stick with Luka's "only the first address is selected" to be clear. > > OK kn with that tweak. > > Doesn't make much sense but "the first line" could be the first line in > the file/input, whereas "the first address" is pretty clear to be the > address one specified with N,M. > > Agree wrt. parentheses. > oh, thanks. that was my mistake! committed with that adjustment. jmc
Re: man sed(1), fix comment about two addresses
On Wed, Sep 14, 2022 at 06:48:48AM +0100, Jason McIntyre wrote: > On Wed, Sep 14, 2022 at 12:40:55AM +0200, Luka Krmpoti?? wrote: > > sed's man(1) page, section "sed addresses", has a note: > > > > https://github.com/openbsd/src/blob/16748403c325ab5b4e9457bf8f0879a6698daba9/usr.bin/sed/sed.1#L175-L176 > > > (If the second address is a number less than or equal to the line number > > first selected, only that line is selected.) > > > > to me, this makes it sound as if the second address would be selected, > > so as if in the range `3,1` line 1 would be selected, when in actuality it > > is line 3 > > ```ksh > > $ sed -n 3,1p < > > one > > > two > > > three > > > EOF > > three > > ``` > > I recommend the note to be made clearer, by being explicit about "that line" > > > (If the second address is a number less than or equal to the line number > > first selected, only the first address is selected.) > > > > Best, > > Luka > > > > P.S. > > it is after midnight here > > hi. > > i rather think i agree. diff below to address that. in addition, i don;t > think it helpes having that entire sentence in brackets, so i removed > them. > > will let this sit a little in case of feedback. > jmc > > Index: sed.1 > === > RCS file: /cvs/src/usr.bin/sed/sed.1,v > retrieving revision 1.61 > diff -u -p -r1.61 sed.1 > --- sed.1 3 Aug 2022 08:16:50 - 1.61 > +++ sed.1 14 Sep 2022 05:47:07 - > @@ -172,8 +172,8 @@ that match the address. > A command line with two addresses selects the inclusive range from > the first pattern space that matches the first address through the next > pattern space that matches the second. > -(If the second address is a number less than or equal to the line number > -first selected, only that line is selected.) > +If the second address is a number less than or equal to the line number > +first selected, only the first line is selected. I'd stick with Luka's "only the first address is selected" to be clear. OK kn with that tweak. Doesn't make much sense but "the first line" could be the first line in the file/input, whereas "the first address" is pretty clear to be the address one specified with N,M. Agree wrt. parentheses. > Starting at the first line following the selected range, > .Nm > starts looking again for the first address. >
