Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 27 Jun 2019, Julien Grall wrote:
> On 6/27/19 7:55 PM, Stefano Stabellini wrote:
> > On Mon, 10 Jun 2019, Julien Grall wrote:
> > > +1:
> > > +        /*
> > > +         * Find the second slot used. Remove the entry for the first
> > > +         * table if the slot is not 1 (runtime Xen mapping is 2M - 4M).
> > > +         * For slot 1, it means the ID map was not created.
> > > +         */
> > > +        lsr   x1, x19, #SECOND_SHIFT
> > > +        and   x1, x1, #LPAE_ENTRY_MASK  /* x1 := first slot */
> > > +        cmp   x1, #1
> > > +        beq   id_map_removed
> > > +        /* It is not in slot 1, remove the entry */
> > > +        ldr   x0, =boot_second          /* x0 := second table */
> > > +        str   xzr, [x0, x1, lsl #3]
> > 
> > Wouldn't it be a bit more reliable if we checked whether the slot in
> > question for x19 (whether zero, first, second) is a pagetable pointer or
> > section map, then zero it if it is a section map, otherwise go down one
> > level? If we did it this way it would be independent from the way
> > create_page_tables is written.
> 
> Your suggestion will not comply with the architecture compliance and how Xen
> is/will be working after the full rework. We want to remove everything
> (mapping + table) added specifically for the 1:1 mapping.
> 
> Otherwise, you may end up in a position where boot_first_id is still in place.
> We would need to use the break-before-make sequence in subsequent code if we
> were about to insert 1GB mapping at the same place.
> 
> After my rework, we would have virtually no place where break-before-make will
> be necessary as it will enforce all the mappings to be destroyed before hand.
> So I would rather avoid to make a specific case for the 1:1 mapping.

I don't fully understand your explanation. I understand the final goal
of "removing everything (mapping + table) added specifically for the 1:1
mapping". I don't understand why my suggestion would be a hindrance
toward that goal, compared to what it is done in this patch.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel