From: Jerry Snitselaar <jsnitsel@redhat.com>
To: Will Deacon <will@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>,
Alex Williamson <alex.williamson@redhat.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
iommu@lists.linux-foundation.org
Subject: Re: [GIT PULL] IOMMU fix for 5.10 (-final)
Date: Wed, 09 Dec 2020 12:12:31 -0700 [thread overview]
Message-ID: <87tusulrog.fsf@redhat.com> (raw)
In-Reply-To: <20201209185020.GC8778@willie-the-truck>
Will Deacon @ 2020-12-09 11:50 MST:
> On Wed, Dec 09, 2020 at 10:07:46AM -0800, Linus Torvalds wrote:
>> On Wed, Dec 9, 2020 at 6:12 AM Will Deacon <will@kernel.org> wrote:
>> >
>> > Please pull this one-liner AMD IOMMU fix for 5.10. It's actually a fix
>> > for a fix, where the size of the interrupt remapping table was increased
>> > but a related constant for the size of the interrupt table was forgotten.
>>
>> Pulled.
>
> Thanks.
>
>> However, why didn't this then add some sanity checking for the two
>> different #defines to be in sync?
>>
>> IOW, something like
>>
>> #define AMD_IOMMU_IRQ_TABLE_SHIFT 9
>>
>> #define MAX_IRQS_PER_TABLE (1 << AMD_IOMMU_IRQ_TABLE_SHIFT)
>> #define DTE_IRQ_TABLE_LEN ((u64)AMD_IOMMU_IRQ_TABLE_SHIFT << 1)
Since the field in the device table entry format expects it to be n
where there are 2^n entries in the table I guess it should be:
#define DTE_IRQ_TABLE_LEN 9
#define MAX_IRQS_PER_TABLE (1 << DTE_IRQ_TABLE_LEN)
>>
>> or whatever. Hmm?
>
> This looks like a worthwhile change to me, but I don't have any hardware
> so I've been very reluctant to make even "obvious" driver changes here.
>
> Suravee -- please can you post a patch implementing the above?
>
>> That way this won't happen again, but perhaps equally importantly the
>> linkage will be more clear, and there won't be those random constants.
>>
>> Naming above is probably garbage - I assume there's some actual
>> architectural name for that irq table length field in the DTE?
>
> The one in the spec is even better: "IntTabLen".
>
> Will
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
WARNING: multiple messages have this Message-ID (diff)
From: Jerry Snitselaar <jsnitsel@redhat.com>
To: Will Deacon <will@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Alex Williamson <alex.williamson@redhat.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Robin Murphy <robin.murphy@arm.com>,
iommu@lists.linux-foundation.org
Subject: Re: [GIT PULL] IOMMU fix for 5.10 (-final)
Date: Wed, 09 Dec 2020 12:12:31 -0700 [thread overview]
Message-ID: <87tusulrog.fsf@redhat.com> (raw)
In-Reply-To: <20201209185020.GC8778@willie-the-truck>
Will Deacon @ 2020-12-09 11:50 MST:
> On Wed, Dec 09, 2020 at 10:07:46AM -0800, Linus Torvalds wrote:
>> On Wed, Dec 9, 2020 at 6:12 AM Will Deacon <will@kernel.org> wrote:
>> >
>> > Please pull this one-liner AMD IOMMU fix for 5.10. It's actually a fix
>> > for a fix, where the size of the interrupt remapping table was increased
>> > but a related constant for the size of the interrupt table was forgotten.
>>
>> Pulled.
>
> Thanks.
>
>> However, why didn't this then add some sanity checking for the two
>> different #defines to be in sync?
>>
>> IOW, something like
>>
>> #define AMD_IOMMU_IRQ_TABLE_SHIFT 9
>>
>> #define MAX_IRQS_PER_TABLE (1 << AMD_IOMMU_IRQ_TABLE_SHIFT)
>> #define DTE_IRQ_TABLE_LEN ((u64)AMD_IOMMU_IRQ_TABLE_SHIFT << 1)
Since the field in the device table entry format expects it to be n
where there are 2^n entries in the table I guess it should be:
#define DTE_IRQ_TABLE_LEN 9
#define MAX_IRQS_PER_TABLE (1 << DTE_IRQ_TABLE_LEN)
>>
>> or whatever. Hmm?
>
> This looks like a worthwhile change to me, but I don't have any hardware
> so I've been very reluctant to make even "obvious" driver changes here.
>
> Suravee -- please can you post a patch implementing the above?
>
>> That way this won't happen again, but perhaps equally importantly the
>> linkage will be more clear, and there won't be those random constants.
>>
>> Naming above is probably garbage - I assume there's some actual
>> architectural name for that irq table length field in the DTE?
>
> The one in the spec is even better: "IntTabLen".
>
> Will
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-12-09 19:12 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-09 14:12 [GIT PULL] IOMMU fix for 5.10 (-final) Will Deacon
2020-12-09 14:12 ` Will Deacon
2020-12-09 18:07 ` Linus Torvalds
2020-12-09 18:07 ` Linus Torvalds
2020-12-09 18:50 ` Will Deacon
2020-12-09 18:50 ` Will Deacon
2020-12-09 19:12 ` Jerry Snitselaar [this message]
2020-12-09 19:12 ` Jerry Snitselaar
2020-12-09 19:15 ` Jerry Snitselaar
2020-12-09 19:15 ` Jerry Snitselaar
2020-12-09 19:17 ` Linus Torvalds
2020-12-09 19:17 ` Linus Torvalds
2020-12-09 19:23 ` Jerry Snitselaar
2020-12-09 19:23 ` Jerry Snitselaar
2020-12-10 11:55 ` Suravee Suthikulpanit
2020-12-10 11:55 ` Suravee Suthikulpanit
2020-12-09 18:09 ` pr-tracker-bot
2020-12-09 18:09 ` pr-tracker-bot
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=87tusulrog.fsf@redhat.com \
--to=jsnitsel@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=iommu@lists.linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robin.murphy@arm.com \
--cc=torvalds@linux-foundation.org \
--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.