[PATCH] iommu/arm-smmu-v3: Add default domain quirk for Arm Fast Models

[PATCH] iommu/arm-smmu-v3: Add default domain quirk for Arm Fast Models

[Robin Murphy]

Arm Fast Models are still implementing legacy virtio-pci devices behind
the SMMU, which continue to be problematic as "real hardware" devices
(from the point of view of the simulated system) without the mitigating
VIRTIO_F_ACCESS_PLATFORM feature.

Since we now have the ability to force passthrough on a device-specific
basis, let's use it to work around this particular oddity so that people
who just want to boot Linux on a model don't have to fiddle around with
things to avoid the SMMU getting in the way by default (and I don't have
to keep telling them which things...)

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 54b2f27b81d4..13cf16e8f45b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2649,6 +2649,20 @@ static int arm_smmu_dev_disable_feature(struct device *dev,
 	}
 }
 
+static int arm_smmu_def_domain_type(struct device *dev)
+{
+	if (dev_is_pci(dev)) {
+		struct pci_dev *pdev = to_pci_dev(dev);
+
+		/* Legacy virtio-block devices on Arm Fast Models */
+		if (pdev->vendor == 0x1af4 && pdev->device == 0x1001 &&
+		    pdev->subsystem_vendor == 0x00ff && pdev->subsystem_device == 0x0002)
+			return IOMMU_DOMAIN_IDENTITY;
+	}
+
+	return 0;
+}
+
 static struct iommu_ops arm_smmu_ops = {
 	.capable		= arm_smmu_capable,
 	.domain_alloc		= arm_smmu_domain_alloc,
@@ -2673,6 +2687,7 @@ static struct iommu_ops arm_smmu_ops = {
 	.sva_bind		= arm_smmu_sva_bind,
 	.sva_unbind		= arm_smmu_sva_unbind,
 	.sva_get_pasid		= arm_smmu_sva_get_pasid,
+	.def_domain_type	= arm_smmu_def_domain_type,
 	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
 	.owner			= THIS_MODULE,
 };
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-v3: Add default domain quirk for Arm Fast Models

[Andre Przywara]

On Fri, 18 Jun 2021 17:24:49 +0100
Robin Murphy <robin.murphy@arm.com> wrote:

Hi Robin,

> Arm Fast Models are still implementing legacy virtio-pci devices behind
> the SMMU, which continue to be problematic as "real hardware" devices
> (from the point of view of the simulated system) without the mitigating
> VIRTIO_F_ACCESS_PLATFORM feature.
> 
> Since we now have the ability to force passthrough on a device-specific
> basis, let's use it to work around this particular oddity so that people
> who just want to boot Linux on a model don't have to fiddle around with
> things to avoid the SMMU getting in the way by default (and I don't have
> to keep telling them which things...)
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

So it looks somewhat quirky, but it indeed fixes the boot problems on
the RevC model for me.
And given that currently an out-of-the-box kernel build fails booting
(with the kernel DT), I'd like to see that merged.

Tested-by: Andre Przywara <andre.przywara@arm.com>

Thanks,
Andre

> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 54b2f27b81d4..13cf16e8f45b 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2649,6 +2649,20 @@ static int arm_smmu_dev_disable_feature(struct device *dev,
>  	}
>  }
>  
> +static int arm_smmu_def_domain_type(struct device *dev)
> +{
> +	if (dev_is_pci(dev)) {
> +		struct pci_dev *pdev = to_pci_dev(dev);
> +
> +		/* Legacy virtio-block devices on Arm Fast Models */
> +		if (pdev->vendor == 0x1af4 && pdev->device == 0x1001 &&
> +		    pdev->subsystem_vendor == 0x00ff && pdev->subsystem_device == 0x0002)
> +			return IOMMU_DOMAIN_IDENTITY;
> +	}
> +
> +	return 0;
> +}
> +
>  static struct iommu_ops arm_smmu_ops = {
>  	.capable		= arm_smmu_capable,
>  	.domain_alloc		= arm_smmu_domain_alloc,
> @@ -2673,6 +2687,7 @@ static struct iommu_ops arm_smmu_ops = {
>  	.sva_bind		= arm_smmu_sva_bind,
>  	.sva_unbind		= arm_smmu_sva_unbind,
>  	.sva_get_pasid		= arm_smmu_sva_get_pasid,
> +	.def_domain_type	= arm_smmu_def_domain_type,
>  	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
>  	.owner			= THIS_MODULE,
>  };


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-v3: Add default domain quirk for Arm Fast Models

[Will Deacon]

On Fri, Jun 18, 2021 at 05:24:49PM +0100, Robin Murphy wrote:
> Arm Fast Models are still implementing legacy virtio-pci devices behind
> the SMMU, which continue to be problematic as "real hardware" devices
> (from the point of view of the simulated system) without the mitigating
> VIRTIO_F_ACCESS_PLATFORM feature.
> 
> Since we now have the ability to force passthrough on a device-specific
> basis, let's use it to work around this particular oddity so that people
> who just want to boot Linux on a model don't have to fiddle around with
> things to avoid the SMMU getting in the way by default (and I don't have
> to keep telling them which things...)
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)

Any chance of getting the fastmodels updated instead? It feels like it
has to happen *eventually*, and then there would be no need for this bodge.

Will

> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 54b2f27b81d4..13cf16e8f45b 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2649,6 +2649,20 @@ static int arm_smmu_dev_disable_feature(struct device *dev,
>  	}
>  }
>  
> +static int arm_smmu_def_domain_type(struct device *dev)
> +{
> +	if (dev_is_pci(dev)) {
> +		struct pci_dev *pdev = to_pci_dev(dev);
> +
> +		/* Legacy virtio-block devices on Arm Fast Models */
> +		if (pdev->vendor == 0x1af4 && pdev->device == 0x1001 &&
> +		    pdev->subsystem_vendor == 0x00ff && pdev->subsystem_device == 0x0002)
> +			return IOMMU_DOMAIN_IDENTITY;
> +	}
> +
> +	return 0;
> +}
> +
>  static struct iommu_ops arm_smmu_ops = {
>  	.capable		= arm_smmu_capable,
>  	.domain_alloc		= arm_smmu_domain_alloc,
> @@ -2673,6 +2687,7 @@ static struct iommu_ops arm_smmu_ops = {
>  	.sva_bind		= arm_smmu_sva_bind,
>  	.sva_unbind		= arm_smmu_sva_unbind,
>  	.sva_get_pasid		= arm_smmu_sva_get_pasid,
> +	.def_domain_type	= arm_smmu_def_domain_type,
>  	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
>  	.owner			= THIS_MODULE,
>  };
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-v3: Add default domain quirk for Arm Fast Models

[Robin Murphy]

On 2021-06-29 18:34, Will Deacon wrote:
> On Fri, Jun 18, 2021 at 05:24:49PM +0100, Robin Murphy wrote:
>> Arm Fast Models are still implementing legacy virtio-pci devices behind
>> the SMMU, which continue to be problematic as "real hardware" devices
>> (from the point of view of the simulated system) without the mitigating
>> VIRTIO_F_ACCESS_PLATFORM feature.
>>
>> Since we now have the ability to force passthrough on a device-specific
>> basis, let's use it to work around this particular oddity so that people
>> who just want to boot Linux on a model don't have to fiddle around with
>> things to avoid the SMMU getting in the way by default (and I don't have
>> to keep telling them which things...)
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
> 
> Any chance of getting the fastmodels updated instead? It feels like it
> has to happen *eventually*, and then there would be no need for this bodge.

If it's any consolation the JIRA ticket you raised in 2017 is still open...

Robin.

> 
> Will
> 
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> index 54b2f27b81d4..13cf16e8f45b 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> @@ -2649,6 +2649,20 @@ static int arm_smmu_dev_disable_feature(struct device *dev,
>>   	}
>>   }
>>   
>> +static int arm_smmu_def_domain_type(struct device *dev)
>> +{
>> +	if (dev_is_pci(dev)) {
>> +		struct pci_dev *pdev = to_pci_dev(dev);
>> +
>> +		/* Legacy virtio-block devices on Arm Fast Models */
>> +		if (pdev->vendor == 0x1af4 && pdev->device == 0x1001 &&
>> +		    pdev->subsystem_vendor == 0x00ff && pdev->subsystem_device == 0x0002)
>> +			return IOMMU_DOMAIN_IDENTITY;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static struct iommu_ops arm_smmu_ops = {
>>   	.capable		= arm_smmu_capable,
>>   	.domain_alloc		= arm_smmu_domain_alloc,
>> @@ -2673,6 +2687,7 @@ static struct iommu_ops arm_smmu_ops = {
>>   	.sva_bind		= arm_smmu_sva_bind,
>>   	.sva_unbind		= arm_smmu_sva_unbind,
>>   	.sva_get_pasid		= arm_smmu_sva_get_pasid,
>> +	.def_domain_type	= arm_smmu_def_domain_type,
>>   	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
>>   	.owner			= THIS_MODULE,
>>   };
>> -- 
>> 2.25.1
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-v3: Add default domain quirk for Arm Fast Models

[Marc Zyngier]

On Tue, 29 Jun 2021 18:34:40 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Fri, Jun 18, 2021 at 05:24:49PM +0100, Robin Murphy wrote:
> > Arm Fast Models are still implementing legacy virtio-pci devices behind
> > the SMMU, which continue to be problematic as "real hardware" devices
> > (from the point of view of the simulated system) without the mitigating
> > VIRTIO_F_ACCESS_PLATFORM feature.
> > 
> > Since we now have the ability to force passthrough on a device-specific
> > basis, let's use it to work around this particular oddity so that people
> > who just want to boot Linux on a model don't have to fiddle around with
> > things to avoid the SMMU getting in the way by default (and I don't have
> > to keep telling them which things...)
> > 
> > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > ---
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> 
> Any chance of getting the fastmodels updated instead? It feels like it
> has to happen *eventually*, and then there would be no need for this bodge.

That'd be ideal. What are the chances of that happening before the Sun
turns into a black hole?

> Will
> 
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index 54b2f27b81d4..13cf16e8f45b 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -2649,6 +2649,20 @@ static int arm_smmu_dev_disable_feature(struct device *dev,
> >  	}
> >  }
> >  
> > +static int arm_smmu_def_domain_type(struct device *dev)
> > +{
> > +	if (dev_is_pci(dev)) {
> > +		struct pci_dev *pdev = to_pci_dev(dev);
> > +
> > +		/* Legacy virtio-block devices on Arm Fast Models */
> > +		if (pdev->vendor == 0x1af4 && pdev->device == 0x1001 &&
> > +		    pdev->subsystem_vendor == 0x00ff && pdev->subsystem_device == 0x0002)
> > +			return IOMMU_DOMAIN_IDENTITY;
> > +	}
> > +
> > +	return 0;
> > +}

Could this be expressed as a PCI quirk instead? It would at least keep
the ID matching out of the SMMU driver...

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-v3: Add default domain quirk for Arm Fast Models

[Robin Murphy]

On 2021-06-30 09:56, Marc Zyngier wrote:
> On Tue, 29 Jun 2021 18:34:40 +0100,
> Will Deacon <will@kernel.org> wrote:
>>
>> On Fri, Jun 18, 2021 at 05:24:49PM +0100, Robin Murphy wrote:
>>> Arm Fast Models are still implementing legacy virtio-pci devices behind
>>> the SMMU, which continue to be problematic as "real hardware" devices
>>> (from the point of view of the simulated system) without the mitigating
>>> VIRTIO_F_ACCESS_PLATFORM feature.
>>>
>>> Since we now have the ability to force passthrough on a device-specific
>>> basis, let's use it to work around this particular oddity so that people
>>> who just want to boot Linux on a model don't have to fiddle around with
>>> things to avoid the SMMU getting in the way by default (and I don't have
>>> to keep telling them which things...)
>>>
>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>> ---
>>>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>
>> Any chance of getting the fastmodels updated instead? It feels like it
>> has to happen *eventually*, and then there would be no need for this bodge.
> 
> That'd be ideal. What are the chances of that happening before the Sun
> turns into a black hole?

We'll try making some noise again internally and see where that goes, 
but given the progress over the last 4 years I'd be inclined to posit a 
new theory of the universe eventually ending in one giant event 0x10.

>>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>> index 54b2f27b81d4..13cf16e8f45b 100644
>>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>> @@ -2649,6 +2649,20 @@ static int arm_smmu_dev_disable_feature(struct device *dev,
>>>   	}
>>>   }
>>>   
>>> +static int arm_smmu_def_domain_type(struct device *dev)
>>> +{
>>> +	if (dev_is_pci(dev)) {
>>> +		struct pci_dev *pdev = to_pci_dev(dev);
>>> +
>>> +		/* Legacy virtio-block devices on Arm Fast Models */
>>> +		if (pdev->vendor == 0x1af4 && pdev->device == 0x1001 &&
>>> +		    pdev->subsystem_vendor == 0x00ff && pdev->subsystem_device == 0x0002)
>>> +			return IOMMU_DOMAIN_IDENTITY;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
> 
> Could this be expressed as a PCI quirk instead? It would at least keep
> the ID matching out of the SMMU driver...

I don't think so - getting the information from the PCI layer to the 
IOMMU layer at the right place and time to influence the default domain 
choice would seemingly need some uglier and even less popular 
infrastructure adding. It's something that only the IOMMU layer has any 
need to be aware of, and the def_domain_type callback essentially exists 
for this kind of special-casing - compare intel-iommu's 
device_def_domain_type() for example. If we accept a workaround at all, 
I do believe this is the least-worst option.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel