Remove physmap page upon granttab xfer

Hi Espen.

I think the issues exist in all functions in linux kernel
which uses MMU_MACHPHYS_UPDATE. gnttab_copy_grant_page(),
netbk_gop_frag(), xennet_get_responses() and
netif_release_rx_bufs_flip()
The p2m/m2p table manipulation is done by
MMU_MACHPHYS_UPDATE on x86.
(More exactly the p2m operation is done in linux kernel on
x86.)
So at the line after steal_page(), the p2m/m2p table is
supposed to be
already updated. It implys that iommu is also supposed to be

updated already.
Although I know that grant table page transfer is
obsoleted,
what is the expected exact semantics of GNTTABOP_transfer
with
the p2m/m2p table?

I suppose the right fix would be 
- revert this patch and
- put the iommu opration of pv domain into
MMU_MACHPHYS_UPDATE.

Some comments on xen/ia64 side:
This patch breaks xen/ia64 grant table page transfer.
When I made netback/netfront work on ia64, I thought the
tranfer
operation shouldn't change the m2p/p2m table.
On the other hand, I didn't want to add new hypecall to
manipulate
the m2p/p2m table for auto translated mode that corresponds
to MMU_MACHPHYS_UPDATE and set_phys_to_machine().
So I worked around it such that xen/ia64 steal_page() does
the p2m/m2p manipulation and fill new page after page owner
change.
(There is a risk to fail to allocate a new page during page
tranfer,
though.)

thanks

On Fri, May 23, 2008 at 01:30:16PM +0100, Espen Skoglund
wrote:
> Mapping should be removed from current domain's p2m
table when doing a
> grant table transfer.
> 
> Signed-off-by: Espen Skoglund <espen.skoglundnetronome.com>
> 
> 
>
============================================================
====
> diff -r 2e6a51378451 xen/common/grant_table.c
> --- a/xen/common/grant_table.c	Thu May 22 15:11:06 2008
+0100
> +++ b/xen/common/grant_table.c	Fri May 23 13:23:56 2008
+0100
>  -1073,6 +1073,8 
>              gop.status = GNTST_bad_page;
>              goto copyback;
>          }
> +
> +        guest_physmap_remove_page(d, gop.mfn, mfn,
0);
>  
>          /* Find the target domain. */
>          if ( unlikely((e =
rcu_lock_domain_by_id(gop.domid)) == NULL) )
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devellists.xensource.com
> http://lists.xen
source.com/xen-devel
> 

-- 
yamahata

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devellists.xensource.com
http://list
s.xensource.com/xen-ia64-devel

On Wed, May 28, 2008 at 11:39:16AM +0100, Keir Fraser
wrote:
> On 28/5/08 11:28, "Isaku Yamahata"
<yamahatavalinux.co.jp> wrote:
> 
> > Although I know that grant table page transfer is
obsoleted,
> > what is the expected exact semantics of
GNTTABOP_transfer with
> > the p2m/m2p table?
> > 
> > I suppose the right fix would be
> > - revert this patch and
> > - put the iommu opration of pv domain into
MMU_MACHPHYS_UPDATE.
> 
> After the grant transfer operation completes the page
no longer belongs to
> the backend domain. Thus, for absolute safety and
isolation, the page must
> be removed from the backend domain's IOMMU mappings
before completion of the
> grant transfer operation. Actually we should be
flushing host CPU TLBs too.

Thank you for explanation.
The patch breaks ia64 granttab xfer breakage so that it have
to be fixed.
However the operation is now obsoleted one, so I don't want
to make
big effort to fix.
What do you think about the following patch?


Signed-off-by: Isaku Yamahata <yamahatavalinux.co.jp>

--- ./grant_table.c     2008-05-28 18:47:40.000000000 +0900
+++ ./grant_table.c-if  2008-05-29 18:27:05.000000000 +0900
 -1112,7
+1112,8  gnttab_transfer(
             goto copyback;
         }
 
-        guest_physmap_remove_page(d, gop.mfn, mfn, 0);
+        if (!paging_mode_translate(d))
+            guest_physmap_remove_page(d, gop.mfn, mfn, 0);
 
         /* Find the target domain. */
         if ( unlikely((e =
rcu_lock_domain_by_id(gop.domid)) == NULL)
)

or #ifndef __ia64__ 

-- 
yamahata

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devellists.xensource.com
http://list
s.xensource.com/xen-ia64-devel