From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH 6/7] xen: Allow hardare domain != dom0 Date: Fri, 11 Apr 2014 11:07:38 -0400 Message-ID: <5348053A.9040703@tycho.nsa.gov> References: <1395921128-7086-1-git-send-email-dgdegra@tycho.nsa.gov> <1395921128-7086-7-git-send-email-dgdegra@tycho.nsa.gov> <5347CE550200007800007E5E@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5347CE550200007800007E5E@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Keir Fraser , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 04/11/2014 05:13 AM, Jan Beulich wrote: >>>> On 27.03.14 at 12:52, wrote: > > I was about to commit this, but further changes are needed (along > with fixing the typo in the title): I noticed the typo soon after posting v3, but didn't want to post v4 with that and some other whitespace. It looks like v4 will have useful changes now, however. >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -84,6 +84,8 @@ unsigned long __initdata highmem_start; >> size_param("highmem-start", highmem_start); >> #endif >> >> +integer_param("hardware_dom", hardware_domid); > > This should be moved alongside the definition of the symbol in > common code, and be enclosed in "#ifdef CONFIG_LATE_HWDOM". > > And docs/misc/xen-command-line.markdown wants an entry for it. OK. >> --- a/xen/include/xen/sched.h >> +++ b/xen/include/xen/sched.h >> @@ -45,6 +45,7 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_runstate_info_compat_t); >> >> /* A global pointer to the hardware domain (usually DOM0). */ >> extern struct domain *hardware_domain; >> +extern domid_t hardware_domid; >> >> #ifndef CONFIG_COMPAT >> #define BITS_PER_EVTCHN_WORD(d) BITS_PER_XEN_ULONG >> @@ -794,7 +795,7 @@ void watchdog_domain_destroy(struct domain *d); >> * (that is, this would not be suitable for a driver domain) >> * - There is never a reason to deny dom0 access to this >> */ >> -#define is_hardware_domain(_d) ((_d)->domain_id == 0) >> +#define is_hardware_domain(d) ((d)->domain_id == hardware_domid) > > This macro should imo evaluate to true for Dom0 until the hardware > domain go created, i.e. you should compare _d with hardware_domain > rather than their IDs. With that the definition of hardware_domid can > then also be moved inside the #ifdef requested above. This isn't quite as simple as changing the function since there are some places where is_hardware_domain needs to return false for domain 0 when a hardware domain is used. Also, the hardware_domain variable is not set until domain_create returns, so there are a few places where the domain ID still needs to be checked explicitly. It should be possible to create an is_hardware_domid function for those cases, if comparing to hardware_domain is preferred for most cases; I think that would belong in a new patch 5.5/7 (i.e. 6/8 in v4). Otherwise, I think the is_hardware_domain definition should be: #ifdef CONFIG_LATE_HWDOM #define is_hardware_domain(_d) ((_d)->domain_id == hardware_domid) #else #define is_hardware_domain(_d) ((_d)->domain_id == 0) #endif This also allows hardware_domid to be declared inside the #ifdef. -- Daniel De Graaf National Security Agency