Re: [Xen-devel] [PATCH 1/6] x86/mm: Use mfn_eq()/mfn_add() rather than opencoded variations

[Jan Beulich]

>>> On 15.08.18 at 20:34,  wrote:
> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -729,7 +729,8 @@ hap_write_p2m_entry(struct domain *d, unsigned long gfn, 
> l1_pgentry_t *p,
>   * unless the only change is an increase in access rights. */
>  mfn_t omfn = l1e_get_mfn(*p);
>  mfn_t nmfn = l1e_get_mfn(new);
> -flush_nestedp2m = !( mfn_x(omfn) == mfn_x(nmfn)
> +
> +flush_nestedp2m = !(mfn_eq(omfn, nmfn)
>  && perms_strictly_increased(old_flags, l1e_get_flags(new)) );

Seeing that you strip the stray leading space, could you strip the stray
trailing one as well, and move the && to its proper place?

With the one previously pointed out issue fixed
Reviewed-by: Jan Beulich 

Jan



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

Re: [Xen-devel] [PATCH 1/6] x86/mm: Use mfn_eq()/mfn_add() rather than opencoded variations

[Roger Pau Monné]

On Wed, Aug 15, 2018 at 07:34:32PM +0100, Andrew Cooper wrote:
> Use l1e_get_mfn() in place of l1e_get_pfn() when applicable, and fix up style
> on affected lines.
> 
> For sh_remove_shadow_via_pointer(), map_domain_page() is guaranteed to succeed
> so there is no need to ASSERT() its success.  This allows the pointer
> arithmetic to folded into the previous expression, and for vaddr to be
> properly typed as l1_pgentry_t, avoiding the cast in l1e_get_mfn().
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Roger Pau Monné 

With one change:

> diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
> index 021ae25..0d74c01 100644
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -960,7 +960,8 @@ static int shadow_set_l4e(struct domain *d,
>  {
>  /* We lost a reference to an old mfn. */
>  mfn_t osl3mfn = shadow_l4e_get_mfn(old_sl4e);
> -if ( (mfn_x(osl3mfn) != mfn_x(shadow_l4e_get_mfn(new_sl4e)))
> +
> +if ( mfn_eq(osl3mfn, shadow_l4e_get_mfn(new_sl4e))

I think this should be !mfn_eq.

Roger.

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