Re: [Xen-devel] [PATCH RFC] xen/grant_table: deprecate GNTTABOP_unmap_and_replace

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.07.13 at 12:15, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
wrote:
> On Sun, 21 Jul 2013, Ian Campbell wrote:
>> On Sun, 2013-07-21 at 18:34 +0100, Stefano Stabellini wrote:
>> > GNTTABOP_unmap_and_replace has two issues:
>> > - it unconditionally replaces the mapping passed in new_addr with 0;
>> > - it doesn't support GNTMAP_contains_pte mappings on x86, returning a
>> > general error instead of some forms of ENOSYS.
>> > 
>> > Deprecate GNTTABOP_unmap_and_replace and introduce a new
>> > GNTTABOP_unmap_and_replace (12) that returns GNTST_enosys for
>> > GNTMAP_contains_pte requests and doesn't zero the mapping at new_addr.
>> > 
>> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>> > ---
>> >  xen/arch/x86/mm.c                |   12 +-----------
>> >  xen/include/public/grant_table.h |    7 ++++---
>> >  2 files changed, 5 insertions(+), 14 deletions(-)
>> > 
>> > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
>> > index 77dcafc..610fd09 100644
>> > --- a/xen/arch/x86/mm.c
>> > +++ b/xen/arch/x86/mm.c
>> > @@ -4064,7 +4064,7 @@ int replace_grant_host_mapping(
>> >              return destroy_grant_pte_mapping(addr, frame, curr->domain);
>> >          
>> >          MEM_LOG("Unsupported grant table operation");
>> > -        return GNTST_general_error;
>> > +        return GNTST_enosys;
>> >      }
>> >  
>> >      if ( !new_addr )
>> > @@ -4102,16 +4102,6 @@ int replace_grant_host_mapping(
>> >  
>> >      ol1e = *pl1e;
>> >  
>> > -    if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, l1e_empty(),
>> > -                                gl1mfn, curr, 0)) )
>> > -    {
>> > -        page_unlock(l1pg);
>> > -        put_page(l1pg);
>> > -        MEM_LOG("Cannot delete PTE entry at %p", (unsigned long *)pl1e);
>> > -        guest_unmap_l1e(curr, pl1e);
>> > -        return GNTST_general_error;
>> > -    }
>> 
>> Doesn't this break all existing users of the subop? I think you need to
>> stick a if (op == unmap_and_replace_legacy) in here and keep the code
>> for that case.
> 
> I don't think there are any existing users, but that's a fair point.

As was hopefully implicit from David's response - there _are_ users
of this interface.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel