From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web10.10850.1599648657378712819 for ; Wed, 09 Sep 2020 03:50:57 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=U9CzTgHe; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1599648656; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6IltBYHWsFKrn6s29+W6QYsVp6wB3Ry7oIzyrWn7SXw=; b=U9CzTgHeBgRxmwfenYmYfHcLi4g9Mud8PsrgKHpGlol4igSnocWeos37C3yyXlxNNjlaRg DId5s5XTy7+987jzZ1HYvHCbrsqPxD4no7+cVyKVgldBza7janyGammk1Bsb86ivPgWvhV W1H7YtyznVKKDDS0u15N0zccZoyg04Q= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-143-rdS1nSt2NmW2Elhw3bLByQ-1; Wed, 09 Sep 2020 06:50:52 -0400 X-MC-Unique: rdS1nSt2NmW2Elhw3bLByQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C244318B9F09; Wed, 9 Sep 2020 10:50:50 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-82.ams2.redhat.com [10.36.114.82]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7A8A960C15; Wed, 9 Sep 2020 10:50:48 +0000 (UTC) Subject: Re: [PATCH v10 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand To: Vladimir Olovyannikov , "Rabeda, Maciej" , devel@edk2.groups.io, Zhichao Gao , Ray Ni Cc: Samer El-Haj-Mahmoud , Jiaxin Wu , Siyuan Fu , Liming Gao , Nd References: <20200902040821.24144-1-vladimir.olovyannikov@broadcom.com> <20200902040821.24144-2-vladimir.olovyannikov@broadcom.com> <30fc1fc2-858f-f0e8-e54f-58b872ad57ae@linux.intel.com> <30233c17-c081-deb6-d0cb-d847c7a9992b@redhat.com> <20ac79e77049e8756de36f50df1e7f36@mail.gmail.com> <8aa008fc-ed91-5118-df37-b24f2eb86c2f@redhat.com> From: "Laszlo Ersek" Message-ID: <5534f1de-b9b7-0981-e8c6-de0e4c25c617@redhat.com> Date: Wed, 9 Sep 2020 12:50:47 +0200 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0.003 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US On 09/08/20 23:04, Vladimir Olovyannikov wrote: >> -----Original Message----- >> From: Laszlo Ersek >> Sent: Monday, September 7, 2020 2:37 AM >> To: Vladimir Olovyannikov ; >> Rabeda, Maciej ; devel@edk2.groups.io; >> Zhichao Gao ; Ray Ni >> Cc: Samer El-Haj-Mahmoud ; Jiaxin Wu >> ; Siyuan Fu ; Liming Gao >> ; Nd >> Subject: Re: [PATCH v10 1/1] ShellPkg/DynamicCommand: add >> HttpDynamicCommand >> >> On 09/04/20 19:55, Vladimir Olovyannikov wrote: >> >>> There is also another issue with the TimebaseLib: inconsistency in >>> return values of the EfiTimeToEpoch (returns UINT32, should return >>> UINTN, as Zhichao pointed out earlier in the previous >>> HttpDynamicCommand patchset). >>> If this one is fixed, I can just use the TimeBaseLib.h header for >>> constants. >> >> Consuming TimeBaseLib.h in this patch would be really nice. > OK, if this can be fixed, I will definitely use TimeBaseLib.h header for > constants, and will drop > duplicate definitions in Http.c/Http.h >> >> There are two EfiTimeToEpoch() call sites in edk2: >> >> ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c >> EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c >> >> The latter stores the return value in a UINTN variable, so that seems OK. >> The >> former is a bit messier, but it seems to ensure that the result fits in 32 >> bits for >> HW reasons anyway: >> >> // Because the PL031 is a 32-bit counter counting seconds, >> // the maximum time span is just over 136 years. >> // Time is stored in Unix Epoch format, so it starts in 1970, >> // Therefore it can not exceed the year 2106. >> if ((Time->Year < 1970) || (Time->Year >= 2106)) { >> return EFI_UNSUPPORTED; >> } >> ... >> EpochSeconds = EfiTimeToEpoch (Time); >> ... >> MmioWrite32 (mPL031RtcBase + PL031_RTC_LR_LOAD_REGISTER, >> EpochSeconds); >> >> So I think we'd need two patches: >> >> (1) add an explicit (UINT32) cast to the EfiTimeToEpoch() call in >> "ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c", >> >> (2) change the return value to UINTN in >> "EmbeddedPkg/Include/Library/TimeBaseLib.h" and >> "EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c". >> >> Hmm wait... There are five more call sites in edk2-platforms. :( OK, I >> give up >> here. Sorry. > OK, so what are the next steps, what do you suggest? You'd have to audit, and if necessary, clean up, the EfiTimeToEpoch() call sites in edk2-platforms. And you'd need to establish a global order between the patch series (plural) such that both edk2 and edk2-platforms should build at any stage across those series. Thanks Laszlo > I saw today that unused macros like SEC_PER_MONTH, etc. were removed from > TimeBaseLib.h. > > Thank you, > Vladimir >> >> Laszlo >