Re: [PATCH 1/2] gnttab: remove guest_physmap_remove_page() call from gnttab_map_frame()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Sep 20, 2021 at 05:27:17PM +0200, Jan Beulich wrote:
> On 20.09.2021 12:20, Roger Pau Monné wrote:
> > On Mon, Sep 13, 2021 at 08:41:47AM +0200, Jan Beulich wrote:
> >> --- a/xen/include/asm-arm/grant_table.h
> >> +++ b/xen/include/asm-arm/grant_table.h
> >> +        if ( gfn_eq(ogfn, INVALID_GFN) || gfn_eq(ogfn, gfn) ||           \
> > 
> > I'm slightly confused by this checks, don't you need to check for
> > gfn_eq(gfn, INVALID_GFN) (not ogfn) in order to call
> > guest_physmap_remove_page?
> 
> Why? It's ogfn which gets passed to the function. And it indeed is the
> prior GFN's mapping that we want to remove here.
> 
> > Or assuming that ogfn is not invalid can be used to imply a removal?
> 
> That implication can be (and on x86 is) used for the incoming argument,
> i.e. "gfn". I don't think "ogfn" can serve this purpose.

I guess I'm confused due to the ogfn checks done on the Arm side that
are not performed on x86. So on Arm you always need to explicitly
unhook the previous GFN before attempting to setup a new mapping,
while on x86 you only need to do this when it's a removal in order to
clear the entry?

So you are effectively only removing the call to
guest_physmap_remove_page in gnttab_map_frame for x86, because Arm
will still perform it in gnttab_set_frame_gfn.

This seems like a limitation of Arm's guest_physmap_add_entry.

Thanks, Roger.