From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120])
 by mx.groups.io with SMTP id smtpd.web10.37776.1599471426464812948
 for <devel@edk2.groups.io>;
 Mon, 07 Sep 2020 02:37:06 -0700
Authentication-Results: mx.groups.io;
 dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Y2nnSRaH;
 spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
	s=mimecast20190719; t=1599471425;
	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=iSUxHcpJibE9NfBk14FfwA5NqJNbhNJhJfQ9mdZJNJE=;
	b=Y2nnSRaHc8UyerQ/7Z559uk5lWlgffkVEdjDSAXAs+lehmK9nchAbGzuUghoSez1B1l/Ex
	n3TuwEwXoGRXF5EIIEEzPru+CmabUcUHJIDjZRDEP7LTGEVdzeIIbIgW8VRfZHnf3sXT0a
	1ux9KRulwXhoCeS4ZB6J7BKi9m4sE6o=
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-378-zPmg-jFFMyOLFgSVRRUUjQ-1; Mon, 07 Sep 2020 05:37:01 -0400
X-MC-Unique: zPmg-jFFMyOLFgSVRRUUjQ-1
Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])
	(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
	(No client certificate requested)
	by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 149CF1DDEC;
	Mon,  7 Sep 2020 09:37:00 +0000 (UTC)
Received: from lacos-laptop-7.usersys.redhat.com (ovpn-115-56.ams2.redhat.com [10.36.115.56])
	by smtp.corp.redhat.com (Postfix) with ESMTP id 9D3FC7ED60;
	Mon,  7 Sep 2020 09:36:57 +0000 (UTC)
Subject: Re: [PATCH v10 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand
To: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>,
 "Rabeda, Maciej" <maciej.rabeda@linux.intel.com>, devel@edk2.groups.io,
 Zhichao Gao <zhichao.gao@intel.com>, Ray Ni <ray.ni@intel.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>,
 Jiaxin Wu <jiaxin.wu@intel.com>, Siyuan Fu <siyuan.fu@intel.com>,
 Liming Gao <liming.gao@intel.com>, Nd <nd@arm.com>
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>
From: "Laszlo Ersek" <lersek@redhat.com>
Message-ID: <8aa008fc-ed91-5118-df37-b24f2eb86c2f@redhat.com>
Date: Mon, 7 Sep 2020 11:36:56 +0200
MIME-Version: 1.0
In-Reply-To: <20ac79e77049e8756de36f50df1e7f36@mail.gmail.com>
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11
Authentication-Results: relay.mimecast.com;
	auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com
X-Mimecast-Spam-Score: 0.002
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-US

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.

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.

Laszlo