From: Jan Beulich <JBeulich@suse.com>
To: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
Brian Woods <brian.woods@amd.com>,
Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Subject: [Xen-devel] [PATCH v4 10/12] AMD/IOMMU: correct IRTE updating
Date: Thu, 25 Jul 2019 13:33:02 +0000 [thread overview]
Message-ID: <0ca33ff2-6a66-fce1-1b62-fb30394398bf@suse.com> (raw)
In-Reply-To: <b8ae110a-f94a-4ed2-5cf9-c00ff1c0cb81@suse.com>
Flushing didn't get done along the lines of what the specification says.
Mark entries to be updated as not remapped (which will result in
interrupt requests to get target aborted, but the interrupts should be
masked anyway at that point in time), issue the flush, and only then
write the new entry.
In update_intremap_entry_from_msi_msg() also fold the duplicate initial
lock determination and acquire into just a single instance.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
RFC: Putting the flush invocations in loops isn't overly nice, but I
don't think this can really be abused, since callers up the stack
hold further locks. Nevertheless I'd like to ask for better
suggestions.
---
v4: Re-base.
v3: Remove stale parts of description. Re-base.
v2: Parts morphed into earlier patch.
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -213,15 +213,13 @@ static void update_intremap_entry(const
},
};
- ACCESS_ONCE(entry.ptr128->raw[0]) = 0;
+ ASSERT(!entry.ptr128->full.remap_en);
+ entry.ptr128->raw[1] = irte.raw[1];
/*
- * Low half, in particular RemapEn, needs to be cleared first. See
+ * High half needs to be set before low one (containing RemapEn). See
* comment in free_intremap_entry() regarding the choice of barrier.
*/
smp_wmb();
- entry.ptr128->raw[1] = irte.raw[1];
- /* High half needs to be set before low one (containing RemapEn). */
- smp_wmb();
ACCESS_ONCE(entry.ptr128->raw[0]) = irte.raw[0];
}
else
@@ -296,6 +294,20 @@ static int update_intremap_entry_from_io
}
entry = get_intremap_entry(iommu, req_id, offset);
+
+ /* The RemapEn fields match for all formats. */
+ while ( iommu->enabled && entry.ptr32->flds.remap_en )
+ {
+ entry.ptr32->flds.remap_en = false;
+ spin_unlock(lock);
+
+ spin_lock(&iommu->lock);
+ amd_iommu_flush_intremap(iommu, req_id);
+ spin_unlock(&iommu->lock);
+
+ spin_lock(lock);
+ }
+
if ( fresh )
/* nothing */;
else if ( !lo_update )
@@ -325,13 +337,6 @@ static int update_intremap_entry_from_io
spin_unlock_irqrestore(lock, flags);
- if ( iommu->enabled && !fresh )
- {
- spin_lock_irqsave(&iommu->lock, flags);
- amd_iommu_flush_intremap(iommu, req_id);
- spin_unlock_irqrestore(&iommu->lock, flags);
- }
-
set_rte_index(rte, offset);
return 0;
@@ -587,19 +592,27 @@ static int update_intremap_entry_from_ms
req_id = get_dma_requestor_id(iommu->seg, bdf);
alias_id = get_intremap_requestor_id(iommu->seg, bdf);
+ lock = get_intremap_lock(iommu->seg, req_id);
+ spin_lock_irqsave(lock, flags);
+
if ( msg == NULL )
{
- lock = get_intremap_lock(iommu->seg, req_id);
- spin_lock_irqsave(lock, flags);
for ( i = 0; i < nr; ++i )
free_intremap_entry(iommu, req_id, *remap_index + i);
spin_unlock_irqrestore(lock, flags);
- goto done;
- }
- lock = get_intremap_lock(iommu->seg, req_id);
+ if ( iommu->enabled )
+ {
+ spin_lock_irqsave(&iommu->lock, flags);
+ amd_iommu_flush_intremap(iommu, req_id);
+ if ( alias_id != req_id )
+ amd_iommu_flush_intremap(iommu, alias_id);
+ spin_unlock_irqrestore(&iommu->lock, flags);
+ }
+
+ return 0;
+ }
- spin_lock_irqsave(lock, flags);
dest_mode = (msg->address_lo >> MSI_ADDR_DESTMODE_SHIFT) & 0x1;
delivery_mode = (msg->data >> MSI_DATA_DELIVERY_MODE_SHIFT) & 0x1;
vector = (msg->data >> MSI_DATA_VECTOR_SHIFT) & MSI_DATA_VECTOR_MASK;
@@ -623,6 +636,22 @@ static int update_intremap_entry_from_ms
}
entry = get_intremap_entry(iommu, req_id, offset);
+
+ /* The RemapEn fields match for all formats. */
+ while ( iommu->enabled && entry.ptr32->flds.remap_en )
+ {
+ entry.ptr32->flds.remap_en = false;
+ spin_unlock(lock);
+
+ spin_lock(&iommu->lock);
+ amd_iommu_flush_intremap(iommu, req_id);
+ if ( alias_id != req_id )
+ amd_iommu_flush_intremap(iommu, alias_id);
+ spin_unlock(&iommu->lock);
+
+ spin_lock(lock);
+ }
+
update_intremap_entry(iommu, entry, vector, delivery_mode, dest_mode, dest);
spin_unlock_irqrestore(lock, flags);
@@ -642,16 +671,6 @@ static int update_intremap_entry_from_ms
get_ivrs_mappings(iommu->seg)[alias_id].intremap_table);
}
-done:
- if ( iommu->enabled )
- {
- spin_lock_irqsave(&iommu->lock, flags);
- amd_iommu_flush_intremap(iommu, req_id);
- if ( alias_id != req_id )
- amd_iommu_flush_intremap(iommu, alias_id);
- spin_unlock_irqrestore(&iommu->lock, flags);
- }
-
return 0;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-07-25 13:44 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-25 13:19 [Xen-devel] [PATCH v4 00/12] x86: AMD x2APIC support / AMD IOMMU improvements Jan Beulich
2019-07-25 13:29 ` [Xen-devel] [PATCH v4 01/12] AMD/IOMMU: use bit field for extended feature register Jan Beulich
2019-07-30 9:14 ` [Xen-devel] Ping: " Jan Beulich
2019-07-30 16:35 ` [Xen-devel] " Woods, Brian
2019-07-25 13:29 ` [Xen-devel] [PATCH v4 02/12] AMD/IOMMU: use bit field for control register Jan Beulich
2019-07-25 13:30 ` [Xen-devel] [PATCH v4 03/12] AMD/IOMMU: use bit field for IRTE Jan Beulich
2019-07-25 13:30 ` [Xen-devel] [PATCH v4 04/12] AMD/IOMMU: pass IOMMU to {get, free, update}_intremap_entry() Jan Beulich
2019-07-25 13:31 ` [Xen-devel] [PATCH v4 05/12] AMD/IOMMU: introduce 128-bit IRTE non-guest-APIC IRTE format Jan Beulich
2019-07-30 10:13 ` Andrew Cooper
2019-07-30 16:36 ` Woods, Brian
2019-07-25 13:31 ` [Xen-devel] [PATCH v4 06/12] AMD/IOMMU: split amd_iommu_init_one() Jan Beulich
2019-07-25 13:31 ` [Xen-devel] [PATCH v4 07/12] AMD/IOMMU: allow enabling with IRQ not yet set up Jan Beulich
2019-07-25 13:32 ` [Xen-devel] [PATCH v4 08/12] AMD/IOMMU: adjust setup of internal interrupt for x2APIC mode Jan Beulich
2019-07-25 13:32 ` [Xen-devel] [PATCH v4 09/12] AMD/IOMMU: enable x2APIC mode when available Jan Beulich
2019-07-25 13:33 ` Jan Beulich [this message]
2019-07-25 14:04 ` [Xen-devel] [PATCH v4 10/12] AMD/IOMMU: correct IRTE updating Woods, Brian
2019-07-25 13:33 ` [Xen-devel] [PATCH v4 11/12] AMD/IOMMU: don't needlessly log headers when dumping IRTs Jan Beulich
2019-07-30 10:25 ` Andrew Cooper
2019-07-30 16:36 ` Woods, Brian
2019-07-25 13:33 ` [Xen-devel] [PATCH v4 12/12] AMD/IOMMU: miscellaneous DTE handling adjustments Jan Beulich
2019-07-30 13:42 ` Andrew Cooper
2019-07-30 14:10 ` Jan Beulich
2019-08-06 19:41 ` Woods, Brian
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=0ca33ff2-6a66-fce1-1b62-fb30394398bf@suse.com \
--to=jbeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=brian.woods@amd.com \
--cc=suravee.suthikulpanit@amd.com \
--cc=xen-devel@lists.xenproject.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.