[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN][PATCH v3] x86: make Viridian support optional
On 18.09.25 18:41, Jan Beulich wrote: On 16.09.2025 15:41, Grygorii Strashko wrote:--- a/xen/arch/x86/hvm/Kconfig +++ b/xen/arch/x86/hvm/Kconfig @@ -62,6 +62,16 @@ config ALTP2MIf unsure, stay with defaults. +config HVM_VIRIDIANI may have said so already on v1: I'm not quite convinced of the need or usefulness of the HVM_ part here. Viridian necessarily means HVM, aiui. sure. --- a/xen/arch/x86/hvm/viridian/viridian.c +++ b/xen/arch/x86/hvm/viridian/viridian.c @@ -1116,14 +1116,14 @@ static int cf_check viridian_save_domain_ctxt( { const struct domain *d = v->domain; const struct viridian_domain *vd = d->arch.hvm.viridian; - struct hvm_viridian_domain_context ctxt = { - .hypercall_gpa = vd->hypercall_gpa.raw, - .guest_os_id = vd->guest_os_id.raw, - }; + struct hvm_viridian_domain_context ctxt = {};if ( !is_viridian_domain(d) )return 0;This check doesn't check for vd being non-NULL, so this still feels a little fragile, even if it looks correct now. Hm. May be I missing smth., but - if is_viridian_domain(d) and viridian_domain_init() succeeded then d->arch.hvm.viridian != NULL, like always (otherwise domain will not be created) - if !is_viridian_domain() then code will not go further so I'm missing to see how !d->arch.hvm.viridian (!vd) can happen here. To be paranoid can also add: if (!vd) return -EINVAL; + ctxt.hypercall_gpa = vd->hypercall_gpa.raw; + ctxt.guest_os_id = vd->guest_os_id.raw, + viridian_time_save_domain_ctxt(d, &ctxt); viridian_synic_save_domain_ctxt(d, &ctxt);Just below here we have viridian_load_domain_ctxt(), which I'm pretty sure now also needs to gain some check: Save records coming from user space, we can't really rely on there being none of this type for a non-Viridian domain. As per my understanding: viridian_load_domain_ctxt() calls hvm_load_entry_zeroextend() which should not succeed if context was not saved (which shouldn't happen for !is_viridian_domain(d) case). To be paranoid can also add in viridian_load_domain_ctxt/viridian_load_vcpu_ctxt: if ( !is_viridian_domain(d) ) return -ENODEV; -- Best regards, -grygorii
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |