Re: [PATCHv4 04/14] xen: remove scratch frames for ballooned pages and m2p override

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Tue, 27 Jan 2015, David Vrabel wrote:
> On 27/01/15 11:10, Stefano Stabellini wrote:
> > On Tue, 27 Jan 2015, David Vrabel wrote:
> >> On 27/01/15 10:57, Stefano Stabellini wrote:
> >>> On Mon, 26 Jan 2015, David Vrabel wrote:
> >>>> The scratch frame mappings for ballooned pages and the m2p override
> >>>> are broken.  Remove them in preparation for replacing them with
> >>>> simpler mechanisms that works.
> >>>>
> >>>> The scratch pages did not ensure that the page was not in use.  In
> >>>> particular, the foreign page could still be in use by hardware.  If
> >>>> the guest reused the frame the hardware could read or write that
> >>>> frame.
> >>>>
> >>>> The m2p override did not handle the same frame being granted by two
> >>>> different grant references.  Trying an M2P override lookup in this
> >>>> case is impossible.
> >>>>
> >>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> >>>
> >>> [...]
> >>>
> >>>>  int set_foreign_p2m_mapping(struct gnttab_map_grant_ref *map_ops,
> >>>>  			    struct gnttab_map_grant_ref *kmap_ops,
> >>>>  			    struct page **pages, unsigned int count)
> >>>>  {
> >>>>  	int i, ret = 0;
> >>>> -	bool lazy = false;
> >>>>  	pte_t *pte;
> >>>>  
> >>>>  	if (xen_feature(XENFEAT_auto_translated_physmap))
> >>>>  		return 0;
> >>>>  
> >>>> -	if (kmap_ops &&
> >>>> -	    !in_interrupt() &&
> >>>> -	    paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
> >>>> -		arch_enter_lazy_mmu_mode();
> >>>> -		lazy = true;
> >>>> +	if (kmap_ops) {
> >>>> +		ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
> >>>> +						kmap_ops, count);
> >>>> +		if (ret)
> >>>> +			goto out;
> >>>>  	}
> >>>
> >>> I think that you should explain why you are adding one more grant table
> >>> mapping to a function meant to work on the p2m/m2p.  Everything else is
> >>> clear.
> >>
> >> I'm not.  This function has always done this.
> > 
> > Can you be more specific please?
> > 
> > I cannot see a call to HYPERVISOR_grant_table_op among the lines you are
> > removing from this function in this patch.
> 
> This function called m2p_add_override() which did the grant map
> hypercall (an op at a time).

OK. Add to the commit message please.