Re: Unlock select(2) and pselect(2)
On Wed, Feb 08, 2023 at 12:47:23PM +0100, Claudio Jeker wrote:
> On Wed, Feb 08, 2023 at 12:40:50PM +0100, Mark Kettenis wrote:
> > > Date: Wed, 8 Feb 2023 14:17:14 +0300
> > > From: Vitaliy Makkoveev
> > >
> > > On Tue, Feb 07, 2023 at 05:42:40PM +0300, Vitaliy Makkoveev wrote:
> > > >
> > > > > > Otherwise, if you are concerning about serialized `p_sigmask' and
> > > > > > P_SIGSUSPEND, dosigsuspend() should be left under kernel lock:
> > > > > >
> > > > > > if (sigmask) {
> > > > > > KERNEL_LOCK();
> > > > > > dosigsuspend(p, *sigmask &~ sigcantmask);
> > > > > > KERNEL_UNLOCK();
> > > > > > }
> > > > >
> > > > > This is a hack but would allow progress. Now I would prefer if
> > > > > dosigsuspend() is made safe so that the KERNEL_LOCK is not needed.
> > > > > Which
> > > > > would also allow sigsuspend to be called with NOLOCK.
> > > > >
> > > >
> > > > Makes sense, but sigsuspend(2) will not be unlocked together with
> > > > select(2), so dosigsuspend() could be left under kernel lock for a
> > > > while.
> > > >
> > >
> > > Now, I want to keep dosigsuspend() under the kernel lock. To me it's
> > > better to unlock it together with sigsuspend(2) to separate possible
> > > signal related fallout. This still makes sense because signal mask is
> > > the optional arg for pselect(2) only. Except the local event data
> > > conversion, poll(2) internals are the same as select(2) internals, it
> > > makes sense to unlock them both.
> > >
> > > This doesn't mean I dropped dosigsuspend(), I'm still waiting feedback
> > > for barriers.
> >
> > Splitting it this way makes sense to me.
>
> I agree. Will throw that onto my bgpd test box that will test the poll
> case.
>
Can I push this forward?
Re: Intel Arc / DG2
On Sun, Feb 12, 2023 at 02:25:47PM -0500, Thomas Frohwein wrote:
> On Tue, Feb 07, 2023 at 08:51:40PM +1100, Jonathan Gray wrote:
>
> [...]
> > > ...
> > > i915_resize_lmem_bar: stub
> > > panic: kernel diagnostic assertion "pdev->pci->sc_bridgetag == NULL"
> > > failed: file "/usr/src/sys/dev/pci/drm/drm_linux.c", line 1277
> > > ...
> >
> > The vga arbiter bits need to change. There isn't an easy way to get
> > a softc to avoid having state shared by multiple inteldrm instances.
> >
> > perhaps they can be skipped for the moment?
>
> Thanks, this leads to a uvm_fault now:
>
> xehp_load_dss_mask: stub
> xehp_load_dss_mask: stub
> intel_slicemask_from_xehp_dssmask: stub
> intel_slicemask_from_xehp_dssmask: stub
> i915_resize_lmem_bar: stub
> uvm_fault(0x825181f8, 0x10, 0, 1) -> e
>
> screen photo at https://thfr.info/tmp/DG2-error-20230212.jpg
unfortunately, that isn't much help
If you boot -d and do:
w db_panic 0
c
you should be able to get a backtrace.
>
> >
> > Index: sys/dev/pci/drm/drm_linux.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
> > retrieving revision 1.95
> > diff -u -p -r1.95 drm_linux.c
> > --- sys/dev/pci/drm/drm_linux.c 1 Jan 2023 01:34:34 - 1.95
> > +++ sys/dev/pci/drm/drm_linux.c 7 Feb 2023 09:31:55 -
> > @@ -1274,7 +1274,8 @@ vga_disable_bridge(struct pci_attach_arg
> > void
> > vga_get_uninterruptible(struct pci_dev *pdev, int rsrc)
> > {
> > - KASSERT(pdev->pci->sc_bridgetag == NULL);
> > + if (pdev->pci->sc_bridgetag != NULL)
> > + return;
> > pci_enumerate_bus(pdev->pci, vga_disable_bridge, NULL);
> > }
> >
> > @@ -1282,6 +1283,9 @@ void
> > vga_put(struct pci_dev *pdev, int rsrc)
> > {
> > pcireg_t bc;
> > +
> > + if (pdev->pci->sc_bridgetag != NULL)
> > + return;
> >
> > if (!vga_bridge_disabled)
> > return;
> > Index: sys/dev/pci/drm/i915/i915_pci.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_pci.c,v
> > retrieving revision 1.15
> > diff -u -p -r1.15 i915_pci.c
> > --- sys/dev/pci/drm/i915/i915_pci.c 25 Jan 2023 01:51:59 - 1.15
> > +++ sys/dev/pci/drm/i915/i915_pci.c 3 Feb 2023 01:43:02 -
> > @@ -1078,7 +1078,6 @@ static const struct intel_device_info dg
> > XE_LPD_FEATURES,
> > .__runtime.cpu_transcoder_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
> >BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
> > - .require_force_probe = 1,
> > };
> >
> > static const struct intel_device_info ats_m_info = {
> > Index: sys/dev/pci/drm/i915/intel_memory_region.h
> > ===
> > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_memory_region.h,v
> > retrieving revision 1.3
> > diff -u -p -r1.3 intel_memory_region.h
> > --- sys/dev/pci/drm/i915/intel_memory_region.h 1 Jan 2023 01:34:55
> > - 1.3
> > +++ sys/dev/pci/drm/i915/intel_memory_region.h 4 Feb 2023 00:59:23
> > -
> > @@ -70,8 +70,13 @@ struct intel_memory_region {
> >
> > const struct intel_memory_region_ops *ops;
> >
> > -#ifdef notyet
> > +#ifdef __linux__
> > struct io_mapping iomap;
> > +#else
> > + struct vm_page *pgs;
> > + struct agp_map *agph;
> > + bus_space_handle_t bsh;
> > + bus_space_tag_t bst;
> > #endif
> > struct resource region;
> >
> > Index: sys/dev/pci/drm/i915/gem/i915_gem_lmem.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/drm/i915/gem/i915_gem_lmem.c,v
> > retrieving revision 1.4
> > diff -u -p -r1.4 i915_gem_lmem.c
> > --- sys/dev/pci/drm/i915/gem/i915_gem_lmem.c1 Jan 2023 01:34:56
> > - 1.4
> > +++ sys/dev/pci/drm/i915/gem/i915_gem_lmem.c4 Feb 2023 00:58:16
> > -
> > @@ -15,9 +15,6 @@ i915_gem_object_lmem_io_map(struct drm_i
> > unsigned long n,
> > unsigned long size)
> > {
> > - STUB();
> > - return NULL;
> > -#ifdef notyet
> > resource_size_t offset;
> >
> > GEM_BUG_ON(!i915_gem_object_is_contiguous(obj));
> > @@ -25,7 +22,11 @@ i915_gem_object_lmem_io_map(struct drm_i
> > offset = i915_gem_object_get_dma_address(obj, n);
> > offset -= obj->mm.region->region.start;
> >
> > +#ifdef __linux__
> > return io_mapping_map_wc(>mm.region->iomap, offset, size);
> > +#else
> > + agp_map_atomic(obj->mm.region->agph, offset, >mm.region->bsh);
> > + return bus_space_vaddr(obj->mm.region->bst, obj->mm.region->bsh);
> > #endif
> > }
> >
> > Index: sys/dev/pci/drm/i915/gem/i915_gem_stolen.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/drm/i915/gem/i915_gem_stolen.c,v
> > retrieving revision 1.5
> > diff -u -p -r1.5 i915_gem_stolen.c
> > ---
pcidevs: add PEX 8311 bridge
Index: sys/dev/pci/pcidevs === RCS file: /cvs/src/sys/dev/pci/pcidevs,v retrieving revision 1.2021 diff -u -p -r1.2021 pcidevs --- sys/dev/pci/pcidevs 7 Feb 2023 07:10:43 - 1.2021 +++ sys/dev/pci/pcidevs 13 Feb 2023 10:06:56 - @@ -8194,6 +8194,7 @@ product PLX PCI_6520 0x6520 PCI 6520 product PLX PEX_8111 0x8111 PEX 8111 product PLX PEX_8112 0x8112 PEX 8112 product PLX PEX_8114 0x8114 PEX 8114 +product PLX PEX_8311 0x86e1 PEX 8311 product PLX PEX_8517 0x8517 PEX 8517 product PLX PEX_8518 0x8518 PEX 8518 product PLX PEX_8524 0x8524 PEX 8524
Re: pcidevs: add PEX 8311 bridge
> Date: Mon, 13 Feb 2023 13:08:54 +0300 > From: Vitaliy Makkoveev What makes you think this is a PEX 8311? The data sheet I found has the PCI device ID down as 0x8311, althogh this can be changed by programming the EEPROM. > Index: sys/dev/pci/pcidevs > === > RCS file: /cvs/src/sys/dev/pci/pcidevs,v > retrieving revision 1.2021 > diff -u -p -r1.2021 pcidevs > --- sys/dev/pci/pcidevs 7 Feb 2023 07:10:43 - 1.2021 > +++ sys/dev/pci/pcidevs 13 Feb 2023 10:06:56 - > @@ -8194,6 +8194,7 @@ product PLX PCI_65200x6520 PCI 6520 > product PLX PEX_8111 0x8111 PEX 8111 > product PLX PEX_8112 0x8112 PEX 8112 > product PLX PEX_8114 0x8114 PEX 8114 > +product PLX PEX_8311 0x86e1 PEX 8311 > product PLX PEX_8517 0x8517 PEX 8517 > product PLX PEX_8518 0x8518 PEX 8518 > product PLX PEX_8524 0x8524 PEX 8524 > >
Re: pcidevs: add PEX 8311 bridge
This isn't sorted by device id and looks wrong. Almost all the other PLX entries have a device id that matches the name. The chip is marked as PEX 8311? On Mon, Feb 13, 2023 at 01:08:54PM +0300, Vitaliy Makkoveev wrote: > Index: sys/dev/pci/pcidevs > === > RCS file: /cvs/src/sys/dev/pci/pcidevs,v > retrieving revision 1.2021 > diff -u -p -r1.2021 pcidevs > --- sys/dev/pci/pcidevs 7 Feb 2023 07:10:43 - 1.2021 > +++ sys/dev/pci/pcidevs 13 Feb 2023 10:06:56 - > @@ -8194,6 +8194,7 @@ product PLX PCI_65200x6520 PCI 6520 > product PLX PEX_8111 0x8111 PEX 8111 > product PLX PEX_8112 0x8112 PEX 8112 > product PLX PEX_8114 0x8114 PEX 8114 > +product PLX PEX_8311 0x86e1 PEX 8311 > product PLX PEX_8517 0x8517 PEX 8517 > product PLX PEX_8518 0x8518 PEX 8518 > product PLX PEX_8524 0x8524 PEX 8524 > >
Re: pcidevs: add PEX 8311 bridge
On Mon, Feb 13, 2023 at 11:22:36AM +0100, Mark Kettenis wrote: > > Date: Mon, 13 Feb 2023 13:08:54 +0300 > > From: Vitaliy Makkoveev > > What makes you think this is a PEX 8311? The data sheet I found has > the PCI device ID down as 0x8311, althogh this can be changed by > programming the EEPROM. > I wrote many drivers for them and PID was always 0x86e1. Just checked datasheets, "PEX 8311RDK Hardware Reference Manual" says PID is 0x86e1, meanwhile "PEX 8311AA Data Book" says the default is 0x9056, which is PID for PCI PLX9056. Guess we could drop this diff. > > Index: sys/dev/pci/pcidevs > > === > > RCS file: /cvs/src/sys/dev/pci/pcidevs,v > > retrieving revision 1.2021 > > diff -u -p -r1.2021 pcidevs > > --- sys/dev/pci/pcidevs 7 Feb 2023 07:10:43 - 1.2021 > > +++ sys/dev/pci/pcidevs 13 Feb 2023 10:06:56 - > > @@ -8194,6 +8194,7 @@ product PLX PCI_6520 0x6520 PCI 6520 > > product PLX PEX_8111 0x8111 PEX 8111 > > product PLX PEX_8112 0x8112 PEX 8112 > > product PLX PEX_8114 0x8114 PEX 8114 > > +product PLX PEX_8311 0x86e1 PEX 8311 > > product PLX PEX_8517 0x8517 PEX 8517 > > product PLX PEX_8518 0x8518 PEX 8518 > > product PLX PEX_8524 0x8524 PEX 8524 > > > >
Re: pcidevs: add PEX 8311 bridge
On Mon, Feb 13, 2023 at 09:35:17PM +1100, Jonathan Gray wrote: > This isn't sorted by device id and looks wrong. Almost all the other > PLX entries have a device id that matches the name. > > The chip is marked as PEX 8311? > Sure. I checked datasheets, PLX used this PID for RDK kit. Meanwhile according "PEX8311AA Data Book" the default PID is 0x9056 as they used for PLX 9056 PCI bridge. Since PEX 8311 is mostly bit to bit identical to PLX 9056 this smells like true. However, want to drop this diff. Sorry for noise. > On Mon, Feb 13, 2023 at 01:08:54PM +0300, Vitaliy Makkoveev wrote: > > Index: sys/dev/pci/pcidevs > > === > > RCS file: /cvs/src/sys/dev/pci/pcidevs,v > > retrieving revision 1.2021 > > diff -u -p -r1.2021 pcidevs > > --- sys/dev/pci/pcidevs 7 Feb 2023 07:10:43 - 1.2021 > > +++ sys/dev/pci/pcidevs 13 Feb 2023 10:06:56 - > > @@ -8194,6 +8194,7 @@ product PLX PCI_6520 0x6520 PCI 6520 > > product PLX PEX_8111 0x8111 PEX 8111 > > product PLX PEX_8112 0x8112 PEX 8112 > > product PLX PEX_8114 0x8114 PEX 8114 > > +product PLX PEX_8311 0x86e1 PEX 8311 > > product PLX PEX_8517 0x8517 PEX 8517 > > product PLX PEX_8518 0x8518 PEX 8518 > > product PLX PEX_8524 0x8524 PEX 8524 > > > > >
bgpd adjust rde_generates_updates arguments
Instead of passing the rib and new and old best prefix just pass the
rib_entry to rde_generate_updates(). This simplifies a few things down
that rabbit hole. This is also a step towards decoupling prefix_evaluate()
and the Loc-RIB from rde_generate_updates() and the Adj-RIB-Out
processing.
Since the rib_entry is passed there is now always a valid prefix pointer
in re->prefix and a lot of the if (new) else if (old) cascades can be
simplified a lot.
There is a trivial fix requried in the regress test, that I have ready but
did not share here.
--
:wq Claudio
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.592
diff -u -p -r1.592 rde.c
--- rde.c 9 Feb 2023 13:43:23 - 1.592
+++ rde.c 13 Feb 2023 11:14:18 -
@@ -3888,14 +3888,13 @@ rde_softreconfig_in(struct rib_entry *re
static void
rde_softreconfig_out(struct rib_entry *re, void *arg)
{
- struct rib *rib = arg;
struct prefix *p;
if ((p = prefix_best(re)) == NULL)
/* no valid path for prefix */
return;
- rde_generate_updates(rib, p, NULL, NULL, NULL, EVAL_RECONF);
+ rde_generate_updates(re, NULL, NULL, EVAL_RECONF);
}
static void
Index: rde.h
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.282
diff -u -p -r1.282 rde.h
--- rde.h 9 Feb 2023 13:43:23 - 1.282
+++ rde.h 13 Feb 2023 11:13:34 -
@@ -392,9 +392,6 @@ voidrde_pftable_add(uint16_t, struct p
void rde_pftable_del(uint16_t, struct prefix *);
intrde_evaluate_all(void);
-void rde_generate_updates(struct rib *, struct prefix *,
- struct prefix *, struct prefix *, struct prefix *,
- enum eval_mode);
uint32_t rde_local_as(void);
intrde_decisionflags(void);
void rde_peer_send_rrefresh(struct rde_peer *, uint8_t, uint8_t);
@@ -412,6 +409,9 @@ struct rde_peer *peer_get(uint32_t);
struct rde_peer *peer_match(struct ctl_neighbor *, uint32_t);
struct rde_peer*peer_add(uint32_t, struct peer_config *);
+voidrde_generate_updates(struct rib_entry *, struct prefix *,
+ struct prefix *, enum eval_mode);
+
voidpeer_up(struct rde_peer *, struct session_up *);
voidpeer_down(struct rde_peer *, void *);
voidpeer_flush(struct rde_peer *, uint8_t, time_t);
@@ -729,11 +729,11 @@ intnexthop_compare(struct nexthop *,
/* rde_update.c */
voidup_init(struct rde_peer *);
voidup_generate_updates(struct filter_head *, struct rde_peer *,
- struct prefix *, struct prefix *);
+ struct rib_entry *);
voidup_generate_addpath(struct filter_head *, struct rde_peer *,
- struct prefix *, struct prefix *);
+ struct rib_entry *);
voidup_generate_addpath_all(struct filter_head *,
- struct rde_peer *, struct prefix *, struct prefix *,
+ struct rde_peer *, struct rib_entry *, struct prefix *,
struct prefix *);
voidup_generate_default(struct filter_head *, struct rde_peer *,
uint8_t);
Index: rde_decide.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_decide.c,v
retrieving revision 1.98
diff -u -p -r1.98 rde_decide.c
--- rde_decide.c23 Sep 2022 15:49:20 - 1.98
+++ rde_decide.c13 Feb 2023 11:14:55 -
@@ -556,10 +556,9 @@ prefix_evaluate(struct rib_entry *re, st
* but remember that newbest may be NULL aka ineligible.
* Additional decision may be made by the called functions.
*/
- rde_generate_updates(rib, newbest, oldbest, new, old,
- EVAL_DEFAULT);
if ((rib->flags & F_RIB_NOFIB) == 0)
rde_send_kroute(rib, newbest, oldbest);
+ rde_generate_updates(re, new, old, EVAL_DEFAULT);
return;
}
@@ -570,8 +569,7 @@ prefix_evaluate(struct rib_entry *re, st
*/
if (rde_evaluate_all())
if ((new != NULL && prefix_eligible(new)) || old != NULL)
- rde_generate_updates(rib, newbest, NULL, new, old,
- EVAL_ALL);
+ rde_generate_updates(re, new, old, EVAL_ALL);
}
void
@@ -630,9 +628,9 @@ prefix_evaluate_nexthop(struct prefix *p
* but remember that newbest may be NULL aka ineligible.
* Additional decision may be made by the called functions.
*/
- rde_generate_updates(rib, newbest, oldbest, p, p, EVAL_DEFAULT);
if
Re: bgpd adjust rde_generates_updates arguments
On Mon, Feb 13, 2023 at 02:33:05PM +0100, Claudio Jeker wrote:
> Instead of passing the rib and new and old best prefix just pass the
> rib_entry to rde_generate_updates(). This simplifies a few things down
> that rabbit hole. This is also a step towards decoupling prefix_evaluate()
> and the Loc-RIB from rde_generate_updates() and the Adj-RIB-Out
> processing.
>
> Since the rib_entry is passed there is now always a valid prefix pointer
> in re->prefix and a lot of the if (new) else if (old) cascades can be
> simplified a lot.
Very nice.
> There is a trivial fix requried in the regress test, that I have ready but
> did not share here.
thanks
ok, just a small comment below
> --
> :wq Claudio
>
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.592
> diff -u -p -r1.592 rde.c
> --- rde.c 9 Feb 2023 13:43:23 - 1.592
> +++ rde.c 13 Feb 2023 11:14:18 -
> @@ -3888,14 +3888,13 @@ rde_softreconfig_in(struct rib_entry *re
> static void
> rde_softreconfig_out(struct rib_entry *re, void *arg)
> {
> - struct rib *rib = arg;
> struct prefix *p;
>
> if ((p = prefix_best(re)) == NULL)
> /* no valid path for prefix */
> return;
>
> - rde_generate_updates(rib, p, NULL, NULL, NULL, EVAL_RECONF);
> + rde_generate_updates(re, NULL, NULL, EVAL_RECONF);
Unless you want to use p in a follow-up diff, I suggest you garbage
collect it.
Re: bgpd adjust rde_generates_updates arguments
On Mon, Feb 13, 2023 at 04:20:00PM +0100, Theo Buehler wrote:
> On Mon, Feb 13, 2023 at 02:33:05PM +0100, Claudio Jeker wrote:
> > Instead of passing the rib and new and old best prefix just pass the
> > rib_entry to rde_generate_updates(). This simplifies a few things down
> > that rabbit hole. This is also a step towards decoupling prefix_evaluate()
> > and the Loc-RIB from rde_generate_updates() and the Adj-RIB-Out
> > processing.
> >
> > Since the rib_entry is passed there is now always a valid prefix pointer
> > in re->prefix and a lot of the if (new) else if (old) cascades can be
> > simplified a lot.
>
> Very nice.
>
> > There is a trivial fix requried in the regress test, that I have ready but
> > did not share here.
>
> thanks
>
> ok, just a small comment below
>
> > --
> > :wq Claudio
> >
> > Index: rde.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> > retrieving revision 1.592
> > diff -u -p -r1.592 rde.c
> > --- rde.c 9 Feb 2023 13:43:23 - 1.592
> > +++ rde.c 13 Feb 2023 11:14:18 -
> > @@ -3888,14 +3888,13 @@ rde_softreconfig_in(struct rib_entry *re
> > static void
> > rde_softreconfig_out(struct rib_entry *re, void *arg)
> > {
> > - struct rib *rib = arg;
> > struct prefix *p;
> >
> > if ((p = prefix_best(re)) == NULL)
> > /* no valid path for prefix */
> > return;
> >
> > - rde_generate_updates(rib, p, NULL, NULL, NULL, EVAL_RECONF);
> > + rde_generate_updates(re, NULL, NULL, EVAL_RECONF);
>
> Unless you want to use p in a follow-up diff, I suggest you garbage
> collect it.
Good point. I removed it.
--
:wq Claudio
Re: pf max-src-{states,conn} without overload/flush useless?
On Thu, 09 Feb 2023 at 11:51:19 +0100, Alexandr Nedvedicky wrote: > I gave it a try after doing a sysupgrade to: > > penBSD 7.2-current (GENERIC.MP) #1025: Wed Feb 8 19:16:09 MST 2023 > > it still works for me as expected: > disk$ for i in `seq 5` ; do nc 192.168.2.175 22 & done > [1] 51566 > [2] 78983 > [3] 77864 > [4] 37474 > [5] 98599 > disk$ SSH-2.0-OpenSSH_9.2 > SSH-2.0-OpenSSH_9.2 > SSH-2.0-OpenSSH_9.2 > > my connection arrives over iwn0 interface which is in egress group > so our environments are almost identical. Ok now with the latest snapshot kernel I can no longer reproduce this. Maybe there was something unrelated in that snapshot that was causing it. I would still like to have it not fully open the new connection when the max-src-* limit is reached rather than opening and closing, but I guess that is a separate discussion to be had. Thanks for looking into it though.
Re: Intel Arc / DG2
On Mon, Feb 13, 2023 at 08:40:22PM +1100, Jonathan Gray wrote:
> On Sun, Feb 12, 2023 at 02:25:47PM -0500, Thomas Frohwein wrote:
> > On Tue, Feb 07, 2023 at 08:51:40PM +1100, Jonathan Gray wrote:
> >
> > [...]
> > > > ...
> > > > i915_resize_lmem_bar: stub
> > > > panic: kernel diagnostic assertion "pdev->pci->sc_bridgetag == NULL"
> > > > failed: file "/usr/src/sys/dev/pci/drm/drm_linux.c", line 1277
> > > > ...
> > >
> > > The vga arbiter bits need to change. There isn't an easy way to get
> > > a softc to avoid having state shared by multiple inteldrm instances.
> > >
> > > perhaps they can be skipped for the moment?
> >
> > Thanks, this leads to a uvm_fault now:
> >
> > xehp_load_dss_mask: stub
> > xehp_load_dss_mask: stub
> > intel_slicemask_from_xehp_dssmask: stub
> > intel_slicemask_from_xehp_dssmask: stub
> > i915_resize_lmem_bar: stub
> > uvm_fault(0x825181f8, 0x10, 0, 1) -> e
> >
> > screen photo at https://thfr.info/tmp/DG2-error-20230212.jpg
>
> unfortunately, that isn't much help
>
> If you boot -d and do:
> w db_panic 0
> c
> you should be able to get a backtrace.
There is no difference if I do this, and still no backtrace.
>
> >
> > >
> > > Index: sys/dev/pci/drm/drm_linux.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
> > > retrieving revision 1.95
> > > diff -u -p -r1.95 drm_linux.c
> > > --- sys/dev/pci/drm/drm_linux.c 1 Jan 2023 01:34:34 - 1.95
> > > +++ sys/dev/pci/drm/drm_linux.c 7 Feb 2023 09:31:55 -
> > > @@ -1274,7 +1274,8 @@ vga_disable_bridge(struct pci_attach_arg
> > > void
> > > vga_get_uninterruptible(struct pci_dev *pdev, int rsrc)
> > > {
> > > - KASSERT(pdev->pci->sc_bridgetag == NULL);
> > > + if (pdev->pci->sc_bridgetag != NULL)
> > > + return;
> > > pci_enumerate_bus(pdev->pci, vga_disable_bridge, NULL);
> > > }
> > >
> > > @@ -1282,6 +1283,9 @@ void
> > > vga_put(struct pci_dev *pdev, int rsrc)
> > > {
> > > pcireg_t bc;
> > > +
> > > + if (pdev->pci->sc_bridgetag != NULL)
> > > + return;
> > >
> > > if (!vga_bridge_disabled)
> > > return;
> > > Index: sys/dev/pci/drm/i915/i915_pci.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_pci.c,v
> > > retrieving revision 1.15
> > > diff -u -p -r1.15 i915_pci.c
> > > --- sys/dev/pci/drm/i915/i915_pci.c 25 Jan 2023 01:51:59 -
> > > 1.15
> > > +++ sys/dev/pci/drm/i915/i915_pci.c 3 Feb 2023 01:43:02 -
> > > @@ -1078,7 +1078,6 @@ static const struct intel_device_info dg
> > > XE_LPD_FEATURES,
> > > .__runtime.cpu_transcoder_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
> > > BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
> > > - .require_force_probe = 1,
> > > };
> > >
> > > static const struct intel_device_info ats_m_info = {
> > > Index: sys/dev/pci/drm/i915/intel_memory_region.h
> > > ===
> > > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_memory_region.h,v
> > > retrieving revision 1.3
> > > diff -u -p -r1.3 intel_memory_region.h
> > > --- sys/dev/pci/drm/i915/intel_memory_region.h1 Jan 2023 01:34:55
> > > - 1.3
> > > +++ sys/dev/pci/drm/i915/intel_memory_region.h4 Feb 2023 00:59:23
> > > -
> > > @@ -70,8 +70,13 @@ struct intel_memory_region {
> > >
> > > const struct intel_memory_region_ops *ops;
> > >
> > > -#ifdef notyet
> > > +#ifdef __linux__
> > > struct io_mapping iomap;
> > > +#else
> > > + struct vm_page *pgs;
> > > + struct agp_map *agph;
> > > + bus_space_handle_t bsh;
> > > + bus_space_tag_t bst;
> > > #endif
> > > struct resource region;
> > >
> > > Index: sys/dev/pci/drm/i915/gem/i915_gem_lmem.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/pci/drm/i915/gem/i915_gem_lmem.c,v
> > > retrieving revision 1.4
> > > diff -u -p -r1.4 i915_gem_lmem.c
> > > --- sys/dev/pci/drm/i915/gem/i915_gem_lmem.c 1 Jan 2023 01:34:56
> > > - 1.4
> > > +++ sys/dev/pci/drm/i915/gem/i915_gem_lmem.c 4 Feb 2023 00:58:16
> > > -
> > > @@ -15,9 +15,6 @@ i915_gem_object_lmem_io_map(struct drm_i
> > > unsigned long n,
> > > unsigned long size)
> > > {
> > > - STUB();
> > > - return NULL;
> > > -#ifdef notyet
> > > resource_size_t offset;
> > >
> > > GEM_BUG_ON(!i915_gem_object_is_contiguous(obj));
> > > @@ -25,7 +22,11 @@ i915_gem_object_lmem_io_map(struct drm_i
> > > offset = i915_gem_object_get_dma_address(obj, n);
> > > offset -= obj->mm.region->region.start;
> > >
> > > +#ifdef __linux__
> > > return io_mapping_map_wc(>mm.region->iomap, offset, size);
> > > +#else
> > > + agp_map_atomic(obj->mm.region->agph, offset, >mm.region->bsh);
> > > + return bus_space_vaddr(obj->mm.region->bst,
Re: fix NULL pointer dereference in pfsync_bulk_update()
On Mon, Feb 13, 2023 at 08:39:39AM +0100, Alexandr Nedvedicky wrote:
> this bug has been found and reported by Hrvoje@ [1].
> I took my chance and asked Hrvoje to test a small diff [2].
> I would like to ask for OK to commit this fix which makes
> Hrvoje's test box happy. Diff below is same to one found
> at bugs@. The thing is that pfsync_bulk_update() function
> must check first if there is anything to update, otherwise
> we may day due to NULL pointer dereference.
Your check makes sense, OK bluhm@
But who is protecting write access to sc->sc_bulk_next ?
I think it is exclusive netlock. This works as pfsync_input() is
a protocol input function which is still not running in parallel.
rw_enter_read(_state_list.pfs_rwl) does not protect sc->sc_bulk_next
it is a read lock. mtx_enter(_state_list.pfs_mtx) is not taken
for all writers to sc->sc_bulk_next.
Do you have plans to relax this code from exclusive to shared
netlock?
bluhm
> 8<---8<---8<--8<
> diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
> index e2c86971336..1fa58f6fab9 100644
> --- a/sys/net/if_pfsync.c
> +++ b/sys/net/if_pfsync.c
> @@ -2464,6 +2464,11 @@ pfsync_bulk_update(void *arg)
> st = sc->sc_bulk_next;
> sc->sc_bulk_next = NULL;
>
> + if (st == NULL) {
> + rw_exit_read(_state_list.pfs_rwl);
> + goto out;
> + }
> +
> for (;;) {
> if (st->sync_state == PFSYNC_S_NONE &&
> st->timeout < PFTM_MAX &&
