From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web12.10704.1584024295999435295 for ; Thu, 12 Mar 2020 07:44:56 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=st5kZ5hO; spf=pass (domain: nuviainc.com, ip: 209.85.221.66, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f66.google.com with SMTP id n15so7786485wrw.13 for ; Thu, 12 Mar 2020 07:44:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=1Ii1H3E9APMURfN6DwlIgEhJA6e2DrSy58F6gYrm8ek=; b=st5kZ5hOu0vvEctsv/bAyXLs2/zWf3ONKZeW2l/ldRouch09SbXnHGTkZEGi2BmCHa PzYme9yFerZFk1igNjUWiGVdmwiUG+cDBv2x6PSxNDBHD4t1FhDboAhkaiS0fzS8a9Kt MC5q3pgFyk/3My8msGTB4a8L9vKglq/CSOSHV5aj+QZ9U76HHzrsYRR7EV5g5Z0p2LbU ZC7PvadCf3BWdKBNPelxeY0xPH0mfayRUTRYdAP2Z4TltROZT92ZHxU5MaEaZ2lcxg+a YlTWpOFKS5qmtUvaGJi2lcJ/dcuYu+2ceeBZktt0n78US80aaJMj8E8ezHGEGYhLaTMn RkEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=1Ii1H3E9APMURfN6DwlIgEhJA6e2DrSy58F6gYrm8ek=; b=Bom3RDNlqkkArYkJC91GFdMF1hpkvmfPTieF5tfcbPMGmW2hJnNybEkXKL6iap1SEl pVyLPjaA6SKQLv4gixmi2pcjtpTyfvSSKzPcKy/W1up+oZ04O1e/MycLcxq0jiSUqwS9 jlDFP4TcHitikMm88h1evvDRuIlBGHlh3SdO13ocjxppd8bsCJV1bb8t6dfxDlYZhQBk 0xzif/FSAiF13K3l6dxDN/vvWLnFrX1tvhdVPfyvWQ6I8QBQGW7Qe1Z9iHTG+tX91knG igyy4IOpOKvyj6YBgYTYO+OINvKn6dY2JD6qFehus463QVlzfWDkqiWGO2pi5+EGYOG9 dPVQ== X-Gm-Message-State: ANhLgQ0i2t6CZR+5dOmNSs8ctueCVNdJlh0MFzEjDajtpHBGUKcDzaoC kwUDrik5g2wjAzsPnCkIe1zkog== X-Google-Smtp-Source: ADFU+vtpcPjRdKQ55BODenGgO2u68brOh3Wxjty4DVHiYKMzPqMyLkpphqT/CCOV7f9v29XLz12+Tg== X-Received: by 2002:adf:ef44:: with SMTP id c4mr11162524wrp.404.1584024294586; Thu, 12 Mar 2020 07:44:54 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id n6sm1941776wmn.13.2020.03.12.07.44.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Mar 2020 07:44:54 -0700 (PDT) Date: Thu, 12 Mar 2020 14:44:52 +0000 From: "Leif Lindholm" To: "Chang, Abner (HPS SW/FW Technologist)" Cc: "Schaefer, Daniel (DualStudy)" , "devel@edk2.groups.io" , "Chen, Gilbert" , "afish@apple.com" , "michael.d.kinney@intel.com" , "pete@akeo.ie" , Ard Biesheuvel , Laszlo Ersek Subject: Re: [edk2-devel] [PATCH v2 3/3] MdeModulePkg: Use CopyMem instead of GUID assignment Message-ID: <20200312144452.GI23627@bivouac.eciton.net> References: <20200302103238.25726-1-daniel.schaefer@hpe.com> <20200302103238.25726-4-daniel.schaefer@hpe.com> <20200312105528.GC23627@bivouac.eciton.net> <539c8673-786c-9c58-98cc-ab470b345740@hpe.com> <20200312140304.GF23627@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline And what would you propose we do the next time the RISC-V toolchain generates a memcpy call based on some other completely valid change to core code? Doing this de-risks the RISC-V upstreaming effort. It's also a trivial move/rename opetation - the only question is where the library should live and be called. / Leif On Thu, Mar 12, 2020 at 14:33:27 +0000, Chang, Abner (HPS SW/FW Technologist) wrote: > I don't prefer to do this at this moment, Leif. I have no problem if > we create a BZ for this and create BaseCompilerIntrinsicsLib in > MdeModulePkg, but please don't bind it with RISC-V EDK2 port. You > know my concern pretty well, we can't hold those RISC-V patches on > hands like forever and address the code structure optimization. We > can still work on BaseCompilerIntrinsicsLib but not part of this > RISC-V submission. We can implement RISC-V variant if necessary > after RISC-V edk2 get in edk2 master. > > Abner > > > -----Original Message----- > > From: Leif Lindholm [mailto:leif@nuviainc.com] > > Sent: Thursday, March 12, 2020 10:03 PM > > To: Schaefer, Daniel (DualStudy) > > Cc: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) > > ; Chen, Gilbert ; > > afish@apple.com; michael.d.kinney@intel.com; pete@akeo.ie; Ard > > Biesheuvel ; Laszlo Ersek > > Subject: Re: [edk2-devel] [PATCH v2 3/3] MdeModulePkg: Use CopyMem > > instead of GUID assignment > > > > +Ard, Laszlo. > > > > I think it would make sense to move it to MdeModulePkg (or MdePkg) and > > rename it BaseCompilerIntrinsicsLib (it *is* a BASE library). > > > > As I alluded in my reply to Ray - x86 also have this problem, but to a lesser > > extent, and ended up creating library functions to call instead of using plain C > > for certain operations. > > Which was probably the right decision when it was restricted to a very few > > corner cases. > > > > / > > Leif > > > > On Thu, Mar 12, 2020 at 13:24:30 +0000, Schaefer, Daniel (DualStudy) wrote: > > > Hi Leif, > > > > > > you're right. If I revert my commit and include > > > NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf > > > without making any changes to it, the build succeeds. > > > > > > What do others think? (cc Michael, Pete, Andrew, Ard, who have made > > > changes to this module) Is this a big hack or should we use it in RISC-V, too > > and move the module to MdeModulePkg? > > > Why isn't this a problem on x86? Was it fine on Itanium? > > > > > > - Daniel > > > > > > ________________________________ > > > From: devel@edk2.groups.io > > > on behalf of Leif > > > Lindholm > > > Sent: Thursday, March 12, 2020 11:55 > > > To: devel@edk2.groups.io > > > ; Schaefer, Daniel > > > (DualStudy) > > > > > Cc: Chang, Abner (HPS SW/FW Technologist) > > > ; Chen, Gilbert > > > ; Dandan Bi > > > ; Eric Dong > > > > > > Subject: Re: [edk2-devel] [PATCH v2 3/3] MdeModulePkg: Use CopyMem > > > instead of GUID assignment > > > > > > Hi Daniel, > > > > > > There is nothing wrong with this patch that just went in (and I should > > > have called out sooner if I wanted to stop it), but I think a better > > > solution is to implement a RISC-V variant of > > > ArmPkg/Library/CompilerIntrinsicsLib/. > > > > > > It is perfectly valid for the compiler to generate memcpy calls in > > > response to struct operations that are perfectly valid C. > > > > > > In fact, we could consider moving the ArmPkg one over into > > > MdeModulePkg. I have a feeling that including a > > > NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf > > > in your current build would be an alternative solution to your > > > compilation error. > > > > > > / > > > Leif > > > > > > On Mon, Mar 02, 2020 at 11:32:38 +0100, Daniel Schaefer wrote: > > > > GCC translates a simple assignment to memcpy, which EDKII doesn't > > provide. > > > > See: > > > > INVALID URI REMOVED > > 2Darch > > > > ive.com_edk2-2Ddevel- > > 40lists.01.org_msg11928.html&d=DwIBAg&c=C5b8zRQ > > > > > > O1miGmBeVZ2LFWg&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3 > > E&m=wjlf > > > > > > QYZsXfc5WWDFmywLOEYlCKvrpYrnaXMIpJcLK7U&s=426yv7VvgHTtgtYaR0f0X > > NiaEA > > > > LgqZkxektjywEwPco&e= > > > > > > > > REF:INVALID URI REMOVED > > > > anocore.org_show-5Fbug.cgi-3Fid- > > 3D2547&d=DwIBAg&c=C5b8zRQO1miGmBeVZ2 > > > > > > LFWg&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=wjlfQYZsX > > fc5WWD > > > > FmywLOEYlCKvrpYrnaXMIpJcLK7U&s=PjC_mIwh0GhNy5np2h7K8l4l- > > pLLBWt1FRuLB > > > > UbULpc&e= > > > > > > > > Signed-off-by: Daniel Schaefer > > > > > > > > Cc: Abner Chang > > > > > > Cc: Gilbert Chen > > > > > > Cc: Leif Lindholm > > > > Cc: Dandan Bi > > > > Cc: Eric Dong > > > > --- > > > > > > > > Notes: > > > > v2: > > > > - Use CopyMem instead of CopyGuid [Dandan] > > > > > > > > MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git > > a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > > > > b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > > > > index 5cc527679a78..0540e6fa8a44 100644 > > > > --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > > > > +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > > > > @@ -619,7 +619,7 @@ CreateDeviceManagerForm( > > > > TokenHelp = HiiSetString (HiiHandle, 0, String, NULL); > > > > FreePool (String); > > > > > > > > - FormSetGuid = ((EFI_IFR_FORM_SET *)Ptr)->Guid; > > > > + CopyMem (&FormSetGuid, &((EFI_IFR_FORM_SET *) Ptr)->Guid, > > > > + sizeof (EFI_GUID)); > > > > > > > > // > > > > // Network device process > > > > -- > > > > 2.25.0 > > > > > > > > > > > > > > > > > > > > > > > > >