Re: [PATCH] MdeModulePkg/NonDiscoverable: Compare SIZE_4GB with address type

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 11 January 2017 at 02:05, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
> One minor comments: put one space after (EFI_PHYSICAL_ADDRESS) and (UINTN) as below.
>> +            (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress + *NumberOfBytes > SIZE_4GB
>

I disagree. The EDK2 coding style does not mandate a space after a
cast, and given the tight binding of the () cast operator in C, it is
counterinituitive.

> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
>
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Hao Wu
>> Sent: Wednesday, January 11, 2017 10:01 AM
>> To: edk2-devel@lists.01.org
>> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Ard
>> Biesheuvel <ard.biesheuvel@linaro.org>
>> Subject: [edk2] [PATCH] MdeModulePkg/NonDiscoverable: Compare
>> SIZE_4GB with address type
>>
>> Refine the codes to compare the definition 'SIZE_4GB' with type
>> EFI_PHYSICAL_ADDRESS.
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
>> ---
>>  .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c  |
>> 4 ++--
>>  .../NonDiscoverableDeviceRegistrationLib.c                            | 2 +-
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git
>> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
>> PciDeviceIo.c
>> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
>> PciDeviceIo.c
>> index b07c129..c836ad6 100644
>> ---
>> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
>> PciDeviceIo.c
>> +++
>> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
>> Pc
>> +++ iDeviceIo.c
>> @@ -598,7 +598,7 @@ CoherentPciIoMap (
>>    //
>>    Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
>>    if ((Dev->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) ==
>> 0 &&
>> -      (UINTN)HostAddress + *NumberOfBytes > SIZE_4GB) {
>> +      (EFI_PHYSICAL_ADDRESS)(UINTN)HostAddress + *NumberOfBytes >
>> + SIZE_4GB) {
>>
>>      //
>>      // Bounce buffering is not possible for consistent mappings @@ -1006,7
>> +1006,7 @@ NonCoherentPciIoMap (
>>    // a bounce buffer and copy over the data in case HostAddress >= 4 GB.
>>    //
>>    Bounce = ((Dev->Attributes &
>> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0 &&
>> -            (UINTN)HostAddress + *NumberOfBytes > SIZE_4GB);
>> +            (EFI_PHYSICAL_ADDRESS)(UINTN)HostAddress + *NumberOfBytes >
>> + SIZE_4GB);
>>
>>    if (!Bounce) {
>>      switch (Operation) {
>> diff --git
>> a/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDisco
>> verableDeviceRegistrationLib.c
>> b/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDisco
>> verableDeviceRegistrationLib.c
>> index 6f46dfa..4180b0a 100644
>> ---
>> a/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDisco
>> verableDeviceRegistrationLib.c
>> +++
>> b/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDisco
>> +++ verableDeviceRegistrationLib.c
>> @@ -163,7 +163,7 @@ RegisterNonDiscoverableMmioDevice (
>>      Desc->AddrLen               = Size;
>>      Desc->AddrRangeMax          = Base + Size - 1;
>>      Desc->ResType               = ACPI_ADDRESS_SPACE_TYPE_MEM;
>> -    Desc->AddrSpaceGranularity  = (Base + Size > SIZE_4GB) ? 64 : 32;
>> +    Desc->AddrSpaceGranularity  = ((EFI_PHYSICAL_ADDRESS) Base + Size >
>> + SIZE_4GB) ? 64 : 32;
>>      Desc->AddrTranslationOffset = 0;
>>    }
>>    VA_END (Args);
>> --
>> 1.9.5.msysgit.0
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel