public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
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>
Subject: Re: [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable() cache support
Date: Thu, 3 Oct 2019 13:00:36 +0200	[thread overview]
Message-ID: <0a1fcdde-1605-4217-b888-0ff5d63f12d9@redhat.com> (raw)
In-Reply-To: <B80AF82E9BFB8E4FBD8C89DA810C6A093C940D78@SHSMSX104.ccr.corp.intel.com>

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

  reply	other threads:[~2019-10-03 11:00 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-28  1:47 [PATCH V2 0/9] UEFI Variable SMI Reduction Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 1/9] MdeModulePkg/Variable: Consolidate common parsing functions Kubacki, Michael A
2019-10-03  8:03   ` Wu, Hao A
2019-10-03 17:35     ` Kubacki, Michael A
2019-10-08  2:11       ` Wu, Hao A
2019-10-08 21:53         ` Kubacki, Michael A
2019-10-08  6:07   ` Wang, Jian J
2019-10-08 22:00     ` Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 2/9] MdeModulePkg/Variable: Parameterize GetNextVariableEx() store list Kubacki, Michael A
2019-10-03  8:03   ` Wu, Hao A
2019-10-03 18:04     ` Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 3/9] MdeModulePkg/Variable: Parameterize VARIABLE_INFO_ENTRY buffer Kubacki, Michael A
2019-10-03  8:03   ` Wu, Hao A
2019-10-03 18:05     ` Kubacki, Michael A
2019-10-08  2:11       ` [edk2-devel] " Wu, Hao A
2019-10-08 21:49         ` Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 4/9] MdeModulePkg/Variable: Add local auth status in VariableParsing Kubacki, Michael A
2019-10-03  8:04   ` [edk2-devel] " Wu, Hao A
2019-10-03 18:35     ` Kubacki, Michael A
2019-10-16  7:55       ` Wu, Hao A
2019-10-16 16:37         ` Kubacki, Michael A
2019-10-17  1:00           ` Wu, Hao A
2019-09-28  1:47 ` [PATCH V2 5/9] MdeModulePkg/Variable: Add a file for NV variable functions Kubacki, Michael A
2019-10-03  8:04   ` Wu, Hao A
2019-10-03 18:43     ` Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 6/9] MdeModulePkg VariableInfo: Always consider RT DXE and SMM stats Kubacki, Michael A
2019-10-03  8:04   ` Wu, Hao A
2019-09-28  1:47 ` [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable() cache support Kubacki, Michael A
2019-10-03  8:04   ` Wu, Hao A
2019-10-03 11:00     ` Laszlo Ersek [this message]
2019-10-03 20:53       ` Kubacki, Michael A
2019-10-03 21:53     ` Kubacki, Michael A
2019-10-03 22:01       ` Michael D Kinney
2019-10-03 23:31         ` Kubacki, Michael A
2019-10-04  6:50           ` Laszlo Ersek
2019-10-04 16:48             ` Kubacki, Michael A
2019-10-04  6:38       ` Laszlo Ersek
2019-10-04 16:48         ` Kubacki, Michael A
2019-10-08  2:12       ` Wu, Hao A
2019-09-28  1:47 ` [PATCH V2 8/9] MdeModulePkg/Variable: Add RT GetNextVariableName() " Kubacki, Michael A
2019-10-03  8:04   ` Wu, Hao A
2019-10-03 18:52     ` Kubacki, Michael A
2019-10-03 18:59       ` [edk2-devel] " Andrew Fish
2019-10-03 20:12         ` Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 9/9] MdeModulePkg/VariableSmm: Remove unused SMI handler functions Kubacki, Michael A

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0a1fcdde-1605-4217-b888-0ff5d63f12d9@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox