Re: [Xen-devel] [PATCH V2] x86/p2m: fixed p2m_change_type_range() start / end check

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 04/18/2018 04:12 PM, Razvan Cojocaru wrote:
> p2m_change_type_range() handles end > max_mapped_pfn, but not
> start > max_mapped_pfn. Check the latter just after grabbing the
> lock and bail if true.
> 
> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> Suggested-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> 
> ---
> Changes since V1:
>  - Added ASSERT()s.
>  - Wrapped the new condition in an unlikely().
> ---
>  xen/arch/x86/mm/p2m.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index c53cab4..e09b256 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -978,8 +978,19 @@ void p2m_change_type_range(struct domain *d,
>      p2m_lock(p2m);
>      p2m->defer_nested_flush = 1;
>  
> +    ASSERT(start < end);
> +
> +    if ( unlikely(start > p2m->max_mapped_pfn) )
> +    {
> +        ASSERT(!p2m_is_hostp2m(p2m));
> +        p2m_unlock(p2m);
> +        return;
> +    }
> +
>      if ( unlikely(end > p2m->max_mapped_pfn) )
>      {
> +        ASSERT(end == ~0UL || !p2m_is_hostp2m(p2m));
> +
>          if ( !gfn )
>          {
>              p2m->change_entry_type_global(p2m, ot, nt);
> 

I think I've addressed George's comments on the altp2m VGA logdirty
issue and I'm ready to send the current version of the patch, however it
depends on this one.

Is the above patch uncontroversial enough to find its way into staging
soon (in which case I should probably wait until it's in to send out the
other patch), or should I perhaps create a series of two patches and
send that out? Or is it maybe customary to proceed somehow else in a
situation like this?


Thank you,
Razvan

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