|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH] xen/domain: make shutdown state explicit
From: Mykola Kvach <mykola_kvach@xxxxxxxx> The domain shutdown flow currently overloads is_shutting_down and is_shut_down to represent multiple phases of the shutdown lifecycle, while some users treat is_shutting_down as a broader "domain is no longer normal" condition. Make the shutdown lifecycle explicit by introducing enum domain_shutdown_state and converting the existing users to helper predicates describing whether shutdown is in progress, complete, or active. At the same time, make domain_resume() validate its input state and return an error to its callers. Resume is now accepted only from the fully shut down state. This removes the implicit coupling between unrelated users of is_shutting_down and makes the shutdown/resume state transitions self-describing. Suggested-by: Jan Beulich <jbeulich@xxxxxxxx> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx> --- Link to discussion: https://patchew.org/Xen/cover.1756392094.git.mykola._5Fkvach@xxxxxxxx/bb53d9911b00879c7b25f5258d0e3e48005671f9.1756392094.git.mykola._5Fkvach@xxxxxxxx/#a64cff9f-df5f-467b-a944-74e803c64ab9@xxxxxxxx --- xen/arch/x86/hvm/viridian/time.c | 2 +- xen/arch/x86/mm.c | 2 +- xen/arch/x86/mm/hap/hap.c | 2 +- xen/arch/x86/mm/shadow/common.c | 5 ++-- xen/arch/x86/mm/shadow/multi.c | 12 +++++---- xen/common/domain.c | 46 +++++++++++++++++++++++--------- xen/common/domctl.c | 4 +-- xen/common/sched/core.c | 2 +- xen/drivers/passthrough/iommu.c | 8 +++--- xen/drivers/passthrough/pci.c | 2 +- xen/include/xen/sched.h | 30 +++++++++++++++++---- 11 files changed, 80 insertions(+), 35 deletions(-) diff --git a/xen/arch/x86/hvm/viridian/time.c b/xen/arch/x86/hvm/viridian/time.c index 9311858d63..c786ebd2af 100644 --- a/xen/arch/x86/hvm/viridian/time.c +++ b/xen/arch/x86/hvm/viridian/time.c @@ -102,7 +102,7 @@ static void time_ref_count_thaw(const struct domain *d) struct viridian_domain *vd = d->arch.hvm.viridian; struct viridian_time_ref_count *trc = &vd->time_ref_count; - if ( d->is_shutting_down || + if ( domain_shutdown_active(d) || test_and_set_bit(_TRC_running, &trc->flags) ) return; diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 4c404b6c13..039b4ffb00 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -1219,7 +1219,7 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner) */ #if _PAGE_GNTTAB if ( (l1e_get_flags(l1e) & _PAGE_GNTTAB) && - !l1e_owner->is_shutting_down && !l1e_owner->is_dying ) + !domain_shutdown_active(l1e_owner) && !l1e_owner->is_dying ) { gprintk(XENLOG_WARNING, "Attempt to implicitly unmap %pd's grant PTE %" PRIpte "\n", diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c index a337752bf4..205b33de0d 100644 --- a/xen/arch/x86/mm/hap/hap.c +++ b/xen/arch/x86/mm/hap/hap.c @@ -414,7 +414,7 @@ static mfn_t hap_make_monitor_table(struct vcpu *v) oom: if ( !d->is_dying && - (!d->is_shutting_down || d->shutdown_code != SHUTDOWN_crash) ) + (!domain_shutdown_active(d) || d->shutdown_code != SHUTDOWN_crash) ) { printk(XENLOG_G_ERR "%pd: out of memory building monitor pagetable\n", d); diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c index dd2d04d049..a09aab46d2 100644 --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -164,7 +164,7 @@ void shadow_promote(struct domain *d, mfn_t gmfn, unsigned int type) /* We should never try to promote a gmfn that has writeable mappings */ ASSERT((page->u.inuse.type_info & PGT_type_mask) != PGT_writable_page || (page->u.inuse.type_info & PGT_count_mask) == 0 - || d->is_shutting_down); + || domain_shutdown_active(d)); /* Is the page already shadowed? */ if ( !test_and_set_bit(_PGC_shadowed_pt, &page->count_info) ) @@ -442,7 +442,8 @@ bool shadow_prealloc(struct domain *d, unsigned int type, unsigned int count) count += paging_logdirty_levels(); ret = _shadow_prealloc(d, count); - if ( !ret && (!d->is_shutting_down || d->shutdown_code != SHUTDOWN_crash) ) + if ( !ret && (!domain_shutdown_active(d) || + d->shutdown_code != SHUTDOWN_crash) ) /* * Failing to allocate memory required for shadow usage can only result in * a domain crash, do it here rather that relying on every caller to do it. diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c index 80cd3299fa..7cc3024455 100644 --- a/xen/arch/x86/mm/shadow/multi.c +++ b/xen/arch/x86/mm/shadow/multi.c @@ -2373,7 +2373,8 @@ static int cf_check sh_page_fault( * already used for some special purpose (ioreq pages, or granted pages). * If that happens we'll have killed the guest already but it's still not * safe to propagate entries out of the guest PT so get out now. */ - if ( unlikely(d->is_shutting_down && d->shutdown_code == SHUTDOWN_crash) ) + if ( unlikely(domain_shutdown_active(d) && + d->shutdown_code == SHUTDOWN_crash) ) { SHADOW_PRINTK("guest is shutting down\n"); goto propagate; @@ -2483,7 +2484,7 @@ static int cf_check sh_page_fault( #if GUEST_PAGING_LEVELS == 3 sh_update_cr3(v, false); #else - ASSERT(d->is_shutting_down); + ASSERT(domain_shutdown_active(d)); sh_trace_va(TRC_SHADOW_DOMF_DYING, va); #endif paging_unlock(d); @@ -2497,7 +2498,8 @@ static int cf_check sh_page_fault( && ft == ft_demand_write ) sh_unsync(v, gmfn); - if ( unlikely(d->is_shutting_down && d->shutdown_code == SHUTDOWN_crash) ) + if ( unlikely(domain_shutdown_active(d) && + d->shutdown_code == SHUTDOWN_crash) ) { /* We might end up with a crashed domain here if * sh_remove_shadows() in a previous sh_resync() call has @@ -3269,7 +3271,7 @@ static pagetable_t cf_check sh_update_cr3(struct vcpu *v, bool noflush) sh_make_shadow); if ( unlikely(pagetable_is_null(v->arch.paging.shadow.shadow_table[0])) ) { - ASSERT(d->is_dying || d->is_shutting_down); + ASSERT(d->is_dying || domain_shutdown_active(d)); return old_entry; } if ( !paging_mode_external(d) && !is_pv_32bit_domain(d) ) @@ -3336,7 +3338,7 @@ static pagetable_t cf_check sh_update_cr3(struct vcpu *v, bool noflush) ASSERT(pagetable_is_null(old_entry)); if ( unlikely(pagetable_is_null(v->arch.paging.shadow.shadow_table[0])) ) { - ASSERT(d->is_dying || d->is_shutting_down); + ASSERT(d->is_dying || domain_shutdown_active(d)); return old_entry; } #else diff --git a/xen/common/domain.c b/xen/common/domain.c index ab910fcf93..fb6eb7b89a 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -190,7 +190,7 @@ static void set_domain_state_info(struct xen_domctl_get_domain_state *info, const struct domain *d) { info->state = XEN_DOMCTL_GETDOMSTATE_STATE_EXIST; - if ( d->is_shut_down ) + if ( domain_shutdown_complete(d) ) info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_SHUTDOWN; if ( d->is_dying == DOMDYING_dying ) info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING; @@ -281,14 +281,14 @@ static void __domain_finalise_shutdown(struct domain *d) BUG_ON(!spin_is_locked(&d->shutdown_lock)); - if ( d->is_shut_down ) + if ( domain_shutdown_complete(d) ) return; for_each_vcpu ( d, v ) if ( !v->paused_for_shutdown ) return; - d->is_shut_down = 1; + d->shutdown_state = DOMSHUTDOWN_complete; domain_changed_state(d); if ( (d->shutdown_code == SHUTDOWN_suspend) && d->suspend_evtchn ) evtchn_send(d, d->suspend_evtchn); @@ -302,7 +302,7 @@ static void vcpu_check_shutdown(struct vcpu *v) spin_lock(&d->shutdown_lock); - if ( d->is_shutting_down ) + if ( domain_shutdown_in_progress(d) ) { if ( !v->paused_for_shutdown ) vcpu_pause_nosync(v); @@ -1355,7 +1355,7 @@ int domain_kill(struct domain *d) void __domain_crash(struct domain *d) { - if ( d->is_shutting_down ) + if ( domain_shutdown_active(d) ) { /* Print nothing: the domain is already shutting down. */ } @@ -1393,13 +1393,13 @@ int domain_shutdown(struct domain *d, u8 reason) if ( is_hardware_domain(d) ) hwdom_shutdown(reason); - if ( d->is_shutting_down ) + if ( domain_shutdown_active(d) ) { spin_unlock(&d->shutdown_lock); return 0; } - d->is_shutting_down = 1; + d->shutdown_state = DOMSHUTDOWN_in_progress; smp_mb(); /* set shutdown status /then/ check for per-cpu deferrals */ @@ -1422,9 +1422,12 @@ int domain_shutdown(struct domain *d, u8 reason) return 0; } -void domain_resume(struct domain *d) +int domain_resume(struct domain *d) { struct vcpu *v; + enum domain_shutdown_state shutdown_state; + unsigned int shutdown_code; + int rc = 0; /* * Some code paths assume that shutdown status does not get reset under @@ -1434,7 +1437,18 @@ void domain_resume(struct domain *d) spin_lock(&d->shutdown_lock); - d->is_shutting_down = d->is_shut_down = 0; + shutdown_state = d->shutdown_state; + shutdown_code = d->shutdown_code; + + if ( shutdown_state != DOMSHUTDOWN_complete || + (shutdown_code != SHUTDOWN_suspend && + shutdown_code != SHUTDOWN_soft_reset) ) + { + rc = -EINVAL; + goto out_unlock; + } + + d->shutdown_state = DOMSHUTDOWN_none; d->shutdown_code = SHUTDOWN_CODE_INVALID; for_each_vcpu ( d, v ) @@ -1444,9 +1458,17 @@ void domain_resume(struct domain *d) v->paused_for_shutdown = 0; } +out_unlock: spin_unlock(&d->shutdown_lock); domain_unpause(d); + + if ( rc ) + dprintk(XENLOG_WARNING, + "%pd: Invalid domain state for resume: shutdown_state=%u, shutdown_code=%u\n", + d, shutdown_state, shutdown_code); + + return rc; } int vcpu_start_shutdown_deferral(struct vcpu *v) @@ -1456,7 +1478,7 @@ int vcpu_start_shutdown_deferral(struct vcpu *v) v->defer_shutdown = 1; smp_mb(); /* set deferral status /then/ check for shutdown */ - if ( unlikely(v->domain->is_shutting_down) ) + if ( unlikely(domain_shutdown_in_progress(v->domain)) ) vcpu_check_shutdown(v); return v->defer_shutdown; @@ -1466,7 +1488,7 @@ void vcpu_end_shutdown_deferral(struct vcpu *v) { v->defer_shutdown = 0; smp_mb(); /* clear deferral status /then/ check for shutdown */ - if ( unlikely(v->domain->is_shutting_down) ) + if ( unlikely(domain_shutdown_in_progress(v->domain)) ) vcpu_check_shutdown(v); } @@ -1798,7 +1820,7 @@ int domain_soft_reset(struct domain *d, bool resuming) rc = arch_domain_soft_reset(d); if ( !rc ) - domain_resume(d); + rc = domain_resume(d); else domain_crash(d); diff --git a/xen/common/domctl.c b/xen/common/domctl.c index 93738931c5..23686bb603 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -86,7 +86,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info) info->flags = (info->nr_online_vcpus ? flags : 0) | ((d->is_dying == DOMDYING_dead) ? XEN_DOMINF_dying : 0) | - (d->is_shut_down ? XEN_DOMINF_shutdown : 0) | + (domain_shutdown_complete(d) ? XEN_DOMINF_shutdown : 0) | (d->controller_pause_count > 0 ? XEN_DOMINF_paused : 0) | (d->debugger_attached ? XEN_DOMINF_debugged : 0) | (is_xenstore_domain(d) ? XEN_DOMINF_xs_domain : 0) | @@ -404,7 +404,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) if ( d == current->domain ) /* no domain_pause() */ ret = -EINVAL; else - domain_resume(d); + ret = domain_resume(d); break; case XEN_DOMCTL_createdomain: diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c index a57d5dd929..48f5b4f738 100644 --- a/xen/common/sched/core.c +++ b/xen/common/sched/core.c @@ -1540,7 +1540,7 @@ static void cf_check domain_watchdog_timeout(void *data) BUILD_BUG_ON(alignof(*d) < PAGE_SIZE); - if ( d->is_shutting_down || d->is_dying ) + if ( domain_shutdown_active(d) || d->is_dying ) return; printk("Watchdog timer %u fired for %pd\n", id, d); diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index c9425d6971..f70d970b0e 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -355,7 +355,7 @@ long iommu_map(struct domain *d, dfn_t dfn0, mfn_t mfn0, if ( likely(!rc) ) continue; - if ( !d->is_shutting_down && printk_ratelimit() ) + if ( !domain_shutdown_active(d) && printk_ratelimit() ) printk(XENLOG_ERR "d%d: IOMMU mapping dfn %"PRI_dfn" to mfn %"PRI_mfn" failed: %d\n", d->domain_id, dfn_x(dfn), mfn_x(mfn), rc); @@ -427,7 +427,7 @@ long iommu_unmap(struct domain *d, dfn_t dfn0, unsigned long page_count, if ( likely(!err) ) continue; - if ( !d->is_shutting_down && printk_ratelimit() ) + if ( !domain_shutdown_active(d) && printk_ratelimit() ) printk(XENLOG_ERR "d%d: IOMMU unmapping dfn %"PRI_dfn" failed: %d\n", d->domain_id, dfn_x(dfn), err); @@ -492,7 +492,7 @@ int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned long page_count, flush_flags); if ( unlikely(rc) ) { - if ( !d->is_shutting_down && printk_ratelimit() ) + if ( !domain_shutdown_active(d) && printk_ratelimit() ) printk(XENLOG_ERR "d%d: IOMMU IOTLB flush failed: %d, dfn %"PRI_dfn", page count %lu flags %x\n", d->domain_id, rc, dfn_x(dfn), page_count, flush_flags); @@ -517,7 +517,7 @@ int iommu_iotlb_flush_all(struct domain *d, unsigned int flush_flags) flush_flags | IOMMU_FLUSHF_all); if ( unlikely(rc) ) { - if ( !d->is_shutting_down && printk_ratelimit() ) + if ( !domain_shutdown_active(d) && printk_ratelimit() ) printk(XENLOG_ERR "d%d: IOMMU IOTLB flush all failed: %d\n", d->domain_id, rc); diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index 464bb0fee4..c22f45109c 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -1746,7 +1746,7 @@ void iommu_dev_iotlb_flush_timeout(struct domain *d, struct pci_dev *pdev) pdev->broken = true; - if ( !d->is_shutting_down && printk_ratelimit() ) + if ( !domain_shutdown_active(d) && printk_ratelimit() ) printk(XENLOG_ERR "dom%d: ATS device %pp flush failed\n", d->domain_id, &pdev->sbdf); if ( !is_hardware_domain(d) ) diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 40a35fc15c..d774fdd43c 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -226,7 +226,7 @@ struct vcpu bool force_context_switch; /* Require shutdown to be deferred for some asynchronous operation? */ bool defer_shutdown; - /* VCPU is paused following shutdown request (d->is_shutting_down)? */ + /* VCPU is paused following a domain shutdown request? */ bool paused_for_shutdown; /* VCPU need affinity restored */ uint8_t affinity_broken; @@ -386,6 +386,12 @@ struct domain_console { char buf[256]; }; +enum domain_shutdown_state { + DOMSHUTDOWN_none, + DOMSHUTDOWN_in_progress, + DOMSHUTDOWN_complete, +}; + struct domain { domid_t domain_id; @@ -552,10 +558,9 @@ struct domain struct rangeset *iomem_caps; struct rangeset *irq_caps; - /* Guest has shut down (inc. reason code)? */ + /* Guest shutdown state and associated reason code. */ spinlock_t shutdown_lock; - bool is_shutting_down; /* in process of shutting down? */ - bool is_shut_down; /* fully shut down? */ + enum domain_shutdown_state shutdown_state; #define SHUTDOWN_CODE_INVALID ~0u unsigned int shutdown_code; @@ -674,6 +679,21 @@ struct domain unsigned int pending_scrub_index; } __aligned(PAGE_SIZE); +static inline bool domain_shutdown_in_progress(const struct domain *d) +{ + return d->shutdown_state == DOMSHUTDOWN_in_progress; +} + +static inline bool domain_shutdown_complete(const struct domain *d) +{ + return d->shutdown_state == DOMSHUTDOWN_complete; +} + +static inline bool domain_shutdown_active(const struct domain *d) +{ + return d->shutdown_state != DOMSHUTDOWN_none; +} + static inline struct page_list_head *page_to_list( struct domain *d, const struct page_info *pg) { @@ -828,7 +848,7 @@ static inline void put_pg_owner(struct domain *pg_owner) void domain_destroy(struct domain *d); int domain_kill(struct domain *d); int domain_shutdown(struct domain *d, u8 reason); -void domain_resume(struct domain *d); +int domain_resume(struct domain *d); int domain_soft_reset(struct domain *d, bool resuming); -- 2.43.0
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |