diff mbox series

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

Message ID 20240308015435.4044339-3-xin.wang2@amd.com (mailing list archive)
State New, archived
Headers show
Series DOMCTL-based guest magic regions allocation for dom0less | expand

Commit Message

Henry Wang March 8, 2024, 1:54 a.m. UTC
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>
---
v2:
- New patch
---
 xen/arch/arm/include/asm/domain.h   | 2 --
 xen/arch/ppc/include/asm/domain.h   | 2 --
 xen/arch/riscv/include/asm/domain.h | 2 --
 xen/arch/x86/include/asm/domain.h   | 1 -
 xen/include/xen/domain.h            | 3 +++
 5 files changed, 3 insertions(+), 7 deletions(-)

Comments

Michal Orzel March 8, 2024, 8:59 a.m. UTC | #1
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?

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

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.

~Michal
Henry Wang March 8, 2024, 9:06 a.m. UTC | #2
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
Jan Beulich March 8, 2024, 9:41 a.m. UTC | #3
On 08.03.2024 10:06, Henry Wang wrote:
> On 3/8/2024 4:59 PM, Michal Orzel wrote:
>> 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.

While I wouldn't strictly mind the addition of an #ifdef, I'd prefer it to
be omitted until such time when an arch would really want to override it.
IOW patch as-is
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
Shawn Anastasio March 11, 2024, 6:02 p.m. UTC | #4
Hi Henry,

On 3/7/24 7:54 PM, 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>

Regardless of whether or not you decide to go with the centralized ifdef
approach suggested by Michal, consider the PPC parts:

Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>

Thanks,
Shawn
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index 8218afb862..f1d72c6e48 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -29,8 +29,6 @@  enum domain_type {
 #define is_64bit_domain(d) (0)
 #endif
 
-#define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
-
 /*
  * Is the domain using the host memory layout?
  *
diff --git a/xen/arch/ppc/include/asm/domain.h b/xen/arch/ppc/include/asm/domain.h
index 573276d0a8..3a447272c6 100644
--- a/xen/arch/ppc/include/asm/domain.h
+++ b/xen/arch/ppc/include/asm/domain.h
@@ -10,8 +10,6 @@  struct hvm_domain
     uint64_t              params[HVM_NR_PARAMS];
 };
 
-#define is_domain_direct_mapped(d) ((void)(d), 0)
-
 /* TODO: Implement */
 #define guest_mode(r) ({ (void)(r); BUG_ON("unimplemented"); 0; })
 
diff --git a/xen/arch/riscv/include/asm/domain.h b/xen/arch/riscv/include/asm/domain.h
index 0f5dc2be40..027bfa8a93 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -10,8 +10,6 @@  struct hvm_domain
     uint64_t              params[HVM_NR_PARAMS];
 };
 
-#define is_domain_direct_mapped(d) ((void)(d), 0)
-
 struct arch_vcpu_io {
 };
 
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index 622d22bef2..4bd78e3a6d 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -22,7 +22,6 @@ 
 #define is_hvm_pv_evtchn_domain(d) (is_hvm_domain(d) && \
         ((d)->arch.hvm.irq->callback_via_type == HVMIRQ_callback_vector || \
          (d)->vcpu[0]->arch.hvm.evtchn_upcall_vector))
-#define is_domain_direct_mapped(d) ((void)(d), 0)
 
 #define VCPU_TRAP_NONE         0
 #define VCPU_TRAP_NMI          1
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index b1a4aa6f38..3de5635291 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -41,6 +41,8 @@  void arch_get_domain_info(const struct domain *d,
 #ifdef CONFIG_ARM
 /* Should domain memory be directly mapped? */
 #define CDF_directmap            (1U << 1)
+#else
+#define CDF_directmap            0
 #endif
 /* Is domain memory on static allocation? */
 #ifdef CONFIG_STATIC_MEMORY
@@ -49,6 +51,7 @@  void arch_get_domain_info(const struct domain *d,
 #define CDF_staticmem            0
 #endif
 
+#define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
 #define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
 
 /*