Re: [Xen-devel] [PATCH v2] VT-d: use two 32-bit writes to update DMAR fault address registers

[Jan Beulich]

>>> On 11.10.17 at 05:03,  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(>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

[Xen-devel] [PATCH v2] VT-d: use two 32-bit writes to update DMAR fault address registers

[Haozhong Zhang]

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é 
Signed-off-by: Haozhong Zhang 
---
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(>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(>register_lock, flags);
 }
 
-- 
2.11.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel