[PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented
@ 2018-09-13  2:10 Star Zeng
  2018-09-13  3:10 ` Ni, Ruiyu
  0 siblings, 1 reply; 6+ messages in thread
From: Star Zeng @ 2018-09-13  2:10 UTC (permalink / raw)
  To: edk2-devel
  Cc: Star Zeng, Jiewen Yao, Rangasai V Chaganty, Tomson Chang,
	Jenny Huang, Amy Chan

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1169

PCI spec:
They are also required to always implement function 0 in the device.
Implementing other functions is optional and may be assigned in any
order (i.e., a two-function device must respond to function 0 but
can choose any of the other possible function numbers (1-7) for the
second function).

This patch updates ScanPciBus() to not scan other functions if
function 0 is not implemented.

Test done:
Added debug code below in the second loop of ScanPciBus(),
compared the debug logs with and without this patch, many
non-0 unimplemented functions are skipped correctly.

  DEBUG ((
    DEBUG_INFO,
    "%a() B%02xD%02xF%02x VendorId: %04x DeviceId: %04x\n",
    __FUNCTION__,
    Bus,
    Device,
    Function,
    VendorID,
    DeviceID
    ));

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Tomson Chang <tomson.chang@intel.com>
Cc: Jenny Huang <jenny.huang@intel.com>
Cc: Amy Chan <amy.chan@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
index 36750b3f1d9c..305995de032c 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018, 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 distribution.  The full text of the license may be found at
@@ -247,6 +247,12 @@ ScanPciBus (
       VendorID  = PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function, PCI_VENDOR_ID_OFFSET));
       DeviceID  = PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function, PCI_DEVICE_ID_OFFSET));
       if (VendorID == 0xFFFF && DeviceID == 0xFFFF) {
+        if (Function == 0) {
+          //
+          // If function 0 is not implemented, do not scan other functions.
+          //
+          break;
+        }
         continue;
       }
 
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented
  2018-09-13  2:10 [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented Star Zeng
@ 2018-09-13  3:10 ` Ni, Ruiyu
  2018-09-13  5:28   ` Yao, Jiewen
  0 siblings, 1 reply; 6+ messages in thread
From: Ni, Ruiyu @ 2018-09-13  3:10 UTC (permalink / raw)
  To: Star Zeng, edk2-devel; +Cc: Tomson Chang, Jiewen Yao, Jenny Huang, Amy Chan

On 9/13/2018 10:10 AM, Star Zeng wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1169
> 
> PCI spec:
> They are also required to always implement function 0 in the device.
> Implementing other functions is optional and may be assigned in any
> order (i.e., a two-function device must respond to function 0 but
> can choose any of the other possible function numbers (1-7) for the
> second function).
> 
> This patch updates ScanPciBus() to not scan other functions if
> function 0 is not implemented.
> 
> Test done:
> Added debug code below in the second loop of ScanPciBus(),
> compared the debug logs with and without this patch, many
> non-0 unimplemented functions are skipped correctly.
> 
>    DEBUG ((
>      DEBUG_INFO,
>      "%a() B%02xD%02xF%02x VendorId: %04x DeviceId: %04x\n",
>      __FUNCTION__,
>      Bus,
>      Device,
>      Function,
>      VendorID,
>      DeviceID
>      ));
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Tomson Chang <tomson.chang@intel.com>
> Cc: Jenny Huang <jenny.huang@intel.com>
> Cc: Amy Chan <amy.chan@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>   IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> index 36750b3f1d9c..305995de032c 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> @@ -1,6 +1,6 @@
>   /** @file
>   
> -  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2017 - 2018, 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 distribution.  The full text of the license may be found at
> @@ -247,6 +247,12 @@ ScanPciBus (
>         VendorID  = PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function, PCI_VENDOR_ID_OFFSET));
>         DeviceID  = PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function, PCI_DEVICE_ID_OFFSET));
>         if (VendorID == 0xFFFF && DeviceID == 0xFFFF) {
> +        if (Function == 0) {
> +          //
> +          // If function 0 is not implemented, do not scan other functions.
> +          //
> +          break;
> +        }
>           continue;
>         }
>   
> 
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

-- 
Thanks,
Ray


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented
  2018-09-13  3:10 ` Ni, Ruiyu
@ 2018-09-13  5:28   ` Yao, Jiewen
  2018-09-13  6:29     ` Zeng, Star
  0 siblings, 1 reply; 6+ messages in thread
From: Yao, Jiewen @ 2018-09-13  5:28 UTC (permalink / raw)
  To: Ni, Ruiyu, Zeng, Star, edk2-devel@lists.01.org
  Cc: Chang, Tomson, Huang, Jenny, Chan, Amy

I checked the UEFI shell implementation. It uses below:

                  //
                  // If this is not a multi-function device, we can leave the loop
                  // to deal with the next device.
                  //
                  if (Func == 0 && ((PciHeader.HeaderType & HEADER_TYPE_MULTI_FUNCTION) == 0x00)) {
                    break;
                  }

Thank you
Yao Jiewen

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Thursday, September 13, 2018 11:11 AM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Chang, Tomson <tomson.chang@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Huang, Jenny <jenny.huang@intel.com>; Chan,
> Amy <amy.chan@intel.com>
> Subject: Re: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func
> 0 is not implemented
> 
> On 9/13/2018 10:10 AM, Star Zeng wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1169
> >
> > PCI spec:
> > They are also required to always implement function 0 in the device.
> > Implementing other functions is optional and may be assigned in any
> > order (i.e., a two-function device must respond to function 0 but
> > can choose any of the other possible function numbers (1-7) for the
> > second function).
> >
> > This patch updates ScanPciBus() to not scan other functions if
> > function 0 is not implemented.
> >
> > Test done:
> > Added debug code below in the second loop of ScanPciBus(),
> > compared the debug logs with and without this patch, many
> > non-0 unimplemented functions are skipped correctly.
> >
> >    DEBUG ((
> >      DEBUG_INFO,
> >      "%a() B%02xD%02xF%02x VendorId: %04x DeviceId: %04x\n",
> >      __FUNCTION__,
> >      Bus,
> >      Device,
> >      Function,
> >      VendorID,
> >      DeviceID
> >      ));
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> > Cc: Tomson Chang <tomson.chang@intel.com>
> > Cc: Jenny Huang <jenny.huang@intel.com>
> > Cc: Amy Chan <amy.chan@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > ---
> >   IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > index 36750b3f1d9c..305995de032c 100644
> > --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > @@ -1,6 +1,6 @@
> >   /** @file
> >
> > -  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> > +  Copyright (c) 2017 - 2018, 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 distribution.  The full text of the license may
> be found at
> > @@ -247,6 +247,12 @@ ScanPciBus (
> >         VendorID  = PciSegmentRead16
> (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function,
> PCI_VENDOR_ID_OFFSET));
> >         DeviceID  = PciSegmentRead16
> (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function,
> PCI_DEVICE_ID_OFFSET));
> >         if (VendorID == 0xFFFF && DeviceID == 0xFFFF) {
> > +        if (Function == 0) {
> > +          //
> > +          // If function 0 is not implemented, do not scan other
> functions.
> > +          //
> > +          break;
> > +        }
> >           continue;
> >         }
> >
> >
> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
> 
> --
> Thanks,
> Ray

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented
  2018-09-13  5:28   ` Yao, Jiewen
@ 2018-09-13  6:29     ` Zeng, Star
  2018-09-13  6:42       ` Yao, Jiewen
  0 siblings, 1 reply; 6+ messages in thread
From: Zeng, Star @ 2018-09-13  6:29 UTC (permalink / raw)
  To: Yao, Jiewen, Ni, Ruiyu, edk2-devel@lists.01.org
  Cc: Chang, Tomson, Huang, Jenny, Chan, Amy, Zeng, Star

Good information. :)
The UEFI shell implementation also has the code below.
                if (PciHeader.VendorId == 0xffff && Func == 0) {
                  break;
                }

                if (PciHeader.VendorId != 0xffff) {


The ScanPciBus() has no functional issue, but has another optimization point.

Current code checks HeaderType of Function 0 even Function 0 is not
implemented. HeaderType value will be 0xFF if Function 0 is not
implemented, then MaxFunction will be set to PCI_MAX_FUNC + 1.

The code can be optimized to only check HeaderType if Function 0 is
implemented.

I just sent another patch for it at https://lists.01.org/pipermail/edk2-devel/2018-September/029636.html.


Thanks,
Star
-----Original Message-----
From: Yao, Jiewen 
Sent: Thursday, September 13, 2018 1:29 PM
To: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Chang, Tomson <tomson.chang@intel.com>; Huang, Jenny <jenny.huang@intel.com>; Chan, Amy <amy.chan@intel.com>
Subject: RE: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented

I checked the UEFI shell implementation. It uses below:

                  //
                  // If this is not a multi-function device, we can leave the loop
                  // to deal with the next device.
                  //
                  if (Func == 0 && ((PciHeader.HeaderType & HEADER_TYPE_MULTI_FUNCTION) == 0x00)) {
                    break;
                  }

Thank you
Yao Jiewen

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Thursday, September 13, 2018 11:11 AM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Chang, Tomson <tomson.chang@intel.com>; Yao, Jiewen 
> <jiewen.yao@intel.com>; Huang, Jenny <jenny.huang@intel.com>; Chan, 
> Amy <amy.chan@intel.com>
> Subject: Re: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when 
> func
> 0 is not implemented
> 
> On 9/13/2018 10:10 AM, Star Zeng wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1169
> >
> > PCI spec:
> > They are also required to always implement function 0 in the device.
> > Implementing other functions is optional and may be assigned in any 
> > order (i.e., a two-function device must respond to function 0 but 
> > can choose any of the other possible function numbers (1-7) for the 
> > second function).
> >
> > This patch updates ScanPciBus() to not scan other functions if 
> > function 0 is not implemented.
> >
> > Test done:
> > Added debug code below in the second loop of ScanPciBus(), compared 
> > the debug logs with and without this patch, many
> > non-0 unimplemented functions are skipped correctly.
> >
> >    DEBUG ((
> >      DEBUG_INFO,
> >      "%a() B%02xD%02xF%02x VendorId: %04x DeviceId: %04x\n",
> >      __FUNCTION__,
> >      Bus,
> >      Device,
> >      Function,
> >      VendorID,
> >      DeviceID
> >      ));
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> > Cc: Tomson Chang <tomson.chang@intel.com>
> > Cc: Jenny Huang <jenny.huang@intel.com>
> > Cc: Amy Chan <amy.chan@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > ---
> >   IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > index 36750b3f1d9c..305995de032c 100644
> > --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > @@ -1,6 +1,6 @@
> >   /** @file
> >
> > -  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> > +  Copyright (c) 2017 - 2018, 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 distribution.  The full text of the 
> > license may
> be found at
> > @@ -247,6 +247,12 @@ ScanPciBus (
> >         VendorID  = PciSegmentRead16
> (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function, 
> PCI_VENDOR_ID_OFFSET));
> >         DeviceID  = PciSegmentRead16
> (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function, 
> PCI_DEVICE_ID_OFFSET));
> >         if (VendorID == 0xFFFF && DeviceID == 0xFFFF) {
> > +        if (Function == 0) {
> > +          //
> > +          // If function 0 is not implemented, do not scan other
> functions.
> > +          //
> > +          break;
> > +        }
> >           continue;
> >         }
> >
> >
> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
> 
> --
> Thanks,
> Ray

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented
  2018-09-13  6:29     ` Zeng, Star
@ 2018-09-13  6:42       ` Yao, Jiewen
  2018-09-13 10:27         ` Zeng, Star
  0 siblings, 1 reply; 6+ messages in thread
From: Yao, Jiewen @ 2018-09-13  6:42 UTC (permalink / raw)
  To: Zeng, Star, Ni, Ruiyu, edk2-devel@lists.01.org
  Cc: Chang, Tomson, Huang, Jenny, Chan, Amy

Sounds good.

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, September 13, 2018 2:30 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> edk2-devel@lists.01.org
> Cc: Chang, Tomson <tomson.chang@intel.com>; Huang, Jenny
> <jenny.huang@intel.com>; Chan, Amy <amy.chan@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func
> 0 is not implemented
> 
> Good information. :)
> The UEFI shell implementation also has the code below.
>                 if (PciHeader.VendorId == 0xffff && Func == 0) {
>                   break;
>                 }
> 
>                 if (PciHeader.VendorId != 0xffff) {
> 
> 
> The ScanPciBus() has no functional issue, but has another optimization point.
> 
> Current code checks HeaderType of Function 0 even Function 0 is not
> implemented. HeaderType value will be 0xFF if Function 0 is not
> implemented, then MaxFunction will be set to PCI_MAX_FUNC + 1.
> 
> The code can be optimized to only check HeaderType if Function 0 is
> implemented.
> 
> I just sent another patch for it at
> https://lists.01.org/pipermail/edk2-devel/2018-September/029636.html.
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, September 13, 2018 1:29 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>;
> edk2-devel@lists.01.org
> Cc: Chang, Tomson <tomson.chang@intel.com>; Huang, Jenny
> <jenny.huang@intel.com>; Chan, Amy <amy.chan@intel.com>
> Subject: RE: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func
> 0 is not implemented
> 
> I checked the UEFI shell implementation. It uses below:
> 
>                   //
>                   // If this is not a multi-function device, we can leave
> the loop
>                   // to deal with the next device.
>                   //
>                   if (Func == 0 && ((PciHeader.HeaderType &
> HEADER_TYPE_MULTI_FUNCTION) == 0x00)) {
>                     break;
>                   }
> 
> Thank you
> Yao Jiewen
> 
> > -----Original Message-----
> > From: Ni, Ruiyu
> > Sent: Thursday, September 13, 2018 11:11 AM
> > To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> > Cc: Chang, Tomson <tomson.chang@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Huang, Jenny <jenny.huang@intel.com>; Chan,
> > Amy <amy.chan@intel.com>
> > Subject: Re: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when
> > func
> > 0 is not implemented
> >
> > On 9/13/2018 10:10 AM, Star Zeng wrote:
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1169
> > >
> > > PCI spec:
> > > They are also required to always implement function 0 in the device.
> > > Implementing other functions is optional and may be assigned in any
> > > order (i.e., a two-function device must respond to function 0 but
> > > can choose any of the other possible function numbers (1-7) for the
> > > second function).
> > >
> > > This patch updates ScanPciBus() to not scan other functions if
> > > function 0 is not implemented.
> > >
> > > Test done:
> > > Added debug code below in the second loop of ScanPciBus(), compared
> > > the debug logs with and without this patch, many
> > > non-0 unimplemented functions are skipped correctly.
> > >
> > >    DEBUG ((
> > >      DEBUG_INFO,
> > >      "%a() B%02xD%02xF%02x VendorId: %04x DeviceId: %04x\n",
> > >      __FUNCTION__,
> > >      Bus,
> > >      Device,
> > >      Function,
> > >      VendorID,
> > >      DeviceID
> > >      ));
> > >
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> > > Cc: Tomson Chang <tomson.chang@intel.com>
> > > Cc: Jenny Huang <jenny.huang@intel.com>
> > > Cc: Amy Chan <amy.chan@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > > ---
> > >   IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c | 8 +++++++-
> > >   1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > > index 36750b3f1d9c..305995de032c 100644
> > > --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > > +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > > @@ -1,6 +1,6 @@
> > >   /** @file
> > >
> > > -  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> > > +  Copyright (c) 2017 - 2018, 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 distribution.  The full text of the
> > > license may
> > be found at
> > > @@ -247,6 +247,12 @@ ScanPciBus (
> > >         VendorID  = PciSegmentRead16
> > (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function,
> > PCI_VENDOR_ID_OFFSET));
> > >         DeviceID  = PciSegmentRead16
> > (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function,
> > PCI_DEVICE_ID_OFFSET));
> > >         if (VendorID == 0xFFFF && DeviceID == 0xFFFF) {
> > > +        if (Function == 0) {
> > > +          //
> > > +          // If function 0 is not implemented, do not scan other
> > functions.
> > > +          //
> > > +          break;
> > > +        }
> > >           continue;
> > >         }
> > >
> > >
> > Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
> >
> > --
> > Thanks,
> > Ray

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented
  2018-09-13  6:42       ` Yao, Jiewen
@ 2018-09-13 10:27         ` Zeng, Star
  0 siblings, 0 replies; 6+ messages in thread
From: Zeng, Star @ 2018-09-13 10:27 UTC (permalink / raw)
  To: Yao, Jiewen, Ni, Ruiyu, edk2-devel@lists.01.org
  Cc: Chang, Tomson, Huang, Jenny, Chan, Amy, Zeng, Star

Thanks.

Please also help review [PATCH] IntelSiliconPkg IntelVTdDxe: Check HeaderType if func 0 is implemented.


Star
-----Original Message-----
From: Yao, Jiewen 
Sent: Thursday, September 13, 2018 2:42 PM
To: Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
Cc: Chang, Tomson <tomson.chang@intel.com>; Huang, Jenny <jenny.huang@intel.com>; Chan, Amy <amy.chan@intel.com>
Subject: RE: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented

Sounds good.

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, September 13, 2018 2:30 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu 
> <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Chang, Tomson <tomson.chang@intel.com>; Huang, Jenny 
> <jenny.huang@intel.com>; Chan, Amy <amy.chan@intel.com>; Zeng, Star 
> <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when 
> func
> 0 is not implemented
> 
> Good information. :)
> The UEFI shell implementation also has the code below.
>                 if (PciHeader.VendorId == 0xffff && Func == 0) {
>                   break;
>                 }
> 
>                 if (PciHeader.VendorId != 0xffff) {
> 
> 
> The ScanPciBus() has no functional issue, but has another optimization point.
> 
> Current code checks HeaderType of Function 0 even Function 0 is not 
> implemented. HeaderType value will be 0xFF if Function 0 is not 
> implemented, then MaxFunction will be set to PCI_MAX_FUNC + 1.
> 
> The code can be optimized to only check HeaderType if Function 0 is 
> implemented.
> 
> I just sent another patch for it at
> https://lists.01.org/pipermail/edk2-devel/2018-September/029636.html.
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, September 13, 2018 1:29 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; 
> edk2-devel@lists.01.org
> Cc: Chang, Tomson <tomson.chang@intel.com>; Huang, Jenny 
> <jenny.huang@intel.com>; Chan, Amy <amy.chan@intel.com>
> Subject: RE: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when 
> func
> 0 is not implemented
> 
> I checked the UEFI shell implementation. It uses below:
> 
>                   //
>                   // If this is not a multi-function device, we can 
> leave the loop
>                   // to deal with the next device.
>                   //
>                   if (Func == 0 && ((PciHeader.HeaderType &
> HEADER_TYPE_MULTI_FUNCTION) == 0x00)) {
>                     break;
>                   }
> 
> Thank you
> Yao Jiewen
> 
> > -----Original Message-----
> > From: Ni, Ruiyu
> > Sent: Thursday, September 13, 2018 11:11 AM
> > To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> > Cc: Chang, Tomson <tomson.chang@intel.com>; Yao, Jiewen 
> > <jiewen.yao@intel.com>; Huang, Jenny <jenny.huang@intel.com>; Chan, 
> > Amy <amy.chan@intel.com>
> > Subject: Re: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize 
> > when func
> > 0 is not implemented
> >
> > On 9/13/2018 10:10 AM, Star Zeng wrote:
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1169
> > >
> > > PCI spec:
> > > They are also required to always implement function 0 in the device.
> > > Implementing other functions is optional and may be assigned in 
> > > any order (i.e., a two-function device must respond to function 0 
> > > but can choose any of the other possible function numbers (1-7) 
> > > for the second function).
> > >
> > > This patch updates ScanPciBus() to not scan other functions if 
> > > function 0 is not implemented.
> > >
> > > Test done:
> > > Added debug code below in the second loop of ScanPciBus(), 
> > > compared the debug logs with and without this patch, many
> > > non-0 unimplemented functions are skipped correctly.
> > >
> > >    DEBUG ((
> > >      DEBUG_INFO,
> > >      "%a() B%02xD%02xF%02x VendorId: %04x DeviceId: %04x\n",
> > >      __FUNCTION__,
> > >      Bus,
> > >      Device,
> > >      Function,
> > >      VendorID,
> > >      DeviceID
> > >      ));
> > >
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> > > Cc: Tomson Chang <tomson.chang@intel.com>
> > > Cc: Jenny Huang <jenny.huang@intel.com>
> > > Cc: Amy Chan <amy.chan@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > > ---
> > >   IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c | 8 +++++++-
> > >   1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > > index 36750b3f1d9c..305995de032c 100644
> > > --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > > +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > > @@ -1,6 +1,6 @@
> > >   /** @file
> > >
> > > -  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> > > +  Copyright (c) 2017 - 2018, 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 distribution.  The full text of the 
> > > license may
> > be found at
> > > @@ -247,6 +247,12 @@ ScanPciBus (
> > >         VendorID  = PciSegmentRead16
> > (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function, 
> > PCI_VENDOR_ID_OFFSET));
> > >         DeviceID  = PciSegmentRead16
> > (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function, 
> > PCI_DEVICE_ID_OFFSET));
> > >         if (VendorID == 0xFFFF && DeviceID == 0xFFFF) {
> > > +        if (Function == 0) {
> > > +          //
> > > +          // If function 0 is not implemented, do not scan other
> > functions.
> > > +          //
> > > +          break;
> > > +        }
> > >           continue;
> > >         }
> > >
> > >
> > Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
> >
> > --
> > Thanks,
> > Ray

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-09-13 10:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-13  2:10 [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented Star Zeng
2018-09-13  3:10 ` Ni, Ruiyu
2018-09-13  5:28   ` Yao, Jiewen
2018-09-13  6:29     ` Zeng, Star
2018-09-13  6:42       ` Yao, Jiewen
2018-09-13 10:27         ` Zeng, Star

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox