On 23/04/2024 3:31 pm, Jan Beulich wrote:
> The latest as of e2b2ff677958 ("x86/P2M: split out init/teardown
> functions") the function is obviously unreachable for PV guests.This doesn't parse. Do you mean "Since e2b2ff677958 ..." ? > Hence > the paging_mode_enabled(d) check is pointless. > > Further host mode of a vCPU is always set, by virtue of > paging_vcpu_init() being part of vCPU creation. Hence the > paging_get_hostmode() check is pointless. > > With that the v local variable is unnecessary too. Drop the "if()" > conditional and its corresponding "else". > > Signed-off-by: Jan Beulich <jbeul...@suse.com> > --- > I have to confess that this if() has been puzzling me before. Puzzling yes, but it can't blindly be dropped. This is the "did the toolstack initiate this update" check. i.e. I think it's "bypass the normal side effects of making this update". I suspect it exists because of improper abstraction between the guest physmap and the shadow pagetables as-were - which were/are tighly coupled to vCPUs even for aspects where they shouldn't have been. For better or worse, the toolstack can add_to_physmap() before it creates vCPUs, and it will take this path you're trying to delete. There may be other cases too; I could see foreign mapping ending up ticking this too. Whether we ought to permit a toolstack to do this is a different question, but seeing as we explicitly intend (eventually for AMX) have a set_policy call between domain_create() and vcpu_create(), I don't think we can reasably restrict other hypercalls too in this period. ~Andrew
