Re: [Xen-devel] [PATCH for-4.5 v7 14/21] xen/arm: p2m changes for mem_access support

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-09-17 at 22:51 +0200, Tamas K Lengyel wrote:
> Add p2m_access_t to struct page_info and add necessary changes for
> page table construction routines to pass the default access information.
> We store the p2m_access_t info in page_info as the PTE lacks enough
> software programmable bits.
> 
> Signed-off-by: Tamas K Lengyel <tklengyel@xxxxxxxxxxxxx>
> ---
> v7: - Remove radix tree init/destroy and move p2m_access_t store to page_info.

I'm afraid I don't think this will work, since a single page can be
mapped into multiple guests (e.g. granted), possibly multiple times per
guest (less likely I admit) and AIUI the access type is at least per
guest if not per p2m mapping.

The radix tree would have been tolerable if there was some sort of quick
to check flag in p2m->foo to indicate if it is in use for a given
domain. (I previously suggested a sentinal p2m_access_t value for
example, but a bool etc would work too).

I don't especially like trusting radix_lookup to be cheap for an empty
tree in such a hotpatch as p2m lookup since a) I couldn't trivially
determine that it was cheap in such cases by inspection and b) we then
become dependent on the radix tree implementation.

>     - Add p2m_gpfn_lock/unlock functions.
>     - Add bool_t lock input to p2m_lookup and apply_p2m_changes so the caller
>       can specify if locking should be performed. This is needed in order to
>       support mem_access_check from common.

Based on the review of the patch which added mem_access_check resulting
in it not being refactored I expect most of this patch will be changing
again, so I won't look at it further unless you ask me to. (except to
note for future reference that if you have a bool_t parameter you should
genherally pass true/false/bool-expr not 0/1)

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel