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 B4D839414B5 for ; Wed, 17 Jan 2024 10:20:23 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=OJ4B0e1hCY54MP8Yj3Spp6JRsSCHhv2VhnAfDnrxmZc=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1705486822; v=1; b=BW8NywhL5eOjNemNTVfkwwAxAbBBJ93PIONMi89vzdunuZ9QTz3zuqyopEGwZR8M7X7STlnM +ErxCtSXgvOWb2OJ4R4KbsUCxhRWcrf6ysnAIpnKr2KH1BkbIi/PN08Ocyyy6ulqIlM9wtTHqXh q4B2z9P2o2agP//6z6n5jTnU= X-Received: by 127.0.0.2 with SMTP id pK4eYY7687511xXWEbInSXOT; Wed, 17 Jan 2024 02:20:22 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web10.6333.1705486821704401453 for ; Wed, 17 Jan 2024 02:20:21 -0800 X-Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-549-_VNrbNO3Np-NW8cPyZRwOA-1; Wed, 17 Jan 2024 05:20:18 -0500 X-MC-Unique: _VNrbNO3Np-NW8cPyZRwOA-1 X-Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id AB5513813BC9 for ; Wed, 17 Jan 2024 10:20:18 +0000 (UTC) X-Received: from [10.39.192.21] (unknown [10.39.192.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 05BC61C066AB; Wed, 17 Jan 2024 10:20:17 +0000 (UTC) Message-ID: <6a7e9ad1-622f-ac78-1df7-94372f160f56@redhat.com> Date: Wed, 17 Jan 2024 11:20:16 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH v3 6/6] OvmfPkg/VirtNorFlashDxe: move DoErase code block into new function To: devel@edk2.groups.io, kraxel@redhat.com Cc: oliver@redhat.com References: <20240116171105.37831-1-kraxel@redhat.com> <20240116171105.37831-7-kraxel@redhat.com> From: "Laszlo Ersek" In-Reply-To: <20240116171105.37831-7-kraxel@redhat.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: rGfrvCbL3gRxAkR3KNqV0sihx7686176AA= Content-Language: en-US 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=BW8NywhL; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (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 1/16/24 18:11, Gerd Hoffmann wrote: > Move the DoErase code block into a separate function, call the function > instead of jumping around with goto. >=20 > Signed-off-by: Gerd Hoffmann > --- > OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c | 76 ++++++++++++++++++-------- > 1 file changed, 52 insertions(+), 24 deletions(-) >=20 > diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c b/OvmfPkg/VirtNorFlas= hDxe/VirtNorFlash.c > index 3d1d20daa1e5..e6aaed27ceba 100644 > --- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c > +++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c > @@ -502,6 +502,38 @@ NorFlashRead ( > return EFI_SUCCESS; > } > =20 > +STATIC > +EFI_STATUS > +NorFlashWriteSingleBlockWithErase ( > + IN NOR_FLASH_INSTANCE *Instance, > + IN EFI_LBA Lba, > + IN UINTN Offset, > + IN OUT UINTN *NumBytes, > + IN UINT8 *Buffer > + ) > +{ > + EFI_STATUS Status; > + > + // Read NOR Flash data into shadow buffer > + Status =3D NorFlashReadBlocks (Instance, Lba, Instance->BlockSize, Ins= tance->ShadowBuffer); > + if (EFI_ERROR (Status)) { > + // Return one of the pre-approved error statuses > + return EFI_DEVICE_ERROR; > + } > + > + // Put the data at the appropriate location inside the buffer area > + CopyMem ((VOID *)((UINTN)Instance->ShadowBuffer + Offset), Buffer, *Nu= mBytes); > + > + // Write the modified buffer back to the NorFlash > + Status =3D NorFlashWriteBlocks (Instance, Lba, Instance->BlockSize, In= stance->ShadowBuffer); > + if (EFI_ERROR (Status)) { > + // Return one of the pre-approved error statuses > + return EFI_DEVICE_ERROR; > + } > + > + return EFI_SUCCESS; > +} > + > /* > Write a full or portion of a block. It must not span block boundaries;= that is, > Offset + *NumBytes <=3D Instance->BlockSize. > @@ -607,7 +639,14 @@ NorFlashWriteSingleBlock ( > // that we want to set. In that case, we will need to erase the bloc= k first. > for (CurOffset =3D 0; CurOffset < *NumBytes; CurOffset++) { > if (~(UINT32)OrigData[CurOffset] & (UINT32)Buffer[CurOffset]) { > - goto DoErase; > + Status =3D NorFlashWriteSingleBlockWithErase ( > + Instance, > + Lba, > + Offset, > + NumBytes, > + Buffer > + ); > + return Status; > } > =20 > OrigData[CurOffset] =3D Buffer[CurOffset]; > @@ -636,33 +675,22 @@ NorFlashWriteSingleBlock ( > goto Exit; > } > } > - > -Exit: > - // Put device back into Read Array mode > - SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY= ); > - > + } else { > + Status =3D NorFlashWriteSingleBlockWithErase ( > + Instance, > + Lba, > + Offset, > + NumBytes, > + Buffer > + ); > return Status; > } > =20 > -DoErase: > - // Read NOR Flash data into shadow buffer > - Status =3D NorFlashReadBlocks (Instance, Lba, BlockSize, Instance->Sha= dowBuffer); > - if (EFI_ERROR (Status)) { > - // Return one of the pre-approved error statuses > - return EFI_DEVICE_ERROR; > - } > +Exit: > + // Put device back into Read Array mode > + SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY); > =20 > - // Put the data at the appropriate location inside the buffer area > - CopyMem ((VOID *)((UINTN)Instance->ShadowBuffer + Offset), Buffer, *Nu= mBytes); > - > - // Write the modified buffer back to the NorFlash > - Status =3D NorFlashWriteBlocks (Instance, Lba, BlockSize, Instance->Sh= adowBuffer); > - if (EFI_ERROR (Status)) { > - // Return one of the pre-approved error statuses > - return EFI_DEVICE_ERROR; > - } > - > - return EFI_SUCCESS; > + return Status; > } > =20 > EFI_STATUS Reviewed-by: Laszlo Ersek This series is now ready to be merged, but I'm unsure what the CI status is, after the recent security fixes seem to have caused breakage with multiple external definitions of the same symbol. For now I'll keep this tagged, as a reminder for myself. Laszlo -=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 (#113944): https://edk2.groups.io/g/devel/message/113944 Mute This Topic: https://groups.io/mt/103766779/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562= 12/xyzzy [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-