* [PATCH] MdeModulePkg/NonDiscoverable: Compare SIZE_4GB with address type
@ 2017-01-11 2:01 Hao Wu
2017-01-11 2:05 ` Ni, Ruiyu
0 siblings, 1 reply; 6+ messages in thread
From: Hao Wu @ 2017-01-11 2:01 UTC (permalink / raw)
To: edk2-devel; +Cc: Hao Wu, Ard Biesheuvel, Ruiyu Ni
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/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index b07c129..c836ad6 100644
--- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.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/NonDiscoverableDeviceRegistrationLib.c b/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.c
index 6f46dfa..4180b0a 100644
--- a/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.c
+++ b/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] MdeModulePkg/NonDiscoverable: Compare SIZE_4GB with address type
2017-01-11 2:01 [PATCH] MdeModulePkg/NonDiscoverable: Compare SIZE_4GB with address type Hao Wu
@ 2017-01-11 2:05 ` Ni, Ruiyu
2017-01-11 2:08 ` Wu, Hao A
2017-01-11 7:55 ` Ard Biesheuvel
0 siblings, 2 replies; 6+ messages in thread
From: Ni, Ruiyu @ 2017-01-11 2:05 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org; +Cc: Wu, Hao A, Ard Biesheuvel
One minor comments: put one space after (EFI_PHYSICAL_ADDRESS) and (UINTN) as below.
> + (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress + *NumberOfBytes > SIZE_4GB
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MdeModulePkg/NonDiscoverable: Compare SIZE_4GB with address type
2017-01-11 2:05 ` Ni, Ruiyu
@ 2017-01-11 2:08 ` Wu, Hao A
2017-01-11 7:55 ` Ard Biesheuvel
1 sibling, 0 replies; 6+ messages in thread
From: Wu, Hao A @ 2017-01-11 2:08 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Ard Biesheuvel
Got it, I will update the codes before check in. Thanks for the feedback.
Best Regards,
Hao Wu
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Wednesday, January 11, 2017 10:05 AM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Wu, Hao A; Ard Biesheuvel
> Subject: RE: [edk2] [PATCH] MdeModulePkg/NonDiscoverable: Compare
> SIZE_4GB with address type
>
> One minor comments: put one space after (EFI_PHYSICAL_ADDRESS) and
> (UINTN) as below.
> > + (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress + *NumberOfBytes >
> SIZE_4GB
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MdeModulePkg/NonDiscoverable: Compare SIZE_4GB with address type
2017-01-11 2:05 ` Ni, Ruiyu
2017-01-11 2:08 ` Wu, Hao A
@ 2017-01-11 7:55 ` Ard Biesheuvel
2017-01-11 10:59 ` Wu, Hao A
1 sibling, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2017-01-11 7:55 UTC (permalink / raw)
To: Ni, Ruiyu, Laszlo Ersek; +Cc: Wu, Hao A, edk2-devel@lists.01.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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MdeModulePkg/NonDiscoverable: Compare SIZE_4GB with address type
2017-01-11 7:55 ` Ard Biesheuvel
@ 2017-01-11 10:59 ` Wu, Hao A
2017-01-11 20:14 ` Laszlo Ersek
0 siblings, 1 reply; 6+ messages in thread
From: Wu, Hao A @ 2017-01-11 10:59 UTC (permalink / raw)
To: Ard Biesheuvel, Ni, Ruiyu, Laszlo Ersek; +Cc: edk2-devel@lists.01.org
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, January 11, 2017 3:55 PM
> To: Ni, Ruiyu; Laszlo Ersek
> Cc: Wu, Hao A; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/NonDiscoverable: Compare
> SIZE_4GB with address type
>
> 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.
Thanks Ard, I agree with your point. I will leave the patch unchanged.
Best Regards,
Hao Wu
>
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MdeModulePkg/NonDiscoverable: Compare SIZE_4GB with address type
2017-01-11 10:59 ` Wu, Hao A
@ 2017-01-11 20:14 ` Laszlo Ersek
0 siblings, 0 replies; 6+ messages in thread
From: Laszlo Ersek @ 2017-01-11 20:14 UTC (permalink / raw)
To: Wu, Hao A, Ard Biesheuvel, Ni, Ruiyu; +Cc: edk2-devel@lists.01.org
On 01/11/17 11:59, Wu, Hao A wrote:
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Wednesday, January 11, 2017 3:55 PM
>> To: Ni, Ruiyu; Laszlo Ersek
>> Cc: Wu, Hao A; edk2-devel@lists.01.org
>> Subject: Re: [edk2] [PATCH] MdeModulePkg/NonDiscoverable: Compare
>> SIZE_4GB with address type
>>
>> 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.
>
> Thanks Ard, I agree with your point. I will leave the patch unchanged.
Thanks! I agree those spaces are super counter-intuitive; in the past
we've had bugs due to them. It's for best to reflect the tight binding
of the cast operator in the source code, by keeping the operand directly
adjacent.
Cheers
Laszlo
> Best Regards,
> Hao Wu
>
>>
>>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-11 20:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-11 2:01 [PATCH] MdeModulePkg/NonDiscoverable: Compare SIZE_4GB with address type Hao Wu
2017-01-11 2:05 ` Ni, Ruiyu
2017-01-11 2:08 ` Wu, Hao A
2017-01-11 7:55 ` Ard Biesheuvel
2017-01-11 10:59 ` Wu, Hao A
2017-01-11 20:14 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox