|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used
Hi Stefano, On 7/31/19 9:40 PM, Stefano Stabellini wrote: On Tue, 30 Jul 2019, Julien Grall wrote: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.I understand your comment now, and of course I agree that both mapping and tables need to be removed. I am careful making suggestions for assembly coding because I don't really want to suggest something that doesn't work, or even if it works that it's worse than the original. It should be possible to remove both the table and the mapping in a generic way. Instead of hardcoding the assemply equivalent of "It is not in slot 0, remove the entry", we could check whether the table offset matches the table offset of the mapping that we want to preserve. That way, "slot 0" would be calculate instead of hardcoded, and the code would be pretty generic. What do you think? It should only be a small addition. It should be feasible and may actually help the next step in my plan where I need to make Xen relocatable. I will have a look for both the arm32 and arm64 code. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |