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.web10.11026.1584025027353645904 for ; Thu, 12 Mar 2020 07:57:07 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=yXDQsjZR; spf=pass (domain: nuviainc.com, ip: 209.85.221.66, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f66.google.com with SMTP id v11so7876936wrm.9 for ; Thu, 12 Mar 2020 07:57:07 -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=IcZuJYnosVTRlSOLb80ErnidM2hIWGBdEZpd09kFwQ0=; b=yXDQsjZRkSLQSeZa14PMqXNSyfgpaI26DjPNV/yFTjQie8mVKrSpil8NqQAe1geP8/ D5ez4idAeV7ymfWZUGblvT2hSlfVdE1HeQ03JvMzyPa3yMxVPOIld03cEjPbrSuJAQM5 f7ZGBbfhZ/3zdtWOetR8eWVCL8o8Gdz9XfBn4ckhjag0xDDPyyD4yUuB9//Z1npY90bc YqRqYCciO21rCUQH4Iz5JLmChK+3VBsZst4fNQtFI8gLHTHw9ZOHe70kD6HUumJaInWf m7aWlX+l4gDfQl94MrVoDRCGXDJclj85vXF16zbeBHPC1DaAFBIrFqvfRVMxXYAho5kq DHPg== 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=IcZuJYnosVTRlSOLb80ErnidM2hIWGBdEZpd09kFwQ0=; b=pwTRHmbgff8QcIkixPmjr+Rm/8elOYDYADlNDB13jPjfi4aBi0Uhd0ePQP65p9Q41Q VjgcI/m52YBJqFEjDr9xHH0iNZHSz9o4qe/KQ1PvRlWQ67mAFcLvayjTw6TNJiRbjhiK hM41h34wKadExWWg9DhkbgtfXWwRdQV64MbQPndbUqPtqrqC65bXo1j1gZZBJbc+ccLP OzZRaV8UOw2hFdAVQCIyzqBojFxni3PcTCUJoVUoOHCP4xCaJoZ+OK5zGmPWtQpOGjVX u4Xf+iZTlZV4ofDXiclWQrbk0oCR4wO8oreLNyUsSghJN1rIiw3xGJkZ5tX3T5LnBNnM u7OA== X-Gm-Message-State: ANhLgQ3nWpWYpO04wx60GlPD0vwHFmV5ucKRf5z8dBs6nGEUneZ6p29a oGF23P6/4/389egsE0UkxuDm1A== X-Google-Smtp-Source: ADFU+vsobr0+o0dBWF+UzoMsGUEsndQFXb9NAnhyzXtd8IjrIzFXhxzXSmM5/hqIxK9TZLntokidzA== X-Received: by 2002:a05:6000:c:: with SMTP id h12mr5636770wrx.168.1584025025933; Thu, 12 Mar 2020 07:57:05 -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 h10sm538542wrb.24.2020.03.12.07.57.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Mar 2020 07:57:05 -0700 (PDT) Date: Thu, 12 Mar 2020 14:57:04 +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: <20200312145704.GJ23627@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> <20200312144452.GI23627@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: <20200312144452.GI23627@bivouac.eciton.net> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline For clarity, I'm suggesting *I* take care of moving this into a generic Intrinsics handling lib, and that HPE take care of including and using it. I'm also not suggesting we revert the CopyMem patch before that is complete. On Thu, Mar 12, 2020 at 14:44:52 +0000, Leif Lindholm wrote: > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >