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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Apr 09, 2019 at 06:56:56AM -0600, Jan Beulich wrote:
> >>> 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?

It can be done when I'm reasonably sure about the final incarnation of
this series.

Wei.

> 
> Jan
> 
> 

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