... now that static initialization is possible. Use RADIX_TREE() for
pci_segments and ivrs_maps.

Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.

--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -31,7 +31,7 @@ static struct tasklet amd_iommu_irq_task
 unsigned int __read_mostly amd_iommu_acpi_info;
 unsigned int __read_mostly ivrs_bdf_entries;
 u8 __read_mostly ivhd_type;
-static struct radix_tree_root ivrs_maps;
+static RADIX_TREE(ivrs_maps);
 LIST_HEAD_RO_AFTER_INIT(amd_iommu_head);
 bool iommuv2_enabled;
 
@@ -1408,7 +1408,6 @@ int __init amd_iommu_prepare(bool xt)
         goto error_out;
     ivrs_bdf_entries = rc;
 
-    radix_tree_init(&ivrs_maps);
     for_each_amd_iommu ( iommu )
     {
         rc = amd_iommu_prepare_one(iommu);
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -68,7 +68,7 @@ bool pcidevs_locked(void)
     return rspin_is_locked(&_pcidevs_lock);
 }
 
-static struct radix_tree_root pci_segments;
+static RADIX_TREE(pci_segments);
 
 static inline struct pci_seg *get_pseg(u16 seg)
 {
@@ -124,7 +124,6 @@ static int pci_segments_iterate(
 
 void __init pci_segments_init(void)
 {
-    radix_tree_init(&pci_segments);
     if ( !alloc_pseg(0) )
         panic("Could not initialize PCI segment 0\n");
 }
--- a/xen/include/xen/radix-tree.h
+++ b/xen/include/xen/radix-tree.h
@@ -72,6 +72,9 @@ struct radix_tree_root {
  *** radix-tree API starts here **
  */
 
+#define RADIX_TREE_INIT() {}
+#define RADIX_TREE(name) struct radix_tree_root name = RADIX_TREE_INIT()
+
 void radix_tree_init(struct radix_tree_root *root);
 
 void radix_tree_destroy(

On 03/02/2025 4:26 pm, Jan Beulich wrote:
> ... now that static initialization is possible. Use RADIX_TREE() for
> pci_segments and ivrs_maps.
>
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I'd not considered having RADIX_TREE() but it's nicer than my attempt.

However,

> --- a/xen/include/xen/radix-tree.h
> +++ b/xen/include/xen/radix-tree.h
> @@ -72,6 +72,9 @@ struct radix_tree_root {
>   *** radix-tree API starts here **
>   */
>  
> +#define RADIX_TREE_INIT() {}

... this ought to be (struct radix_tree_root){} so it can't be used with
other types, and radix_tree_init() wants to become:

void radix_tree_init(struct radix_tree_root *root)
{
        *root = RADIX_TREE_INIT();
}

instead of the raw memset(), so the connection is retained.

Assuming you're happy with these adjustments, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>

~Andrew

On 03.02.2025 17:48, Andrew Cooper wrote:
> On 03/02/2025 4:26 pm, Jan Beulich wrote:
>> ... now that static initialization is possible. Use RADIX_TREE() for
>> pci_segments and ivrs_maps.
>>
>> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> I'd not considered having RADIX_TREE() but it's nicer than my attempt.
> 
> However,
> 
>> --- a/xen/include/xen/radix-tree.h
>> +++ b/xen/include/xen/radix-tree.h
>> @@ -72,6 +72,9 @@ struct radix_tree_root {
>>   *** radix-tree API starts here **
>>   */
>>  
>> +#define RADIX_TREE_INIT() {}
> 
> ... this ought to be (struct radix_tree_root){} so it can't be used with
> other types, and radix_tree_init() wants to become:
> 
> void radix_tree_init(struct radix_tree_root *root)
> {
>         *root = RADIX_TREE_INIT();
> }
> 
> instead of the raw memset(), so the connection is retained.

Can do; in fact I did consider both, but omitted them for simplicity.

> Assuming you're happy with these adjustments, Reviewed-by: Andrew Cooper
> <andrew.cooper3@citrix.com>

Thanks.

Jan

On 03.02.2025 17:58, Jan Beulich wrote:
> On 03.02.2025 17:48, Andrew Cooper wrote:
>> On 03/02/2025 4:26 pm, Jan Beulich wrote:
>>> ... now that static initialization is possible. Use RADIX_TREE() for
>>> pci_segments and ivrs_maps.
>>>
>>> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> I'd not considered having RADIX_TREE() but it's nicer than my attempt.
>>
>> However,
>>
>>> --- a/xen/include/xen/radix-tree.h
>>> +++ b/xen/include/xen/radix-tree.h
>>> @@ -72,6 +72,9 @@ struct radix_tree_root {
>>>   *** radix-tree API starts here **
>>>   */
>>>  
>>> +#define RADIX_TREE_INIT() {}
>>
>> ... this ought to be (struct radix_tree_root){} so it can't be used with
>> other types, and radix_tree_init() wants to become:
>>
>> void radix_tree_init(struct radix_tree_root *root)
>> {
>>         *root = RADIX_TREE_INIT();
>> }
>>
>> instead of the raw memset(), so the connection is retained.
> 
> Can do; in fact I did consider both, but omitted them for simplicity.

Sadly I've now learned the hard way that the former can't be done. Gcc
4.3 complains "initializer element is not constant", which - aiui - is
correct when considering plain C99 (and apparently the GNU99 extension
to this was introduced only later).

What I can do is make this compiler version dependent, adding type-
safety on at least all more modern compilers. Thoughts?

Jan

On 04/02/2025 8:36 am, Jan Beulich wrote:
> On 03.02.2025 17:58, Jan Beulich wrote:
>> On 03.02.2025 17:48, Andrew Cooper wrote:
>>> On 03/02/2025 4:26 pm, Jan Beulich wrote:
>>>> ... now that static initialization is possible. Use RADIX_TREE() for
>>>> pci_segments and ivrs_maps.
>>>>
>>>> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> I'd not considered having RADIX_TREE() but it's nicer than my attempt.
>>>
>>> However,
>>>
>>>> --- a/xen/include/xen/radix-tree.h
>>>> +++ b/xen/include/xen/radix-tree.h
>>>> @@ -72,6 +72,9 @@ struct radix_tree_root {
>>>>   *** radix-tree API starts here **
>>>>   */
>>>>  
>>>> +#define RADIX_TREE_INIT() {}
>>> ... this ought to be (struct radix_tree_root){} so it can't be used with
>>> other types, and radix_tree_init() wants to become:
>>>
>>> void radix_tree_init(struct radix_tree_root *root)
>>> {
>>>         *root = RADIX_TREE_INIT();
>>> }
>>>
>>> instead of the raw memset(), so the connection is retained.
>> Can do; in fact I did consider both, but omitted them for simplicity.
> Sadly I've now learned the hard way that the former can't be done. Gcc
> 4.3 complains "initializer element is not constant", which - aiui - is
> correct when considering plain C99 (and apparently the GNU99 extension
> to this was introduced only later).
>
> What I can do is make this compiler version dependent, adding type-
> safety on at least all more modern compilers. Thoughts?

I think you can guess what my view is about us still supporting GCC 4.3.

As this needs backporting, lets just go with {} for now.

~Andrew

On 04.02.2025 09:36, Jan Beulich wrote:
> On 03.02.2025 17:58, Jan Beulich wrote:
>> On 03.02.2025 17:48, Andrew Cooper wrote:
>>> On 03/02/2025 4:26 pm, Jan Beulich wrote:
>>>> ... now that static initialization is possible. Use RADIX_TREE() for
>>>> pci_segments and ivrs_maps.
>>>>
>>>> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> I'd not considered having RADIX_TREE() but it's nicer than my attempt.
>>>
>>> However,
>>>
>>>> --- a/xen/include/xen/radix-tree.h
>>>> +++ b/xen/include/xen/radix-tree.h
>>>> @@ -72,6 +72,9 @@ struct radix_tree_root {
>>>>   *** radix-tree API starts here **
>>>>   */
>>>>  
>>>> +#define RADIX_TREE_INIT() {}
>>>
>>> ... this ought to be (struct radix_tree_root){} so it can't be used with
>>> other types, and radix_tree_init() wants to become:
>>>
>>> void radix_tree_init(struct radix_tree_root *root)
>>> {
>>>         *root = RADIX_TREE_INIT();
>>> }
>>>
>>> instead of the raw memset(), so the connection is retained.
>>
>> Can do; in fact I did consider both, but omitted them for simplicity.
> 
> Sadly I've now learned the hard way that the former can't be done. Gcc
> 4.3 complains "initializer element is not constant", which - aiui - is
> correct when considering plain C99 (and apparently the GNU99 extension
> to this was introduced only later).

And then I can't use RADIX_TREE_INIT() in radix_tree_init() anymore. All
quite unhelpful.

Jan

> What I can do is make this compiler version dependent, adding type-
> safety on at least all more modern compilers. Thoughts?
> 
> Jan