[PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

[PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

[Anthony PERARD]

The macro SERIAL_DXE_FILE_GUID is already been defined twice and the GUID is
been used once without defining the macro. This patch series define the macro
in MdeModulePkg where the SerialDxe is, and replace all other use by this new
one.

Note that I haven't build/test those changes, but I have test the first patch
by applying a similar change to a patch series I'm working on.

Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/ovmf.git br.serial-dxe-guid-v1

Anthony PERARD (4):
  MdeModulePkg: Add SERIAL_DXE_FILE_GUID
  ArmVirtPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID from
    MdeModulePkg
  ArmPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID from
    MdeModulePkg
  UefiPayloadPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID from
    MdeModulePkg

 MdeModulePkg/MdeModulePkg.dec                 |  3 +++
 MdeModulePkg/Include/Guid/SerialDxe.h         | 19 +++++++++++++++++++
 .../PlatformBootManagerLib/PlatformBm.c       |  6 +-----
 .../PlatformBootManagerLib/PlatformBm.c       |  6 +-----
 .../PlatformBootManagerLib/PlatformConsole.c  |  3 ++-
 5 files changed, 26 insertions(+), 11 deletions(-)
 create mode 100644 MdeModulePkg/Include/Guid/SerialDxe.h

-- 
Anthony PERARD

[PATCH 1/4] MdeModulePkg: Add SERIAL_DXE_FILE_GUID

[Anthony PERARD]

SERIAL_DXE_FILE_GUID is used in different places, create a single
define that other can use.

Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    Suggested in: Message-ID: <7d6adf5d-baca-7e9c-68ef-2f8479bbd902@redhat.com>

 MdeModulePkg/MdeModulePkg.dec         |  3 +++
 MdeModulePkg/Include/Guid/SerialDxe.h | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+)
 create mode 100644 MdeModulePkg/Include/Guid/SerialDxe.h

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 6cba729982..6c5736618e 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -396,6 +396,9 @@ [Guids]
   ## Include/Guid/S3StorageDeviceInitList.h
   gS3StorageDeviceInitListGuid = { 0x310e9b8c, 0xcf90, 0x421e, { 0x8e, 0x9b, 0x9e, 0xef, 0xb6, 0x17, 0xc8, 0xef } }
 
+  ## Include/Guid/SerialDxe.h
+  gSerialDxeFileGuid = { 0xD3987D4B, 0x971A, 0x435F, { 0x8C, 0xAF, 0x49, 0x67, 0xEB, 0x62, 0x72, 0x41 } }
+
 [Ppis]
   ## Include/Ppi/AtaController.h
   gPeiAtaControllerPpiGuid       = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a, 0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
diff --git a/MdeModulePkg/Include/Guid/SerialDxe.h b/MdeModulePkg/Include/Guid/SerialDxe.h
new file mode 100644
index 0000000000..4d53f4877f
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/SerialDxe.h
@@ -0,0 +1,19 @@
+/** @file
+  Define the SerialDxe GUID.
+
+  Copyright (c) 2019, Citrix Systems, Inc.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef __SERIAL_DXE_H__
+#define __SERIAL_DXE_H__
+
+#define SERIAL_DXE_FILE_GUID { \
+          0xD3987D4B, 0x971A, 0x435F, \
+          { 0x8C, 0xAF, 0x49, 0x67, 0xEB, 0x62, 0x72, 0x41 } \
+          }
+
+extern EFI_GUID gSerialDxeFileGuid;
+
+#endif // __SERIAL_DXE_H__
-- 
Anthony PERARD

[PATCH 2/4] ArmVirtPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID from MdeModulePkg

[Anthony PERARD]

SERIAL_DXE_FILE_GUID is now defined in MdeModulePkg, simply use it.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
index b8f50ea96b..d7697c93c3 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -26,6 +26,7 @@
 #include <Protocol/VirtioDevice.h>
 #include <Guid/EventGroup.h>
 #include <Guid/RootBridgesConnectedEventGroup.h>
+#include <Guid/SerialDxe.h>
 
 #include "PlatformBm.h"
 
@@ -41,11 +42,6 @@ typedef struct {
 } PLATFORM_SERIAL_CONSOLE;
 #pragma pack ()
 
-#define SERIAL_DXE_FILE_GUID { \
-          0xD3987D4B, 0x971A, 0x435F, \
-          { 0x8C, 0xAF, 0x49, 0x67, 0xEB, 0x62, 0x72, 0x41 } \
-          }
-
 STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
   //
   // VENDOR_DEVICE_PATH SerialDxe
-- 
Anthony PERARD

[PATCH 3/4] ArmPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID from MdeModulePkg

[Anthony PERARD]

SERIAL_DXE_FILE_GUID is now defined in MdeModulePkg, simply use it.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 0f91692c1a..668c6cb1b8 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -28,6 +28,7 @@
 #include <Protocol/PlatformBootManager.h>
 #include <Guid/EventGroup.h>
 #include <Guid/TtyTerm.h>
+#include <Guid/SerialDxe.h>
 
 #include "PlatformBm.h"
 
@@ -42,11 +43,6 @@ typedef struct {
 } PLATFORM_SERIAL_CONSOLE;
 #pragma pack ()
 
-#define SERIAL_DXE_FILE_GUID { \
-          0xD3987D4B, 0x971A, 0x435F, \
-          { 0x8C, 0xAF, 0x49, 0x67, 0xEB, 0x62, 0x72, 0x41 } \
-          }
-
 STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
   //
   // VENDOR_DEVICE_PATH SerialDxe
-- 
Anthony PERARD

[PATCH 4/4] UefiPayloadPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID from MdeModulePkg

[Anthony PERARD]

SERIAL_DXE_FILE_GUID is now defined in MdeModulePkg, simply use it.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 .../Library/PlatformBootManagerLib/PlatformConsole.c           | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
index 80a11d7451..b7c36f37d1 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
@@ -8,6 +8,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include "PlatformBootManager.h"
 #include "PlatformConsole.h"
+#include <Guid/SerialDxe.h>
 
 #define PCI_DEVICE_PATH_NODE(Func, Dev) \
   { \
@@ -53,7 +54,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
         (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8) \
       } \
     }, \
-    {0xD3987D4B, 0x971A, 0x435F, {0x8C, 0xAF, 0x49, 0x67, 0xEB, 0x62, 0x72, 0x41}} \
+    SERIAL_DXE_FILE_GUID \
   }
 
 #define gUart \
-- 
Anthony PERARD

Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

[Ard Biesheuvel]

On Wed, 29 May 2019 at 13:37, Anthony PERARD <anthony.perard@citrix.com> wrote:
>
> The macro SERIAL_DXE_FILE_GUID is already been defined twice and the GUID is
> been used once without defining the macro. This patch series define the macro
> in MdeModulePkg where the SerialDxe is, and replace all other use by this new
> one.
>
> Note that I haven't build/test those changes, but I have test the first patch
> by applying a similar change to a patch series I'm working on.
>
> Patch series available in this git branch:
> https://xenbits.xen.org/git-http/people/aperard/ovmf.git br.serial-dxe-guid-v1
>
> Anthony PERARD (4):
>   MdeModulePkg: Add SERIAL_DXE_FILE_GUID
>   ArmVirtPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID from
>     MdeModulePkg
>   ArmPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID from
>     MdeModulePkg
>   UefiPayloadPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID from
>     MdeModulePkg
>
>  MdeModulePkg/MdeModulePkg.dec                 |  3 +++
>  MdeModulePkg/Include/Guid/SerialDxe.h         | 19 +++++++++++++++++++
>  .../PlatformBootManagerLib/PlatformBm.c       |  6 +-----
>  .../PlatformBootManagerLib/PlatformBm.c       |  6 +-----
>  .../PlatformBootManagerLib/PlatformConsole.c  |  3 ++-
>  5 files changed, 26 insertions(+), 11 deletions(-)
>  create mode 100644 MdeModulePkg/Include/Guid/SerialDxe.h
>

Hello Anthony,

If the MdeModulePkg maintainers are ok with this, then I'm fine with it as well.

I would like to suggest to include a patch that does

diff --git a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
index 81066a26a278..761c2e6649de 100644
--- a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
+++ b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
@@ -11,7 +11,7 @@
   INF_VERSION                    = 0x00010005
   BASE_NAME                      = SerialDxe
   MODULE_UNI_FILE                = SerialDxe.uni
-  FILE_GUID                      = D3987D4B-971A-435F-8CAF-4967EB627241
+  FILE_GUID                      =
D3987D4B-971A-435F-8CAF-4967EB627241 # SERIAL_DXE_FILE_GUID
   MODULE_TYPE                    = DXE_DRIVER
   VERSION_STRING                 = 1.0

so that it is clear for future users that there is a symbolic GUID
constant they can use.

Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

[Zeng, Star]

I would suggest using gEdkii (for global variable) and EDKII (for definition) as the prefixes.

Thanks,
Star
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Ard Biesheuvel
> Sent: Friday, May 31, 2019 5:12 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>; Anthony PERARD
> <anthony.perard@citrix.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng,
> Star <star.zeng@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Julien Grall
> <julien.grall@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only
> once
> 
> On Wed, 29 May 2019 at 13:37, Anthony PERARD
> <anthony.perard@citrix.com> wrote:
> >
> > The macro SERIAL_DXE_FILE_GUID is already been defined twice and the
> > GUID is been used once without defining the macro. This patch series
> > define the macro in MdeModulePkg where the SerialDxe is, and replace
> > all other use by this new one.
> >
> > Note that I haven't build/test those changes, but I have test the
> > first patch by applying a similar change to a patch series I'm working on.
> >
> > Patch series available in this git branch:
> > https://xenbits.xen.org/git-http/people/aperard/ovmf.git
> > br.serial-dxe-guid-v1
> >
> > Anthony PERARD (4):
> >   MdeModulePkg: Add SERIAL_DXE_FILE_GUID
> >   ArmVirtPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID from
> >     MdeModulePkg
> >   ArmPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID from
> >     MdeModulePkg
> >   UefiPayloadPkg/PlatformBootManagerLib: Use SERIAL_DXE_FILE_GUID
> from
> >     MdeModulePkg
> >
> >  MdeModulePkg/MdeModulePkg.dec                 |  3 +++
> >  MdeModulePkg/Include/Guid/SerialDxe.h         | 19 +++++++++++++++++++
> >  .../PlatformBootManagerLib/PlatformBm.c       |  6 +-----
> >  .../PlatformBootManagerLib/PlatformBm.c       |  6 +-----
> >  .../PlatformBootManagerLib/PlatformConsole.c  |  3 ++-
> >  5 files changed, 26 insertions(+), 11 deletions(-)  create mode
> > 100644 MdeModulePkg/Include/Guid/SerialDxe.h
> >
> 
> Hello Anthony,
> 
> If the MdeModulePkg maintainers are ok with this, then I'm fine with it as
> well.
> 
> I would like to suggest to include a patch that does
> 
> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> index 81066a26a278..761c2e6649de 100644
> --- a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +++ b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> @@ -11,7 +11,7 @@
>    INF_VERSION                    = 0x00010005
>    BASE_NAME                      = SerialDxe
>    MODULE_UNI_FILE                = SerialDxe.uni
> -  FILE_GUID                      = D3987D4B-971A-435F-8CAF-4967EB627241
> +  FILE_GUID                      =
> D3987D4B-971A-435F-8CAF-4967EB627241 # SERIAL_DXE_FILE_GUID
>    MODULE_TYPE                    = DXE_DRIVER
>    VERSION_STRING                 = 1.0
> 
> so that it is clear for future users that there is a symbolic GUID constant they
> can use.
> 
> 

Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

[Michael D Kinney]

Anthony,

I am curious.  I agree that a GUID can be defined in DEC file and
in an include file that is used as a Vendor GUID in a device path.

Is there any reason that the FILE_GUID for the module needs to be
the same GUID value?  Is there any code that looks for an FFS file
with that GUID value as the FFS file name?

If not, then it would be better to not over use that GUID value.
The FILE_GUID of the SerialDxe can be different.

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io
> [mailto:devel@edk2.groups.io] On Behalf Of Anthony
> PERARD
> Sent: Wednesday, May 29, 2019 4:37 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Zeng, Star
> <star.zeng@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Julien Grall <julien.grall@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Laszlo Ersek
> <lersek@redhat.com>; Anthony PERARD
> <anthony.perard@citrix.com>
> Subject: [edk2-devel] [PATCH 0/4] Define
> SERIAL_DXE_FILE_GUID only once
> 
> The macro SERIAL_DXE_FILE_GUID is already been defined
> twice and the GUID is
> been used once without defining the macro. This patch
> series define the macro
> in MdeModulePkg where the SerialDxe is, and replace all
> other use by this new
> one.
> 
> Note that I haven't build/test those changes, but I
> have test the first patch
> by applying a similar change to a patch series I'm
> working on.
> 
> Patch series available in this git branch:
> https://xenbits.xen.org/git-
> http/people/aperard/ovmf.git br.serial-dxe-guid-v1
> 
> Anthony PERARD (4):
>   MdeModulePkg: Add SERIAL_DXE_FILE_GUID
>   ArmVirtPkg/PlatformBootManagerLib: Use
> SERIAL_DXE_FILE_GUID from
>     MdeModulePkg
>   ArmPkg/PlatformBootManagerLib: Use
> SERIAL_DXE_FILE_GUID from
>     MdeModulePkg
>   UefiPayloadPkg/PlatformBootManagerLib: Use
> SERIAL_DXE_FILE_GUID from
>     MdeModulePkg
> 
>  MdeModulePkg/MdeModulePkg.dec                 |  3 +++
>  MdeModulePkg/Include/Guid/SerialDxe.h         | 19
> +++++++++++++++++++
>  .../PlatformBootManagerLib/PlatformBm.c       |  6 +--
> ---
>  .../PlatformBootManagerLib/PlatformBm.c       |  6 +--
> ---
>  .../PlatformBootManagerLib/PlatformConsole.c  |  3 ++-
>  5 files changed, 26 insertions(+), 11 deletions(-)
>  create mode 100644
> MdeModulePkg/Include/Guid/SerialDxe.h
> 
> --
> Anthony PERARD
> 
> 
> 

Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

[Anthony PERARD]

On Sat, Jun 01, 2019 at 05:00:37PM +0000, Kinney, Michael D wrote:
> I am curious.  I agree that a GUID can be defined in DEC file and
> in an include file that is used as a Vendor GUID in a device path.
> 
> Is there any reason that the FILE_GUID for the module needs to be
> the same GUID value?  Is there any code that looks for an FFS file
> with that GUID value as the FFS file name?

Sorry, I think you lost me here. The way I see it is that the GUID is
"produced" only by "MdeModulePkg/Universal/SerialDxe/SerialDxe.inf",
and then all the "PlatformBootManagerLib" are looking for exactly that
driver (SerialDxe), so the FILE_GUID is used.

There is probably a reason why this is already done three times in the
edk2.git repo.

Maybe you could have a closer look at
"ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c" to see how
SERIAL_DXE_FILE_GUID is used.

BTW, I've added "gSerialDxeFileGuid" as it seems there's often both a
"#define" and a variable for the GUID, but only the "#define" is used so
far. So maybe I should remove the variable?

> If not, then it would be better to not over use that GUID value.
> The FILE_GUID of the SerialDxe can be different.

Thanks for the review,

-- 
Anthony PERARD

Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

[Laszlo Ersek]

On 06/01/19 19:00, Kinney, Michael D wrote:
> Anthony,
> 
> I am curious.  I agree that a GUID can be defined in DEC file and
> in an include file that is used as a Vendor GUID in a device path.
> 
> Is there any reason that the FILE_GUID for the module needs to be
> the same GUID value?  Is there any code that looks for an FFS file
> with that GUID value as the FFS file name?

It's the other way around. "MdeModulePkg/Universal/SerialDxe" uses its own FILE_GUID for populating a device path node on a handle that it creates. Please see in "SerialIo.c":

SERIAL_DEVICE_PATH mSerialDevicePath = {
  {
    { HARDWARE_DEVICE_PATH, HW_VENDOR_DP, { sizeof (VENDOR_DEVICE_PATH), 0} },
    EFI_CALLER_ID_GUID  // Use the driver's GUID
  },

In turn, PlatformBootManagerLib instances have to refer to this device path (including said FILE_GUID) for setting up their console UEFI variables.

> If not, then it would be better to not over use that GUID value.
> The FILE_GUID of the SerialDxe can be different.

This would be a valid change, but for that, the device path protocol installed by SerialDxe should be decoupled from SerialDxe's own FILE_GUID.

Dependent on platform circumstances, that could be a compatibility issue, because the old ConIn / ConOut / ErrOut variables could stop working (still referencing the original FILE_GUID in the devpath).

Thanks!
Laszlo

> 
> Thanks,
> 
> Mike
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io
>> [mailto:devel@edk2.groups.io] On Behalf Of Anthony
>> PERARD
>> Sent: Wednesday, May 29, 2019 4:37 AM
>> To: devel@edk2.groups.io
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray
>> <ray.ni@intel.com>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>; Zeng, Star
>> <star.zeng@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
>> Julien Grall <julien.grall@arm.com>; Leif Lindholm
>> <leif.lindholm@linaro.org>; Laszlo Ersek
>> <lersek@redhat.com>; Anthony PERARD
>> <anthony.perard@citrix.com>
>> Subject: [edk2-devel] [PATCH 0/4] Define
>> SERIAL_DXE_FILE_GUID only once
>>
>> The macro SERIAL_DXE_FILE_GUID is already been defined
>> twice and the GUID is
>> been used once without defining the macro. This patch
>> series define the macro
>> in MdeModulePkg where the SerialDxe is, and replace all
>> other use by this new
>> one.
>>
>> Note that I haven't build/test those changes, but I
>> have test the first patch
>> by applying a similar change to a patch series I'm
>> working on.
>>
>> Patch series available in this git branch:
>> https://xenbits.xen.org/git-
>> http/people/aperard/ovmf.git br.serial-dxe-guid-v1
>>
>> Anthony PERARD (4):
>>   MdeModulePkg: Add SERIAL_DXE_FILE_GUID
>>   ArmVirtPkg/PlatformBootManagerLib: Use
>> SERIAL_DXE_FILE_GUID from
>>     MdeModulePkg
>>   ArmPkg/PlatformBootManagerLib: Use
>> SERIAL_DXE_FILE_GUID from
>>     MdeModulePkg
>>   UefiPayloadPkg/PlatformBootManagerLib: Use
>> SERIAL_DXE_FILE_GUID from
>>     MdeModulePkg
>>
>>  MdeModulePkg/MdeModulePkg.dec                 |  3 +++
>>  MdeModulePkg/Include/Guid/SerialDxe.h         | 19
>> +++++++++++++++++++
>>  .../PlatformBootManagerLib/PlatformBm.c       |  6 +--
>> ---
>>  .../PlatformBootManagerLib/PlatformBm.c       |  6 +--
>> ---
>>  .../PlatformBootManagerLib/PlatformConsole.c  |  3 ++-
>>  5 files changed, 26 insertions(+), 11 deletions(-)
>>  create mode 100644
>> MdeModulePkg/Include/Guid/SerialDxe.h
>>
>> --
>> Anthony PERARD
>>
>>
>> 
> 

Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

[Michael D Kinney]

Laszlo,

We can keep compatibility.

The value of the GUID in the DEC/Include files can be the
current SerialDxe FFS FILE_GUID.  Then the device path can
use the GUID defined in the DEC/Include file and it will
have the same value as today.

In the future, the SerialDxe FILE_GUID can be changed as
needed with no impacts.

There is actually a build feature in the DSC file to
override the FILE_GUID value of a single module in the
module scoped <Defines> section.  If these feature was
ever used with SerialDxe, then the generated device path
would be wrong if EFI_CALLER_ID_GUID is used.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io
> [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
> Sent: Monday, June 3, 2019 5:02 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io; anthony.perard@citrix.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Zeng, Star
> <star.zeng@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Julien Grall <julien.grall@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>
> Subject: Re: [edk2-devel] [PATCH 0/4] Define
> SERIAL_DXE_FILE_GUID only once
> 
> On 06/01/19 19:00, Kinney, Michael D wrote:
> > Anthony,
> >
> > I am curious.  I agree that a GUID can be defined in
> DEC file and
> > in an include file that is used as a Vendor GUID in a
> device path.
> >
> > Is there any reason that the FILE_GUID for the module
> needs to be
> > the same GUID value?  Is there any code that looks
> for an FFS file
> > with that GUID value as the FFS file name?
> 
> It's the other way around.
> "MdeModulePkg/Universal/SerialDxe" uses its own
> FILE_GUID for populating a device path node on a handle
> that it creates. Please see in "SerialIo.c":
> 
> SERIAL_DEVICE_PATH mSerialDevicePath = {
>   {
>     { HARDWARE_DEVICE_PATH, HW_VENDOR_DP, { sizeof
> (VENDOR_DEVICE_PATH), 0} },
>     EFI_CALLER_ID_GUID  // Use the driver's GUID
>   },
> 
> In turn, PlatformBootManagerLib instances have to refer
> to this device path (including said FILE_GUID) for
> setting up their console UEFI variables.
> 
> > If not, then it would be better to not over use that
> GUID value.
> > The FILE_GUID of the SerialDxe can be different.
> 
> This would be a valid change, but for that, the device
> path protocol installed by SerialDxe should be
> decoupled from SerialDxe's own FILE_GUID.
> 
> Dependent on platform circumstances, that could be a
> compatibility issue, because the old ConIn / ConOut /
> ErrOut variables could stop working (still referencing
> the original FILE_GUID in the devpath).
> 
> Thanks!
> Laszlo
> 
> >
> > Thanks,
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io
> >> [mailto:devel@edk2.groups.io] On Behalf Of Anthony
> >> PERARD
> >> Sent: Wednesday, May 29, 2019 4:37 AM
> >> To: devel@edk2.groups.io
> >> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray
> >> <ray.ni@intel.com>; Ard Biesheuvel
> >> <ard.biesheuvel@linaro.org>; Zeng, Star
> >> <star.zeng@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>;
> >> Julien Grall <julien.grall@arm.com>; Leif Lindholm
> >> <leif.lindholm@linaro.org>; Laszlo Ersek
> >> <lersek@redhat.com>; Anthony PERARD
> >> <anthony.perard@citrix.com>
> >> Subject: [edk2-devel] [PATCH 0/4] Define
> >> SERIAL_DXE_FILE_GUID only once
> >>
> >> The macro SERIAL_DXE_FILE_GUID is already been
> defined
> >> twice and the GUID is
> >> been used once without defining the macro. This
> patch
> >> series define the macro
> >> in MdeModulePkg where the SerialDxe is, and replace
> all
> >> other use by this new
> >> one.
> >>
> >> Note that I haven't build/test those changes, but I
> >> have test the first patch
> >> by applying a similar change to a patch series I'm
> >> working on.
> >>
> >> Patch series available in this git branch:
> >> https://xenbits.xen.org/git-
> >> http/people/aperard/ovmf.git br.serial-dxe-guid-v1
> >>
> >> Anthony PERARD (4):
> >>   MdeModulePkg: Add SERIAL_DXE_FILE_GUID
> >>   ArmVirtPkg/PlatformBootManagerLib: Use
> >> SERIAL_DXE_FILE_GUID from
> >>     MdeModulePkg
> >>   ArmPkg/PlatformBootManagerLib: Use
> >> SERIAL_DXE_FILE_GUID from
> >>     MdeModulePkg
> >>   UefiPayloadPkg/PlatformBootManagerLib: Use
> >> SERIAL_DXE_FILE_GUID from
> >>     MdeModulePkg
> >>
> >>  MdeModulePkg/MdeModulePkg.dec                 |  3
> +++
> >>  MdeModulePkg/Include/Guid/SerialDxe.h         | 19
> >> +++++++++++++++++++
> >>  .../PlatformBootManagerLib/PlatformBm.c       |  6
> +--
> >> ---
> >>  .../PlatformBootManagerLib/PlatformBm.c       |  6
> +--
> >> ---
> >>  .../PlatformBootManagerLib/PlatformConsole.c  |  3
> ++-
> >>  5 files changed, 26 insertions(+), 11 deletions(-)
> >>  create mode 100644
> >> MdeModulePkg/Include/Guid/SerialDxe.h
> >>
> >> --
> >> Anthony PERARD
> >>
> >>
> >>
> >
> 
> 
> 

Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

[Laszlo Ersek]

On 06/03/19 16:50, Kinney, Michael D wrote:
> Laszlo,
> 
> We can keep compatibility.
> 
> The value of the GUID in the DEC/Include files can be the
> current SerialDxe FFS FILE_GUID.  Then the device path can
> use the GUID defined in the DEC/Include file and it will
> have the same value as today.
> 
> In the future, the SerialDxe FILE_GUID can be changed as
> needed with no impacts.
> 
> There is actually a build feature in the DSC file to
> override the FILE_GUID value of a single module in the
> module scoped <Defines> section.  If these feature was
> ever used with SerialDxe, then the generated device path
> would be wrong if EFI_CALLER_ID_GUID is used.

Right.

So, ultimately, we are asking Anthony to:

(1) please introduce the new GUID as EDKII_SERIAL_VENDOR_GUID and
gEdkiiSerialVendorGuid,

(2) please append another patch to this series, replacing
EFI_CALLER_ID_GUID -- and the comment! -- with EDKII_SERIAL_VENDOR_GUID,
in the "mSerialDevicePath" initializer (in
"MdeModulePkg/Universal/SerialDxe/SerialIo.c").

Correct?

Can we suggest an include file name too, in place of
"MdeModulePkg/Include/Guid/SerialDxe.h"?

Thanks!
Laszlo


>> -----Original Message-----
>> From: devel@edk2.groups.io
>> [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
>> Sent: Monday, June 3, 2019 5:02 AM
>> To: Kinney, Michael D <michael.d.kinney@intel.com>;
>> devel@edk2.groups.io; anthony.perard@citrix.com
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray
>> <ray.ni@intel.com>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>; Zeng, Star
>> <star.zeng@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
>> Julien Grall <julien.grall@arm.com>; Leif Lindholm
>> <leif.lindholm@linaro.org>
>> Subject: Re: [edk2-devel] [PATCH 0/4] Define
>> SERIAL_DXE_FILE_GUID only once
>>
>> On 06/01/19 19:00, Kinney, Michael D wrote:
>>> Anthony,
>>>
>>> I am curious.  I agree that a GUID can be defined in
>> DEC file and
>>> in an include file that is used as a Vendor GUID in a
>> device path.
>>>
>>> Is there any reason that the FILE_GUID for the module
>> needs to be
>>> the same GUID value?  Is there any code that looks
>> for an FFS file
>>> with that GUID value as the FFS file name?
>>
>> It's the other way around.
>> "MdeModulePkg/Universal/SerialDxe" uses its own
>> FILE_GUID for populating a device path node on a handle
>> that it creates. Please see in "SerialIo.c":
>>
>> SERIAL_DEVICE_PATH mSerialDevicePath = {
>>   {
>>     { HARDWARE_DEVICE_PATH, HW_VENDOR_DP, { sizeof
>> (VENDOR_DEVICE_PATH), 0} },
>>     EFI_CALLER_ID_GUID  // Use the driver's GUID
>>   },
>>
>> In turn, PlatformBootManagerLib instances have to refer
>> to this device path (including said FILE_GUID) for
>> setting up their console UEFI variables.
>>
>>> If not, then it would be better to not over use that
>> GUID value.
>>> The FILE_GUID of the SerialDxe can be different.
>>
>> This would be a valid change, but for that, the device
>> path protocol installed by SerialDxe should be
>> decoupled from SerialDxe's own FILE_GUID.
>>
>> Dependent on platform circumstances, that could be a
>> compatibility issue, because the old ConIn / ConOut /
>> ErrOut variables could stop working (still referencing
>> the original FILE_GUID in the devpath).
>>
>> Thanks!
>> Laszlo
>>
>>>
>>> Thanks,
>>>
>>> Mike
>>>
>>>> -----Original Message-----
>>>> From: devel@edk2.groups.io
>>>> [mailto:devel@edk2.groups.io] On Behalf Of Anthony
>>>> PERARD
>>>> Sent: Wednesday, May 29, 2019 4:37 AM
>>>> To: devel@edk2.groups.io
>>>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray
>>>> <ray.ni@intel.com>; Ard Biesheuvel
>>>> <ard.biesheuvel@linaro.org>; Zeng, Star
>>>> <star.zeng@intel.com>; Wu, Hao A
>> <hao.a.wu@intel.com>;
>>>> Julien Grall <julien.grall@arm.com>; Leif Lindholm
>>>> <leif.lindholm@linaro.org>; Laszlo Ersek
>>>> <lersek@redhat.com>; Anthony PERARD
>>>> <anthony.perard@citrix.com>
>>>> Subject: [edk2-devel] [PATCH 0/4] Define
>>>> SERIAL_DXE_FILE_GUID only once
>>>>
>>>> The macro SERIAL_DXE_FILE_GUID is already been
>> defined
>>>> twice and the GUID is
>>>> been used once without defining the macro. This
>> patch
>>>> series define the macro
>>>> in MdeModulePkg where the SerialDxe is, and replace
>> all
>>>> other use by this new
>>>> one.
>>>>
>>>> Note that I haven't build/test those changes, but I
>>>> have test the first patch
>>>> by applying a similar change to a patch series I'm
>>>> working on.
>>>>
>>>> Patch series available in this git branch:
>>>> https://xenbits.xen.org/git-
>>>> http/people/aperard/ovmf.git br.serial-dxe-guid-v1
>>>>
>>>> Anthony PERARD (4):
>>>>   MdeModulePkg: Add SERIAL_DXE_FILE_GUID
>>>>   ArmVirtPkg/PlatformBootManagerLib: Use
>>>> SERIAL_DXE_FILE_GUID from
>>>>     MdeModulePkg
>>>>   ArmPkg/PlatformBootManagerLib: Use
>>>> SERIAL_DXE_FILE_GUID from
>>>>     MdeModulePkg
>>>>   UefiPayloadPkg/PlatformBootManagerLib: Use
>>>> SERIAL_DXE_FILE_GUID from
>>>>     MdeModulePkg
>>>>
>>>>  MdeModulePkg/MdeModulePkg.dec                 |  3
>> +++
>>>>  MdeModulePkg/Include/Guid/SerialDxe.h         | 19
>>>> +++++++++++++++++++
>>>>  .../PlatformBootManagerLib/PlatformBm.c       |  6
>> +--
>>>> ---
>>>>  .../PlatformBootManagerLib/PlatformBm.c       |  6
>> +--
>>>> ---
>>>>  .../PlatformBootManagerLib/PlatformConsole.c  |  3
>> ++-
>>>>  5 files changed, 26 insertions(+), 11 deletions(-)
>>>>  create mode 100644
>>>> MdeModulePkg/Include/Guid/SerialDxe.h
>>>>
>>>> --
>>>> Anthony PERARD
>>>>
>>>>
>>>>
>>>
>>
>>
>> 
> 

Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

[Michael D Kinney]

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io]
> On Behalf Of Laszlo Ersek
> Sent: Monday, June 3, 2019 9:32 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io; anthony.perard@citrix.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Zeng, Star
> <star.zeng@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Julien Grall <julien.grall@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>
> Subject: Re: [edk2-devel] [PATCH 0/4] Define
> SERIAL_DXE_FILE_GUID only once
> 
> On 06/03/19 16:50, Kinney, Michael D wrote:
> > Laszlo,
> >
> > We can keep compatibility.
> >
> > The value of the GUID in the DEC/Include files can be
> the current
> > SerialDxe FFS FILE_GUID.  Then the device path can use
> the GUID
> > defined in the DEC/Include file and it will have the
> same value as
> > today.
> >
> > In the future, the SerialDxe FILE_GUID can be changed
> as needed with
> > no impacts.
> >
> > There is actually a build feature in the DSC file to
> override the
> > FILE_GUID value of a single module in the module
> scoped <Defines>
> > section.  If these feature was ever used with
> SerialDxe, then the
> > generated device path would be wrong if
> EFI_CALLER_ID_GUID is used.
> 
> Right.
> 
> So, ultimately, we are asking Anthony to:
> 
> (1) please introduce the new GUID as
> EDKII_SERIAL_VENDOR_GUID and gEdkiiSerialVendorGuid,

Yes.

> 
> (2) please append another patch to this series,
> replacing EFI_CALLER_ID_GUID -- and the comment! -- with
> EDKII_SERIAL_VENDOR_GUID, in the "mSerialDevicePath"
> initializer (in
> "MdeModulePkg/Universal/SerialDxe/SerialIo.c").
> 
> Correct?

Yes.

> 
> Can we suggest an include file name too, in place of
> "MdeModulePkg/Include/Guid/SerialDxe.h"?

The SerialDxe module uses the services of the SeriaPortLib to
produce the Serial I/O Protocol.  Instead of a physical register
interface such as UART in HW, the SW APIs of the SerialPortLib are
used.  The GUIDed Vendor HW node in the device path for the HW
register case would be a GUID value and name(s) that represents
the HW device used to perform the Serial I/O actions.  Applying
this to the SerialPortLib  access would imply GUID and Include
file names that are associated with the SerialPortLib. Perhaps:

  MdeModulePkg/Include/Guid/SerialPortLibVendor.h

  EDKII_SERIAL_PORT_LIB_VENDOR_GUID

  gEdkiiSerialPortLibVendorGuid

> 
> Thanks!
> Laszlo
> 
> 
> >> -----Original Message-----
> >> From: devel@edk2.groups.io
> >> [mailto:devel@edk2.groups.io] On Behalf Of Laszlo
> Ersek
> >> Sent: Monday, June 3, 2019 5:02 AM
> >> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> >> devel@edk2.groups.io; anthony.perard@citrix.com
> >> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray
> <ray.ni@intel.com>;
> >> Ard Biesheuvel <ard.biesheuvel@linaro.org>; Zeng,
> Star
> >> <star.zeng@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Julien Grall
> >> <julien.grall@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>
> >> Subject: Re: [edk2-devel] [PATCH 0/4] Define
> SERIAL_DXE_FILE_GUID
> >> only once
> >>
> >> On 06/01/19 19:00, Kinney, Michael D wrote:
> >>> Anthony,
> >>>
> >>> I am curious.  I agree that a GUID can be defined in
> >> DEC file and
> >>> in an include file that is used as a Vendor GUID in
> a
> >> device path.
> >>>
> >>> Is there any reason that the FILE_GUID for the
> module
> >> needs to be
> >>> the same GUID value?  Is there any code that looks
> >> for an FFS file
> >>> with that GUID value as the FFS file name?
> >>
> >> It's the other way around.
> >> "MdeModulePkg/Universal/SerialDxe" uses its own
> FILE_GUID for
> >> populating a device path node on a handle that it
> creates. Please see
> >> in "SerialIo.c":
> >>
> >> SERIAL_DEVICE_PATH mSerialDevicePath = {
> >>   {
> >>     { HARDWARE_DEVICE_PATH, HW_VENDOR_DP, { sizeof
> >> (VENDOR_DEVICE_PATH), 0} },
> >>     EFI_CALLER_ID_GUID  // Use the driver's GUID
> >>   },
> >>
> >> In turn, PlatformBootManagerLib instances have to
> refer to this
> >> device path (including said FILE_GUID) for setting up
> their console
> >> UEFI variables.
> >>
> >>> If not, then it would be better to not over use that
> >> GUID value.
> >>> The FILE_GUID of the SerialDxe can be different.
> >>
> >> This would be a valid change, but for that, the
> device path protocol
> >> installed by SerialDxe should be decoupled from
> SerialDxe's own
> >> FILE_GUID.
> >>
> >> Dependent on platform circumstances, that could be a
> compatibility
> >> issue, because the old ConIn / ConOut / ErrOut
> variables could stop
> >> working (still referencing the original FILE_GUID in
> the devpath).
> >>
> >> Thanks!
> >> Laszlo
> >>
> >>>
> >>> Thanks,
> >>>
> >>> Mike
> >>>
> >>>> -----Original Message-----
> >>>> From: devel@edk2.groups.io
> >>>> [mailto:devel@edk2.groups.io] On Behalf Of Anthony
> PERARD
> >>>> Sent: Wednesday, May 29, 2019 4:37 AM
> >>>> To: devel@edk2.groups.io
> >>>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray
> >>>> <ray.ni@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>;
> >>>> Zeng, Star <star.zeng@intel.com>; Wu, Hao A
> >> <hao.a.wu@intel.com>;
> >>>> Julien Grall <julien.grall@arm.com>; Leif Lindholm
> >>>> <leif.lindholm@linaro.org>; Laszlo Ersek
> <lersek@redhat.com>;
> >>>> Anthony PERARD <anthony.perard@citrix.com>
> >>>> Subject: [edk2-devel] [PATCH 0/4] Define
> SERIAL_DXE_FILE_GUID only
> >>>> once
> >>>>
> >>>> The macro SERIAL_DXE_FILE_GUID is already been
> >> defined
> >>>> twice and the GUID is
> >>>> been used once without defining the macro. This
> >> patch
> >>>> series define the macro
> >>>> in MdeModulePkg where the SerialDxe is, and replace
> >> all
> >>>> other use by this new
> >>>> one.
> >>>>
> >>>> Note that I haven't build/test those changes, but I
> have test the
> >>>> first patch by applying a similar change to a patch
> series I'm
> >>>> working on.
> >>>>
> >>>> Patch series available in this git branch:
> >>>> https://xenbits.xen.org/git-
> >>>> http/people/aperard/ovmf.git br.serial-dxe-guid-v1
> >>>>
> >>>> Anthony PERARD (4):
> >>>>   MdeModulePkg: Add SERIAL_DXE_FILE_GUID
> >>>>   ArmVirtPkg/PlatformBootManagerLib: Use
> SERIAL_DXE_FILE_GUID from
> >>>>     MdeModulePkg
> >>>>   ArmPkg/PlatformBootManagerLib: Use
> SERIAL_DXE_FILE_GUID from
> >>>>     MdeModulePkg
> >>>>   UefiPayloadPkg/PlatformBootManagerLib: Use
> SERIAL_DXE_FILE_GUID
> >>>> from
> >>>>     MdeModulePkg
> >>>>
> >>>>  MdeModulePkg/MdeModulePkg.dec                 |  3
> >> +++
> >>>>  MdeModulePkg/Include/Guid/SerialDxe.h         | 19
> >>>> +++++++++++++++++++
> >>>>  .../PlatformBootManagerLib/PlatformBm.c       |  6
> >> +--
> >>>> ---
> >>>>  .../PlatformBootManagerLib/PlatformBm.c       |  6
> >> +--
> >>>> ---
> >>>>  .../PlatformBootManagerLib/PlatformConsole.c  |  3
> >> ++-
> >>>>  5 files changed, 26 insertions(+), 11 deletions(-)
> create mode
> >>>> 100644 MdeModulePkg/Include/Guid/SerialDxe.h
> >>>>
> >>>> --
> >>>> Anthony PERARD
> >>>>
> >>>>
> >>>>
> >>>
> >>
> >>
> >>
> >
> 
> 
> 

Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

[Anthony PERARD]

> > So, ultimately, we are asking Anthony to:
> > 
> > (1) please introduce the new GUID as
> > EDKII_SERIAL_VENDOR_GUID and gEdkiiSerialVendorGuid,
> 
> Yes.
> 
> > 
> > (2) please append another patch to this series,
> > replacing EFI_CALLER_ID_GUID -- and the comment! -- with
> > EDKII_SERIAL_VENDOR_GUID, in the "mSerialDevicePath"
> > initializer (in
> > "MdeModulePkg/Universal/SerialDxe/SerialIo.c").
> > 
> > Correct?
> 
> Yes.
> 
> > 
> > Can we suggest an include file name too, in place of
> > "MdeModulePkg/Include/Guid/SerialDxe.h"?
> 
> The SerialDxe module uses the services of the SeriaPortLib to
> produce the Serial I/O Protocol.  Instead of a physical register
> interface such as UART in HW, the SW APIs of the SerialPortLib are
> used.  The GUIDed Vendor HW node in the device path for the HW
> register case would be a GUID value and name(s) that represents
> the HW device used to perform the Serial I/O actions.  Applying
> this to the SerialPortLib  access would imply GUID and Include
> file names that are associated with the SerialPortLib. Perhaps:
> 
>   MdeModulePkg/Include/Guid/SerialPortLibVendor.h
> 
>   EDKII_SERIAL_PORT_LIB_VENDOR_GUID
> 
>   gEdkiiSerialPortLibVendorGuid

Thank you all, I'll update the patches.

-- 
Anthony PERARD