From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id E3EBB7803D7 for ; Sat, 29 Jul 2023 07:06:34 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=SgvVIWrxThNQ4dg9zfkJsBAvxdN04rx1aiOXQmaA+Do=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:X-Received:X-Received:X-Received:X-Gm-Message-State:X-Google-Smtp-Source:X-Received:MIME-Version:References:In-Reply-To:From:Date:X-Gmail-Original-Message-ID:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20140610; t=1690614393; v=1; b=uJOB7VdMB356WEXVZ29FsJ2W9Rj1b4zVaA8GX+LWwneTxmlSKlC8I6E272QgEtdrGjmPKa50 ASqVr6nzj76wc9Zj+pxbX8R6sj35oBGvDhaN6Gvj8A6hGnNL2SXdMZRwrnG0VRwDr6mYNbStkjq d1eUS9pRVKciDWirzPicqkho= X-Received: by 127.0.0.2 with SMTP id Kz3ZYY7687511xlev7s2CIRm; Sat, 29 Jul 2023 00:06:33 -0700 X-Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.50217.1690614392998132056 for ; Sat, 29 Jul 2023 00:06:33 -0700 X-Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 56CA5608CC for ; Sat, 29 Jul 2023 07:06:32 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4A815C433CA for ; Sat, 29 Jul 2023 07:06:31 +0000 (UTC) X-Received: by mail-lf1-f43.google.com with SMTP id 2adb3069b0e04-4fcd615d7d6so4634127e87.3 for ; Sat, 29 Jul 2023 00:06:31 -0700 (PDT) X-Gm-Message-State: LpwfpuCpJEUqKZyy9X7H0XAXx7686176AA= X-Google-Smtp-Source: APBJJlH0465B5vAkmvhBQxTJ+2lqOzX1h3pVleZf8gardBQuwb9SQmSmlP/slFfNAfKartLZ6mQ+3wbTP79XlxQfh6g= X-Received: by 2002:ac2:4bca:0:b0:4fb:772a:af13 with SMTP id o10-20020ac24bca000000b004fb772aaf13mr3491157lfq.28.1690614389270; Sat, 29 Jul 2023 00:06:29 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: "Ard Biesheuvel" Date: Sat, 29 Jul 2023 09:06:17 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1] Silicon/Synopsys/DesignWare: DwEmacSnpDxe: Fix bug in EmacGetDmaStatus To: Pedro Falcato Cc: devel@edk2.groups.io, wangyzhaoz@163.com, Leif Lindholm , Ran Wang Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=uJOB7VdM; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Fri, 28 Jul 2023 at 21:40, Pedro Falcato wrote= : > > On Fri, Jul 28, 2023 at 5:00=E2=80=AFAM wangy wrote: > > > > From: Yang Wang > > > > If IrqStat is NULL, the interrupt status will not be > > read from the device.When the interrupt status is read, > > it will also be cleared. > > Let's improve the commit message a bit, something like: > > The EFI spec (see UEFI 2.10, 24.1.12) requires > EFI_SIMPLE_NETWORK.GetStatus() to handle NULL InterruptStatus pointers > by not reading nor clearing the interrupt status from the device. > > However, EmacGetDmaStatus (part of the DwEmacSnpDxe GetStatus() > implementation) did not correctly handle NULL IrqStat, despite already > being tagged as an OPTIONAL argument. This made calling GetStatus() > with a NULL pointer (for example, the call in MnpRecycleTxBuf) either > corrupt memory or straight-up crash. > > Make it EFI spec compliant, by adding proper NULL pointer checks > around RI_SET_MSK and TI_SET_MSK retrieval/clearing. > > -- > > In any case, take my: > > Acked-by: Pedro Falcato > Thanks, I've pushed this as 4f316893c9ed..cbab3c40f76e with the suggested update for the commit message. > > Cc: Leif Lindholm > > Cc: Ard Biesheuvel > > Cc: Ran Wang > > > > Signed-off-by: Yang Wang > > --- > > .../Drivers/DwEmacSnpDxe/EmacDxeUtil.c | 22 ++++++++++++------- > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > diff --git a/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUt= il.c b/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c > > index 3b982ce984..26d3ff6138 100755 > > --- a/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c > > +++ b/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c > > @@ -500,24 +500,30 @@ EmacGetDmaStatus ( > > UINT32 ErrorBit; > > UINT32 Mask =3D 0; > > > > + if (IrqStat !=3D NULL) { > > + *IrqStat =3D 0; > > + } > > + > > DmaStatus =3D MmioRead32 (MacBaseAddress + > > DW_EMAC_DMAGRP_STATUS_OFST); > > if (DmaStatus & DW_EMAC_DMAGRP_STATUS_NIS_SET_MSK) { > > Mask |=3D DW_EMAC_DMAGRP_STATUS_NIS_SET_MSK; > > // Rx interrupt > > if (DmaStatus & DW_EMAC_DMAGRP_STATUS_RI_SET_MSK) { > > - *IrqStat |=3D EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT; > > - Mask |=3D DW_EMAC_DMAGRP_STATUS_RI_SET_MSK; > > - } else { > > - *IrqStat &=3D ~EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT; > > + if (IrqStat !=3D NULL) { > > + *IrqStat |=3D EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT; > > + Mask |=3D DW_EMAC_DMAGRP_STATUS_RI_SET_MSK; > > + } > > } > > + > > // Tx interrupt > > if (DmaStatus & DW_EMAC_DMAGRP_STATUS_TI_SET_MSK) { > > - *IrqStat |=3D EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT; > > - Mask |=3D DW_EMAC_DMAGRP_STATUS_TI_SET_MSK; > > - } else { > > - *IrqStat &=3D ~EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT; > > + if (IrqStat !=3D NULL) { > > + *IrqStat |=3D EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT; > > + Mask |=3D DW_EMAC_DMAGRP_STATUS_TI_SET_MSK; > > + } > > } > > + > > // Tx Buffer > > if (DmaStatus & DW_EMAC_DMAGRP_STATUS_TU_SET_MSK){ > > Mask |=3D DW_EMAC_DMAGRP_STATUS_TU_SET_MSK; > > -- > > 2.25.1 > > > > > > > > ------------ > > Groups.io Links: You receive all messages sent to this group. > > View/Reply Online (#107311): https://edk2.groups.io/g/devel/message/107= 311 > > Mute This Topic: https://groups.io/mt/100404855/5946980 > > Group Owner: devel+owner@edk2.groups.io > > Unsubscribe: https://edk2.groups.io/g/devel/unsub [pedro.falcato@gmail.= com] > > ------------ > > > > > > > -- > Pedro -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107377): https://edk2.groups.io/g/devel/message/107377 Mute This Topic: https://groups.io/mt/100404855/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-