On 9/15/20 8:19 PM, Joao Martins wrote:
On 9/15/20 1:30 PM, Suravee Suthikulpanit wrote:
On 9/15/20 6:25 PM, Maxim Levitsky wrote:
On Mon, 2020-09-14 at 21:48 +0700, Suravee Suthikulpanit wrote:
Could you please try with the following patch instead?
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3840,14 +3840,18 @@ int amd_iommu_activate_guest_mode(void *data)
{
struct amd_ir_data *ir_data = (struct amd_ir_data *)data;
struct irte_ga *entry = (struct irte_ga *) ir_data->entry;
+ u64 valid;
if (!AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) ||
!entry || entry->lo.fields_vapic.guest_mode)
return 0;
+ valid = entry->lo.fields_vapic.valid;
+
entry->lo.val = 0;
entry->hi.val = 0;
+ entry->lo.fields_vapic.valid = valid;
entry->lo.fields_vapic.guest_mode = 1;
entry->lo.fields_vapic.ga_log_intr = 1;
entry->hi.fields.ga_root_ptr = ir_data->ga_root_ptr;
@@ -3864,12 +3868,14 @@ int amd_iommu_deactivate_guest_mode(void *data)
struct amd_ir_data *ir_data = (struct amd_ir_data *)data;
struct irte_ga *entry = (struct irte_ga *) ir_data->entry;
struct irq_cfg *cfg = ir_data->cfg;
- u64 valid = entry->lo.fields_remap.valid;
+ u64 valid;
if (!AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) ||
!entry || !entry->lo.fields_vapic.guest_mode)
return 0;
+ valid = entry->lo.fields_remap.valid;
+
entry->lo.val = 0;
entry->hi.val = 0;
I see. I based my approach on the fact that valid bit was
set always to true anyway before, plus that amd_iommu_activate_guest_mode
should be really only called when someone activates a valid interrupt remapping
entry, but IMHO the approach of preserving the valid bit is safer anyway.
It works on my system (I applied the patch manually, since either your or my
email client,
seems to mangle the patch)
Sorry for the mangled patch. I'll submit the patch w/ your information. Thanks
for your help reporting, debugging, and
testing the patch.
I assume you're only doing the valid bit preservation in
amd_iommu_activate_guest_mode() ?
The null deref fix in amd_iommu_deactivate_guest_mode() was fixed elsewhere[0],
or are you
planning on merging both changes like the diff you attached?
I am planning to send a separate patch just for amd_iommu_activate_guest_mode().
Asking also because commit 26e495f341 ("iommu/amd: Restore IRTE.RemapEn bit
after
programming IRTE") was added in v5.4 and v5.8 stable trees but the v5.4
backport didn't
include e52d58d54a321 ("iommu/amd: Use cmpxchg_double() when updating 128-bit
IRTE").
We should probably backport the e52d58d54a321 along with the fixes in amd_iommu_activate_guest_mode() and
amd_iommu_deactivate_guest_mode(). I'll work with the community to get these back-ported.
Thanks,
Suravee
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
