Re: [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable() cache support

["Kubacki, Michael A" <michael.a.kubacki@intel.com>]

I understand the concern. I'm aware code is sometimes marked as runtime compatible
though it is actually not and meeting all the criteria to work in the runtime environment
is non-trivial as you noted.

In response to series #2 patch #8, Andrew noted that the UEFI spec defines restrictions
on Runtime Services callers that prohibit re-entry into the GetVariable () and
GetNextVariableName () services. With this restriction in mind, I believe the polling can be
removed and the lock simply serve as an indicator to SMM whether a SMI that invokes
SetVariable () interrupted a runtime read.

Thanks,
Michael

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Thursday, October 3, 2019 4:01 AM
> To: Wu, Hao A <hao.a.wu@intel.com>; Kubacki, Michael A
> <michael.a.kubacki@intel.com>; 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
> 
> 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