iommu/amd: Fix GT feature enablement again
Subsystems (2)
Vasant Hegde
maintainerPATCH 1/1 iommu/amd: Fix GT feature enablement again
Current code configures GCR3 even when device is attached to identity domain. So that we can support SVA with identity domain. This means in attach device path it updates Guest Translation related bits in DTE.
Commit de111f6b4f6a ("iommu/amd: Enable Guest Translation after reading IOMMU feature register") missed to enable Control[GT] bit in resume path. Its causing certain laptop to fail to resume after suspend.
This is because we have inconsistency between between control register (GT is disabled) and DTE (where we have enabled guest translation related bits) in resume path. And IOMMU hardware throws ILLEGAL_DEV_TABLE_ENTRY.
Fix it by enabling GT bit in resume path.
--- drivers/iommu/amd/init.c | 1 + 1 file changed, 1 insertion(+)
drivers/iommu/amd/init.c +1 -0
@@ -2743,6 +2743,7 @@ static void early_enable_iommu(struct amd_iommu *iommu)iommu_enable_command_buffer (iommu);iommu_enable_event_buffer (iommu);iommu_set_exclusion_range (iommu); +iommu_enable_gt (iommu);iommu_enable_ga (iommu);iommu_enable_xt (iommu);iommu_enable_irtcachedis (iommu);
Signature · 1 line
2.31.1
Quoted 8 lines · Vasant Hegde
Current code configures GCR3 even when device is attached to identity domain. So that we can support SVA with identity domain. This means in attach device path it updates Guest Translation related bits in DTE.
Commit
de111f6b4f6a("iommu/amd: Enable Guest Translation after reading IOMMU feature register") missed to enable Control[GT] bit in resume path. Its causing certain laptop to fail to resume after suspend.
Does this bring back the issue that de111f6b4f6a was trying to fix in relation to getting a bad value in the IVRS table?
Quoted 30 lines
This is because we have inconsistency between between control register (GT is disabled) and DTE (where we have enabled guest translation related bits) in resume path. And IOMMU hardware throws
ILLEGAL_DEV_TABLE_ENTRY.Fix it by enabling GT bit in resume path.
Reported-by Błażej Szczygieł <[email protected]>Fixes de111f6b4f6a ("iommu/amd: Enable Guest Translation after reading IOMMU feature register")Tested-by Błażej Szczygieł <[email protected]>Signed-off-by Vasant Hegde <[email protected]>--- drivers/iommu/amd/init.c | 1 + 1 file changed, 1 insertion(+)drivers/iommu/amd/init.c +1
-0@@ -2743,6 +2743,7 @@ static void early_enable_iommu(struct amd_iommu *iommu)iommu_enable_command_buffer (iommu);iommu_enable_event_buffer (iommu);iommu_set_exclusion_range (iommu); +iommu_enable_gt (iommu);iommu_enable_ga (iommu);iommu_enable_xt (iommu);iommu_enable_irtcachedis (iommu);Signature · 1 line
2.31.1
Vasant Hegde
maintainerHi Jerry,
Quoted 5 lines · Jerry Snitselaar
Quoted 4 lines · Vasant Hegde
Current code configures GCR3 even when device is attached to identity domain. So that we can support SVA with identity domain. This means in attach device path it updates Guest Translation related bits in DTE.
Quoted 7 lines
Quoted 4 lines
Commit
de111f6b4f6a("iommu/amd: Enable Guest Translation after reading IOMMU feature register") missed to enable Control[GT] bit in resume path. Its causing certain laptop to fail to resume after suspend.Does this bring back the issue that
de111f6b4f6awas trying to fix in relation to getting a bad value in the IVRS table?
Actually it will not bring back that issue as I have kept the GT enablement in iommu_init_pci() as well.
So if we have bad IVRS table, then it makes second attempt in iommu_init_pci() path. So we are good.
We may have to rework some part of init code.. I will look into it later.
-Vasant
Quoted 33 lines
Quoted 31 lines
This is because we have inconsistency between between control register (GT is disabled) and DTE (where we have enabled guest translation related bits) in resume path. And IOMMU hardware throws
ILLEGAL_DEV_TABLE_ENTRY.Fix it by enabling GT bit in resume path.
Reported-by Błażej Szczygieł <[email protected]>Fixes de111f6b4f6a ("iommu/amd: Enable Guest Translation after reading IOMMU feature register")Tested-by Błażej Szczygieł <[email protected]>Signed-off-by Vasant Hegde <[email protected]>--- drivers/iommu/amd/init.c | 1 + 1 file changed, 1 insertion(+)drivers/iommu/amd/init.c +1
-0@@ -2743,6 +2743,7 @@ static void early_enable_iommu(struct amd_iommu *iommu)iommu_enable_command_buffer (iommu);iommu_enable_event_buffer (iommu);iommu_set_exclusion_range (iommu); +iommu_enable_gt (iommu);iommu_enable_ga (iommu);iommu_enable_xt (iommu);iommu_enable_irtcachedis (iommu);Signature · 1 line
2.31.1
Quoted 12 lines · Vasant Hegde
Current code configures GCR3 even when device is attached to identity domain. So that we can support SVA with identity domain. This means in attach device path it updates Guest Translation related bits in DTE.
Commit
de111f6b4f6a("iommu/amd: Enable Guest Translation after reading IOMMU feature register") missed to enable Control[GT] bit in resume path. Its causing certain laptop to fail to resume after suspend.This is because we have inconsistency between between control register (GT is disabled) and DTE (where we have enabled guest translation related bits) in resume path. And IOMMU hardware throws
ILLEGAL_DEV_TABLE_ENTRY.
Quoted 7 lines
Fix it by enabling GT bit in resume path.
Reported-by Błażej Szczygieł <[email protected]>Fixes de111f6b4f6a ("iommu/amd: Enable Guest Translation after reading IOMMU feature register")Tested-by Błażej Szczygieł <[email protected]>Signed-off-by Vasant Hegde <[email protected]>
Quoted 19 lines
--- drivers/iommu/amd/init.c | 1 + 1 file changed, 1 insertion(+)drivers/iommu/amd/init.c +1
-0@@ -2743,6 +2743,7 @@ static void early_enable_iommu(struct amd_iommu *iommu)iommu_enable_command_buffer (iommu);iommu_enable_event_buffer (iommu);iommu_set_exclusion_range (iommu); +iommu_enable_gt (iommu);iommu_enable_ga (iommu);iommu_enable_xt (iommu);iommu_enable_irtcachedis (iommu);Signature · 1 line
2.31.1
Quoted 2 lines · Vasant Hegde
drivers/iommu/amd/init.c | 1 + 1 file changed, 1 insertion(+)
Applied, thanks.