On 1/15/24 03:59, Liu, Zhiguang wrote: > Hi Laszlo, > > I don't think it is a good idea to explicitly mask out the Accessed/Dirty > bit. We can't assume Accessed/Dirty bit are only changed by hardware, because > the caller also can change the Accessed/Dirty bit. > > For API PageTableMap, the IsModified is already set as False in the beginning > of the function. > For internal function PageTableLibMapInLevel, we don't set IsModified as > False in the beginning on purpose, because it keeps the global state of > whether the PageTable is changed. > > I plan to change the comment as below to explicitly explain the behavior: > > For internal function PageTableLibMapInLevel, the description of IsModified > should be: > @param[in, out] IsModified change IsModified to True if page table is > modified and input parameter Modify is TRUE. > > For API PageTableMap, the description of IsModified should be: > @param[out] IsModified TRUE means page table is modified by software > or hardware. FALSE means page table is not modified by software. > If the output IsModified is FALSE, there is possibility that the page table > is changed by hardware. It is ok because page table can be changed by > hardware anytime, and we don't need to Flush TLB. > > With these comments changed, I don't need to change C code in my patch.
OK, sounds reasonable to me. Thanks. > > BTW, I assume IsModified can be used as an indicator whether the caller need > to flush TLB. Do you prefer to change the parameter name to IsFlushTlbNeeded? > I am both fine. I think the new description of the parameter suffices (without renaming the parameter). Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113838): https://edk2.groups.io/g/devel/message/113838 Mute This Topic: https://groups.io/mt/103636407/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
