Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

[Haojian Zhuang]

On 02/15/2018 11:41 PM, Leif Lindholm wrote:
> On Mon, Feb 12, 2018 at 11:45:06AM +, Leif Lindholm wrote:
>> On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>>> be executed well on HiKey. Serial logs are missing.
>>
>> "Can't be executed well"? Does this mean it crashes?
> 
> You replied to this question, but not the further ones below.
> Can you have a look, please?

Excuse me that I missed comment in below.

> 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Haojian Zhuang 
>>> ---
>>>   Platform/Hisilicon/D02/Pv660D02.dsc | 2 ++
>>>   Platform/Hisilicon/D03/D03.dsc  | 1 +
>>>   Platform/Hisilicon/D05/D05.dsc  | 1 +
>>>   Silicon/Hisilicon/Hisilicon.dsc.inc | 2 --
>>>   4 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Platform/Hisilicon/D02/Pv660D02.dsc 
>>> b/Platform/Hisilicon/D02/Pv660D02.dsc
>>> index 9e826ae..018e149 100644
>>> --- a/Platform/Hisilicon/D02/Pv660D02.dsc
>>> +++ b/Platform/Hisilicon/D02/Pv660D02.dsc
>>> @@ -80,6 +80,8 @@
>>>   
>>>   [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>>> I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>>> +  
>>> SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
>>
>> This change I agree with - this is a clear fix.
>>
>>> +  
>>> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>>
>> But this one just causes duplication of boilerplate.
>> Could you instead put the fragment in Hisilicon.dsc.inc ...
>>
>>>   
>>>   [BuildOptions]
>>> GCC:*_*_AARCH64_PLATFORM_FLAGS == 
>>> -I$(WORKSPACE)/Silicon/Hisilicon/Pv660/Include
>>> diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
>>> index c496306..b9bce66 100644
>>> --- a/Platform/Hisilicon/D03/D03.dsc
>>> +++ b/Platform/Hisilicon/D03/D03.dsc
>>> @@ -97,6 +97,7 @@
>>>   [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>>> I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>>> 
>>> SerialPortLib|Silicon/Hisilicon/Hi1610/Library/Uart/LpcSerialPortLib/LpcSerialPortLib.inf
>>> +  
>>> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>>>   
>>>   [BuildOptions]
>>> GCC:*_*_AARCH64_PLATFORM_FLAGS == 
>>> -I$(WORKSPACE)/Silicon/Hisilicon/Hi1610/Include
>>> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
>>> index 0792b08..dfee09b 100644
>>> --- a/Platform/Hisilicon/D05/D05.dsc
>>> +++ b/Platform/Hisilicon/D05/D05.dsc
>>> @@ -105,6 +105,7 @@
>>>   [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>>> I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>>> 
>>> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>>> +  
>>> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>>>   
>>>   [BuildOptions]
>>> GCC:*_*_AARCH64_PLATFORM_FLAGS == 
>>> -I$(WORKSPACE)/Silicon/Hisilicon/Hi1616/Include
>>> diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc 
>>> b/Silicon/Hisilicon/Hisilicon.dsc.inc
>>> index 5766829..b5b9e7e 100644
>>> --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
>>> +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
>>> @@ -208,8 +208,6 @@
>>> 
>>> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>>> 
>>> ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
>>> 
>>> CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
>>> -  
>>> SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
>>
>> ... behind a conditional like
>>
>> !ifndef CONFIG_NO_DEBUGLIB
>>> -  
>>> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>> !endif
>>
>> ?
>>
>> And correspondingly set CONFIG_NO_DEBUGLIB in hikey/hikey960 .dscs?

OK. I'll create it.

>>
>> Also, if this is a problem causes by using Hisilicon.dsc.inc, it
>> should really go in before 2/4.

OK. I'll move it before [2/4].

>> This is great, by the way - I was not expecting that you would be able
>> to reuse that, I thought there would be a separate .inc for
>> hikey/hikey960.
>>
>> /
>>  Leif
>>
>>>   
>>>   [LibraryClasses.AARCH64]
>>> 
>>> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
>>> -- 
>>> 2.7.4
>>>

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

[Leif Lindholm]

On Mon, Feb 12, 2018 at 11:45:06AM +, Leif Lindholm wrote:
> On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
> > With the SerialPortLib and DebugLib, Dxe runtime driver can't
> > be executed well on HiKey. Serial logs are missing.
> 
> "Can't be executed well"? Does this mean it crashes?

You replied to this question, but not the further ones below.
Can you have a look, please?

/
Leif

> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Haojian Zhuang 
> > ---
> >  Platform/Hisilicon/D02/Pv660D02.dsc | 2 ++
> >  Platform/Hisilicon/D03/D03.dsc  | 1 +
> >  Platform/Hisilicon/D05/D05.dsc  | 1 +
> >  Silicon/Hisilicon/Hisilicon.dsc.inc | 2 --
> >  4 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Platform/Hisilicon/D02/Pv660D02.dsc 
> > b/Platform/Hisilicon/D02/Pv660D02.dsc
> > index 9e826ae..018e149 100644
> > --- a/Platform/Hisilicon/D02/Pv660D02.dsc
> > +++ b/Platform/Hisilicon/D02/Pv660D02.dsc
> > @@ -80,6 +80,8 @@
> >  
> >  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> >I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
> > +  
> > SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
> 
> This change I agree with - this is a clear fix.
> 
> > +  
> > DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
> 
> But this one just causes duplication of boilerplate.
> Could you instead put the fragment in Hisilicon.dsc.inc ...
> 
> >  
> >  [BuildOptions]
> >GCC:*_*_AARCH64_PLATFORM_FLAGS == 
> > -I$(WORKSPACE)/Silicon/Hisilicon/Pv660/Include
> > diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
> > index c496306..b9bce66 100644
> > --- a/Platform/Hisilicon/D03/D03.dsc
> > +++ b/Platform/Hisilicon/D03/D03.dsc
> > @@ -97,6 +97,7 @@
> >  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> >I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
> >
> > SerialPortLib|Silicon/Hisilicon/Hi1610/Library/Uart/LpcSerialPortLib/LpcSerialPortLib.inf
> > +  
> > DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
> >  
> >  [BuildOptions]
> >GCC:*_*_AARCH64_PLATFORM_FLAGS == 
> > -I$(WORKSPACE)/Silicon/Hisilicon/Hi1610/Include
> > diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> > index 0792b08..dfee09b 100644
> > --- a/Platform/Hisilicon/D05/D05.dsc
> > +++ b/Platform/Hisilicon/D05/D05.dsc
> > @@ -105,6 +105,7 @@
> >  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> >I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
> >
> > SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> > +  
> > DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
> >  
> >  [BuildOptions]
> >GCC:*_*_AARCH64_PLATFORM_FLAGS == 
> > -I$(WORKSPACE)/Silicon/Hisilicon/Hi1616/Include
> > diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc 
> > b/Silicon/Hisilicon/Hisilicon.dsc.inc
> > index 5766829..b5b9e7e 100644
> > --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
> > +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
> > @@ -208,8 +208,6 @@
> >
> > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> >
> > ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
> >CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> > -  
> > SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
> 
> ... behind a conditional like
> 
> !ifndef CONFIG_NO_DEBUGLIB
> > -  
> > DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
> !endif
> 
> ?
> 
> And correspondingly set CONFIG_NO_DEBUGLIB in hikey/hikey960 .dscs?
> 
> Also, if this is a problem causes by using Hisilicon.dsc.inc, it
> should really go in before 2/4.
> This is great, by the way - I was not expecting that you would be able
> to reuse that, I thought there would be a separate .inc for
> hikey/hikey960.
> 
> /
> Leif
> 
> >  
> >  [LibraryClasses.AARCH64]
> >
> > ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
> > -- 
> > 2.7.4
> > 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

[Guo Heyi]

On Tue, Feb 13, 2018 at 12:59:50AM +, Haojian Zhuang wrote:
> On 02/13/2018 08:23 AM, Guo Heyi wrote:
> > Hi Haojian,
> > 
> > Dw8250SerialPortRuntimeLib actually depends on DW8250 hardware IP; if there
> > isn't such device on Hikey, then you can't use this library instance indeed.
> > 
> > But I think PeiDxeDebugLibReportStatusCode should be some common code, 
> > however
> > it depends on ReportStatusCodeLib and Status Code PEIM and Status code DXE
> > driver. Have you added them too?
> >
> If I leave PeiDxeDebugLibReportStatusCode and move Dw8250SerialPortRuntimeLib 
> out,
> I'll meet UEFI crash. It seems the debug lib is depended on serial port lib.
This is strange, because you should have your own SerialPortLib of your
platform, otherwise error will occur when you build your UEFI.

> 
> And I consider that Dw8250 serial port is only valid on D02. So I decide to 
> move them out.

I agree.

Regards,

Heyi

> 
> Best Regards
> Haojian
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

[Haojian Zhuang]

On 02/13/2018 08:23 AM, Guo Heyi wrote:
> Hi Haojian,
> 
> Dw8250SerialPortRuntimeLib actually depends on DW8250 hardware IP; if there
> isn't such device on Hikey, then you can't use this library instance indeed.
> 
> But I think PeiDxeDebugLibReportStatusCode should be some common code, however
> it depends on ReportStatusCodeLib and Status Code PEIM and Status code DXE
> driver. Have you added them too?
>
If I leave PeiDxeDebugLibReportStatusCode and move Dw8250SerialPortRuntimeLib 
out,
I'll meet UEFI crash. It seems the debug lib is depended on serial port lib.

And I consider that Dw8250 serial port is only valid on D02. So I decide to 
move them out.

Best Regards
Haojian
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

[Guo Heyi]

Hi Haojian,

Dw8250SerialPortRuntimeLib actually depends on DW8250 hardware IP; if there
isn't such device on Hikey, then you can't use this library instance indeed.

But I think PeiDxeDebugLibReportStatusCode should be some common code, however
it depends on ReportStatusCodeLib and Status Code PEIM and Status code DXE
driver. Have you added them too?

Heyi

On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
> With the SerialPortLib and DebugLib, Dxe runtime driver can't
> be executed well on HiKey. Serial logs are missing.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang 
> ---
>  Platform/Hisilicon/D02/Pv660D02.dsc | 2 ++
>  Platform/Hisilicon/D03/D03.dsc  | 1 +
>  Platform/Hisilicon/D05/D05.dsc  | 1 +
>  Silicon/Hisilicon/Hisilicon.dsc.inc | 2 --
>  4 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/Hisilicon/D02/Pv660D02.dsc 
> b/Platform/Hisilicon/D02/Pv660D02.dsc
> index 9e826ae..018e149 100644
> --- a/Platform/Hisilicon/D02/Pv660D02.dsc
> +++ b/Platform/Hisilicon/D02/Pv660D02.dsc
> @@ -80,6 +80,8 @@
>  
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
> +  
> SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
> +  
> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [BuildOptions]
>GCC:*_*_AARCH64_PLATFORM_FLAGS == 
> -I$(WORKSPACE)/Silicon/Hisilicon/Pv660/Include
> diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
> index c496306..b9bce66 100644
> --- a/Platform/Hisilicon/D03/D03.dsc
> +++ b/Platform/Hisilicon/D03/D03.dsc
> @@ -97,6 +97,7 @@
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>
> SerialPortLib|Silicon/Hisilicon/Hi1610/Library/Uart/LpcSerialPortLib/LpcSerialPortLib.inf
> +  
> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [BuildOptions]
>GCC:*_*_AARCH64_PLATFORM_FLAGS == 
> -I$(WORKSPACE)/Silicon/Hisilicon/Hi1610/Include
> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> index 0792b08..dfee09b 100644
> --- a/Platform/Hisilicon/D05/D05.dsc
> +++ b/Platform/Hisilicon/D05/D05.dsc
> @@ -105,6 +105,7 @@
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>
> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +  
> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [BuildOptions]
>GCC:*_*_AARCH64_PLATFORM_FLAGS == 
> -I$(WORKSPACE)/Silicon/Hisilicon/Hi1616/Include
> diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc 
> b/Silicon/Hisilicon/Hisilicon.dsc.inc
> index 5766829..b5b9e7e 100644
> --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
> +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
> @@ -208,8 +208,6 @@
>
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>
> ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
>CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> -  
> SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
> -  
> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [LibraryClasses.AARCH64]
>
> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

[Haojian Zhuang]

>From: Ard Biesheuvel <ard.biesheu...@linaro.org>
>Sent: Monday, February 12, 2018 12:22 PM
>To: Haojian Zhuang
>Cc: Leif Lindholm; edk2-devel@lists.01.org; heyi@linaro.org; 
>linaro-u...@lists.linaro.org
>Subject: Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime 
>lib from common file
>
>On 12 February 2018 at 12:19, Haojian Zhuang <haojian.zhu...@linaro.org> wrote:
>>>From: Ard Biesheuvel <ard.biesheu...@linaro.org>
>>>Sent: Monday, February 12, 2018 12:05 PM
>>>To: Haojian Zhuang
>>>Cc: Leif Lindholm; edk2-devel@lists.01.org; heyi@linaro.org; 
>>>ard.sheu...@linaro.org; linaro-u...@lists.linaro.org
>>>Subject: Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime 
>>>lib from common file
>>>
>>>>On 12 February 2018 at 11:47, Haojian Zhuang <haojian.zhu...@linaro.org> 
>>>>wrote:
>>>>>From: Leif Lindholm <leif.lindh...@linaro.org>
>>>>>Sent: Monday, February 12, 2018 11:45 AM
>>>>>To: Haojian Zhuang
>>>>>Cc: edk2-devel@lists.01.org; linaro-u...@lists.linaro.org; 
>>>>>ard.sheu...@linaro.org; heyi@linaro.org
>>>>>Subject: Re: [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib 
>>>>>from common file
>>>>>
>>>>>On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>>>>>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>>>>>> be executed well on HiKey. Serial logs are missing.
>>>>>
>>>>>"Can't be executed well"? Does this mean it crashes?
>>>>
>>>> No crash. Serial output are missing since SerialPortLib is different.
>>>>
>>>
>>>Does this driver take care to only create serial output at boot time?
>>>Does it, e.g., call EfiAtRuntime() or use a notification callback at
>>>ExitBootServices to make absolutely sure the serial port is no longer
>>>used?
>>
>> These drivers don't use serial port directly. But I tried to use DEBUG ()
>> function to dump some debug informations in these drivers. I found
>> that I can't output anything on serial console.
>>
>
>But do those DEBUG() calls only occur at boot time? Or could they be
>called at runtime as well?

Excuse me that I didn't explain it clearly. 

At first, I need to make sure everything executed well when I switch to
the common dsc file. So I added some debug messages in those key
drivers.
In the second, I need to debug the boot flow later. I mean that I need some
debug message in the initialization of DXE runtime driver. I'm considering
to make use of EmuVariable and predefined emu variable region in RAM.
Then I could store the predefined boot options in emu variable region.
And I could re-use PlatformBootManager in ArmPlatformPkg without big
changes. So I need to add some debug messages in these DXE runtime
driver.

These two drivers in the common dsc file blocks me enabling debug
messages in the initialization code of DXE runtime driver. So I have
to move them out.

Best Regards
Haojian
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

[Ard Biesheuvel]

On 12 February 2018 at 12:19, Haojian Zhuang <haojian.zhu...@linaro.org> wrote:
>>From: Ard Biesheuvel <ard.biesheu...@linaro.org>
>>Sent: Monday, February 12, 2018 12:05 PM
>>To: Haojian Zhuang
>>Cc: Leif Lindholm; edk2-devel@lists.01.org; heyi@linaro.org; 
>>ard.sheu...@linaro.org; linaro-u...@lists.linaro.org
>>Subject: Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime 
>>lib from common file
>>
>>On 12 February 2018 at 11:47, Haojian Zhuang <haojian.zhu...@linaro.org> 
>>wrote:
>>>>From: Leif Lindholm <leif.lindh...@linaro.org>
>>>>Sent: Monday, February 12, 2018 11:45 AM
>>>>To: Haojian Zhuang
>>>>Cc: edk2-devel@lists.01.org; linaro-u...@lists.linaro.org; 
>>>>ard.sheu...@linaro.org; heyi@linaro.org
>>>>Subject: Re: [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib 
>>>>from common file
>>>>
>>>>On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>>>>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>>>>> be executed well on HiKey. Serial logs are missing.
>>>>
>>>>"Can't be executed well"? Does this mean it crashes?
>>>
>>> No crash. Serial output are missing since SerialPortLib is different.
>>>
>>
>>Does this driver take care to only create serial output at boot time?
>>Does it, e.g., call EfiAtRuntime() or use a notification callback at
>>ExitBootServices to make absolutely sure the serial port is no longer
>>used?
>
> These drivers don't use serial port directly. But I tried to use DEBUG ()
> function to dump some debug informations in these drivers. I found
> that I can't output anything on serial console.
>

But do those DEBUG() calls only occur at boot time? Or could they be
called at runtime as well?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

[Haojian Zhuang]

>From: Ard Biesheuvel <ard.biesheu...@linaro.org>
>Sent: Monday, February 12, 2018 12:05 PM
>To: Haojian Zhuang
>Cc: Leif Lindholm; edk2-devel@lists.01.org; heyi@linaro.org; 
>ard.sheu...@linaro.org; linaro-u...@lists.linaro.org
>Subject: Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime 
>lib from common file
>
>On 12 February 2018 at 11:47, Haojian Zhuang <haojian.zhu...@linaro.org> wrote:
>>>From: Leif Lindholm <leif.lindh...@linaro.org>
>>>Sent: Monday, February 12, 2018 11:45 AM
>>>To: Haojian Zhuang
>>>Cc: edk2-devel@lists.01.org; linaro-u...@lists.linaro.org; 
>>>ard.sheu...@linaro.org; heyi@linaro.org
>>>Subject: Re: [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib 
>>>from common file
>>>
>>>On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>>>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>>>> be executed well on HiKey. Serial logs are missing.
>>>
>>>"Can't be executed well"? Does this mean it crashes?
>>
>> No crash. Serial output are missing since SerialPortLib is different.
>>
>
>Does this driver take care to only create serial output at boot time?
>Does it, e.g., call EfiAtRuntime() or use a notification callback at
>ExitBootServices to make absolutely sure the serial port is no longer
>used?

These drivers don't use serial port directly. But I tried to use DEBUG () 
function to dump some debug informations in these drivers. I found 
that I can't output anything on serial console.

Best Regards
Haojian
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

[Ard Biesheuvel]

On 12 February 2018 at 11:47, Haojian Zhuang  wrote:
>>From: Leif Lindholm 
>>Sent: Monday, February 12, 2018 11:45 AM
>>To: Haojian Zhuang
>>Cc: edk2-devel@lists.01.org; linaro-u...@lists.linaro.org; 
>>ard.sheu...@linaro.org; heyi@linaro.org
>>Subject: Re: [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from 
>>common file
>>
>>On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>>> be executed well on HiKey. Serial logs are missing.
>>
>>"Can't be executed well"? Does this mean it crashes?
>
> No crash. Serial output are missing since SerialPortLib is different.
>

Does this driver take care to only create serial output at boot time?
Does it, e.g., call EfiAtRuntime() or use a notification callback at
ExitBootServices to make absolutely sure the serial port is no longer
used?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

[Haojian Zhuang]

>From: Leif Lindholm 
>Sent: Monday, February 12, 2018 11:45 AM
>To: Haojian Zhuang
>Cc: edk2-devel@lists.01.org; linaro-u...@lists.linaro.org; 
>ard.sheu...@linaro.org; heyi@linaro.org
>Subject: Re: [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from 
>common file
>
>On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>> be executed well on HiKey. Serial logs are missing.
>
>"Can't be executed well"? Does this mean it crashes?

No crash. Serial output are missing since SerialPortLib is different.

Best Regards
Haojian
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

[Leif Lindholm]

On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
> With the SerialPortLib and DebugLib, Dxe runtime driver can't
> be executed well on HiKey. Serial logs are missing.

"Can't be executed well"? Does this mean it crashes?

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang 
> ---
>  Platform/Hisilicon/D02/Pv660D02.dsc | 2 ++
>  Platform/Hisilicon/D03/D03.dsc  | 1 +
>  Platform/Hisilicon/D05/D05.dsc  | 1 +
>  Silicon/Hisilicon/Hisilicon.dsc.inc | 2 --
>  4 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/Hisilicon/D02/Pv660D02.dsc 
> b/Platform/Hisilicon/D02/Pv660D02.dsc
> index 9e826ae..018e149 100644
> --- a/Platform/Hisilicon/D02/Pv660D02.dsc
> +++ b/Platform/Hisilicon/D02/Pv660D02.dsc
> @@ -80,6 +80,8 @@
>  
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
> +  
> SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf

This change I agree with - this is a clear fix.

> +  
> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf

But this one just causes duplication of boilerplate.
Could you instead put the fragment in Hisilicon.dsc.inc ...

>  
>  [BuildOptions]
>GCC:*_*_AARCH64_PLATFORM_FLAGS == 
> -I$(WORKSPACE)/Silicon/Hisilicon/Pv660/Include
> diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
> index c496306..b9bce66 100644
> --- a/Platform/Hisilicon/D03/D03.dsc
> +++ b/Platform/Hisilicon/D03/D03.dsc
> @@ -97,6 +97,7 @@
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>
> SerialPortLib|Silicon/Hisilicon/Hi1610/Library/Uart/LpcSerialPortLib/LpcSerialPortLib.inf
> +  
> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [BuildOptions]
>GCC:*_*_AARCH64_PLATFORM_FLAGS == 
> -I$(WORKSPACE)/Silicon/Hisilicon/Hi1610/Include
> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> index 0792b08..dfee09b 100644
> --- a/Platform/Hisilicon/D05/D05.dsc
> +++ b/Platform/Hisilicon/D05/D05.dsc
> @@ -105,6 +105,7 @@
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>
> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +  
> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [BuildOptions]
>GCC:*_*_AARCH64_PLATFORM_FLAGS == 
> -I$(WORKSPACE)/Silicon/Hisilicon/Hi1616/Include
> diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc 
> b/Silicon/Hisilicon/Hisilicon.dsc.inc
> index 5766829..b5b9e7e 100644
> --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
> +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
> @@ -208,8 +208,6 @@
>
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>
> ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
>CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> -  
> SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf

... behind a conditional like

!ifndef CONFIG_NO_DEBUGLIB
> -  
> DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
!endif

?

And correspondingly set CONFIG_NO_DEBUGLIB in hikey/hikey960 .dscs?

Also, if this is a problem causes by using Hisilicon.dsc.inc, it
should really go in before 2/4.
This is great, by the way - I was not expecting that you would be able
to reuse that, I thought there would be a separate .inc for
hikey/hikey960.

/
Leif

>  
>  [LibraryClasses.AARCH64]
>
> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel