Hi,
On 03/10/2019 13:18, Oleksandr wrote:
>
> On 01.10.19 22:07, Julien Grall wrote:
>> On 10/1/19 5:07 PM, Oleksandr wrote:
>>>
>>> On 01.10.19 18:36, Julien Grall wrote:
>>>> On 01/10/2019 16:25, Oleksandr wrote:
>>>>>
>>>>> On 01.10.19 18:04, Julien Grall wrote:
>>> > 1. Giving the IOMMU to Dom0 is a bad idea.
>>
>> Please to try expand your thoughts in the same e-mail when you say
>> "this is a bad idea".
>
> Well, this was a conclusion I had got from the discussion [1]. Sorry for
> not being clear here.
>
>
>>
>> But, this is clearly what happen in current Xen setup if the driver is
>> not enabled. What I want to avoid is exposing an half complete
>> bindings to the guest (you don't know how it will behave).
>>
>> So we either remove all the properties and node related to the IOMMUs
>> or nothing.
> I think, I got it. Our target is *not* to add a way for Dom0 to use
> IOMMU, but to be consistent in removing IOMMU node/master device
> properties. Now, we remove the IOMMU node if Xen identifies it (the
> IOMMU driver is present in Xen),
> so looks like we have to remove master device properties only if this
> master device is behind the IOMMU which node is removed. This, I hope,
> will avoid exposing an half complete bindings to guest. Am I right?
>
>
> [1]
> https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg00858.html
>
>
> ----------
>
> If you happy with that logic I will craft a proper patch.
The logic looks alright to me. One comment below, I will look at the
rest once this is formally sent.
>
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 67021d9..6d45e55 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -480,10 +480,26 @@ static int __init write_properties(struct domain
> *d, struct kernel_info *kinfo,
> const struct dt_property *prop, *status = NULL;
> int res = 0;
> int had_dom0_bootargs = 0;
> + struct dt_device_node *iommu_node;
>
> if ( kinfo->cmdline && kinfo->cmdline[0] )
> bootargs = &kinfo->cmdline[0];
>
> + /*
> + * If we skip the IOMMU device when creating DT for Dom0 (even if
I would prefer if we use "hwdom" over "Dom0". They are both the same on
Arm, but this may change in the future (we may actually drop Dom0 ;)).
> + * the IOMMU device is not used by Xen), we should also skip the IOMMU
> + * specific properties of the master device behind it in order to
> avoid
> + * exposing an half complete IOMMU bindings to Dom0.
> + * Use "iommu_node" as an indicator of the master device which
> properties
> + * should be skipped.
> + */
> + iommu_node = dt_parse_phandle(node, "iommus", 0);
> + if ( iommu_node )
> + {
> + if ( device_get_class(iommu_node) != DEVICE_IOMMU )
> + iommu_node = NULL;
> + }
> +
> dt_for_each_property_node (node, prop)
> {
> const void *prop_data = prop->value;
> @@ -540,6 +556,19 @@ static int __init write_properties(struct domain
> *d, struct kernel_info *kinfo,
> continue;
> }
>
> + if ( iommu_node )
> + {
> + /* Don't expose IOMMU specific properties to Dom0 */
> + if ( dt_property_name_is_equal(prop, "iommus") )
> + continue;
> +
> + if ( dt_property_name_is_equal(prop, "iommu-map") )
> + continue;
> +
> + if ( dt_property_name_is_equal(prop, "iommu-map-mask") )
> + continue;
> + }
> +
> res = fdt_property(kinfo->fdt, prop->name, prop_data, prop_len);
>
> if ( res )
>
>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
