On Thu, Oct 13, 2022 at 08:48:21AM +0200, Jan Beulich wrote: > Just like domain_soft_reset() properly zaps runstate area handles, the > secondary time area ones also need discarding to prevent guest memory > corruption once the guest is re-started. > > Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Roger Pau Monné <roger....@citrix.com> > --- > To avoid another for_each_vcpu() here, domain_soft_reset() could also > be made call a new arch_vcpu_soft_reset() out of its already present > loop. Yet that would make the change less isolated. > > In domain_soft_reset() I wonder whether, just like done here, the > zapping of runstate area handles and vCPU info mappings wouldn't better > be done after all operations which can fail. But perhaps for this to > matter the domain is left in too inconsistent a state anyway if the > function fails ... We would need some kind of recovery anyway, so given the current code and lack of recovery it doesn't seem to matter much. > However, at the very least I wonder whether x86'es > restriction to HVM shouldn't leave PV guests undisturbed if a soft-reset > was attempted on them. Right now they not only have state partially > clobbered, but (if the arch function is reached) they would be crashed > unconditionally. It's a toolstack initiated operation by a domctl, so I'm fine with saying that it's up for the toolstack to prevent soft resets from being attempted against PV domains. Would be nice to reject the operation earlier on the hypervisor, maybe by moving arch_domain_soft_reset() earlier in domain_soft_reset() so that we can return without crashing? In any case it's unlikely for a domain that was attempting a soft reset to survive the hypervisor rejecting the operation, so it doesn't matter much whether the domain is crashed by Xen or left as-is I would think. Thanks, Roger.
