* [GIT PULL] IOMMU fix for 5.10 (-final)
@ 2020-12-09 14:12 Will Deacon
2020-12-09 18:07 ` Linus Torvalds
2020-12-09 18:09 ` pr-tracker-bot
0 siblings, 2 replies; 9+ messages in thread
From: Will Deacon @ 2020-12-09 14:12 UTC (permalink / raw)
To: torvalds
Cc: iommu, linux-kernel, joro, Alex Williamson, Robin Murphy, Lu Baolu
Hi Linus,
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.
Cheers,
Will
--->8
The following changes since commit d76b42e92780c3587c1a998a3a943b501c137553:
iommu/vt-d: Don't read VCCAP register unless it exists (2020-11-26 14:50:24 +0000)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git tags/iommu-fixes
for you to fetch changes up to 4165bf015ba9454f45beaad621d16c516d5c5afe:
iommu/amd: Set DTE[IntTabLen] to represent 512 IRTEs (2020-12-07 11:00:24 +0000)
----------------------------------------------------------------
iommu fix for 5.10
- Fix interrupt table length definition for AMD IOMMU
----------------------------------------------------------------
Suravee Suthikulpanit (1):
iommu/amd: Set DTE[IntTabLen] to represent 512 IRTEs
drivers/iommu/amd/amd_iommu_types.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] IOMMU fix for 5.10 (-final)
2020-12-09 14:12 [GIT PULL] IOMMU fix for 5.10 (-final) Will Deacon
@ 2020-12-09 18:07 ` Linus Torvalds
2020-12-09 18:50 ` Will Deacon
2020-12-09 18:09 ` pr-tracker-bot
1 sibling, 1 reply; 9+ messages in thread
From: Linus Torvalds @ 2020-12-09 18:07 UTC (permalink / raw)
To: Will Deacon, Suravee Suthikulpanit
Cc: iommu, Linux Kernel Mailing List, Joerg Roedel, Alex Williamson,
Robin Murphy, Lu Baolu
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.
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)
or whatever. Hmm?
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?
Linus
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] IOMMU fix for 5.10 (-final)
2020-12-09 18:07 ` Linus Torvalds
@ 2020-12-09 18:50 ` Will Deacon
2020-12-09 19:12 ` Jerry Snitselaar
2020-12-10 11:55 ` Suravee Suthikulpanit
0 siblings, 2 replies; 9+ messages in thread
From: Will Deacon @ 2020-12-09 18:50 UTC (permalink / raw)
To: Linus Torvalds
Cc: Suravee Suthikulpanit, iommu, Linux Kernel Mailing List,
Joerg Roedel, Alex Williamson, Robin Murphy, Lu Baolu
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)
>
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] IOMMU fix for 5.10 (-final)
2020-12-09 18:50 ` Will Deacon
@ 2020-12-09 19:12 ` Jerry Snitselaar
2020-12-09 19:15 ` Jerry Snitselaar
2020-12-09 19:17 ` Linus Torvalds
2020-12-10 11:55 ` Suravee Suthikulpanit
1 sibling, 2 replies; 9+ messages in thread
From: Jerry Snitselaar @ 2020-12-09 19:12 UTC (permalink / raw)
To: Will Deacon
Cc: Linus Torvalds, Alex Williamson, Linux Kernel Mailing List,
Robin Murphy, iommu
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] IOMMU fix for 5.10 (-final)
2020-12-09 19:12 ` Jerry Snitselaar
@ 2020-12-09 19:15 ` Jerry Snitselaar
2020-12-09 19:17 ` Linus Torvalds
1 sibling, 0 replies; 9+ messages in thread
From: Jerry Snitselaar @ 2020-12-09 19:15 UTC (permalink / raw)
To: Will Deacon
Cc: Linus Torvalds, Alex Williamson, Linux Kernel Mailing List,
Robin Murphy, iommu
On Wed, Dec 9, 2020 at 12:12 PM Jerry Snitselaar <jsnitsel@redhat.com> wrote:
>
>
> 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)
>
No, ignore that. I'm being stupid.
> >>
> >> 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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] IOMMU fix for 5.10 (-final)
2020-12-09 19:12 ` Jerry Snitselaar
2020-12-09 19:15 ` Jerry Snitselaar
@ 2020-12-09 19:17 ` Linus Torvalds
2020-12-09 19:23 ` Jerry Snitselaar
1 sibling, 1 reply; 9+ messages in thread
From: Linus Torvalds @ 2020-12-09 19:17 UTC (permalink / raw)
To: Jerry Snitselaar
Cc: Will Deacon, Alex Williamson, Linux Kernel Mailing List,
Robin Murphy, iommu
On Wed, Dec 9, 2020 at 11:12 AM Jerry Snitselaar <jsnitsel@redhat.com> wrote:
>
> 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)
No, that "DTE_IRQ_TABLE_LEN" is not the size shift - it's the size
shift value in that DTE field, which is shifted up by 1.
That's why the current code does that
#define DTE_IRQ_TABLE_LEN (9ULL << 1)
there..
Which was why I suggested that new #define that is the *actual* shift
value, and then the DTE thing and the MAX_IRQS_PER_TABLE values would
depend on that.
Linus
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] IOMMU fix for 5.10 (-final)
2020-12-09 19:17 ` Linus Torvalds
@ 2020-12-09 19:23 ` Jerry Snitselaar
0 siblings, 0 replies; 9+ messages in thread
From: Jerry Snitselaar @ 2020-12-09 19:23 UTC (permalink / raw)
To: Linus Torvalds
Cc: Will Deacon, Alex Williamson, Linux Kernel Mailing List,
Robin Murphy, iommu
On Wed, Dec 9, 2020 at 12:18 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Dec 9, 2020 at 11:12 AM Jerry Snitselaar <jsnitsel@redhat.com> wrote:
> >
> > 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)
>
> No, that "DTE_IRQ_TABLE_LEN" is not the size shift - it's the size
> shift value in that DTE field, which is shifted up by 1.
>
> That's why the current code does that
>
> #define DTE_IRQ_TABLE_LEN (9ULL << 1)
>
> there..
>
> Which was why I suggested that new #define that is the *actual* shift
> value, and then the DTE thing and the MAX_IRQS_PER_TABLE values would
> depend on that.
>
> Linus
>
Yes, when I read it my head was translating it as setting them both to
512 and then
I forgot that it gets shifted over 1. Which considering I was the once
who noticed the
original problem of it still being 8 was a nice brain fart. This
should be fixed like you suggest.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] IOMMU fix for 5.10 (-final)
2020-12-09 18:50 ` Will Deacon
2020-12-09 19:12 ` Jerry Snitselaar
@ 2020-12-10 11:55 ` Suravee Suthikulpanit
1 sibling, 0 replies; 9+ messages in thread
From: Suravee Suthikulpanit @ 2020-12-10 11:55 UTC (permalink / raw)
To: Will Deacon, Linus Torvalds
Cc: iommu, Linux Kernel Mailing List, Joerg Roedel, Alex Williamson,
Robin Murphy, Lu Baolu
Hi All,
On 12/10/20 1:50 AM, Will Deacon wrote:
> 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)
>>
>> 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?
I'll send one out ASAP.
>
>> 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
Thanks,
Suravee
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] IOMMU fix for 5.10 (-final)
2020-12-09 14:12 [GIT PULL] IOMMU fix for 5.10 (-final) Will Deacon
2020-12-09 18:07 ` Linus Torvalds
@ 2020-12-09 18:09 ` pr-tracker-bot
1 sibling, 0 replies; 9+ messages in thread
From: pr-tracker-bot @ 2020-12-09 18:09 UTC (permalink / raw)
To: Will Deacon
Cc: torvalds, iommu, linux-kernel, joro, Alex Williamson,
Robin Murphy, Lu Baolu
The pull request you sent on Wed, 9 Dec 2020 14:12:38 +0000:
> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git tags/iommu-fixes
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/ca4bbdaf171604841f77648a2877e2e43db69b71
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-12-10 11:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-09 14:12 [GIT PULL] IOMMU fix for 5.10 (-final) Will Deacon
2020-12-09 18:07 ` Linus Torvalds
2020-12-09 18:50 ` Will Deacon
2020-12-09 19:12 ` Jerry Snitselaar
2020-12-09 19:15 ` Jerry Snitselaar
2020-12-09 19:17 ` Linus Torvalds
2020-12-09 19:23 ` Jerry Snitselaar
2020-12-10 11:55 ` Suravee Suthikulpanit
2020-12-09 18:09 ` pr-tracker-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).