Re: [PATCH v2 2/5] xen/domain.h: Centrialize is_domain_direct_mapped()

From: Henry Wang <xin.wang2@amd.com>
To: Michal Orzel <michal.orzel@amd.com>, <xen-devel@lists.xenproject.org>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>,
	"Bertrand Marquis" <bertrand.marquis@arm.com>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>, "Wei Liu" <wl@xen.org>,
	"Shawn Anastasio" <sanastasio@raptorengineering.com>,
	"Alistair Francis" <alistair.francis@wdc.com>,
	"Bob Eshleman" <bobbyeshleman@gmail.com>,
	"Connor Davis" <connojdavis@gmail.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [PATCH v2 2/5] xen/domain.h: Centrialize is_domain_direct_mapped()
Date: Fri, 8 Mar 2024 17:06:08 +0800	[thread overview]
Message-ID: <4cd6aec3-aa5e-47fd-9d33-c03848e3f563@amd.com> (raw)
In-Reply-To: <9b55829a-3248-4bd4-b36b-a709debf46f3@amd.com>

Hi Michal,

On 3/8/2024 4:59 PM, Michal Orzel wrote:
> Hi Henry,
>
> On 08/03/2024 02:54, Henry Wang wrote:
>> Currently direct mapped domain is only supported by the Arm
>> architecture at the domain creation time by setting the CDF_directmap
>> flag. There is not a need for every non-Arm architecture, i.e. x86,
>> RISC-V and PPC, to define a stub is_domain_direct_mapped() in arch
>> header.
>>
>> Move is_domain_direct_mapped() to a centralized place at xen/domain.h
>> and evaluate CDF_directmap for non-Arm architecture to 0.
>>
>> Signed-off-by: Henry Wang <xin.wang2@amd.com>
> Shouldn't you add Suggested-by: Jan?

Yeah indeed I should have added it. I always have a hard time to 
determine if I should add "Suggested-by"/"Reported-by" tags, sorry for 
missing it this time.

I will add it in the next version.

> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Thanks!

> Another alternative would be to let the arch header define it if needed and use a centralized stub in xen/domain.h:
>
> #ifndef is_domain_direct_mapped
> #define is_domain_direct_mapped(d) ((void)(d), 0)
> #endif
>
> I'm not sure which solution is better.

Thanks for the suggestion. I am fine with either way, so let's see what 
the others would say and I will do the update of this patch accordingly.

Kind regards,
Henry

> ~Michal