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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 7/30/19 6:33 PM, Stefano Stabellini wrote:
> 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.
Because, AFAICT, your suggestion will only remove the mapping and not 
the tables (such as boot_first_id). This is different from this patch 
where both mapping and tables are removed.
So yes, my suggestion is not generic, but at least it does the job that 
is expected by this function. I.e removing anything that was 
specifically created for the identity mapping.
Cheers,

--
Julien Grall

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