From owner-svn-src-all@freebsd.org Tue Feb 20 20:04:23 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 16ABCF2669A; Tue, 20 Feb 2018 20:04:23 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: from mail-io0-f169.google.com (mail-io0-f169.google.com [209.85.223.169]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 74AE96A5EA; Tue, 20 Feb 2018 20:04:22 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mail-io0-f169.google.com with SMTP id t126so16234299iof.4; Tue, 20 Feb 2018 12:04:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:reply-to:in-reply-to:references :from:date:message-id:subject:to:cc:content-transfer-encoding; bh=KLQIBAfwPDn7fbufCIPjUDrwyOCzZOJazSticwGkNSI=; b=uWuJw78hXkEbRx4eEMUVkm+k25olO7fO1pgjgOD+Kyli9RrNsAristRw1gp25b7lB4 5lFiaw9jc0DRir4pQ/b+ixvFPBiP8UAwwvPRg1T5ZZ9DLAwfBr3/C2BMBb5QqjdMcuJl /6A22HTX/KtgRVdp3p25pgbeiCyMPjlQIgEV9wlJcnbAbRJiqsB/BRIiJwmmKCz9+HVu yR3nIcY/0iwgmBN7BhzFZ+ofLW1bY270tjUQZW/dOTm51ZFPkj9yzXioBRdp9SFxtYgA G5soqnf4kL/KnYsFGuB8m93rBJ4LTmg7J4YDbVtoPak5bU30ERsfQ9H7GYM2X+Ogg0Jp lWsw== X-Gm-Message-State: APf1xPAdyCp0YLafvF6KNQJDCHbYid04xoBuAz1erhNmEM5fm4A012x3 ZG3/F6G/4j0Y2ts6+TA9w6iAAq9q X-Google-Smtp-Source: AG47ELu3/79r2oX0sKTt8cBPuBJhpsBele9rahpvrf3G8T6EZvFYcLTEoznVh/blcgAQJpHx2u0GVQ== X-Received: by 10.107.59.130 with SMTP id i124mr1045034ioa.129.1519156650108; Tue, 20 Feb 2018 11:57:30 -0800 (PST) Received: from mail-io0-f175.google.com (mail-io0-f175.google.com. [209.85.223.175]) by smtp.gmail.com with ESMTPSA id k66sm23020452itk.5.2018.02.20.11.57.29 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 20 Feb 2018 11:57:29 -0800 (PST) Received: by mail-io0-f175.google.com with SMTP id u84so16237058iod.9; Tue, 20 Feb 2018 11:57:29 -0800 (PST) X-Received: by 10.107.41.16 with SMTP id p16mr1066096iop.173.1519156649473; Tue, 20 Feb 2018 11:57:29 -0800 (PST) MIME-Version: 1.0 Reply-To: cem@freebsd.org Received: by 10.2.30.149 with HTTP; Tue, 20 Feb 2018 11:57:28 -0800 (PST) In-Reply-To: <201802160517.w1G5H1XH047278@repo.freebsd.org> References: <201802160517.w1G5H1XH047278@repo.freebsd.org> From: Conrad Meyer Date: Tue, 20 Feb 2018 11:57:28 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: svn commit: r329360 - in head/sys: amd64/vmm/amd contrib/dev/acpica/include To: Anish Gupta Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Feb 2018 20:04:23 -0000 Hi Anish, Some coverity nits inline: On Thu, Feb 15, 2018 at 9:17 PM, Anish Gupta wrote: > Author: anish > Date: Fri Feb 16 05:17:00 2018 > New Revision: 329360 > URL: https://svnweb.freebsd.org/changeset/base/329360 > > Log: > This change fixes duplicate detection of same IOMMU/AMD-Vi device for R= yzen with EFR support. > > IVRS can have entry of type legacy and non-legacy present at same time = for same AMD-Vi device. ivhd driver will ignore legacy if new IVHD type is = present as specified in AMD-Vi specification. Earlier both of IVHD entries = used and two ivhd devices were created. > Add support for new IVHD type 0x11 and 0x40 in ACPI. Create new struct = of type acpi_ivrs_hardware_new for these new type of IVHDs. Legacy type 0x1= 0 will continue to use acpi_ivrs_hardware. > > Reviewed by: avg > Approved by: grehan > Differential Revision:https://reviews.freebsd.org/D13160 > > Modified: > head/sys/amd64/vmm/amd/amdvi_hw.c > head/sys/amd64/vmm/amd/amdvi_priv.h > head/sys/amd64/vmm/amd/ivrs_drv.c > head/sys/contrib/dev/acpica/include/actbl2.h > > ... > Modified: head/sys/amd64/vmm/amd/ivrs_drv.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/sys/amd64/vmm/amd/ivrs_drv.c Fri Feb 16 04:59:21 2018 (= r329359) > +++ head/sys/amd64/vmm/amd/ivrs_drv.c Fri Feb 16 05:17:00 2018 (= r329360) > ... > @@ -196,11 +205,26 @@ ivhd_dev_parse(ACPI_IVRS_HARDWARE * ivhd, struct am= dvi > softc->start_dev_rid =3D ~0; > softc->end_dev_rid =3D 0; > > - /* > - * XXX The following actually depends on Header.Type and > - * is only true for 0x10. > - */ > - p =3D (uint8_t *)ivhd + sizeof(ACPI_IVRS_HARDWARE); > + switch (ivhd->Header.Type) { > + case ACPI_IVRS_TYPE_HARDWARE_EXT1: > + case ACPI_IVRS_TYPE_HARDWARE_EXT2: > + p =3D (uint8_t *)ivhd + sizeof(ACPI_IVRS_HARDWARE= _NEW); > + de =3D (ACPI_IVRS_DE_HEADER *) ((uint8_t *)ivhd + > + sizeof(ACPI_IVRS_HARDWARE_NEW)); > + break; > + > + case ACPI_IVRS_TYPE_HARDWARE: > + p =3D (uint8_t *)ivhd + sizeof(ACPI_IVRS_HARDWARE= ); > + de =3D (ACPI_IVRS_DE_HEADER *) ((uint8_t *)ivhd + > + sizeof(ACPI_IVRS_HARDWARE)); Coverity points out that initializing 'de' in these cases is pointless, as the value is never used before it is overridden in the immediately subsequent loop. > ... > @@ -285,14 +309,30 @@ ivhd_dev_parse(ACPI_IVRS_HARDWARE * ivhd, struct am= dvi > return (0); > } > > +static bool > +ivhd_is_newer(ACPI_IVRS_HEADER *old, ACPI_IVRS_HEADER *new) > +{ > + /* > + * Newer IVRS header type take precedence. > + */ > + if ((old->DeviceId =3D=3D new->DeviceId) && > + (old->Type =3D=3D ACPI_IVRS_TYPE_HARDWARE) && > + ((new->Type =3D=3D ACPI_IVRS_TYPE_HARDWARE_EXT1) || > + (new->Type =3D=3D ACPI_IVRS_TYPE_HARDWARE_EXT1))) { Coverity also points out that both sides of this OR are the same, ACPI_IVRS_TYPE_HARDWARE_EXT1. Logically this is redundant but probably indicates a typo? Perhaps one should be ACPI_IVRS_TYPE_HARDWARE_EXT2? Thanks, Conrad