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 18CE4D802C7 for ; Fri, 28 Jul 2023 19:40:29 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=LBI7cCS8v8BIObmI3cdV6FtlRhqXVaqSXRke6h7Cx30=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:X-Received:X-Gm-Message-State:X-Google-Smtp-Source:X-Received:MIME-Version:References:In-Reply-To:From:Date: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=1690573228; v=1; b=O47b8YyS8p/769UjYRhFJpyvA52euCMdLiw+9m1RfdJbjsaNqTevuQMfAKkjtU9gabeI5oka Pd+/c4YL1bnJR9vE61OACLmNhXsN0mnijxYuWkOvrm0v1gTXILLBCD/UrYXy63ojK7QVzpVxV/Z L1f0+saEpw7RMuLdXqqdM4io= X-Received: by 127.0.0.2 with SMTP id bw1yYY7687511xzrgiNdrNnd; Fri, 28 Jul 2023 12:40:28 -0700 X-Received: from mail-vk1-f170.google.com (mail-vk1-f170.google.com [209.85.221.170]) by mx.groups.io with SMTP id smtpd.web11.41224.1690573227951437741 for ; Fri, 28 Jul 2023 12:40:28 -0700 X-Received: by mail-vk1-f170.google.com with SMTP id 71dfb90a1353d-48651709fa5so898714e0c.1 for ; Fri, 28 Jul 2023 12:40:27 -0700 (PDT) X-Gm-Message-State: saLvGYpLEPQgy1PZzrXea7vjx7686176AA= X-Google-Smtp-Source: APBJJlFDOTmzFst/mUpISf3y3v4otrQgJuRqTh6+5fBG9aPuVrfzSz0cOnQRY3fJfU5ln8zTzOF/1npCner2tbvBTNI= X-Received: by 2002:a1f:c485:0:b0:486:4220:a46f with SMTP id u127-20020a1fc485000000b004864220a46fmr2688153vkf.2.1690573226668; Fri, 28 Jul 2023 12:40:26 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: "Pedro Falcato" Date: Fri, 28 Jul 2023 20:40:15 +0100 Message-ID: Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1] Silicon/Synopsys/DesignWare: DwEmacSnpDxe: Fix bug in EmacGetDmaStatus To: devel@edk2.groups.io, wangyzhaoz@163.com Cc: Leif Lindholm , Ard Biesheuvel , 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,pedro.falcato@gmail.com 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=O47b8YyS; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none) 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 > 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/EmacDxeUtil= .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/10731= 1 > 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.co= m] > ------------ > > --=20 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 (#107376): https://edk2.groups.io/g/devel/message/107376 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-