Re: [Xen-devel] [PATCH] tools/libxl: Reposition build_pre() logic between architectures

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Julien Grall <julien@xen.org>,
	Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>, "Jan Beulich" <JBeulich@suse.com>,
	"Anthony Perard" <anthony.perard@citrix.com>,
	"Ian Jackson" <Ian.Jackson@citrix.com>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH] tools/libxl: Reposition build_pre() logic between architectures
Date: Mon, 30 Dec 2019 13:38:12 +0000	[thread overview]
Message-ID: <967aad91-812f-3178-e8f5-1b787127ee8d@citrix.com> (raw)
In-Reply-To: <b190ca53-3541-438f-7a61-723a51e0d57d@xen.org>

On 30/12/2019 13:15, Julien Grall wrote:
> Hi Andrew,
>
> On 27/12/2019 13:45, Andrew Cooper wrote:
>> The call to xc_domain_disable_migrate() is made only from x86, while its
>> handling in Xen is common.  Move it to the libxl__build_pre().
>>
>> hvm_set_conf_params(), hvm_set_viridian_features(),
>> hvm_set_mca_capabilities(), and the altp2m logic is all in common
>> code (parts ifdef'd) but despite this, is all actually x86 specific.
>
> While altp2m is only supported on x86, the concept is not
> x86-specific. I am actually aware of people using altp2m on Arm,
> althought the support is not upstream yet.
>
>>
>> Move it into x86 specific code, and fold all of the
>> xc_hvm_param_set() calls
>> together into hvm_set_conf_params() in a far more coherent way.
>>
>> Finally - ensure that all hypercalls have their return values checked.
>>
>> No practical change in constructed domains.  Fewer useless hypercalls
>> now to
>> construct an ARM guest.
>
> I think it would be best to keep anything that we know can be used on
> arm (or new architecture) in common code. I am thinking about
> "nestedhvm" and "altp2m".

Neither of those options are going to survive in this form.

Also, the checks can't stay in common code.  Currently, Xen doesn't
reject bad parameters, and the toolstack doesn't check return values. 
Frankly, neither of these bugs should ever have got through code review,
seeing as we were doing rather better code review by the time the ARM
port came about.

I've fixed the libxl code to check return values, but when the
hypervisor has its
"not-quite-an-XSA-because-the-guest-induced-damage-is-in-unsupported-subsystems"
bugs fixed, these hypercalls will start failing.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel