diff mbox series

[v2,03/15] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>

Message ID cd1341ae337fc9a1619d747b3db1a7a5cd6ad2fb.1699633310.git.oleksii.kurochko@gmail.com (mailing list archive)
State Superseded
Headers show
Series Introduce generic headers | expand

Commit Message

Oleksii Kurochko Nov. 10, 2023, 4:30 p.m. UTC
Ifdefing 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.

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
	- ifdef inclusion of asm/grant_table.h in xen/grant_table.h to avoid
	  generation of empty headers for PPC and RISC-V archs.
	- update commit message
	- add Suggested-by: Jan Beulich <jbeulich@suse.com>
	- Remove provided before asm-generic/grant_table.h header.
---
 xen/include/xen/grant_table.h | 3 +++
 1 file changed, 3 insertions(+)

Comments

Oleksii Kurochko Nov. 11, 2023, 10:25 a.m. UTC | #1
I missed to check the patch properly.

The patch fails for Arm randconfigs:
https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/1068865674

I need to do an additional investigation.

Does it make sense to re-send this patch separately out of this patch
series?

~ Oleksii

On Fri, 2023-11-10 at 18:30 +0200, Oleksii Kurochko wrote:
> Ifdefing 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.
> 
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V2:
>         - ifdef inclusion of asm/grant_table.h in xen/grant_table.h
> to avoid
>           generation of empty headers for PPC and RISC-V archs.
>         - update commit message
>         - add Suggested-by: Jan Beulich <jbeulich@suse.com>
>         - Remove provided before asm-generic/grant_table.h header.
> ---
>  xen/include/xen/grant_table.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> 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;
>
Oleksii Kurochko Nov. 13, 2023, 1:13 p.m. UTC | #2
On Sat, 2023-11-11 at 12:25 +0200, Oleksii wrote:
> I missed to check the patch properly.
> 
> The patch fails for Arm randconfigs:
> https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/1068865674
> 
> I need to do an additional investigation.
So the only one macro cause compile issue if move it to
xen/grant_table.h compilation will pass:

--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -23,10 +23,14 @@
 #ifndef __XEN_GRANT_TABLE_H__
 #define __XEN_GRANT_TABLE_H__
 
+#include <xen/kernel.h>
 #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;
 
@@ -112,6 +116,16 @@ static inline int gnttab_acquire_resource(
     return -EINVAL;
 }
 
+/*
+ * The region used by Xen on the memory will never be mapped in DOM0
+ * memory layout. Therefore it can be used for the grant table.
+ *
+ * Only use the text section as it's always present and will contain
+ * enough space for a large grant table
+ */
+#define gnttab_dom0_frames()                                         
\
+    min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext -
_stext))
+
 #endif /* CONFIG_GRANT_TABLE */
 
 #endif /* __XEN_GRANT_TABLE_H__ */


But gnttab_dom0_frames() is used only for ARM, so probably moving it to
<xen/grant_table.h> is not a good idea.

Could you please tell me your opinion? Thanks in advance.

~ Oleksii

> 
> Does it make sense to re-send this patch separately out of this patch
> series?
> 
> ~ Oleksii
> 
> On Fri, 2023-11-10 at 18:30 +0200, Oleksii Kurochko wrote:
> > Ifdefing 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.
> > 
> > Suggested-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> > Changes in V2:
> >         - ifdef inclusion of asm/grant_table.h in xen/grant_table.h
> > to avoid
> >           generation of empty headers for PPC and RISC-V archs.
> >         - update commit message
> >         - add Suggested-by: Jan Beulich <jbeulich@suse.com>
> >         - Remove provided before asm-generic/grant_table.h header.
> > ---
> >  xen/include/xen/grant_table.h | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > 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;
> >  
>
Jan Beulich Nov. 13, 2023, 1:29 p.m. UTC | #3
On 13.11.2023 14:13, Oleksii wrote:
> On Sat, 2023-11-11 at 12:25 +0200, Oleksii wrote:
>> I missed to check the patch properly.
>>
>> The patch fails for Arm randconfigs:
>> https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/1068865674
>>
>> I need to do an additional investigation.
> So the only one macro cause compile issue if move it to
> xen/grant_table.h compilation will pass:
> 
> --- a/xen/include/xen/grant_table.h
> +++ b/xen/include/xen/grant_table.h
> @@ -23,10 +23,14 @@
>  #ifndef __XEN_GRANT_TABLE_H__
>  #define __XEN_GRANT_TABLE_H__
>  
> +#include <xen/kernel.h>
>  #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;
>  
> @@ -112,6 +116,16 @@ static inline int gnttab_acquire_resource(
>      return -EINVAL;
>  }
>  
> +/*
> + * The region used by Xen on the memory will never be mapped in DOM0
> + * memory layout. Therefore it can be used for the grant table.
> + *
> + * Only use the text section as it's always present and will contain
> + * enough space for a large grant table
> + */
> +#define gnttab_dom0_frames()                                         
> \
> +    min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext -
> _stext))
> +
>  #endif /* CONFIG_GRANT_TABLE */
>  
>  #endif /* __XEN_GRANT_TABLE_H__ */
> 
> 
> But gnttab_dom0_frames() is used only for ARM, so probably moving it to
> <xen/grant_table.h> is not a good idea.

Indeed. But wouldn't dealing with this again be a matter of having
Arm's domain_build.c simply include asm/grant_table.h explicitly, if need
be alongside xen/grant_table.h?

Jan
Oleksii Kurochko Nov. 13, 2023, 3:13 p.m. UTC | #4
On Mon, 2023-11-13 at 14:29 +0100, Jan Beulich wrote:
> On 13.11.2023 14:13, Oleksii wrote:
> > On Sat, 2023-11-11 at 12:25 +0200, Oleksii wrote:
> > > I missed to check the patch properly.
> > > 
> > > The patch fails for Arm randconfigs:
> > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/1068865674
> > > 
> > > I need to do an additional investigation.
> > So the only one macro cause compile issue if move it to
> > xen/grant_table.h compilation will pass:
> > 
> > --- a/xen/include/xen/grant_table.h
> > +++ b/xen/include/xen/grant_table.h
> > @@ -23,10 +23,14 @@
> >  #ifndef __XEN_GRANT_TABLE_H__
> >  #define __XEN_GRANT_TABLE_H__
> >  
> > +#include <xen/kernel.h>
> >  #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;
> >  
> > @@ -112,6 +116,16 @@ static inline int gnttab_acquire_resource(
> >      return -EINVAL;
> >  }
> >  
> > +/*
> > + * The region used by Xen on the memory will never be mapped in
> > DOM0
> > + * memory layout. Therefore it can be used for the grant table.
> > + *
> > + * Only use the text section as it's always present and will
> > contain
> > + * enough space for a large grant table
> > + */
> > +#define
> > gnttab_dom0_frames()                                         
> > \
> > +    min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext -
> > _stext))
> > +
> >  #endif /* CONFIG_GRANT_TABLE */
> >  
> >  #endif /* __XEN_GRANT_TABLE_H__ */
> > 
> > 
> > But gnttab_dom0_frames() is used only for ARM, so probably moving
> > it to
> > <xen/grant_table.h> is not a good idea.
> 
> Indeed. But wouldn't dealing with this again be a matter of having
> Arm's domain_build.c simply include asm/grant_table.h explicitly, if
> need
> be alongside xen/grant_table.h?
It can be a solution. Then I'll send a separate patch.

Thanks.

~ Oleksii
diff mbox series

Patch

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;