From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x230.google.com (mail-it0-x230.google.com [IPv6:2607:f8b0:4001:c0b::230]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 672AC21DFA7B8 for ; Mon, 27 Mar 2017 00:00:54 -0700 (PDT) Received: by mail-it0-x230.google.com with SMTP id 190so42531107itm.0 for ; Mon, 27 Mar 2017 00:00:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=1OtymE8KMP7gtEfwNu2bsIMSnEFvLgI0FrOIcm+I8II=; b=dwdhw+llVyFF8c92djUxQ9Eiz316TWlcKFm3XlsaX9Nxf4fnwouv1g90tRVxrU4itv dYd+K+yFmkx77KfH3kaLeagmUHJEr44qzK8kOyOh0OIXPXvVwkxMPPuG2JOYs+53zC0+ zMFq+j6RjO6PaDdPYPNQS8jObsYOTFS+EfESI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=1OtymE8KMP7gtEfwNu2bsIMSnEFvLgI0FrOIcm+I8II=; b=pmZOz0fTNhc4GpNAu1EeGYBHN1TOlJri5jxIjMSA0ExXlVZp+W1+0P/Hg0Hi/UBmH5 GXUnvkPgD9oBjXgK8IPh//g2ScqmEdyQcMidmMmOYFN2SAH0w92sk9HmYjSvNBMHrM5g aY4/Ni8Lo4ahs2zex4hRBbFhy8TFaD19kqTfq86e/wjY6F6g4B4wzSy0cfdAFk16Oofp +TjOVUDYZC94GzwJpzp1yx5iJyr/Jt4IyMsfVogchohK/sbXsJVQAbzEFvOY4gvh/elK Z/EQHjGNAj8Le+ev1K/aOn96Bewh8DTNYleVgL1re0XdpayPUeYJqbJwOzz3demNku36 g4DQ== X-Gm-Message-State: AFeK/H1xC0Lc0ofotNweA4opL724J43FsrSD4F0rtkCsBgxmKRpJV1X3ZA8PMRnHAKXClU+gLuDfcmimeAlS/MYx X-Received: by 10.36.89.209 with SMTP id p200mr7849122itb.59.1490598053596; Mon, 27 Mar 2017 00:00:53 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.10.27 with HTTP; Mon, 27 Mar 2017 00:00:52 -0700 (PDT) In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14D702AE8@shsmsx102.ccr.corp.intel.com> References: <20170317204731.31488-1-lersek@redhat.com> <20170317204731.31488-6-lersek@redhat.com> <20170318150041.GL16034@bivouac.eciton.net> <1bc7b29c-7a71-86cb-adce-5a14de129c63@redhat.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B8377AB@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B84CA03@shsmsx102.ccr.corp.intel.com> <5b2c0152-85fe-db6b-e173-0e4b0891e0f2@redhat.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14D702AE8@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Mon, 27 Mar 2017 08:00:52 +0100 Message-ID: To: "Gao, Liming" Cc: Laszlo Ersek , "Zeng, Star" , "Kinney, Michael D" , "afish@apple.com" , "Tian, Feng" , edk2-devel-01 , Leif Lindholm Subject: Re: [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Mar 2017 07:00:54 -0000 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 27 March 2017 at 03:42, Gao, Liming wrote: > Laszlo: > On GUID header file, some header file are here, because they are added = before BaseTools supports it. Now, we allow not to add header file if the h= eader file only has GUID value definition. I have to agree with Laszlo here. The BaseTools support is incomplete, since it does not add a #define for the GUID to AutoGen.h. This makes it impossible to initialize static structures containing GUIDs, such as templates containing vendor device paths. For instance, the following typedef struct { EFI_GUID foo; } TYPE; TYPE mFoo { SOME_GUID }; is not possible without a Guid/xxx.h include file containing a #define for SOME_GUID. > On GUID C MACRO, we suggest to use GuidCName in every C source code. So= , we don't suggest to add it. As you know, some existing header files have= GuidCName and GUID Macro. Now, we have no plan to clean up the existing on= e. But, we expect new added file follows this rule. > > Thanks > Liming >>-----Original Message----- >>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of >>Laszlo Ersek >>Sent: Saturday, March 25, 2017 1:09 AM >>To: Zeng, Star ; Ard Biesheuvel >>; Kinney, Michael D >>; afish@apple.com >>Cc: Tian, Feng ; Gao, Liming ; >>edk2-devel-01 ; Leif Lindholm >> >>Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has >>ACPI Protocol, and plug-in library >> >>On 03/24/17 04:44, Zeng, Star wrote: >>> I just think it seems a generic problem. >>> Some platforms may dynamically determine whether ACPI should be >>supported or not. >>> Some platforms may dynamically determine whether SMBIOS should be >>supported or not. >>> Some platforms may dynamically determine whether HII should be >>supported or not. >>> ... >>> >>> We are thinking whether we can have a generic NULL instance to support = all >>this kind of cases, for example: >>> 1. Define a FixedAtBuild PCD PcdDepexGuid that will be a VOID* type PCD >>point to a depex GUID. >>> 2. Implement a NULL instance DepexLib. >>> [Defines] >>> INF_VERSION =3D 0x00010005 >>> BASE_NAME =3D DepexLib >>> FILE_GUID =3D XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXX= XX >>> MODULE_TYPE =3D BASE >>> VERSION_STRING =3D 1.0 >>> LIBRARY_CLASS =3D NULL >>> >>> [Sources] >>> DepexLib.c >>> >>> [Packages] >>> XXXPkg/XXXPkg.dec >>> >>> [Depex] >>> PcdDepexGuid >>> >>> 3. Link NULL instance and configure PCD in dsc. >>> MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf { >>> >>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf >>> >>> PcdDepexGuid|gEdkiiPlatformHasAcpiGuid >>> } >>> MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf { >>> >>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf >>> >>> PcdDepexGuid|gEdkiiPlatformHasSmbiosGuid >>> } >>> >>> But current BaseTools does not support the syntax to declare PCD in [De= pex] >>section of inf yet. >>> >>> Based on the statements above, I have below comments. >> >>> 1. I agree to add the GUID into MdeModulePkg, but how about using >>> gEdkiiPlatformHasAcpiGuid instead of >>> gEdkiiPlatformHasAcpiProtocolGuid and add the GUID into [Guids] >>> section of MdeModulePkg.dec? As Platform can install >>> gEdkiiPlatformHasAcpiGuid PPI to control PEIM and install >>> gEdkiiPlatformHasAcpiGuid PROTOCOL to control DRIVER. >> >>Sounds reasonable, I'll do this. >> >>> And another, no >>> need to add a include file PlatformHasAcpi.h as BaseTools supports to >>> add the GUID definitions to AutoGen files from package dec. >> >>I disagree with this. I mean, I *personally* wouldn't mind, but this >>would be inconsistent with existing MdeModulePkg practice. For example, >>see >> >>MdeModulePkg/Include/Guid/TtyTerm.h >> >>Similarly to the current case, that GUID ("gEfiTtyTermGuid") has no data >>structures associated with it; the header file only has an extern >>declaration, and -- importantly -- an array initializer macro called >>EFI_TTY_TERM_GUID, which can be used in the following syntax: >> >>STATIC CONST EFI_GUID mThisIsMyGuid =3D EFI_TTY_TERM_GUID; >> >>So, I think that the GUID header file is required. I will add it in v3. >> >>Once we generalize this idea to the design that you laid out above, we >>can perhaps eliminate the header file. But, for now, I think we should >>remain consistent with other MdeModulePkg GUID definitions. >> >>> 2. You can file bugzilla bug to request BaseTools syntax support to >>> declare PCD in [Depex] section of inf. Then PcdDepexGuid and DepexLib >>> could be finally added into core package MdeModulePkg or even MdePkg >>> (I prefer). >> >>I filed: >> >>https://bugzilla.tianocore.org/show_bug.cgi?id=3D443 -- for BaseTools, >>https://bugzilla.tianocore.org/show_bug.cgi?id=3D444 -- for the INF spec >> >>> Before that, how about implementing the >>> PlatformHasAcpiLib in none core package? >> >>Works for me; I'll split that part off and keep it under ArmPkg. >> >>Thanks >>Laszlo >> >>> >>> >>> Thanks, >>> Star >>> -----Original Message----- >>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of >>Ard Biesheuvel >>> Sent: Thursday, March 23, 2017 5:09 PM >>> To: Zeng, Star ; Kinney, Michael D >>; afish@apple.com >>> Cc: Tian, Feng ; Laszlo Ersek ; >>edk2-devel-01 ; Leif Lindholm >> >>> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform H= as >>ACPI Protocol, and plug-in library >>> >>> On 23 March 2017 at 01:41, Zeng, Star wrote: >>>> I prefer to do not include the protocol definition and the library ins= tance >>into MdeModulePkg at this moment until they need to be used by multiple >>platforms/archs. >>>> >>> >>> I disagree. Nowhere in the PI or UEFI spec is there any mention (for an= y >>architecture) that ACPI is mandatory. Given that EDK2 is the reference >>platform UEFI/PI, and not for ACPI or the PC/x86 platform, I think it is >>unreasonable to have lots and lots of PC quirks (i.e., allocations below = 4 GB) in >>MdeModulePkg, but then disallow a clean approach to make ACPI selectable, >>in a way that is true to the spirit of PI (i.e., using protocol dependenc= ies) >>> >>> So I think at least the protocol definitions belong in MdeModulePkg. >>> >>> Thanks, >>> Ard. >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel >>> >> >>_______________________________________________ >>edk2-devel mailing list >>edk2-devel@lists.01.org >>https://lists.01.org/mailman/listinfo/edk2-devel