Re: [Xen-devel] [PATCH 2/3] Rename p2m_mmio_write_dm to p2m_ioreq_server

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 16/03/16 12:21, Yu Zhang wrote:
> Previously p2m type p2m_mmio_write_dm was introduced for write-
> protected memory pages whose write operations are supposed to be
> forwarded to and emulated by an ioreq server. Yet limitations of
> rangeset restricts the number of guest pages to be write-protected.
> 
> This patch replace the p2m type p2m_mmio_write_dm with a new name:
> p2m_ioreq_server, which means this p2m type can be claimed by one
> ioreq server, instead of being tracked inside the rangeset of ioreq
> server. Patches following up will add the related hvmop handling
> code which maps type p2m_ioreq_server to an ioreq server.
> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Signed-off-by: Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx>
> ---

[snip]

> @@ -174,7 +174,8 @@ typedef unsigned int p2m_query_t;
>  
>  #define p2m_is_any_ram(_t)  (p2m_to_mask(_t) &                   \
>                               (P2M_RAM_TYPES | P2M_GRANT_TYPES |  \
> -                              p2m_to_mask(p2m_map_foreign)))
> +                              p2m_to_mask(p2m_map_foreign) | \
> +                              p2m_to_mask(p2m_ioreq_server)))

So this is the only bit that doesn't seem to be a straight rename.
What's the rationale for adding this in this patch?

And in any case, we want to add this to P2M_RAM_TYPES, don't we, so that
p2m_is_ram() returns true?

For example, if p2m_ioreq_server is *not* marked as p2m_is_ram(), then
when it's ballooned out the m2p mapping won't be updated properly (see
xen/arch/x86/mm/p2m.c:set_typed_p2m_entry()); and it looks like there
could be issues with emulation when running in shadow mode (see
xen/arch/x86/mm/common.c:emulate_gva_to_mfn()).  Other examples of this
sort of thing abound.

Everything else looks fine to me.

 -George


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