Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 15 Mar 2019 11:07:26 -0600
From:      Warner Losh <imp@bsdimp.com>
To:        "Rodney W. Grimes" <rgrimes@freebsd.org>
Cc:        Chuck Tuffli <chuck@freebsd.org>, src-committers <src-committers@freebsd.org>,  svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r345171 - head/usr.sbin/bhyve
Message-ID:  <CANCZdfqFCrVVgpY4cuFwpTHwigXcE7L=aL_44Cy4KY2G4WuP0g@mail.gmail.com>
In-Reply-To: <201903151556.x2FFuAA4030779@gndrsh.dnsmgr.net>
References:  <CANCZdfqNAQi5BTrvjriXYBo8640fUGvdqEYjEjA1JWV5nPDSuA@mail.gmail.com> <201903151556.x2FFuAA4030779@gndrsh.dnsmgr.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Mar 15, 2019 at 9:56 AM Rodney W. Grimes <freebsd@gndrsh.dnsmgr.net>
wrote:

> > On Thu, Mar 14, 2019 at 8:32 PM Rodney W. Grimes <
> freebsd@gndrsh.dnsmgr.net>
> > wrote:
> >
> > > > Author: chuck
> > > > Date: Fri Mar 15 02:11:28 2019
> > > > New Revision: 345171
> > > > URL: https://svnweb.freebsd.org/changeset/base/345171
> > > >
> > > > Log:
> > > >   Fix bhyve PCIe capability emulation
> > > >
> > > >   PCIe devices starting with version 1.1 must set the Role-Based
> Error
> > > >   Reporting bit.
> > > >
> > > >   And while we're in the neighborhood, generalize the code assigning
> the
> > > >   device type.
> > > >
> > > >   Reviewed by:        imp, araujo, rgrimes
> > > >   Approved by:        imp (mentor)
> > > >   MFC after:  1 week
> > > >   Differential Revision: https://reviews.freebsd.org/D19580
> > >
> > > This code requires maintainer approval before a commit,
> > > though this was well reviewed that doesnt exclude it
> > > from the MAINTAINERS entry.
> > >
> > > Leave it for now, I am sure jhb or thyco are fine with it,
> > > this is just a heads up FYI for future commits.
> > >
> > > Bhyve code has been and still is under a fairly tight
> > > MAINTAINER status.
> > >
> >
> > There is no such thing as a hard lock in FreeBSD. This sounds like you
> are
> > advocating for that, but that's not the case.
> >
> > Stop this stupid nitpicking for single line commits. We don't have that
> > culture any more and it's really pissing a lot of people off.
> >
> > The MAINTAINERS file even says this:
> >
> > Please note that the content of this file is strictly advisory.
> >
> > And the entry for bhyve doesn't say things are mandatory, just requested.
> >
> > Jumping on people's case like this, for a review you yourself were on and
> > approved but made no mention of seeking further review / approval, is
> > demotivating and toxic. Please stop.
>
> I explicitly DID add jhb to the review.
> I also explicitly did not mark the bhyve# box that is added by
> the hearald rules.
>
> I did not jump on him, I informed him of the entry, and told him to leave
> it.
> You how ever have infact jumped on me, repeatedly, if you want to talk
> about
> discouraging tones of behavior I suggest you look at yourself as well.
>

Weird that so many other people in the thread read it the same way that I
did, and not only this time. That suggests that you're not aware that your
behavior is annoying others, which I contend is a problem that needs
looking into.

As always, I'm open to constructive, actionable feedback about my actions.

Warner


> > Warner
> >
> > >   head/usr.sbin/bhyve/pci_emul.c
> > > >
> > > > Modified: head/usr.sbin/bhyve/pci_emul.c
> > > >
> > >
> ==============================================================================
> > > > --- head/usr.sbin/bhyve/pci_emul.c    Fri Mar 15 02:11:27 2019
> > > (r345170)
> > > > +++ head/usr.sbin/bhyve/pci_emul.c    Fri Mar 15 02:11:28 2019
> > > (r345171)
> > > > @@ -953,7 +953,10 @@ pci_emul_add_pciecap(struct pci_devinst *pi, int
> > > type)
> > > >       bzero(&pciecap, sizeof(pciecap));
> > > >
> > > >       pciecap.capid = PCIY_EXPRESS;
> > > > -     pciecap.pcie_capabilities = PCIECAP_VERSION |
> PCIEM_TYPE_ROOT_PORT;
> > > > +     pciecap.pcie_capabilities = PCIECAP_VERSION | type;
> > > > +     /* Devices starting with version 1.1 must set the RBER bit */
> > > > +     if (PCIECAP_VERSION >= 1)
> > > > +             pciecap.dev_capabilities = PCIEM_CAP_ROLE_ERR_RPT;
> > > >       pciecap.link_capabilities = 0x411;      /* gen1, x1 */
> > > >       pciecap.link_status = 0x11;             /* gen1, x1 */
> > > >
> > > >
> > > >
> > >
> > > --
> > > Rod Grimes
> > > rgrimes@freebsd.org
> > >
> > >
>
> --
> Rod Grimes
> rgrimes@freebsd.org
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfqFCrVVgpY4cuFwpTHwigXcE7L=aL_44Cy4KY2G4WuP0g>