Re: [PATCH v2 06/14] x86/P2M: PoD, altp2m, and nested-p2m are HVM-only

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Feb 23, 2022, at 4:01 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> 
> There's no need to initialize respective data for PV domains. Note that
> p2m_teardown_{alt,nested}p2m() will handle the lack-of-initialization
> case fine.
> 
> As a result, despite PV domains having a host P2M associated with them
> and hence using XENMEM_get_pod_target on such may not be a real problem,
> calling p2m_pod_set_mem_target() for a PV domain is surely wrong, even
> if benign at present. Add a guard there as well.
> 
> In p2m_pod_demand_populate() the situation is a little different: This
> function is reachable only for HVM domains anyway, but following from
> other PoD functions only ever acting on the host P2M (and hence PoD
> entries only ever existing in host P2Ms), assert and bail from there for
> non-host-P2Ms.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks,

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>

> Perhaps p2m_pod_init() could be invoked from p2m_init_hostp2m(), leaving
> all other p2m's PoD state uninitialized. Of course at that point the
> question would be whether the PoD pieces of struct p2m_domain wouldn't
> better move into a separate structure, present only for host P2Ms.
> Together with the p2m_pod_demand_populate() adjustment this might then
> better be a separate change ...

I’d certainly be tempted to do that kind of clean-up.

I would just check this patch in as-is, but if you really want to pull the 
p2m_pod_demand_populate() adjustment into a separate patch to keep everything 
in the same place, feel free to drop that while retaining my R-b.

 -George

Attachment: signature.asc
Description: Message signed with OpenPGP