> On Jul 18, 2015, at 11:23 AM, Smith, Jonathan D <jonathan.d.sm...@intel.com> > wrote: > > The issues I ran into were after solving issues with the /D options. I’ve > been working with /D _EDK2_EFI /D _ACPI_DUMP_APP so far. A lot of warnings > escalated to errors were simple fixes, such as unsigned-to-signed > comparisons. I was asking whether the parameter I mentioned called “Address” > which was declared as type ACPI_SIZE should have been declared > ACPI_PHYSICAL_ADDRESS, since every use of that function (OslMapTable) passes > in type ACPI_PHYSICAL_ADDRESS. Changing to the address type fixes some > compile warnings, but I was wondering if there was some implicit reason why > we used the size type instead. >
Jonathan, 1st off this code comes from acpica, so the experts about this code don’t subscribe to this mailing list. You would need to ask questions about why on some acpica list. I only looked at the acpiaca code for the 1st time to try and port it. ACPI_PHYSICAL_ADDRESS is defined in actypes.h. This file supports different modes of acpica, but it is tool agnostic. So I did not change it.This file is modified by #define’s so you probably have the wrong combination? You probably have some more fundamental bug that you are hiding by changing the common code. The acpica code supports being compiled for VC++ and gcc, so it “should work”? The edk2 defines EFI_PHYSICAL_ADDERSS as: typedef UINT64 EFI_PHYSICAL_ADDRESS; ~/work/src/edk2/acpica(acpica)>git grep --untracked ACPI_PHYSICAL_ADDRESS -- *.h | grep typedef source/include/actypes.h:238:typedef UINT64 ACPI_PHYSICAL_ADDRESS; source/include/actypes.h:285:typedef UINT32 ACPI_PHYSICAL_ADDRESS; source/include/actypes.h:295:typedef UINT64 ACPI_PHYSICAL_ADDRESS; Thus you don’t really need to set things to EFI_PHYSICAL_ADDRESS, you just need to make sure acpica has the right settings. It looks like ACPI_32BIT_PHYSICAL_ADDRESS getting set will cause a 32-bit address. If EFI_PHYSICAL_ADDERSS and EFI_PHYSICAL_ADDERSS are both defined to be UINT64 I would not thing you would have an issue? > I’m running into further issues now with the ACPICA package trying to define > functions like memcpy which are intrinsic and cause a fatal error on compile What error are you getting? You should fix the thing that is throwing the error, and not modify the code. I thin the gcc side might depend on that code? The edk2 StdLib includes a memcpy, so this issue must have been solved? > . My workaround is to do #if !defined(_EDK2_EFI) guards since I searched the > package and didn’t find any uses of those functions. I added the _EDK2_EFI, I don’t think it is a good idea to work around some compiler specific issue you are hitting by adding it in new locations. > Currently I’m just combing the code for any other compile warnings/errors and > not making any functional changes outside that. I will need a lot more help > when it comes to debugging the function of this J > I’d suggest not doing a lot of warning fixes in the code. You would be better off suppressing the warnings via command line or #pragma. For example I had to turn off these warnings to get clang to compile: -Wno-unused-function -Wno-unused-const-variable. Here is an example of the edk2 suppressing a warning via #pragma https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Base.h #include <ProcessorBind.h> #if defined(_MSC_EXTENSIONS) // // Disable warning when last field of data structure is a zero sized array. // #pragma warning ( disable : 4200 ) #endif I started trying to do zero changes, but I was forced to add the _EDK2_EFI. In general if we keep the edk2 changes small and contained we have a good chance of getting the integrated into the acpiaca project. This will make it easy for the edk2 to stay in sync with the acpica project, and it will also make it easier to port new tools or features. So in general we should only touch acpica files that already contain compiler, or OS specific porting. > Also, I’m replying this back onto the new edk2-devel mailing list. > <> Thanks, Andrew Fish > -Jonathan > > From: Andrew Fish [mailto:af...@apple.com] > Sent: Thursday, July 16, 2015 4:48 PM > To: Smith, Jonathan D > Cc: Shubha Ramani; edk2-de...@lists.sourceforge.net; Zheng, Lv > Subject: Re: [edk2] efi acpidump port > > > On Jul 16, 2015, at 4:07 PM, Smith, Jonathan D <jonathan.d.sm...@intel.com > <mailto:jonathan.d.sm...@intel.com>> wrote: > > From his post: > “Sorry I don’t have time to debug it, or even run it, but maybe some one else > could take it on” > > I’m going to take this on, and try getting these changes debugged, cleaned, > and pushed up. I’ve been trying to build it on the VS2013x86 tool chain, but > I get the following warning-treated-as-error when I build: > > c:\users\jdsmith1\edk2\acpica\source\os_specific\service_layers\osefitbl.c(238) > : warning C4244: 'function' : conversion from 'ACPI_PHYSICAL_ADDRESS' to > 'ACPI_SIZE', possible loss of data > > It’s from this declaration: > > static ACPI_STATUS > OslMapTable ( > ACPI_SIZE Address, > char *Signature, > ACPI_TABLE_HEADER **Table) > { > > Is there any reason the Address parameter is ACPI_SIZE type and not > ACPI_PHYSICAL_ADDRESS ? > > > So you have to think of acpica as a complete self contained world. It is use > for the ACPI infrastructure in Linux, and OS based tools in may different > operating systems. I ran into these kind of issues and you have to pick the > right set of /D compiler options to get things defined properly. > > If you look in: acpidump.inf > [BuildOptions] > GCC: *_*_*_CC_FLAGS = -D_EDK2_EFI -DACPI_DUMP_APP > -DACPI_USE_SYSTEM_INTTYPES -Wno-unused-function -Wno-unused-const-variable > > I only set the flags for the GCC family, so you will need to add MSFT > version. I’d start with this: > MSFT:*_*_*_CC_FLAGS = /D _EDK2_EFI /D ACPI_DUMP_APP > > Basically acpica has lots of build flags to support building in different > environments. I added the _EDK2_EFI to try and pick up as much stuff as > possible from the edk2. > > Thanks, > > Andrew Fish > > PS I’ve been burned by folks not using the EFI type system before…. long on > VC++ is 4 bytes, but for GCC/clang it can be 8-bytes. That might be what you > are hitting, but hopefully turning on _EDK2_EFI will make it go away. > > > From: Shubha Ramani [mailto:shubharam...@yahoo.com > <mailto:shubharam...@yahoo.com>] > Sent: Thursday, July 16, 2015 10:44 AM > To: Smith, Jonathan D; edk2-de...@lists.sourceforge.net > <mailto:edk2-de...@lists.sourceforge.net> > Cc: Zheng, Lv; Andrew Fish > Subject: Re: [edk2] efi acpidump port > > Andrew Fish has done most of it already. Please check his last post. > > > Shubha D. Ramani > shubharam...@gmail.com <mailto:shubharam...@gmail.com> > shubharam...@yahoo.com <mailto:shubharam...@yahoo.com> > > > > On Thursday, July 16, 2015 10:33 AM, "Smith, Jonathan D" > <jonathan.d.sm...@intel.com <mailto:jonathan.d.sm...@intel.com>> wrote: > > > Hi, > > There’s been talk about making an EDK2 port of acpidump. I can try making the > port. What has been done so far, and where should I start? > > Thanks, > Jonathan Smith _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
