From mboxrd@z Thu Jan  1 00:00:00 1970
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com)
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
 by groups.io with SMTP; Thu, 03 Oct 2019 04:00:40 -0700
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 mx1.redhat.com (Postfix) with ESMTPS id CB30A883CA;
	Thu,  3 Oct 2019 11:00:39 +0000 (UTC)
Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-154.rdu2.redhat.com [10.10.120.154])
	by smtp.corp.redhat.com (Postfix) with ESMTP id 7525F6012C;
	Thu,  3 Oct 2019 11:00:37 +0000 (UTC)
Subject: Re: [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable() cache support
To: "Wu, Hao A" <hao.a.wu@intel.com>,
 "Kubacki, Michael A" <michael.a.kubacki@intel.com>,
 "devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Bi, Dandan" <dandan.bi@intel.com>,
 Ard Biesheuvel <ard.biesheuvel@linaro.org>, "Dong, Eric"
 <eric.dong@intel.com>, "Gao, Liming" <liming.gao@intel.com>,
 "Kinney, Michael D" <michael.d.kinney@intel.com>, "Ni, Ray"
 <ray.ni@intel.com>, "Wang, Jian J" <jian.j.wang@intel.com>,
 "Yao, Jiewen" <jiewen.yao@intel.com>
References: <20190928014717.31372-1-michael.a.kubacki@intel.com>
 <20190928014717.31372-8-michael.a.kubacki@intel.com>
 <B80AF82E9BFB8E4FBD8C89DA810C6A093C940D78@SHSMSX104.ccr.corp.intel.com>
From: "Laszlo Ersek" <lersek@redhat.com>
Message-ID: <0a1fcdde-1605-4217-b888-0ff5d63f12d9@redhat.com>
Date: Thu, 3 Oct 2019 13:00:36 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <B80AF82E9BFB8E4FBD8C89DA810C6A093C940D78@SHSMSX104.ccr.corp.intel.com>
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 03 Oct 2019 11:00:39 +0000 (UTC)
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit

On 10/03/19 10:04, Wu, Hao A wrote:
> Before any comment on the patch, since I am not experienced in the Variable
> driver, I would like to ask for help from other reviewers to look into this
> patch and provide feedbacks as well. Thanks in advance.
> 
> With the above fact, some comments provided below maybe wrong. So please help
> to kindly correct me.
> 
> 
> Some general comments:
> 1. I am not sure if bringing the TimerLib dependency (delay in acquiring the
>    runtime cache read lock) to variable driver (a software driver for the most
>    part) is a good idea.

I agree. Most TimerLib instances do not expect sharing the hardware with
the OS.


Another complication is if the hardware is accessed via MMIO (that is,
not IO ports). MMIO accesses are subject to page tables.

Assuming that MicroSecondDelay() is invoked from the runtime DXE driver
at OS runtime, a platform would have to expose the MMIO area of the
timer hardware in the UEFI memory map as "runtime MMIO". (Via GCD memory
space operations in a platform driver or in the TimerLib constructor.)

Furthermore, the constructor function of the TimerLib instance would
have to register a VirtualAddressChange event handler, and convert the
MMIO address.

Thanks
Laszlo