| Message ID | e49bae656ed6c0fe689703f78df4e815b955f5b2.1703072575.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce generic headers | expand |
Hi Oleksii, On 20/12/2023 14:08, Oleksii Kurochko wrote: > Ifdef-ing inclusion of <asm/grant_table.h> allows to avoid > generation of empty <asm/grant_table.h> for cases when > CONFIG_GRANT_TABLE is not enabled. It would have been nice to explain the reason of this change. Is this a compilation error or just a nice thing to have? The reason I am asking is... > > The following changes were done for Arm: > <asm/grant_table.h> should be included directly because it contains > gnttab_dom0_frames() macros which is unique for Arm and is used in > arch/arm/domain_build.c. > <asm/grant_table.h> is #ifdef-ed with CONFIG_GRANT_TABLE in > <xen/grant_table.h> so in case of !CONFIG_GRANT_TABLE gnttab_dom0_frames > won't be available for use in arch/arm/domain_build.c. ... I find rather ugly that we require domain_build.c to include both asm/grant_table.h and xen/grant_table.h. Right now, I don't have a better approach, so I would be ok so long the rationale of the change is explained in the commit message. > > Suggested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Acked-by: Julien Grall <jgrall@amazon.com> Cheers,
On 21/12/2023 19:19, Julien Grall wrote: > Hi Oleksii, > > On 20/12/2023 14:08, Oleksii Kurochko wrote: >> Ifdef-ing inclusion of <asm/grant_table.h> allows to avoid >> generation of empty <asm/grant_table.h> for cases when >> CONFIG_GRANT_TABLE is not enabled. > > It would have been nice to explain the reason of this change. Is this a > compilation error or just a nice thing to have? > > The reason I am asking is... > >> >> The following changes were done for Arm: >> <asm/grant_table.h> should be included directly because it contains >> gnttab_dom0_frames() macros which is unique for Arm and is used in >> arch/arm/domain_build.c. >> <asm/grant_table.h> is #ifdef-ed with CONFIG_GRANT_TABLE in >> <xen/grant_table.h> so in case of !CONFIG_GRANT_TABLE gnttab_dom0_frames >> won't be available for use in arch/arm/domain_build.c. > > ... I find rather ugly that we require domain_build.c to include both > asm/grant_table.h and xen/grant_table.h. > > Right now, I don't have a better approach, so I would be ok so long the > rationale of the change is explained in the commit message. Urgh, I just realized that this is explained in the commit message. Please ignore my comment about expanding the commit message. Sorry for the noise. Cheers,
Hi Julien, On Thu, 2023-12-21 at 19:20 +0000, Julien Grall wrote: > > > On 21/12/2023 19:19, Julien Grall wrote: > > Hi Oleksii, > > > > On 20/12/2023 14:08, Oleksii Kurochko wrote: > > > Ifdef-ing inclusion of <asm/grant_table.h> allows to avoid > > > generation of empty <asm/grant_table.h> for cases when > > > CONFIG_GRANT_TABLE is not enabled. > > > > It would have been nice to explain the reason of this change. Is > > this a > > compilation error or just a nice thing to have? > > > > The reason I am asking is... > > > > > > > > The following changes were done for Arm: > > > <asm/grant_table.h> should be included directly because it > > > contains > > > gnttab_dom0_frames() macros which is unique for Arm and is used > > > in > > > arch/arm/domain_build.c. > > > <asm/grant_table.h> is #ifdef-ed with CONFIG_GRANT_TABLE in > > > <xen/grant_table.h> so in case of !CONFIG_GRANT_TABLE > > > gnttab_dom0_frames > > > won't be available for use in arch/arm/domain_build.c. > > > > ... I find rather ugly that we require domain_build.c to include > > both > > asm/grant_table.h and xen/grant_table.h. > > > > Right now, I don't have a better approach, so I would be ok so long > > the > > rationale of the change is explained in the commit message. > > Urgh, I just realized that this is explained in the commit message. > Please ignore my comment about expanding the commit message. Sorry > for > the noise. It's OK. Thanks for review! ~ Oleksii
Hi Oleksii, On 12/20/23 8:08 AM, Oleksii Kurochko wrote: > Ifdef-ing inclusion of <asm/grant_table.h> allows to avoid > generation of empty <asm/grant_table.h> for cases when > CONFIG_GRANT_TABLE is not enabled. > > The following changes were done for Arm: > <asm/grant_table.h> should be included directly because it contains > gnttab_dom0_frames() macros which is unique for Arm and is used in > arch/arm/domain_build.c. > <asm/grant_table.h> is #ifdef-ed with CONFIG_GRANT_TABLE in > <xen/grant_table.h> so in case of !CONFIG_GRANT_TABLE gnttab_dom0_frames > won't be available for use in arch/arm/domain_build.c. > > Suggested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com> Thanks, Shawn
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 6945b9755d..46161848dc 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -36,6 +36,7 @@ #include <xen/irq.h> #include <xen/grant_table.h> +#include <asm/grant_table.h> #include <xen/serial.h> static unsigned int __initdata opt_dom0_max_vcpus; diff --git a/xen/arch/ppc/include/asm/grant_table.h b/xen/arch/ppc/include/asm/grant_table.h deleted file mode 100644 index d0ff58dd3d..0000000000 --- a/xen/arch/ppc/include/asm/grant_table.h +++ /dev/null @@ -1,5 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -#ifndef __ASM_PPC_GRANT_TABLE_H__ -#define __ASM_PPC_GRANT_TABLE_H__ - -#endif /* __ASM_PPC_GRANT_TABLE_H__ */ diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h index 85fe6b7b5e..50edfecfb6 100644 --- a/xen/include/xen/grant_table.h +++ b/xen/include/xen/grant_table.h @@ -26,7 +26,10 @@ #include <xen/mm-frame.h> #include <xen/rwlock.h> #include <public/grant_table.h> + +#ifdef CONFIG_GRANT_TABLE #include <asm/grant_table.h> +#endif struct grant_table;
Ifdef-ing inclusion of <asm/grant_table.h> allows to avoid generation of empty <asm/grant_table.h> for cases when CONFIG_GRANT_TABLE is not enabled. The following changes were done for Arm: <asm/grant_table.h> should be included directly because it contains gnttab_dom0_frames() macros which is unique for Arm and is used in arch/arm/domain_build.c. <asm/grant_table.h> is #ifdef-ed with CONFIG_GRANT_TABLE in <xen/grant_table.h> so in case of !CONFIG_GRANT_TABLE gnttab_dom0_frames won't be available for use in arch/arm/domain_build.c. Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in V6: - Remove the way how CONFIG_GRANT_TABLE is disabled for PPC and RISC-V. --- Changes in V5: - Added dependencies for "Config GRANT_TABLE" to be sure that randconfig will not turn on the config. --- Changes in V4: - Nothing changed. Only rebase. --- Changes in V3: - Remove unnecessary comment. --- xen/arch/arm/domain_build.c | 1 + xen/arch/ppc/include/asm/grant_table.h | 5 ----- xen/include/xen/grant_table.h | 3 +++ 3 files changed, 4 insertions(+), 5 deletions(-) delete mode 100644 xen/arch/ppc/include/asm/grant_table.h