From nobody Tue Jun 06 04:31:56 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4QZyHY6Zkhz4bld0 for ; Tue, 6 Jun 2023 04:32:09 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com [IPv6:2a00:1450:4864:20::62e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4QZyHY4ZSKz3QG1 for ; Tue, 6 Jun 2023 04:32:09 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-ej1-x62e.google.com with SMTP id a640c23a62f3a-977d02931d1so342018966b.0 for ; Mon, 05 Jun 2023 21:32:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20221208.gappssmtp.com; s=20221208; t=1686025928; x=1688617928; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=II1vapOVkldguiLbfYwly/YEq8lJBMvVWYgW8C8Mwh4=; b=GnZQyp9Jbi5AgYltIdLMu5T/dZsyUmTz68heP8dMJJZ1SXm0g/K6NDyEcj43yf6yp6 A6fYMt5t1hv18OTMelgPFRarZ5nk5xnGQ4Zw4g7eSNv/sICgvSZ0xWMuxKXLSvy480Yj RM72Rgfs/Fh23fk3IHH3Xr76QGuSy9xMSWYvsP02Hv7LyOCNAgoplZhOQ2e07WxPvYI1 d0GR35Vek7ZNkrpSTNsxbS0IuI7mUPRucBPpkuqUhSAjacvdmwtmtvRiKbEzg3yd0kEP +CgPTiaXKzL4+MT8Hqe2CwM0W5EbxKnbAEyGzySnVdoJvHJSNuf8flGNje7w3Jx63V8P oinA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686025928; x=1688617928; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=II1vapOVkldguiLbfYwly/YEq8lJBMvVWYgW8C8Mwh4=; b=YtptNWM6acHuGGqG2pQg/IDAn2a7MGaOkrIQi7t8DXK45KTQ0u/uN0l3mWJxM+Vsha AHJx/P28AXIBmrRkopc0O/AhevFhdLQ5AWfmlJbmxtdnJnb/Ht90q0PLSSyJpIiq37TW +lK1KT7gtG0nhTuFHbYncATzyxoJ2oBV8C3crF64AfmfmK7gi98uhjmvI8E/wHLHJi3v eWFfJzUarc6bwl8ICzmLo2BhDeOmg/vKlkDn3LgEqyuWx7MMIb8abzpCCzp/hbddzMgN G+rx+Sqgg9FCVSEBNgqrEPdbcuJp4yGgxyrzIEjePHWEu+VJY358e6boc6rByaqhmrwm Hc/Q== X-Gm-Message-State: AC+VfDx8DOL02yrnCyU+VCAs6A+O0N9f8cKWBp/LCGZR1um7c4JjT4uU E6ieA1uQ5HPXWyjsKkEiQN4L/R8Shbec1k7p7kZdzA== X-Google-Smtp-Source: ACHHUZ6UQdlF59Sov8dtWvX1hz0r2oIIG0+vnYyrMMhzmk30E93uzMmTASC2Ngw9b/33oK0RSyWZ0NXBOxCL6pdFygg= X-Received: by 2002:a17:906:dc8e:b0:970:482:9fcd with SMTP id cs14-20020a170906dc8e00b0097004829fcdmr1143223ejc.24.1686025927900; Mon, 05 Jun 2023 21:32:07 -0700 (PDT) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 References: <202306060406.35646hbA078041@gitrepo.freebsd.org> <6A5F859A-033A-43E1-81CB-B364662DE2F3@freebsd.org> In-Reply-To: <6A5F859A-033A-43E1-81CB-B364662DE2F3@freebsd.org> From: Warner Losh Date: Mon, 5 Jun 2023 22:31:56 -0600 Message-ID: Subject: Re: git: 1bbdfb0b4386 - main - gve: Add PNP info to PCI attachment of gve(4) driver. To: Jessica Clarke Cc: Xin LI , src-committers , "" , "" Content-Type: multipart/alternative; boundary="00000000000083cf7805fd6e8084" X-Rspamd-Queue-Id: 4QZyHY4ZSKz3QG1 X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N --00000000000083cf7805fd6e8084 Content-Type: text/plain; charset="UTF-8" On Mon, Jun 5, 2023, 10:15 PM Jessica Clarke wrote: > On 6 Jun 2023, at 05:06, Xin LI wrote: > > > > The branch main has been updated by delphij: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=1bbdfb0b438689a839e29094fcb582a104cbabd9 > > > > commit 1bbdfb0b438689a839e29094fcb582a104cbabd9 > > Author: Xin LI > > AuthorDate: 2023-06-06 00:58:43 +0000 > > Commit: Xin LI > > CommitDate: 2023-06-06 04:05:55 +0000 > > > > gve: Add PNP info to PCI attachment of gve(4) driver. > > > > Reviewed-by: imp > > Differential Revision: https://reviews.freebsd.org/D40429 > > --- > > sys/dev/gve/gve_main.c | 28 ++++++++++++++++++++++++---- > > 1 file changed, 24 insertions(+), 4 deletions(-) > > > > diff --git a/sys/dev/gve/gve_main.c b/sys/dev/gve/gve_main.c > > index ae45a0cfc24a..383fd326d33a 100644 > > --- a/sys/dev/gve/gve_main.c > > +++ b/sys/dev/gve/gve_main.c > > @@ -38,6 +38,16 @@ > > > > #define GVE_DEFAULT_RX_COPYBREAK 256 > > > > +/* Devices supported by this driver. */ > > +static struct gve_dev { > > + uint16_t vendor_id; > > + uint16_t device_id; > > + const char *name; > > +} gve_devs[] = { > > + { PCI_VENDOR_ID_GOOGLE, PCI_DEV_ID_GVNIC, "gVNIC" } > > +}; > > +#define GVE_DEVS_COUNT nitems(gve_devs) > > IMO this obfuscates rather than helps given the loop iterates over > gve_devs (and the macro references it). > > > + > > struct sx gve_global_lock; > > > > static int > > @@ -701,10 +711,18 @@ gve_service_task(void *arg, int pending) > > static int > > gve_probe(device_t dev) > > { > > - if (pci_get_vendor(dev) == PCI_VENDOR_ID_GOOGLE && > > - pci_get_device(dev) == PCI_DEV_ID_GVNIC) { > > - device_set_desc(dev, "gVNIC"); > > - return (BUS_PROBE_DEFAULT); > > + uint16_t deviceid, vendorid; > > + int i; > > + > > + vendorid = pci_get_vendor(dev); > > + deviceid = pci_get_device(dev); > > + > > + for (i = 0; i < GVE_DEVS_COUNT; i++) { > > + if (vendorid == gve_devs[i].vendor_id && > > + deviceid == gve_devs[i].device_id) { > > + device_set_desc(dev, gve_devs[i].name); > > + return (BUS_PROBE_DEFAULT); > > + } > > } > > return (ENXIO); > > } > > @@ -851,3 +869,5 @@ DRIVER_MODULE(gve, pci, gve_driver, gve_devclass, 0, > 0); > > #else > > DRIVER_MODULE(gve, pci, gve_driver, 0, 0); > > #endif > > +MODULE_PNP_INFO("U16:vendor;U16:device", pci, gve, gve_devs, > > This is missing ;D:# so it will break as soon as you have a second > entry in the table. > While the first bit is true, I though we encoded the per entry size so trailing garbage didn't matter.. d# will be more useful in the future maybe. Warner Jess > > > + GVE_DEVS_COUNT); > --00000000000083cf7805fd6e8084 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Mon, Jun 5, 2023, 10:15 PM Jessica Clarke <jrtc27@freebsd.org> wrote:
On 6 Jun 2023, at 05:06, Xin LI <delphij= @FreeBSD.org> wrote:
>
> The branch main has been updated by delphij:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=3D1bbdfb0b438689a839e29094fcb582= a104cbabd9
>
> commit 1bbdfb0b438689a839e29094fcb582a104cbabd9
> Author:=C2=A0 =C2=A0 =C2=A0Xin LI <delphij@FreeBSD.org>
> AuthorDate: 2023-06-06 00:58:43 +0000
> Commit:=C2=A0 =C2=A0 =C2=A0Xin LI <delphij@FreeBSD.org>
> CommitDate: 2023-06-06 04:05:55 +0000
>
>=C2=A0 =C2=A0 gve: Add PNP info to PCI attachment of gve(4) driver.
>
>=C2=A0 =C2=A0 Reviewed-by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 imp=
>=C2=A0 =C2=A0 Differential Revision:=C2=A0 https://= reviews.freebsd.org/D40429
> ---
> sys/dev/gve/gve_main.c | 28 ++++++++++++++++++++++++----
> 1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/sys/dev/gve/gve_main.c b/sys/dev/gve/gve_main.c
> index ae45a0cfc24a..383fd326d33a 100644
> --- a/sys/dev/gve/gve_main.c
> +++ b/sys/dev/gve/gve_main.c
> @@ -38,6 +38,16 @@
>
> #define GVE_DEFAULT_RX_COPYBREAK 256
>
> +/* Devices supported by this driver. */
> +static struct gve_dev {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 uint16_t vendor_id;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 uint16_t device_id;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 const char *name;
> +} gve_devs[] =3D {
> + { PCI_VENDOR_ID_GOOGLE, PCI_DEV_ID_GVNIC, "gVNIC" }
> +};
> +#define GVE_DEVS_COUNT nitems(gve_devs)

IMO this obfuscates rather than helps given the loop iterates over
gve_devs (and the macro references it).

> +
> struct sx gve_global_lock;
>
> static int
> @@ -701,10 +711,18 @@ gve_service_task(void *arg, int pending)
> static int
> gve_probe(device_t dev)
> {
> - if (pci_get_vendor(dev) =3D=3D PCI_VENDOR_ID_GOOGLE &&
> -=C2=A0 =C2=A0 pci_get_device(dev) =3D=3D PCI_DEV_ID_GVNIC) {
> - device_set_desc(dev, "gVNIC");
> - return (BUS_PROBE_DEFAULT);
> + uint16_t deviceid, vendorid;
> + int i;
> +
> + vendorid =3D pci_get_vendor(dev);
> + deviceid =3D pci_get_device(dev);
> +
> + for (i =3D 0; i < GVE_DEVS_COUNT; i++) {
> + if (vendorid =3D=3D gve_devs[i].vendor_id &&
> +=C2=A0 =C2=A0 deviceid =3D=3D gve_devs[i].device_id) {
> + device_set_desc(dev, gve_devs[i].name);
> + return (BUS_PROBE_DEFAULT);
> + }
> }
> return (ENXIO);
> }
> @@ -851,3 +869,5 @@ DRIVER_MODULE(gve, pci, gve_driver, gve_devclass, = 0, 0);
> #else
> DRIVER_MODULE(gve, pci, gve_driver, 0, 0);
> #endif
> +MODULE_PNP_INFO("U16:vendor;U16:device", pci, gve, gve_devs= ,

This is missing ;D:# so it will break as soon as you have a second
entry in the table.

While the first bit is true, I though we encoded the per= entry size so trailing garbage didn't matter.. d# will be more useful = in the future maybe.

War= ner

Jess

> +=C2=A0 =C2=A0 GVE_DEVS_COUNT);
--00000000000083cf7805fd6e8084--