Re: [Xen-devel] [PATCH RFC 32/55] efi: switch EFI L4 table to use new APIs

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.04.19 at 14:23, <wei.liu2@xxxxxxxxxx> wrote:
> On Mon, Mar 18, 2019 at 09:14:44PM +0000, Nuernberger, Stefan wrote:
>> On Thu, 2019-02-07 at 16:44 +0000, Wei Liu wrote:
>> > This requires storing the MFN instead of linear address of the L4
>> > table. Adjust code accordingly.
>> > 
>> > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
>> > ---
>> >  xen/arch/x86/efi/runtime.h | 12 +++++++++---
>> >  xen/common/efi/boot.c      |  8 ++++++--
>> >  xen/common/efi/efi.h       |  3 ++-
>> >  xen/common/efi/runtime.c   |  8 ++++----
>> >  4 files changed, 21 insertions(+), 10 deletions(-)
>> > 
>> > diff --git a/xen/arch/x86/efi/runtime.h b/xen/arch/x86/efi/runtime.h
>> > index d9eb8f5c27..277d237953 100644
>> > --- a/xen/arch/x86/efi/runtime.h
>> > +++ b/xen/arch/x86/efi/runtime.h
>> > @@ -2,11 +2,17 @@
>> >  #include <asm/mc146818rtc.h>
>> >  
>> >  #ifndef COMPAT
>> > -l4_pgentry_t *__read_mostly efi_l4_pgtable;
>> > +mfn_t __read_mostly efi_l4_mfn = INVALID_MFN_INITIALIZER;
>> >  
>> >  void efi_update_l4_pgtable(unsigned int l4idx, l4_pgentry_t l4e)
>> >  {
>> > -    if ( efi_l4_pgtable )
>> > -        l4e_write(efi_l4_pgtable + l4idx, l4e);
>> > +    if ( !mfn_eq(efi_l4_mfn, INVALID_MFN) )
>> > +    {
>> > +        l4_pgentry_t *l4t;
>> > +
>> > +        l4t = map_xen_pagetable_new(efi_l4_mfn);
>> > +        l4e_write(l4t + l4idx, l4e);
>> > +        UNMAP_XEN_PAGETABLE_NEW(l4t);
>> 
>> nit: This doesn't need the implicit NULL assignment. The non-macro
>> unmap_xen_pagetable_new is sufficient here. Do you have a guideline on
>> when to use the macro over the function call? I assume the compiler
>> eliminates most of the dead stores on function return.
>> 
> 
> I introduced the macro such that any misuse of stale pointer could be
> caught as early as possible. We've got similar construct -- see XFREE.
> 
> I certainly hope compilers can become smart enough. We can also
> eliminate useless assignment in a later series. I didn't spend much time
> going over all the places during the development of this series.

But it surely wouldn't be too hard to switch to the lower-case
sibling function in cases where the NULL assignment is pointless?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel