| Message ID | 20181029033249.45363-1-ming.huang@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Ming, On Mon, Oct 29, 2018 at 11:32:37AM +0800, Ming Huang wrote: > The major features of this patchset include: > 1. Modify acpi table for ACS test; > 2. Enable secure boot for SBBR-SCT; > 3. Other change for ACS test; > > For this SCT issue: > RT.SetVariable - Create one Time Base Auth Variable, the expect return status > should be EFI_SUCCESS – FAILURE > > The resule of fail is effected by the edk2 commit(67943427). > If Modify Variable.c as below, this case will pass. > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > @@ -3188,7 +3188,7 @@ VariableServiceSetVariable ( > // Maybe it's the delete operation of common authenticated variable at > // user physical presence. > // > if (DataSize != AUTHINFO_SIZE) { > - return EFI_UNSUPPORTED; > + return EFI_SECURITY_VIOLATION; > > I supect ACS SCT compatible with UEFI 2.7 spec. We will analyze this issue > continue. > > Code can also be found in github: > https://github.com/hisilicon/OpenPlatformPkg.git > branch: d06-acs-platforms > > > Ming Huang (12): > Silicon/Hisilicon/D06: Add watchdog to GTDT > Silicon/Hisilicon/D06: Drop _CID for fwts issue > Silicon/Hisilicon/D06: Fix fwts issue in Dbg2 > Silicon/Hisilicon/D06: Fix fwts issue in FADT > Hisilicon/D06: Move some functions to OemMiscLib > Silicon/Hisilicon: Modify for SBBR fwts SetTime_Func test case > Hisilicon/D0x: Fix secure boot bug in FlashFvbDxe > Hisilicon/D06: Fix SBBR-SCT AuthVar issue > Silicon/Hisilicon/D06: Reserve ECAM resource in DSDT > Silicon/Hisilicon/D06: Modify GTDT timer flag > Hisilicon/D06: Modify Gic base > Silicon/Hisilicon/D06: Set TA as Node 0 for TA boot > > Silicon/Hisilicon/HisiPkg.dec | 1 + > Silicon/Hisilicon/Hisilicon.dsc.inc | 16 ++ > Platform/Hisilicon/D03/D03.dsc | 5 + > Platform/Hisilicon/D05/D05.dsc | 5 + > Platform/Hisilicon/D06/D06.dsc | 9 +- > .../Drivers/FlashFvbDxe/FlashFvbDxe.inf | 2 + > .../M41T83RealTimeClockLib.inf | 3 +- > .../Hi1620/Hi1620AcpiTables/Hi1620Platform.h | 2 +- > .../Hisilicon/Include/Library/OemMiscLib.h | 9 + > .../M41T83RealTimeClock.h | 8 +- > .../D06/Library/OemMiscLibD06/OemMiscLibD06.c | 82 ++++++ > .../Drivers/FlashFvbDxe/FlashFvbDxe.c | 14 +- > .../M41T83RealTimeClockLib.c | 263 ++++++++++++------ > .../Hi1620/Hi1620AcpiTables/Dsdt/Com.asl | 1 - > .../Hi1620AcpiTables/Dsdt/Hi1620Mbig.asl | 48 ---- > .../Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 36 ++- > .../Hi1620/Hi1620AcpiTables/Fadt.aslc | 2 +- > .../Hi1620/Hi1620AcpiTables/Gtdt.aslc | 35 +-- > .../Hi1620/Hi1620AcpiTables/Hi1620Dbg2.aslc | 4 +- > .../Hi1620/Hi1620AcpiTables/Hi1620Iort.asl | 18 +- > .../Hi1620/Hi1620AcpiTables/Hi1620Srat.aslc | 194 ++++++------- > .../Hi1620/Hi1620AcpiTables/MadtHi1620.aslc | 2 +- Can you ensure you use the options specified in https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23 when generating your patches? This way we don't need to guess which files are being modified when looking at the summary. Regards, Leif > 22 files changed, 475 insertions(+), 284 deletions(-) > > -- > 2.18.0 >
Hi Leif, Yes, I generated this patchset with the same git configuration as previous patchset: ([PATCH edk2-platforms v5 00/28] Upload for D06 platform) and use the same command: git format-patch --stat=1000 --stat-graph-width=20 --cover-letter --no-binary --subject-prefix="PATCH edk2-platforms" -12 -v 1 -o v1 I check the URL below, no important different configuration found. Have problems with this patchset? Thanks, Ming On 10/29/2018 7:43 PM, Leif Lindholm wrote: > Hi Ming, > > On Mon, Oct 29, 2018 at 11:32:37AM +0800, Ming Huang wrote: >> The major features of this patchset include: >> 1. Modify acpi table for ACS test; >> 2. Enable secure boot for SBBR-SCT; >> 3. Other change for ACS test; >> >> For this SCT issue: >> RT.SetVariable - Create one Time Base Auth Variable, the expect return status >> should be EFI_SUCCESS – FAILURE >> >> The resule of fail is effected by the edk2 commit(67943427). >> If Modify Variable.c as below, this case will pass. >> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >> @@ -3188,7 +3188,7 @@ VariableServiceSetVariable ( >> // Maybe it's the delete operation of common authenticated variable at >> // user physical presence. >> // >> if (DataSize != AUTHINFO_SIZE) { >> - return EFI_UNSUPPORTED; >> + return EFI_SECURITY_VIOLATION; >> >> I supect ACS SCT compatible with UEFI 2.7 spec. We will analyze this issue >> continue. >> >> Code can also be found in github: >> https://github.com/hisilicon/OpenPlatformPkg.git >> branch: d06-acs-platforms >> >> >> Ming Huang (12): >> Silicon/Hisilicon/D06: Add watchdog to GTDT >> Silicon/Hisilicon/D06: Drop _CID for fwts issue >> Silicon/Hisilicon/D06: Fix fwts issue in Dbg2 >> Silicon/Hisilicon/D06: Fix fwts issue in FADT >> Hisilicon/D06: Move some functions to OemMiscLib >> Silicon/Hisilicon: Modify for SBBR fwts SetTime_Func test case >> Hisilicon/D0x: Fix secure boot bug in FlashFvbDxe >> Hisilicon/D06: Fix SBBR-SCT AuthVar issue >> Silicon/Hisilicon/D06: Reserve ECAM resource in DSDT >> Silicon/Hisilicon/D06: Modify GTDT timer flag >> Hisilicon/D06: Modify Gic base >> Silicon/Hisilicon/D06: Set TA as Node 0 for TA boot >> >> Silicon/Hisilicon/HisiPkg.dec | 1 + >> Silicon/Hisilicon/Hisilicon.dsc.inc | 16 ++ >> Platform/Hisilicon/D03/D03.dsc | 5 + >> Platform/Hisilicon/D05/D05.dsc | 5 + >> Platform/Hisilicon/D06/D06.dsc | 9 +- >> .../Drivers/FlashFvbDxe/FlashFvbDxe.inf | 2 + >> .../M41T83RealTimeClockLib.inf | 3 +- >> .../Hi1620/Hi1620AcpiTables/Hi1620Platform.h | 2 +- >> .../Hisilicon/Include/Library/OemMiscLib.h | 9 + >> .../M41T83RealTimeClock.h | 8 +- >> .../D06/Library/OemMiscLibD06/OemMiscLibD06.c | 82 ++++++ >> .../Drivers/FlashFvbDxe/FlashFvbDxe.c | 14 +- >> .../M41T83RealTimeClockLib.c | 263 ++++++++++++------ >> .../Hi1620/Hi1620AcpiTables/Dsdt/Com.asl | 1 - >> .../Hi1620AcpiTables/Dsdt/Hi1620Mbig.asl | 48 ---- >> .../Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 36 ++- >> .../Hi1620/Hi1620AcpiTables/Fadt.aslc | 2 +- >> .../Hi1620/Hi1620AcpiTables/Gtdt.aslc | 35 +-- >> .../Hi1620/Hi1620AcpiTables/Hi1620Dbg2.aslc | 4 +- >> .../Hi1620/Hi1620AcpiTables/Hi1620Iort.asl | 18 +- >> .../Hi1620/Hi1620AcpiTables/Hi1620Srat.aslc | 194 ++++++------- >> .../Hi1620/Hi1620AcpiTables/MadtHi1620.aslc | 2 +- > > Can you ensure you use the options specified in > https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23 > when generating your patches? > > This way we don't need to guess which files are being modified when > looking at the summary. > > Regards, > > Leif > >> 22 files changed, 475 insertions(+), 284 deletions(-) >> >> -- >> 2.18.0 >>
On Mon, Oct 29, 2018 at 11:01:19PM +0800, Ming Huang wrote: > Hi Leif, > > Yes, I generated this patchset with the same git configuration as previous patchset: > ([PATCH edk2-platforms v5 00/28] Upload for D06 platform) > and use the same command: > git format-patch --stat=1000 --stat-graph-width=20 --cover-letter --no-binary --subject-prefix="PATCH edk2-platforms" -12 -v 1 -o v1 > > I check the URL below, no important different configuration found. > Have problems with this patchset? See below: > Thanks, > Ming > > On 10/29/2018 7:43 PM, Leif Lindholm wrote: > > Hi Ming, > > > > On Mon, Oct 29, 2018 at 11:32:37AM +0800, Ming Huang wrote: > >> The major features of this patchset include: > >> 1. Modify acpi table for ACS test; > >> 2. Enable secure boot for SBBR-SCT; > >> 3. Other change for ACS test; > >> > >> For this SCT issue: > >> RT.SetVariable - Create one Time Base Auth Variable, the expect return status > >> should be EFI_SUCCESS – FAILURE > >> > >> The resule of fail is effected by the edk2 commit(67943427). > >> If Modify Variable.c as below, this case will pass. > >> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > >> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > >> @@ -3188,7 +3188,7 @@ VariableServiceSetVariable ( > >> // Maybe it's the delete operation of common authenticated variable at > >> // user physical presence. > >> // > >> if (DataSize != AUTHINFO_SIZE) { > >> - return EFI_UNSUPPORTED; > >> + return EFI_SECURITY_VIOLATION; > >> > >> I supect ACS SCT compatible with UEFI 2.7 spec. We will analyze this issue > >> continue. > >> > >> Code can also be found in github: > >> https://github.com/hisilicon/OpenPlatformPkg.git > >> branch: d06-acs-platforms > >> > >> > >> Ming Huang (12): > >> Silicon/Hisilicon/D06: Add watchdog to GTDT > >> Silicon/Hisilicon/D06: Drop _CID for fwts issue > >> Silicon/Hisilicon/D06: Fix fwts issue in Dbg2 > >> Silicon/Hisilicon/D06: Fix fwts issue in FADT > >> Hisilicon/D06: Move some functions to OemMiscLib > >> Silicon/Hisilicon: Modify for SBBR fwts SetTime_Func test case > >> Hisilicon/D0x: Fix secure boot bug in FlashFvbDxe > >> Hisilicon/D06: Fix SBBR-SCT AuthVar issue > >> Silicon/Hisilicon/D06: Reserve ECAM resource in DSDT > >> Silicon/Hisilicon/D06: Modify GTDT timer flag > >> Hisilicon/D06: Modify Gic base > >> Silicon/Hisilicon/D06: Set TA as Node 0 for TA boot > >> > >> Silicon/Hisilicon/HisiPkg.dec | 1 + > >> Silicon/Hisilicon/Hisilicon.dsc.inc | 16 ++ > >> Platform/Hisilicon/D03/D03.dsc | 5 + > >> Platform/Hisilicon/D05/D05.dsc | 5 + > >> Platform/Hisilicon/D06/D06.dsc | 9 +- > >> .../Drivers/FlashFvbDxe/FlashFvbDxe.inf | 2 + > >> .../M41T83RealTimeClockLib.inf | 3 +- > >> .../Hi1620/Hi1620AcpiTables/Hi1620Platform.h | 2 +- > >> .../Hisilicon/Include/Library/OemMiscLib.h | 9 + > >> .../M41T83RealTimeClock.h | 8 +- > >> .../D06/Library/OemMiscLibD06/OemMiscLibD06.c | 82 ++++++ > >> .../Drivers/FlashFvbDxe/FlashFvbDxe.c | 14 +- > >> .../M41T83RealTimeClockLib.c | 263 ++++++++++++------ > >> .../Hi1620/Hi1620AcpiTables/Dsdt/Com.asl | 1 - > >> .../Hi1620AcpiTables/Dsdt/Hi1620Mbig.asl | 48 ---- > >> .../Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 36 ++- > >> .../Hi1620/Hi1620AcpiTables/Fadt.aslc | 2 +- > >> .../Hi1620/Hi1620AcpiTables/Gtdt.aslc | 35 +-- > >> .../Hi1620/Hi1620AcpiTables/Hi1620Dbg2.aslc | 4 +- > >> .../Hi1620/Hi1620AcpiTables/Hi1620Iort.asl | 18 +- > >> .../Hi1620/Hi1620AcpiTables/Hi1620Srat.aslc | 194 ++++++------- > >> .../Hi1620/Hi1620AcpiTables/MadtHi1620.aslc | 2 +- These ... suggest --stat=1000 was not used. Can you verify please? Regards, Leif
Hi Ming, I don't know when --stat was introduced, but it was a very long time ago. The oldest version of git I have easily available is 2.1.4, and that handles it properly. Something else must be going on. Regards, Leif On Tue, Oct 30, 2018 at 02:54:42PM +0800, Huangming (Mark) wrote: > Hi Leif, > > The cause of '...' is the git version, it is 2.18.0 in my build server. > I have ask the administrator to upgrade git. > Should I re-send this set(edk2-platforms) with new git version? > > Thanks, > Ming > > On 2018/10/30 0:14, Leif Lindholm wrote: > > On Mon, Oct 29, 2018 at 11:01:19PM +0800, Ming Huang wrote: > >> Hi Leif, > >> > >> Yes, I generated this patchset with the same git configuration as previous patchset: > >> ([PATCH edk2-platforms v5 00/28] Upload for D06 platform) > >> and use the same command: > >> git format-patch --stat=1000 --stat-graph-width=20 --cover-letter --no-binary --subject-prefix="PATCH edk2-platforms" -12 -v 1 -o v1 > >> > >> I check the URL below, no important different configuration found. > >> Have problems with this patchset? > > > > See below: > > > >> Thanks, > >> Ming > >> > >> On 10/29/2018 7:43 PM, Leif Lindholm wrote: > >>> Hi Ming, > >>> > >>> On Mon, Oct 29, 2018 at 11:32:37AM +0800, Ming Huang wrote: > >>>> The major features of this patchset include: > >>>> 1. Modify acpi table for ACS test; > >>>> 2. Enable secure boot for SBBR-SCT; > >>>> 3. Other change for ACS test; > >>>> > >>>> For this SCT issue: > >>>> RT.SetVariable - Create one Time Base Auth Variable, the expect return status > >>>> should be EFI_SUCCESS – FAILURE > >>>> > >>>> The resule of fail is effected by the edk2 commit(67943427). > >>>> If Modify Variable.c as below, this case will pass. > >>>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > >>>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > >>>> @@ -3188,7 +3188,7 @@ VariableServiceSetVariable ( > >>>> // Maybe it's the delete operation of common authenticated variable at > >>>> // user physical presence. > >>>> // > >>>> if (DataSize != AUTHINFO_SIZE) { > >>>> - return EFI_UNSUPPORTED; > >>>> + return EFI_SECURITY_VIOLATION; > >>>> > >>>> I supect ACS SCT compatible with UEFI 2.7 spec. We will analyze this issue > >>>> continue. > >>>> > >>>> Code can also be found in github: > >>>> https://github.com/hisilicon/OpenPlatformPkg.git > >>>> branch: d06-acs-platforms > >>>> > >>>> > >>>> Ming Huang (12): > >>>> Silicon/Hisilicon/D06: Add watchdog to GTDT > >>>> Silicon/Hisilicon/D06: Drop _CID for fwts issue > >>>> Silicon/Hisilicon/D06: Fix fwts issue in Dbg2 > >>>> Silicon/Hisilicon/D06: Fix fwts issue in FADT > >>>> Hisilicon/D06: Move some functions to OemMiscLib > >>>> Silicon/Hisilicon: Modify for SBBR fwts SetTime_Func test case > >>>> Hisilicon/D0x: Fix secure boot bug in FlashFvbDxe > >>>> Hisilicon/D06: Fix SBBR-SCT AuthVar issue > >>>> Silicon/Hisilicon/D06: Reserve ECAM resource in DSDT > >>>> Silicon/Hisilicon/D06: Modify GTDT timer flag > >>>> Hisilicon/D06: Modify Gic base > >>>> Silicon/Hisilicon/D06: Set TA as Node 0 for TA boot > >>>> > >>>> Silicon/Hisilicon/HisiPkg.dec | 1 + > >>>> Silicon/Hisilicon/Hisilicon.dsc.inc | 16 ++ > >>>> Platform/Hisilicon/D03/D03.dsc | 5 + > >>>> Platform/Hisilicon/D05/D05.dsc | 5 + > >>>> Platform/Hisilicon/D06/D06.dsc | 9 +- > >>>> .../Drivers/FlashFvbDxe/FlashFvbDxe.inf | 2 + > >>>> .../M41T83RealTimeClockLib.inf | 3 +- > >>>> .../Hi1620/Hi1620AcpiTables/Hi1620Platform.h | 2 +- > >>>> .../Hisilicon/Include/Library/OemMiscLib.h | 9 + > >>>> .../M41T83RealTimeClock.h | 8 +- > >>>> .../D06/Library/OemMiscLibD06/OemMiscLibD06.c | 82 ++++++ > >>>> .../Drivers/FlashFvbDxe/FlashFvbDxe.c | 14 +- > >>>> .../M41T83RealTimeClockLib.c | 263 ++++++++++++------ > >>>> .../Hi1620/Hi1620AcpiTables/Dsdt/Com.asl | 1 - > >>>> .../Hi1620AcpiTables/Dsdt/Hi1620Mbig.asl | 48 ---- > >>>> .../Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 36 ++- > >>>> .../Hi1620/Hi1620AcpiTables/Fadt.aslc | 2 +- > >>>> .../Hi1620/Hi1620AcpiTables/Gtdt.aslc | 35 +-- > >>>> .../Hi1620/Hi1620AcpiTables/Hi1620Dbg2.aslc | 4 +- > >>>> .../Hi1620/Hi1620AcpiTables/Hi1620Iort.asl | 18 +- > >>>> .../Hi1620/Hi1620AcpiTables/Hi1620Srat.aslc | 194 ++++++------- > >>>> .../Hi1620/Hi1620AcpiTables/MadtHi1620.aslc | 2 +- > > > > These ... suggest --stat=1000 was not used. Can you verify please? > > > > Regards, > > > > Leif > > > > . > > > > -- > Best Regards, > > Ming >
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c @@ -3188,7 +3188,7 @@ VariableServiceSetVariable ( // Maybe it's the delete operation of common authenticated variable at // user physical presence. // if (DataSize != AUTHINFO_SIZE) { - return EFI_UNSUPPORTED; + return EFI_SECURITY_VIOLATION; I supect ACS SCT compatible with UEFI 2.7 spec. We will analyze this issue