[PATCH 6/8] iommu/dart: Move the blocked domain support to a global static

[Robin Murphy]

On 2023-09-26 20:34, Robin Murphy wrote:
> On 2023-09-26 20:05, Janne Grunau wrote:
>> Hej,
>>
>> On Fri, Sep 22, 2023 at 02:07:57PM -0300, Jason Gunthorpe wrote:
>>> Move to the new static global for blocked domains. Move the blocked
>>> specific code to apple_dart_attach_dev_blocked().
>>>
>>> Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
>>> ---
>>>   drivers/iommu/apple-dart.c | 36 ++++++++++++++++++++++++++----------
>>>   1 file changed, 26 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
>>> index 424f779ccc34df..376f4c5461e8f7 100644
>>> --- a/drivers/iommu/apple-dart.c
>>> +++ b/drivers/iommu/apple-dart.c
>>> @@ -675,10 +675,6 @@ static int apple_dart_attach_dev(struct 
>>> iommu_domain *domain,
>>>           for_each_stream_map(i, cfg, stream_map)
>>>               apple_dart_setup_translation(dart_domain, stream_map);
>>>           break;
>>> -    case IOMMU_DOMAIN_BLOCKED:
>>> -        for_each_stream_map(i, cfg, stream_map)
>>> -            apple_dart_hw_disable_dma(stream_map);
>>> -        break;
>>>       default:
>>>           return -EINVAL;
>>>       }
>>> @@ -710,6 +706,30 @@ static struct iommu_domain 
>>> apple_dart_identity_domain = {
>>>       .ops = &apple_dart_identity_ops,
>>>   };
>>> +static int apple_dart_attach_dev_blocked(struct iommu_domain *domain,
>>> +                     struct device *dev)
>>> +{
>>> +    struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
>>> +    struct apple_dart_stream_map *stream_map;
>>> +    int i;
>>> +
>>> +    if (cfg->stream_maps[0].dart->force_bypass)
>>> +        return -EINVAL;
>>
>> unrelated to this change as this keeps the current behavior but I think
>> force_bypass should not override IOMMU_DOMAIN_BLOCKED. It is set if the
>> CPU page size is smaller than dart's page size. Obviously dart can't
>> translate in that situation but it should be still possible to block it
>> completely.
>>
>> How do we manage this? I can write a patch either to the current state
>> or based on this series.
> 
> The series is queued already, so best to send a patch based on 
> iommu/core (I guess just removing these lines?).

Um, what? This isn't the domain_alloc_paging series itself, Robin you 
fool. Clearly it's time to close the computer and try again tomorrow...

Cheers,
Robin.

> It won't be 
> super-useful in practice since the blocking domain is normally only used 
> to transition to an unmanaged domain which in the force_bypass situation 
> can't be used anyway, but it's still nice on principle not to have 
> unnecessary reasons for attach to fail.
> 
> Thanks,
> Robin.
> 
>>
>>> +
>>> +    for_each_stream_map(i, cfg, stream_map)
>>> +        apple_dart_hw_disable_dma(stream_map);
>>> +    return 0;
>>> +}
>>> +
>>> +static const struct iommu_domain_ops apple_dart_blocked_ops = {
>>> +    .attach_dev = apple_dart_attach_dev_blocked,
>>> +};
>>> +
>>> +static struct iommu_domain apple_dart_blocked_domain = {
>>> +    .type = IOMMU_DOMAIN_BLOCKED,
>>> +    .ops = &apple_dart_blocked_ops,
>>> +};
>>> +
>>>   static struct iommu_device *apple_dart_probe_device(struct device 
>>> *dev)
>>>   {
>>>       struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
>>> @@ -739,8 +759,7 @@ static struct iommu_domain 
>>> *apple_dart_domain_alloc(unsigned int type)
>>>   {
>>>       struct apple_dart_domain *dart_domain;
>>> -    if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED &&
>>> -        type != IOMMU_DOMAIN_BLOCKED)
>>> +    if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED)
>>>           return NULL;
>>>       dart_domain = kzalloc(sizeof(*dart_domain), GFP_KERNEL);
>>> @@ -749,10 +768,6 @@ static struct iommu_domain 
>>> *apple_dart_domain_alloc(unsigned int type)
>>>       mutex_init(&dart_domain->init_lock);
>>> -    /* no need to allocate pgtbl_ops or do any other finalization 
>>> steps */
>>> -    if (type == IOMMU_DOMAIN_BLOCKED)
>>> -        dart_domain->finalized = true;
>>> -
>>>       return &dart_domain->domain;
>>>   }
>>> @@ -966,6 +981,7 @@ static void apple_dart_get_resv_regions(struct 
>>> device *dev,
>>>   static const struct iommu_ops apple_dart_iommu_ops = {
>>>       .identity_domain = &apple_dart_identity_domain,
>>> +    .blocked_domain = &apple_dart_blocked_domain,
>>>       .domain_alloc = apple_dart_domain_alloc,
>>>       .probe_device = apple_dart_probe_device,
>>>       .release_device = apple_dart_release_device,
>>> -- 
>>> 2.42.0
>>
>> Reviewed-by: Janne Grunau <j at jannau.net>
>>
>> best regards
>> Janne
>>
>> ps: I sent the reply to [Patch 4/8] accidentally with an incorrect from
>> address but the correct Reviewed-by:. I can resend if necessary.
>