Re: [Xen-devel] [PATCH RFC v4] x86/mm: Clean up p2m_finish_type_change return value

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.03.19 at 16:21, <aisaila@xxxxxxxxxxxxxxx> wrote:
> @@ -621,7 +623,7 @@ bool_t ept_handle_misconfig(uint64_t gpa)
>  
>      p2m_unlock(p2m);
>  
> -    return spurious ? (rc >= 0) : (rc > 0);
> +    return spurious && !rc;
>  }

I think you've gone too far now: This is - afaict - the one place
where the distinction matters. Looking back at Paul's reply and
my subsequent one on v3, I'm afraid I've managed to misguide
you by not looking closely enough at what Paul did sketch out.
I'm sorry for this.

I think you either want to leave EPT code untouched, and zap
the positive return value in finish_type_change() instead. Or
EPT's resolve_misconfig() would need to gain a wrapper for use
as the ->recalc hook, to squash the positive value for the outside
world. Iirc I've avoided introducing such a wrapper originally
just to limit the number of functions layered on top of one
another, while using resolve_misconfig() directly appeared to
be fine.

> @@ -668,7 +670,7 @@ ept_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t 
> mfn,
>  
>      /* Carry out any eventually pending earlier changes first. */
>      ret = resolve_misconfig(p2m, gfn);
> -    if ( ret < 0 )
> +    if ( ret )
>          return ret;

This would then need undoing as well.

> @@ -1201,7 +1201,7 @@ int p2m_finish_type_change(struct domain *d,
>  
>      rc = finish_type_change(hostp2m, first_gfn, max_nr);
>  
> -    if ( rc < 0 )
> +    if ( rc )
>          goto out;

While I don't really object to this change, I also don't think it's
strictly necessary.

Jan



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