Re: [PATCH 1/2] x86: Introduce arch_domain_teardown()

From: Jan Beulich <jbeulich@suse.com>
To: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>, Wei Liu <wl@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Jens Wiklander <jens.wiklander@linaro.org>
Subject: Re: [PATCH 1/2] x86: Introduce arch_domain_teardown()
Date: Wed, 7 Jun 2023 10:43:24 +0200	[thread overview]
Message-ID: <9a34e419-ce1d-e452-47e8-fe220d600ab6@suse.com> (raw)
In-Reply-To: <18a46079-8fa5-5e43-9296-56a8c6457133@citrix.com>

On 05.06.2023 17:23, Andrew Cooper wrote:
> On 05/06/2023 4:19 pm, Roger Pau Monné wrote:
>> On Mon, Jun 05, 2023 at 03:43:30PM +0100, Andrew Cooper wrote:
>>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>>> index 85242a73d374..854f3e32c00e 100644
>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -589,6 +589,7 @@ struct domain
>>>       */
>>>      struct {
>>>          unsigned int val;
>>> +        unsigned int arch_val;
>> While I haven't looked at patch 2, wouldn't such continuation
>> information better be encoded in arch_domain in whatever way is more
>> suitable for each architecture?
> 
> Well, it's filling a hole here on 64bit builds, which it couldn't do in
> arch_domain.
> 
> And it's contained inside teardown{} which already signals it as fairly
> magic.

Plus why have each architecture duplicate the field? I expect none of
the arch_domain_teardown() instances will remain without an actual
use of the new field, mid to long term.

I don't want to override Roger's concern, but from my pov
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan