Re: [PATCH v3 5/6] OvmfPkg/IncompatiblePci: Do not use deprecated macros

[Laszlo Ersek <lersek@redhat.com>]

On 02/07/17 04:33, Ruiyu Ni wrote:
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
>  .../IncompatiblePciDeviceSupport.c                          | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
> index b6ff128..df9eb60 100644
> --- a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
> +++ b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
> @@ -4,6 +4,7 @@
>    is not present), conserving 32-bit MMIO aperture for 32-bit BARs.
>  
>    Copyright (C) 2016, Red Hat, Inc.
> +  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
>  
>    This program and the accompanying materials are licensed and made available
>    under the terms and conditions of the BSD License which accompanies this
> @@ -66,19 +67,19 @@ STATIC CONST MMIO64_PREFERENCE mConfiguration = {
>          )
>        ),
>      ACPI_ADDRESS_SPACE_TYPE_MEM,                   // ResType
> -    PCI_ACPI_UNUSED,                               // GenFlag
> -    PCI_ACPI_UNUSED,                               // SpecificFlag
> +    0,                                             // GenFlag
> +    0,                                             // SpecificFlag
>      64,                                            // AddrSpaceGranularity:
>                                                     //   aperture selection hint
>                                                     //   for BAR allocation

PCI_ACPI_UNUSED expands to 0, so this is fine.


> -    PCI_ACPI_UNUSED,                               // AddrRangeMin
> -    PCI_BAR_OLD_ALIGN,                             // AddrRangeMax:
> +    0,                                             // AddrRangeMin
> +    0,                                             // AddrRangeMax:
>                                                     //   no special alignment
>                                                     //   for affected BARs

PCI_BAR_OLD_ALIGN used to expand to 0xFFFFFFFFFFFFFFFFULL.

In patch #2, the condtion that checks PCI_BAR_OLD_ALIGN is extended to
cover both 0 (employed here) and the new macro OLD_ALIGN (which is being
introduced in the same patch #2, for the old compat value
0xFFFFFFFFFFFFFFFFULL).

So this looks good too.


> -    PCI_BAR_ALL,                                   // AddrTranslationOffset:
> +    MAX_UINT64,                                   // AddrTranslationOffset:
>                                                     //   hint covers all
>                                                     //   eligible BARs

PCI_BAR_ALL expands to 0xFF. Patch #2 updates PciBusDxe to cover both
0xFF and MAX_UINT64. Okay.

However, I notice whitespace corruption here: the "//" to the right of
MAX_UINT64 no longer lines up with the rest of the comments.

> -    PCI_BAR_NOCHANGE                               // AddrLen:
> +    0                                              // AddrLen:
>                                                     //   use probed BAR size
>    },
>    //
> 

PCI_BAR_NOCHANGE expands to 0, so this is fine.

Ray, please fix up the whitespace corruption on the MAX_UINT64 line,
before committing the patch. With that fixed:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo