1. Patchew
  2. Xen
  3. [PATCH v2 00/15] Allow x86 to unflatten DTs
  4. View patch

[PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h

Alejandro Vallejo posted 15 patches 4 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
Posted by Alejandro Vallejo 4 months, 3 weeks ago 
Not a functional change

Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
v2:
  * Remove both links rather than just one.
---
 xen/arch/arm/include/asm/grant_table.h  | 1 -
 xen/common/device-tree/dom0less-build.c | 1 +
 xen/common/grant_table.c                | 2 ++
 xen/include/xen/grant_table.h           | 4 ----
 4 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/include/asm/grant_table.h b/xen/arch/arm/include/asm/grant_table.h
index c5d87b60c4..c47058a3a0 100644
--- a/xen/arch/arm/include/asm/grant_table.h
+++ b/xen/arch/arm/include/asm/grant_table.h
@@ -1,7 +1,6 @@
 #ifndef __ASM_GRANT_TABLE_H__
 #define __ASM_GRANT_TABLE_H__
 
-#include <xen/grant_table.h>
 #include <xen/kernel.h>
 #include <xen/pfn.h>
 #include <xen/sched.h>
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index 3d503c6973..d5bb1d5d35 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -26,6 +26,7 @@
 #include <public/io/xs_wire.h>
 
 #include <asm/dom0less-build.h>
+#include <asm/grant_table.h>
 #include <asm/setup.h>
 
 #include <xen/static-memory.h>
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index cf131c43a1..1e437eff50 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -42,8 +42,10 @@
 #include <xen/xvmalloc.h>
 #include <xen/nospec.h>
 #include <xsm/xsm.h>
+
 #include <asm/flushtlb.h>
 #include <asm/guest_atomics.h>
+#include <asm/grant_table.h>
 
 #ifdef CONFIG_PV_SHIM
 #include <asm/guest.h>
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 297d7669e9..98c4f4136b 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -27,10 +27,6 @@
 #include <xen/rwlock.h>
 #include <public/grant_table.h>
 
-#ifdef CONFIG_GRANT_TABLE
-#include <asm/grant_table.h>
-#endif
-
 struct grant_table;
 
 /* Seed a gnttab entry for Hyperlaunch/dom0less. */
-- 
2.43.0
Re: [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
Posted by Jan Beulich 4 months, 3 weeks ago 
On 05.06.2025 21:47, Alejandro Vallejo wrote:
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -42,8 +42,10 @@
>  #include <xen/xvmalloc.h>
>  #include <xen/nospec.h>
>  #include <xsm/xsm.h>
> +
>  #include <asm/flushtlb.h>
>  #include <asm/guest_atomics.h>
> +#include <asm/grant_table.h>
>  
>  #ifdef CONFIG_PV_SHIM
>  #include <asm/guest.h>
> --- a/xen/include/xen/grant_table.h
> +++ b/xen/include/xen/grant_table.h
> @@ -27,10 +27,6 @@
>  #include <xen/rwlock.h>
>  #include <public/grant_table.h>
>  
> -#ifdef CONFIG_GRANT_TABLE
> -#include <asm/grant_table.h>
> -#endif
> -
>  struct grant_table;
>  
>  /* Seed a gnttab entry for Hyperlaunch/dom0less. */

The description doesn't make clear why these two files need changing.

Jan
Re: [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
Posted by Alejandro Vallejo 4 months, 3 weeks ago 
On Fri Jun 6, 2025 at 8:52 AM CEST, Jan Beulich wrote:
> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -42,8 +42,10 @@
>>  #include <xen/xvmalloc.h>
>>  #include <xen/nospec.h>
>>  #include <xsm/xsm.h>
>> +
>>  #include <asm/flushtlb.h>
>>  #include <asm/guest_atomics.h>
>> +#include <asm/grant_table.h>
>>  
>>  #ifdef CONFIG_PV_SHIM
>>  #include <asm/guest.h>
>> --- a/xen/include/xen/grant_table.h
>> +++ b/xen/include/xen/grant_table.h
>> @@ -27,10 +27,6 @@
>>  #include <xen/rwlock.h>
>>  #include <public/grant_table.h>
>>  
>> -#ifdef CONFIG_GRANT_TABLE
>> -#include <asm/grant_table.h>
>> -#endif
>> -
>>  struct grant_table;
>>  
>>  /* Seed a gnttab entry for Hyperlaunch/dom0less. */
>
> The description doesn't make clear why these two files need changing.
>
> Jan

What sort of description? I removed a conditional include  and added it to one
of the few places it didn't include it already along with xen/grant_table.h.

The title does say the patch removes the crossed includes in asm/grant_table.h
and xen/grant_table.h.

It's, I hope, self-explanatory regular spring cleanup.

Cheers,
Alejandro
Re: [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
Posted by Jan Beulich 4 months, 3 weeks ago 
On 06.06.2025 12:02, Alejandro Vallejo wrote:
> On Fri Jun 6, 2025 at 8:52 AM CEST, Jan Beulich wrote:
>> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>>> --- a/xen/common/grant_table.c
>>> +++ b/xen/common/grant_table.c
>>> @@ -42,8 +42,10 @@
>>>  #include <xen/xvmalloc.h>
>>>  #include <xen/nospec.h>
>>>  #include <xsm/xsm.h>
>>> +
>>>  #include <asm/flushtlb.h>
>>>  #include <asm/guest_atomics.h>
>>> +#include <asm/grant_table.h>
>>>  
>>>  #ifdef CONFIG_PV_SHIM
>>>  #include <asm/guest.h>
>>> --- a/xen/include/xen/grant_table.h
>>> +++ b/xen/include/xen/grant_table.h
>>> @@ -27,10 +27,6 @@
>>>  #include <xen/rwlock.h>
>>>  #include <public/grant_table.h>
>>>  
>>> -#ifdef CONFIG_GRANT_TABLE
>>> -#include <asm/grant_table.h>
>>> -#endif
>>> -
>>>  struct grant_table;
>>>  
>>>  /* Seed a gnttab entry for Hyperlaunch/dom0less. */
>>
>> The description doesn't make clear why these two files need changing.
> 
> What sort of description? I removed a conditional include  and added it to one
> of the few places it didn't include it already along with xen/grant_table.h.
> 
> The title does say the patch removes the crossed includes in asm/grant_table.h
> and xen/grant_table.h.
> 
> It's, I hope, self-explanatory regular spring cleanup.

Then I'm sorry, to me it isn't. "Break links" has an entirely different (file
system) meaning to me, in the common case. Plus that says what is being done,
but not why. And it's the "why" that I'm seeking clarification on. From your
response to my remarks on v1 I was concluding that the issue is that in a few
places asm/grant_table.h would need including additionally. I didn't expect
any #include to (need to) go away.

Jan
Re: [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
Posted by Alejandro Vallejo 4 months, 3 weeks ago 
On Fri Jun 6, 2025 at 12:07 PM CEST, Jan Beulich wrote:
> On 06.06.2025 12:02, Alejandro Vallejo wrote:
>> On Fri Jun 6, 2025 at 8:52 AM CEST, Jan Beulich wrote:
>>> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>>>> --- a/xen/common/grant_table.c
>>>> +++ b/xen/common/grant_table.c
>>>> @@ -42,8 +42,10 @@
>>>>  #include <xen/xvmalloc.h>
>>>>  #include <xen/nospec.h>
>>>>  #include <xsm/xsm.h>
>>>> +
>>>>  #include <asm/flushtlb.h>
>>>>  #include <asm/guest_atomics.h>
>>>> +#include <asm/grant_table.h>
>>>>  
>>>>  #ifdef CONFIG_PV_SHIM
>>>>  #include <asm/guest.h>
>>>> --- a/xen/include/xen/grant_table.h
>>>> +++ b/xen/include/xen/grant_table.h
>>>> @@ -27,10 +27,6 @@
>>>>  #include <xen/rwlock.h>
>>>>  #include <public/grant_table.h>
>>>>  
>>>> -#ifdef CONFIG_GRANT_TABLE
>>>> -#include <asm/grant_table.h>
>>>> -#endif
>>>> -
>>>>  struct grant_table;
>>>>  
>>>>  /* Seed a gnttab entry for Hyperlaunch/dom0less. */
>>>
>>> The description doesn't make clear why these two files need changing.
>> 
>> What sort of description? I removed a conditional include  and added it to one
>> of the few places it didn't include it already along with xen/grant_table.h.
>> 
>> The title does say the patch removes the crossed includes in asm/grant_table.h
>> and xen/grant_table.h.
>> 
>> It's, I hope, self-explanatory regular spring cleanup.
>
> Then I'm sorry, to me it isn't. "Break links" has an entirely different (file
> system) meaning to me, in the common case. Plus that says what is being done,
> but not why. And it's the "why" that I'm seeking clarification on. From your
> response to my remarks on v1 I was concluding that the issue is that in a few
> places asm/grant_table.h would need including additionally. I didn't expect
> any #include to (need to) go away.
>
> Jan

Let me take a step back then. How about this commit message for this same patch?

  xen/gnttab: Remove cyclic includes in xen/grant_table.h and arm's asm/grant_table.h

  The way they currently include each other, with one of the includes being conditional
  on CONFIG_GRANT_TABLE, makes it hard to know which contents are included when.

  Seeing how nothing in either header depends on the other, let the include sites
  include both if both are needed.

  Signed-off-by: Alejandro Vallejo <agarciav@amd.com>

Cheers,
Alejandro
Re: [PATCH v2 02/15] arm/gnttab: Break links between asm/grant_table.h and xen/grant_table.h
Posted by Jan Beulich 4 months, 3 weeks ago 
On 06.06.2025 12:30, Alejandro Vallejo wrote:
> On Fri Jun 6, 2025 at 12:07 PM CEST, Jan Beulich wrote:
>> On 06.06.2025 12:02, Alejandro Vallejo wrote:
>>> On Fri Jun 6, 2025 at 8:52 AM CEST, Jan Beulich wrote:
>>>> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>>>>> --- a/xen/common/grant_table.c
>>>>> +++ b/xen/common/grant_table.c
>>>>> @@ -42,8 +42,10 @@
>>>>>  #include <xen/xvmalloc.h>
>>>>>  #include <xen/nospec.h>
>>>>>  #include <xsm/xsm.h>
>>>>> +
>>>>>  #include <asm/flushtlb.h>
>>>>>  #include <asm/guest_atomics.h>
>>>>> +#include <asm/grant_table.h>
>>>>>  
>>>>>  #ifdef CONFIG_PV_SHIM
>>>>>  #include <asm/guest.h>
>>>>> --- a/xen/include/xen/grant_table.h
>>>>> +++ b/xen/include/xen/grant_table.h
>>>>> @@ -27,10 +27,6 @@
>>>>>  #include <xen/rwlock.h>
>>>>>  #include <public/grant_table.h>
>>>>>  
>>>>> -#ifdef CONFIG_GRANT_TABLE
>>>>> -#include <asm/grant_table.h>
>>>>> -#endif
>>>>> -
>>>>>  struct grant_table;
>>>>>  
>>>>>  /* Seed a gnttab entry for Hyperlaunch/dom0less. */
>>>>
>>>> The description doesn't make clear why these two files need changing.
>>>
>>> What sort of description? I removed a conditional include  and added it to one
>>> of the few places it didn't include it already along with xen/grant_table.h.
>>>
>>> The title does say the patch removes the crossed includes in asm/grant_table.h
>>> and xen/grant_table.h.
>>>
>>> It's, I hope, self-explanatory regular spring cleanup.
>>
>> Then I'm sorry, to me it isn't. "Break links" has an entirely different (file
>> system) meaning to me, in the common case. Plus that says what is being done,
>> but not why. And it's the "why" that I'm seeking clarification on. From your
>> response to my remarks on v1 I was concluding that the issue is that in a few
>> places asm/grant_table.h would need including additionally. I didn't expect
>> any #include to (need to) go away.
> 
> Let me take a step back then. How about this commit message for this same patch?
> 
>   xen/gnttab: Remove cyclic includes in xen/grant_table.h and arm's asm/grant_table.h
> 
>   The way they currently include each other, with one of the includes being conditional
>   on CONFIG_GRANT_TABLE, makes it hard to know which contents are included when.
> 
>   Seeing how nothing in either header depends on the other, let the include sites
>   include both if both are needed.
> 
>   Signed-off-by: Alejandro Vallejo <agarciav@amd.com>

While definitely better as a description, I still don't see the point of removing
the conditional #include from xen/grant_table.h.

Jan

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.