[v1,04/14] iommu/arm-smmu-v3: Add arm_smmu_hw_info

Message ID 494e36cbb77d49e11427b308868dbc1b0e19fe18.1678348754.git.nicolinc@nvidia.com
State New
Headers
Series Add Nested Translation Support for SMMUv3 |

Commit Message

Nicolin Chen March 9, 2023, 10:53 a.m. UTC
  This is used to forward the host IDR values to the user space, so the
hypervisor and the guest VM can learn about the underlying hardware's
capabilities.

Also, set the driver_type to IOMMU_HW_INFO_TYPE_ARM_SMMUV3 to pass the
corresponding type sanity in the core.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 +++++++++++++++++++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  2 ++
 include/uapi/linux/iommufd.h                | 14 ++++++++++++
 3 files changed, 41 insertions(+)
  

Comments

Robin Murphy March 9, 2023, 1:03 p.m. UTC | #1
On 2023-03-09 10:53, Nicolin Chen wrote:
> This is used to forward the host IDR values to the user space, so the
> hypervisor and the guest VM can learn about the underlying hardware's
> capabilities.
> 
> Also, set the driver_type to IOMMU_HW_INFO_TYPE_ARM_SMMUV3 to pass the
> corresponding type sanity in the core.
> 
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 +++++++++++++++++++++
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  2 ++
>   include/uapi/linux/iommufd.h                | 14 ++++++++++++
>   3 files changed, 41 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 f2425b0f0cd6..c1aac695ae0d 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2005,6 +2005,29 @@ static bool arm_smmu_capable(struct device *dev, enum iommu_cap cap)
>   	}
>   }
>   
> +static void *arm_smmu_hw_info(struct device *dev, u32 *length)
> +{
> +	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> +	struct iommu_hw_info_smmuv3 *info;
> +	void *base_idr;
> +	int i;
> +
> +	if (!master || !master->smmu)
> +		return ERR_PTR(-ENODEV);
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return ERR_PTR(-ENOMEM);
> +
> +	base_idr = master->smmu->base + ARM_SMMU_IDR0;
> +	for (i = 0; i <= 5; i++)
> +		info->idr[i] = readl_relaxed(base_idr + 0x4 * i);

You need to take firmware overrides etc. into account here. In 
particular, features like BTM may need to be hidden to work around 
errata either in the system integration or the SMMU itself. It isn't 
reasonable to expect every VMM to be aware of every erratum and 
workaround, and there may even be workarounds where we need to go out of 
our way to prevent guests from trying to use certain features in order 
to maintain correctness at S2.

In general this should probably follow the same principle as KVM, where 
we only expose sanitised feature registers representing the 
functionality the host understands. Code written today is almost 
guaranteed to be running on hardware released in 2030, at least *somewhere*.

Thanks,
Robin.

> +
> +	*length = sizeof(*info);
> +
> +	return info;
> +}
> +
>   static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
>   {
>   	struct arm_smmu_domain *smmu_domain;
> @@ -2845,6 +2868,7 @@ static void arm_smmu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
>   
>   static struct iommu_ops arm_smmu_ops = {
>   	.capable		= arm_smmu_capable,
> +	.hw_info		= arm_smmu_hw_info,
>   	.domain_alloc		= arm_smmu_domain_alloc,
>   	.probe_device		= arm_smmu_probe_device,
>   	.release_device		= arm_smmu_release_device,
> @@ -2857,6 +2881,7 @@ static struct iommu_ops arm_smmu_ops = {
>   	.page_response		= arm_smmu_page_response,
>   	.def_domain_type	= arm_smmu_def_domain_type,
>   	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
> +	.driver_type		= IOMMU_HW_INFO_TYPE_ARM_SMMUV3,
>   	.owner			= THIS_MODULE,
>   	.default_domain_ops = &(const struct iommu_domain_ops) {
>   		.attach_dev		= arm_smmu_attach_dev,
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 8d772ea8a583..ba2b4562f4b2 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -14,6 +14,8 @@
>   #include <linux/mmzone.h>
>   #include <linux/sizes.h>
>   
> +#include <uapi/linux/iommufd.h>
> +
>   /* MMIO registers */
>   #define ARM_SMMU_IDR0			0x0
>   #define IDR0_ST_LVL			GENMASK(28, 27)
> diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
> index 0d5551b1b2be..c7a37915b49c 100644
> --- a/include/uapi/linux/iommufd.h
> +++ b/include/uapi/linux/iommufd.h
> @@ -519,6 +519,20 @@ struct iommu_hw_info_vtd {
>   	__aligned_u64 ecap_reg;
>   };
>   
> +/**
> + * struct iommu_hw_info_smmuv3 - ARM SMMUv3 device info
> + *
> + * @flags: Must be set to 0
> + * @__reserved: Must be 0
> + * @idr: Implemented features for the SMMU Non-secure programming interface.
> + *       Please refer to the chapters from 6.3.1 to 6.3.6 in the SMMUv3 Spec.
> + */
> +struct iommu_hw_info_smmuv3 {
> +	__u32 flags;
> +	__u32 __reserved;
> +	__u32 idr[6];
> +};
> +
>   /**
>    * struct iommu_hw_info - ioctl(IOMMU_DEVICE_GET_HW_INFO)
>    * @size: sizeof(struct iommu_hw_info)
  
Nicolin Chen March 10, 2023, 1:17 a.m. UTC | #2
Hi Robin,

Thanks for the inputs.

On Thu, Mar 09, 2023 at 01:03:41PM +0000, Robin Murphy wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 2023-03-09 10:53, Nicolin Chen wrote:
> > This is used to forward the host IDR values to the user space, so the
> > hypervisor and the guest VM can learn about the underlying hardware's
> > capabilities.
> > 
> > Also, set the driver_type to IOMMU_HW_INFO_TYPE_ARM_SMMUV3 to pass the
> > corresponding type sanity in the core.
> > 
> > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> > ---
> >   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 +++++++++++++++++++++
> >   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  2 ++
> >   include/uapi/linux/iommufd.h                | 14 ++++++++++++
> >   3 files changed, 41 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 f2425b0f0cd6..c1aac695ae0d 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -2005,6 +2005,29 @@ static bool arm_smmu_capable(struct device *dev, enum iommu_cap cap)
> >       }
> >   }
> > 
> > +static void *arm_smmu_hw_info(struct device *dev, u32 *length)
> > +{
> > +     struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> > +     struct iommu_hw_info_smmuv3 *info;
> > +     void *base_idr;
> > +     int i;
> > +
> > +     if (!master || !master->smmu)
> > +             return ERR_PTR(-ENODEV);
> > +
> > +     info = kzalloc(sizeof(*info), GFP_KERNEL);
> > +     if (!info)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     base_idr = master->smmu->base + ARM_SMMU_IDR0;
> > +     for (i = 0; i <= 5; i++)
> > +             info->idr[i] = readl_relaxed(base_idr + 0x4 * i);
> 
> You need to take firmware overrides etc. into account here. In
> particular, features like BTM may need to be hidden to work around
> errata either in the system integration or the SMMU itself. It isn't
> reasonable to expect every VMM to be aware of every erratum and
> workaround, and there may even be workarounds where we need to go out of
> our way to prevent guests from trying to use certain features in order
> to maintain correctness at S2.

We can add a bit of overrides after this for errata, perhaps?

I have some trouble with finding the errata docs. Would it be
possible for you to direct me to it with a link maybe?

> In general this should probably follow the same principle as KVM, where
> we only expose sanitised feature registers representing the
> functionality the host understands. Code written today is almost
> guaranteed to be running on hardware released in 2030, at least *somewhere*.

Yes.

Thanks
Nicolin
  
Robin Murphy March 10, 2023, 3:28 p.m. UTC | #3
On 2023-03-10 01:17, Nicolin Chen wrote:
> Hi Robin,
> 
> Thanks for the inputs.
> 
> On Thu, Mar 09, 2023 at 01:03:41PM +0000, Robin Murphy wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 2023-03-09 10:53, Nicolin Chen wrote:
>>> This is used to forward the host IDR values to the user space, so the
>>> hypervisor and the guest VM can learn about the underlying hardware's
>>> capabilities.
>>>
>>> Also, set the driver_type to IOMMU_HW_INFO_TYPE_ARM_SMMUV3 to pass the
>>> corresponding type sanity in the core.
>>>
>>> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
>>> ---
>>>    drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 +++++++++++++++++++++
>>>    drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  2 ++
>>>    include/uapi/linux/iommufd.h                | 14 ++++++++++++
>>>    3 files changed, 41 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 f2425b0f0cd6..c1aac695ae0d 100644
>>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>> @@ -2005,6 +2005,29 @@ static bool arm_smmu_capable(struct device *dev, enum iommu_cap cap)
>>>        }
>>>    }
>>>
>>> +static void *arm_smmu_hw_info(struct device *dev, u32 *length)
>>> +{
>>> +     struct arm_smmu_master *master = dev_iommu_priv_get(dev);
>>> +     struct iommu_hw_info_smmuv3 *info;
>>> +     void *base_idr;
>>> +     int i;
>>> +
>>> +     if (!master || !master->smmu)
>>> +             return ERR_PTR(-ENODEV);
>>> +
>>> +     info = kzalloc(sizeof(*info), GFP_KERNEL);
>>> +     if (!info)
>>> +             return ERR_PTR(-ENOMEM);
>>> +
>>> +     base_idr = master->smmu->base + ARM_SMMU_IDR0;
>>> +     for (i = 0; i <= 5; i++)
>>> +             info->idr[i] = readl_relaxed(base_idr + 0x4 * i);
>>
>> You need to take firmware overrides etc. into account here. In
>> particular, features like BTM may need to be hidden to work around
>> errata either in the system integration or the SMMU itself. It isn't
>> reasonable to expect every VMM to be aware of every erratum and
>> workaround, and there may even be workarounds where we need to go out of
>> our way to prevent guests from trying to use certain features in order
>> to maintain correctness at S2.
> 
> We can add a bit of overrides after this for errata, perhaps?
> 
> I have some trouble with finding the errata docs. Would it be
> possible for you to direct me to it with a link maybe?

The key Arm term is "Software Developer Errata Notice", or just SDEN. 
Here's the ones for MMU-600 and MMU-700:

https://developer.arm.com/documentation/SDEN-946810/latest/
https://developer.arm.com/documentation/SDEN-1786925/latest/

Note that until now it has been extremely fortunate that in pretty much 
every case Linux either hasn't supported the affected feature at all, or 
has happened to avoid meeting the conditions. Once we do introduce 
nesting support that all goes out the window (and I'll have to think 
more when reviewing new errata in future...)

I've been putting off revisiting all the existing errata to figure out 
what we'd need to do until new nesting patches appeared, so I'll try to 
get to that soon now. I think in many cases it's likely to be best to 
just disallowing nesting entirely on affected implementations.

Thanks,
Robin.

>> In general this should probably follow the same principle as KVM, where
>> we only expose sanitised feature registers representing the
>> functionality the host understands. Code written today is almost
>> guaranteed to be running on hardware released in 2030, at least *somewhere*.
> 
> Yes.
> 
> Thanks
> Nicolin
  
Nicolin Chen March 16, 2023, 12:13 a.m. UTC | #4
On Fri, Mar 10, 2023 at 03:28:56PM +0000, Robin Murphy wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 2023-03-10 01:17, Nicolin Chen wrote:
> > Hi Robin,
> > 
> > Thanks for the inputs.
> > 
> > On Thu, Mar 09, 2023 at 01:03:41PM +0000, Robin Murphy wrote:
> > > External email: Use caution opening links or attachments
> > > 
> > > 
> > > On 2023-03-09 10:53, Nicolin Chen wrote:
> > > > This is used to forward the host IDR values to the user space, so the
> > > > hypervisor and the guest VM can learn about the underlying hardware's
> > > > capabilities.
> > > > 
> > > > Also, set the driver_type to IOMMU_HW_INFO_TYPE_ARM_SMMUV3 to pass the
> > > > corresponding type sanity in the core.
> > > > 
> > > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> > > > ---
> > > >    drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 +++++++++++++++++++++
> > > >    drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  2 ++
> > > >    include/uapi/linux/iommufd.h                | 14 ++++++++++++
> > > >    3 files changed, 41 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 f2425b0f0cd6..c1aac695ae0d 100644
> > > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > > @@ -2005,6 +2005,29 @@ static bool arm_smmu_capable(struct device *dev, enum iommu_cap cap)
> > > >        }
> > > >    }
> > > > 
> > > > +static void *arm_smmu_hw_info(struct device *dev, u32 *length)
> > > > +{
> > > > +     struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> > > > +     struct iommu_hw_info_smmuv3 *info;
> > > > +     void *base_idr;
> > > > +     int i;
> > > > +
> > > > +     if (!master || !master->smmu)
> > > > +             return ERR_PTR(-ENODEV);
> > > > +
> > > > +     info = kzalloc(sizeof(*info), GFP_KERNEL);
> > > > +     if (!info)
> > > > +             return ERR_PTR(-ENOMEM);
> > > > +
> > > > +     base_idr = master->smmu->base + ARM_SMMU_IDR0;
> > > > +     for (i = 0; i <= 5; i++)
> > > > +             info->idr[i] = readl_relaxed(base_idr + 0x4 * i);
> > > 
> > > You need to take firmware overrides etc. into account here. In
> > > particular, features like BTM may need to be hidden to work around
> > > errata either in the system integration or the SMMU itself. It isn't
> > > reasonable to expect every VMM to be aware of every erratum and
> > > workaround, and there may even be workarounds where we need to go out of
> > > our way to prevent guests from trying to use certain features in order
> > > to maintain correctness at S2.
> > 
> > We can add a bit of overrides after this for errata, perhaps?
> > 
> > I have some trouble with finding the errata docs. Would it be
> > possible for you to direct me to it with a link maybe?
> 
> The key Arm term is "Software Developer Errata Notice", or just SDEN.
> Here's the ones for MMU-600 and MMU-700:
> 
> https://developer.arm.com/documentation/SDEN-946810/latest/

This page shows "Arm CoreLink MMU-600 System Memory Management
Unit Software Developer Errata Notice" but the downloaded file
is "Arm CoreLink CI-700 Coherent Interconnect" errata notice.
And I don't quite understand what it's about.

> https://developer.arm.com/documentation/SDEN-1786925/latest/

Yea, this one I got an "MMU-700 System Memory Management Unit"
SMMU errata file that I can read and understand.

> Note that until now it has been extremely fortunate that in pretty much
> every case Linux either hasn't supported the affected feature at all, or
> has happened to avoid meeting the conditions. Once we do introduce
> nesting support that all goes out the window (and I'll have to think
> more when reviewing new errata in future...)
> 
> I've been putting off revisiting all the existing errata to figure out
> what we'd need to do until new nesting patches appeared, so I'll try to
> get to that soon now. I think in many cases it's likely to be best to
> just disallowing nesting entirely on affected implementations.

Do we have already a list of "affected implementations"? Or,
we would need to make such a list now? In a latter case, can
these affected implementations be detected from their IRD0-5
registers, so that we can simply do something in hw_info()?

Thanks
Nic
  
Robin Murphy March 16, 2023, 3:19 p.m. UTC | #5
On 16/03/2023 12:13 am, Nicolin Chen wrote:
> On Fri, Mar 10, 2023 at 03:28:56PM +0000, Robin Murphy wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 2023-03-10 01:17, Nicolin Chen wrote:
>>> Hi Robin,
>>>
>>> Thanks for the inputs.
>>>
>>> On Thu, Mar 09, 2023 at 01:03:41PM +0000, Robin Murphy wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On 2023-03-09 10:53, Nicolin Chen wrote:
>>>>> This is used to forward the host IDR values to the user space, so the
>>>>> hypervisor and the guest VM can learn about the underlying hardware's
>>>>> capabilities.
>>>>>
>>>>> Also, set the driver_type to IOMMU_HW_INFO_TYPE_ARM_SMMUV3 to pass the
>>>>> corresponding type sanity in the core.
>>>>>
>>>>> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
>>>>> ---
>>>>>     drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 +++++++++++++++++++++
>>>>>     drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  2 ++
>>>>>     include/uapi/linux/iommufd.h                | 14 ++++++++++++
>>>>>     3 files changed, 41 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 f2425b0f0cd6..c1aac695ae0d 100644
>>>>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>>> @@ -2005,6 +2005,29 @@ static bool arm_smmu_capable(struct device *dev, enum iommu_cap cap)
>>>>>         }
>>>>>     }
>>>>>
>>>>> +static void *arm_smmu_hw_info(struct device *dev, u32 *length)
>>>>> +{
>>>>> +     struct arm_smmu_master *master = dev_iommu_priv_get(dev);
>>>>> +     struct iommu_hw_info_smmuv3 *info;
>>>>> +     void *base_idr;
>>>>> +     int i;
>>>>> +
>>>>> +     if (!master || !master->smmu)
>>>>> +             return ERR_PTR(-ENODEV);
>>>>> +
>>>>> +     info = kzalloc(sizeof(*info), GFP_KERNEL);
>>>>> +     if (!info)
>>>>> +             return ERR_PTR(-ENOMEM);
>>>>> +
>>>>> +     base_idr = master->smmu->base + ARM_SMMU_IDR0;
>>>>> +     for (i = 0; i <= 5; i++)
>>>>> +             info->idr[i] = readl_relaxed(base_idr + 0x4 * i);
>>>>
>>>> You need to take firmware overrides etc. into account here. In
>>>> particular, features like BTM may need to be hidden to work around
>>>> errata either in the system integration or the SMMU itself. It isn't
>>>> reasonable to expect every VMM to be aware of every erratum and
>>>> workaround, and there may even be workarounds where we need to go out of
>>>> our way to prevent guests from trying to use certain features in order
>>>> to maintain correctness at S2.
>>>
>>> We can add a bit of overrides after this for errata, perhaps?
>>>
>>> I have some trouble with finding the errata docs. Would it be
>>> possible for you to direct me to it with a link maybe?
>>
>> The key Arm term is "Software Developer Errata Notice", or just SDEN.
>> Here's the ones for MMU-600 and MMU-700:
>>
>> https://developer.arm.com/documentation/SDEN-946810/latest/
> 
> This page shows "Arm CoreLink MMU-600 System Memory Management
> Unit Software Developer Errata Notice" but the downloaded file
> is "Arm CoreLink CI-700 Coherent Interconnect" errata notice.
> And I don't quite understand what it's about.

Oh, wonderful... I've reported that now, hopefully it gets fixed soon...

>> https://developer.arm.com/documentation/SDEN-1786925/latest/
> 
> Yea, this one I got an "MMU-700 System Memory Management Unit"
> SMMU errata file that I can read and understand.
> 
>> Note that until now it has been extremely fortunate that in pretty much
>> every case Linux either hasn't supported the affected feature at all, or
>> has happened to avoid meeting the conditions. Once we do introduce
>> nesting support that all goes out the window (and I'll have to think
>> more when reviewing new errata in future...)
>>
>> I've been putting off revisiting all the existing errata to figure out
>> what we'd need to do until new nesting patches appeared, so I'll try to
>> get to that soon now. I think in many cases it's likely to be best to
>> just disallowing nesting entirely on affected implementations.
> 
> Do we have already a list of "affected implementations"? Or,
> we would need to make such a list now? In a latter case, can
> these affected implementations be detected from their IRD0-5
> registers, so that we can simply do something in hw_info()?

Somewhere I have a patch that adds all the IIDR stuff needed for this, 
but I never sent it upstream since the erratum itself was an early 
MMU-600 one which in practice doesn't matter. I'll dig that out and 
update it with what I have in mind.

Thanks,
Robin.
  
Nicolin Chen March 16, 2023, 8:06 p.m. UTC | #6
On Thu, Mar 16, 2023 at 03:19:27PM +0000, Robin Murphy wrote:

> > > Note that until now it has been extremely fortunate that in pretty much
> > > every case Linux either hasn't supported the affected feature at all, or
> > > has happened to avoid meeting the conditions. Once we do introduce
> > > nesting support that all goes out the window (and I'll have to think
> > > more when reviewing new errata in future...)
> > > 
> > > I've been putting off revisiting all the existing errata to figure out
> > > what we'd need to do until new nesting patches appeared, so I'll try to
> > > get to that soon now. I think in many cases it's likely to be best to
> > > just disallowing nesting entirely on affected implementations.
> > 
> > Do we have already a list of "affected implementations"? Or,
> > we would need to make such a list now? In a latter case, can
> > these affected implementations be detected from their IRD0-5
> > registers, so that we can simply do something in hw_info()?
> 
> Somewhere I have a patch that adds all the IIDR stuff needed for this,
> but I never sent it upstream since the erratum itself was an early
> MMU-600 one which in practice doesn't matter. I'll dig that out and
> update it with what I have in mind.

Nice!

Perhaps we should merge that first, or include in this series
if you don't mind, so that we would be less worried about any
affected platform when releasing the new Linux version having
this nesting feature.

Thanks!
Nic
  
Nicolin Chen April 12, 2023, 7:47 a.m. UTC | #7
Hi Robin,

On Thu, Mar 16, 2023 at 01:06:17PM -0700, Nicolin Chen wrote:
> On Thu, Mar 16, 2023 at 03:19:27PM +0000, Robin Murphy wrote:
> 
> > > > Note that until now it has been extremely fortunate that in pretty much
> > > > every case Linux either hasn't supported the affected feature at all, or
> > > > has happened to avoid meeting the conditions. Once we do introduce
> > > > nesting support that all goes out the window (and I'll have to think
> > > > more when reviewing new errata in future...)
> > > > 
> > > > I've been putting off revisiting all the existing errata to figure out
> > > > what we'd need to do until new nesting patches appeared, so I'll try to
> > > > get to that soon now. I think in many cases it's likely to be best to
> > > > just disallowing nesting entirely on affected implementations.
> > > 
> > > Do we have already a list of "affected implementations"? Or,
> > > we would need to make such a list now? In a latter case, can
> > > these affected implementations be detected from their IRD0-5
> > > registers, so that we can simply do something in hw_info()?
> > 
> > Somewhere I have a patch that adds all the IIDR stuff needed for this,
> > but I never sent it upstream since the erratum itself was an early
> > MMU-600 one which in practice doesn't matter. I'll dig that out and
> > update it with what I have in mind.
> 
> Nice!
> 
> Perhaps we should merge that first, or include in this series
> if you don't mind, so that we would be less worried about any
> affected platform when releasing the new Linux version having
> this nesting feature.

I just want to see if there's a possibility of adding the
patch that you mentioned above in the near term?

I'd like to send a v2 of this series for another round of
review before the next -rc1, so it'd be nicer to include
that.

Thanks
Nic
  

Patch

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 f2425b0f0cd6..c1aac695ae0d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2005,6 +2005,29 @@  static bool arm_smmu_capable(struct device *dev, enum iommu_cap cap)
 	}
 }
 
+static void *arm_smmu_hw_info(struct device *dev, u32 *length)
+{
+	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+	struct iommu_hw_info_smmuv3 *info;
+	void *base_idr;
+	int i;
+
+	if (!master || !master->smmu)
+		return ERR_PTR(-ENODEV);
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return ERR_PTR(-ENOMEM);
+
+	base_idr = master->smmu->base + ARM_SMMU_IDR0;
+	for (i = 0; i <= 5; i++)
+		info->idr[i] = readl_relaxed(base_idr + 0x4 * i);
+
+	*length = sizeof(*info);
+
+	return info;
+}
+
 static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
@@ -2845,6 +2868,7 @@  static void arm_smmu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
 
 static struct iommu_ops arm_smmu_ops = {
 	.capable		= arm_smmu_capable,
+	.hw_info		= arm_smmu_hw_info,
 	.domain_alloc		= arm_smmu_domain_alloc,
 	.probe_device		= arm_smmu_probe_device,
 	.release_device		= arm_smmu_release_device,
@@ -2857,6 +2881,7 @@  static struct iommu_ops arm_smmu_ops = {
 	.page_response		= arm_smmu_page_response,
 	.def_domain_type	= arm_smmu_def_domain_type,
 	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
+	.driver_type		= IOMMU_HW_INFO_TYPE_ARM_SMMUV3,
 	.owner			= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev		= arm_smmu_attach_dev,
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 8d772ea8a583..ba2b4562f4b2 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -14,6 +14,8 @@ 
 #include <linux/mmzone.h>
 #include <linux/sizes.h>
 
+#include <uapi/linux/iommufd.h>
+
 /* MMIO registers */
 #define ARM_SMMU_IDR0			0x0
 #define IDR0_ST_LVL			GENMASK(28, 27)
diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index 0d5551b1b2be..c7a37915b49c 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -519,6 +519,20 @@  struct iommu_hw_info_vtd {
 	__aligned_u64 ecap_reg;
 };
 
+/**
+ * struct iommu_hw_info_smmuv3 - ARM SMMUv3 device info
+ *
+ * @flags: Must be set to 0
+ * @__reserved: Must be 0
+ * @idr: Implemented features for the SMMU Non-secure programming interface.
+ *       Please refer to the chapters from 6.3.1 to 6.3.6 in the SMMUv3 Spec.
+ */
+struct iommu_hw_info_smmuv3 {
+	__u32 flags;
+	__u32 __reserved;
+	__u32 idr[6];
+};
+
 /**
  * struct iommu_hw_info - ioctl(IOMMU_DEVICE_GET_HW_INFO)
  * @size: sizeof(struct iommu_hw_info)