* [PATCH v2] VT-d: use two 32-bit writes to update DMAR fault address registers
@ 2017-10-11 3:03 Haozhong Zhang
2017-10-11 11:27 ` Jan Beulich
0 siblings, 1 reply; 5+ messages in thread
From: Haozhong Zhang @ 2017-10-11 3:03 UTC (permalink / raw)
To: xen-devel
Cc: Andrew Cooper, Kevin Tian, Haozhong Zhang, Jan Beulich,
Roger Pau Monné
The 64-bit DMAR fault address is composed of two 32 bits registers
DMAR_FEADDR_REG and DMAR_FEUADDR_REG. According to VT-d spec:
"Software is expected to access 32-bit registers as aligned doublewords",
a hypervisor should use two 32-bit writes to DMAR_FEADDR_REG and
DMAR_FEUADDR_REG separately in order to update a 64-bit fault address,
rather than a 64-bit write to DMAR_FEADDR_REG. Note that when x2APIC
is not enabled DMAR_FEUADDR_REG is reserved and it's not necessary to
update it.
Though I haven't seen any errors caused by such one 64-bit write on
real machines, it's still better to follow the specification.
Fixes: ae05fd3912b ("VT-d: use qword MMIO access for MSI address writes")
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v2:
* Explain in commit message and code comment why not updating DMAR_FEUADDR_REG
when x2APIC is not enabled
This patch actually reverts part of commit ae05fd3912b
("VT-d: use qword MMIO access for MSI address writes"). The latter
was included in XSA-120, 128..131 follow-up patch series [1]. I
don't know whether my patch breaks those XSA fixes. If it does,
please drop my patch.
[1] https://lists.xenproject.org/archives/html/xen-devel/2015-06/msg00638.html
---
xen/drivers/passthrough/vtd/iommu.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index daaed0abbd..81dd2085c7 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1105,7 +1105,13 @@ static void dma_msi_set_affinity(struct irq_desc *desc, const cpumask_t *mask)
spin_lock_irqsave(&iommu->register_lock, flags);
dmar_writel(iommu->reg, DMAR_FEDATA_REG, msg.data);
- dmar_writeq(iommu->reg, DMAR_FEADDR_REG, msg.address);
+ dmar_writel(iommu->reg, DMAR_FEADDR_REG, msg.address_lo);
+ /*
+ * When x2APIC is not enabled, DMAR_FEUADDR_REG is reserved and
+ * it's not necessary to update it.
+ */
+ if (x2apic_enabled)
+ dmar_writel(iommu->reg, DMAR_FEUADDR_REG, msg.address_hi);
spin_unlock_irqrestore(&iommu->register_lock, flags);
}
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] VT-d: use two 32-bit writes to update DMAR fault address registers
2017-10-11 3:03 [PATCH v2] VT-d: use two 32-bit writes to update DMAR fault address registers Haozhong Zhang
@ 2017-10-11 11:27 ` Jan Beulich
2018-02-22 9:44 ` Roger Pau Monné
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2017-10-11 11:27 UTC (permalink / raw)
To: Haozhong Zhang; +Cc: Andrew Cooper, Kevin Tian, xen-devel, Roger Pau Monné
>>> On 11.10.17 at 05:03, <haozhong.zhang@intel.com> wrote:
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1105,7 +1105,13 @@ static void dma_msi_set_affinity(struct irq_desc
> *desc, const cpumask_t *mask)
>
> spin_lock_irqsave(&iommu->register_lock, flags);
> dmar_writel(iommu->reg, DMAR_FEDATA_REG, msg.data);
> - dmar_writeq(iommu->reg, DMAR_FEADDR_REG, msg.address);
> + dmar_writel(iommu->reg, DMAR_FEADDR_REG, msg.address_lo);
> + /*
> + * When x2APIC is not enabled, DMAR_FEUADDR_REG is reserved and
> + * it's not necessary to update it.
> + */
> + if (x2apic_enabled)
I'm pretty sure it was pointed out before that the style here is
wrong (missing spaces). That's easy to fix while committing, but
anyway.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] VT-d: use two 32-bit writes to update DMAR fault address registers
2017-10-11 11:27 ` Jan Beulich
@ 2018-02-22 9:44 ` Roger Pau Monné
2018-02-22 9:50 ` Jan Beulich
0 siblings, 1 reply; 5+ messages in thread
From: Roger Pau Monné @ 2018-02-22 9:44 UTC (permalink / raw)
To: Jan Beulich; +Cc: Haozhong Zhang, Kevin Tian, xen-devel, Andrew Cooper
On Wed, Oct 11, 2017 at 05:27:05AM -0600, Jan Beulich wrote:
> >>> On 11.10.17 at 05:03, <haozhong.zhang@intel.com> wrote:
> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -1105,7 +1105,13 @@ static void dma_msi_set_affinity(struct irq_desc
> > *desc, const cpumask_t *mask)
> >
> > spin_lock_irqsave(&iommu->register_lock, flags);
> > dmar_writel(iommu->reg, DMAR_FEDATA_REG, msg.data);
> > - dmar_writeq(iommu->reg, DMAR_FEADDR_REG, msg.address);
> > + dmar_writel(iommu->reg, DMAR_FEADDR_REG, msg.address_lo);
> > + /*
> > + * When x2APIC is not enabled, DMAR_FEUADDR_REG is reserved and
> > + * it's not necessary to update it.
> > + */
> > + if (x2apic_enabled)
>
> I'm pretty sure it was pointed out before that the style here is
> wrong (missing spaces). That's easy to fix while committing, but
> anyway.
It seems like this was not applied. Can someone fix the style and
apply it?
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] VT-d: use two 32-bit writes to update DMAR fault address registers
2018-02-22 9:44 ` Roger Pau Monné
@ 2018-02-22 9:50 ` Jan Beulich
2018-02-23 2:39 ` Tian, Kevin
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2018-02-22 9:50 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Andrew Cooper, Kevin Tian, xen-devel, Haozhong Zhang
>>> On 22.02.18 at 10:44, <roger.pau@citrix.com> wrote:
> On Wed, Oct 11, 2017 at 05:27:05AM -0600, Jan Beulich wrote:
>> >>> On 11.10.17 at 05:03, <haozhong.zhang@intel.com> wrote:
>> > --- a/xen/drivers/passthrough/vtd/iommu.c
>> > +++ b/xen/drivers/passthrough/vtd/iommu.c
>> > @@ -1105,7 +1105,13 @@ static void dma_msi_set_affinity(struct irq_desc
>> > *desc, const cpumask_t *mask)
>> >
>> > spin_lock_irqsave(&iommu->register_lock, flags);
>> > dmar_writel(iommu->reg, DMAR_FEDATA_REG, msg.data);
>> > - dmar_writeq(iommu->reg, DMAR_FEADDR_REG, msg.address);
>> > + dmar_writel(iommu->reg, DMAR_FEADDR_REG, msg.address_lo);
>> > + /*
>> > + * When x2APIC is not enabled, DMAR_FEUADDR_REG is reserved and
>> > + * it's not necessary to update it.
>> > + */
>> > + if (x2apic_enabled)
>>
>> I'm pretty sure it was pointed out before that the style here is
>> wrong (missing spaces). That's easy to fix while committing, but
>> anyway.
>
> It seems like this was not applied. Can someone fix the style and
> apply it?
I would have fixed the style issue, but I have no VT-d maintainer
ack on record. It is the patch submitter to go hunt for acks.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] VT-d: use two 32-bit writes to update DMAR fault address registers
2018-02-22 9:50 ` Jan Beulich
@ 2018-02-23 2:39 ` Tian, Kevin
0 siblings, 0 replies; 5+ messages in thread
From: Tian, Kevin @ 2018-02-23 2:39 UTC (permalink / raw)
To: Jan Beulich, Roger Pau Monné
Cc: Andrew Cooper, xen-devel@lists.xen.org, Zhang, Haozhong
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Thursday, February 22, 2018 5:50 PM
>
> >>> On 22.02.18 at 10:44, <roger.pau@citrix.com> wrote:
> > On Wed, Oct 11, 2017 at 05:27:05AM -0600, Jan Beulich wrote:
> >> >>> On 11.10.17 at 05:03, <haozhong.zhang@intel.com> wrote:
> >> > --- a/xen/drivers/passthrough/vtd/iommu.c
> >> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> >> > @@ -1105,7 +1105,13 @@ static void dma_msi_set_affinity(struct
> irq_desc
> >> > *desc, const cpumask_t *mask)
> >> >
> >> > spin_lock_irqsave(&iommu->register_lock, flags);
> >> > dmar_writel(iommu->reg, DMAR_FEDATA_REG, msg.data);
> >> > - dmar_writeq(iommu->reg, DMAR_FEADDR_REG, msg.address);
> >> > + dmar_writel(iommu->reg, DMAR_FEADDR_REG, msg.address_lo);
> >> > + /*
> >> > + * When x2APIC is not enabled, DMAR_FEUADDR_REG is reserved
> and
> >> > + * it's not necessary to update it.
> >> > + */
> >> > + if (x2apic_enabled)
> >>
> >> I'm pretty sure it was pointed out before that the style here is
> >> wrong (missing spaces). That's easy to fix while committing, but
> >> anyway.
> >
> > It seems like this was not applied. Can someone fix the style and
> > apply it?
>
> I would have fixed the style issue, but I have no VT-d maintainer
> ack on record. It is the patch submitter to go hunt for acks.
>
I didn't realize no ack from me. Anyway, here it is:
Acked-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-02-23 2:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-11 3:03 [PATCH v2] VT-d: use two 32-bit writes to update DMAR fault address registers Haozhong Zhang
2017-10-11 11:27 ` Jan Beulich
2018-02-22 9:44 ` Roger Pau Monné
2018-02-22 9:50 ` Jan Beulich
2018-02-23 2:39 ` Tian, Kevin
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).