On 21.09.2021 10:32, Roger Pau Monné wrote: > 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?
The difference isn't with guest_physmap_add_entry() (both x86 and Arm only insert a new mapping there), but with xenmem_add_to_physmap_one(): Arm's variant doesn't care about prior mappings. And gnttab_map_frame() gets called only from there. This is effectively what the first paragraph of the description is about. > 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. Yes. > This seems like a limitation of Arm's guest_physmap_add_entry. As per above I'm viewing this as a limitation of Arm's xenmem_add_to_physmap_one(). Jan
