From: Yang Shi <yang@os.amperecomputing.com>
To: Daniel Mentz <danielmentz@google.com>
Cc: jgg@ziepe.ca, nicolinc@nvidia.com, james.morse@arm.com,
will@kernel.org, robin.murphy@arm.com,
linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [v3 PATCH] iommu/arm-smmu-v3: Fix L1 stream table index calculation for 32-bit sid size
Date: Mon, 7 Oct 2024 10:49:57 -0700 [thread overview]
Message-ID: <f434f714-b38f-4ed2-97f4-8f00a03b855b@os.amperecomputing.com> (raw)
In-Reply-To: <CAE2F3rBBx_bMgVi5R1G7d-B+c3UdXiUB4sEL6KnsNc4gWJHroQ@mail.gmail.com>
On 10/7/24 9:36 AM, Daniel Mentz wrote:
> On Fri, Oct 4, 2024 at 6:53 PM Yang Shi <yang@os.amperecomputing.com> wrote:
>>>>> On a related note, in arm_smmu_init_strtab_2lvl() we're capping the
>>>>> number of l1 entries at STRTAB_MAX_L1_ENTRIES for 2 level stream
>>>>> tables. I'm thinking it would make sense to limit the size of linear
>>>>> stream tables for the same reasons.
>>>> Yes, this also works. But I don't know what value should be used. Jason
>>>> actually suggested (size > SIZE_512M) in v2 review, but I thought the
>>>> value is a magic number. Why 512M? Just because it is too large for
>>>> allocation. So I picked up SIZE_MAX, just because it is the largest size
>>>> supported by size_t type.
>>> I think it should be capped to STRTAB_MAX_L1_ENTRIES
>> I'm not expert on SMMU. Does the linear stream table have the same cap
>> as 2-level stream table? Is this defined by the hardware spec? If it is
>> not, why should we pick this value?
> No. I don't think it's defined by the architecture specification. I
> don't have a strong opinion on the particular value for the size limit
> of linear Stream tables. However, I do believe that we should pick a
> size limit. Today, the driver limits the number of Level-1 Stream
> Table Descriptors in a 2-level Stream table. For consistency, we
> should limit the size of linear Stream tables, too.
We are on the same page regarding having the size limit. Took a look at
the definition of STRTAB_MAX_L1_ENTRIES, I saw this comment:
/*
* Stream table.
*
* Linear: Enough to cover 1 << IDR1.SIDSIZE entries
* 2lvl: 128k L1 entries,
* 256 lazy entries per table (each table covers a PCI bus)
*/
I'm not sure whether the comment for linear is still true or not with
large IDR1.SIDSIZE. But using STRTAB_MAX_L1_ENTRIES for linear does have
conflict with the comment. I will pick U32_MAX for now since it is the
largest size on 32-bit and good enough to prevent from overflow.
next prev parent reply other threads:[~2024-10-07 17:50 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-04 18:04 [v3 PATCH] iommu/arm-smmu-v3: Fix L1 stream table index calculation for 32-bit sid size Yang Shi
2024-10-04 21:14 ` Daniel Mentz
2024-10-04 21:47 ` Yang Shi
2024-10-05 1:03 ` Daniel Mentz
2024-10-05 1:53 ` Yang Shi
2024-10-07 16:36 ` Daniel Mentz
2024-10-07 17:49 ` Yang Shi [this message]
2024-10-07 17:50 ` Jason Gunthorpe
2024-10-07 18:43 ` Yang Shi
2024-10-08 13:34 ` Will Deacon
2024-10-08 15:15 ` Jason Gunthorpe
2024-10-08 17:04 ` Yang Shi
2024-10-08 17:41 ` Will Deacon
2024-10-08 17:42 ` Will Deacon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f434f714-b38f-4ed2-97f4-8f00a03b855b@os.amperecomputing.com \
--to=yang@os.amperecomputing.com \
--cc=danielmentz@google.com \
--cc=iommu@lists.linux.dev \
--cc=james.morse@arm.com \
--cc=jgg@ziepe.ca \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nicolinc@nvidia.com \
--cc=robin.murphy@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.