[PATCH] amd iommu: re-enable iommu msi if dom0 disabled it

[PATCH] amd iommu: re-enable iommu msi if dom0 disabled it

[Wei Wang]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]

Hi Jan,
I found that recent dom0 (e.g 3.4 pv_ops) disables iommu msi capability 
for some reasons and iommu cannot generate any interrupts in this case.
Attached patch re-enables it in device assignment.

Thanks,
Wei

--
Advanced Micro Devices GmbH
Sitz: Dornach, Gemeinde Aschheim,
Landkreis München Registergericht München,
HRB Nr. 43632 WEEE Registrierungsnummer 129 19551
Geschäftsführer:
Alberto Bozzo

[-- Attachment #2: iommu-msi.patch --]
[-- Type: text/x-patch, Size: 1785 bytes --]

# HG changeset patch
# Parent f6bfaf9daa508c31b2bca0e461202db2759426fc
# User Wei Wang <wei.wang2@amd.com>
Re-enable iommu msi capability block if it is disabled by dom0

Signed-off-by: Wei Wang <wei.wang2@amd.com>

diff -r f6bfaf9daa50 xen/drivers/passthrough/amd/pci_amd_iommu.c
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c	Wed Jun 06 16:37:05 2012 +0100
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c	Fri Jun 08 15:13:11 2012 +0200
@@ -81,6 +81,30 @@ static void disable_translation(u32 *dte
     dte[0] = entry;
 }
 
+static void iommu_msi_check_enable(struct amd_iommu *iommu)
+{
+    unsigned long flags;
+    uint16_t control;
+    uint8_t bus = PCI_BUS(iommu->bdf);
+    uint8_t dev = PCI_SLOT(iommu->bdf);
+    uint8_t func = PCI_FUNC(iommu->bdf);
+
+    ASSERT( iommu->msi_cap );
+    
+    spin_lock_irqsave(&iommu->lock, flags);
+
+    control = pci_conf_read16(iommu->seg, bus, dev, func,
+                              iommu->msi_cap + PCI_MSI_FLAGS);
+    if ( !(control & IOMMU_CONTROL_ENABLED) )
+    {    
+        control |= IOMMU_CONTROL_ENABLED;
+        pci_conf_write16(iommu->seg, bus, dev, func,
+                         iommu->msi_cap + PCI_MSI_FLAGS, control);
+    }
+
+    spin_unlock_irqrestore(&iommu->lock, flags);
+}
+
 static void amd_iommu_setup_domain_device(
     struct domain *domain, struct amd_iommu *iommu, int bdf)
 {
@@ -101,6 +125,12 @@ static void amd_iommu_setup_domain_devic
     if ( ats_enabled )
         dte_i = 1;
 
+    /* 
+     * In some cases, dom0 disables iommu msi capability, 
+     * re-enable it here.
+     */
+    iommu_msi_check_enable(iommu); 
+
     /* get device-table entry */
     req_id = get_dma_requestor_id(iommu->seg, bdf);
     dte = iommu->dev_table.buffer + (req_id * IOMMU_DEV_TABLE_ENTRY_SIZE);

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] amd iommu: re-enable iommu msi if dom0 disabled it

[Andrew Cooper]

On 08/06/12 14:28, Wei Wang wrote:
> Hi Jan,
> I found that recent dom0 (e.g 3.4 pv_ops) disables iommu msi capability 
> for some reasons and iommu cannot generate any interrupts in this case.
> Attached patch re-enables it in device assignment.

Under which circumstances should dom0 able to play with the IOMMUs ? 
Surely the fact that dom0 can play with the IOMMUs is a bug in itself.

>
> Thanks,
> Wei
>
> --
> Advanced Micro Devices GmbH
> Sitz: Dornach, Gemeinde Aschheim,
> Landkreis München Registergericht München,
> HRB Nr. 43632 WEEE Registrierungsnummer 129 19551
> Geschäftsführer:
> Alberto Bozzo

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

Re: [PATCH] amd iommu: re-enable iommu msi if dom0 disabled it

[Wei Wang]

On 06/08/2012 03:31 PM, Andrew Cooper wrote:
> On 08/06/12 14:28, Wei Wang wrote:
>> Hi Jan,
>> I found that recent dom0 (e.g 3.4 pv_ops) disables iommu msi capability
>> for some reasons and iommu cannot generate any interrupts in this case.
>> Attached patch re-enables it in device assignment.
>
> Under which circumstances should dom0 able to play with the IOMMUs ?
> Surely the fact that dom0 can play with the IOMMUs is a bug in itself.

It looks like some generic msi/pci codes disable it, not the Linux iommu 
driver itself, which is only loaded on bare metal. AMD IOMMU expose 
interrupt capability as a normal msi block. So the general pci/msi layer 
of dom0 might touch it...

Thanks,
Wei


>>
>> Thanks,
>> Wei
>>
>> --
>> Advanced Micro Devices GmbH
>> Sitz: Dornach, Gemeinde Aschheim,
>> Landkreis München Registergericht München,
>> HRB Nr. 43632 WEEE Registrierungsnummer 129 19551
>> Geschäftsführer:
>> Alberto Bozzo
>

Re: [PATCH] amd iommu: re-enable iommu msi if dom0 disabled it

[Jan Beulich]

>>> On 08.06.12 at 15:41, Wei Wang <wei.wang2@amd.com> wrote:
> On 06/08/2012 03:31 PM, Andrew Cooper wrote:
>> On 08/06/12 14:28, Wei Wang wrote:
>>> I found that recent dom0 (e.g 3.4 pv_ops) disables iommu msi capability
>>> for some reasons and iommu cannot generate any interrupts in this case.
>>> Attached patch re-enables it in device assignment.
>>
>> Under which circumstances should dom0 able to play with the IOMMUs ?
>> Surely the fact that dom0 can play with the IOMMUs is a bug in itself.
> 
> It looks like some generic msi/pci codes disable it, not the Linux iommu 
> driver itself, which is only loaded on bare metal. AMD IOMMU expose 
> interrupt capability as a normal msi block. So the general pci/msi layer 
> of dom0 might touch it...

In that case it wouldn't be on pv-ops alone (which your patch
says).

Also, your patch using IOMMU_CONTROL_ENABLED instead of
PCI_MSI_FLAGS_ENABLE is quite misleading (as it hides the fact
the what you play with is not IOMMU-specific).

Jan

Re: [PATCH] amd iommu: re-enable iommu msi if dom0 disabled it

[Wei Wang]

On 06/08/2012 04:08 PM, Jan Beulich wrote:
>>>> On 08.06.12 at 15:41, Wei Wang<wei.wang2@amd.com>  wrote:
>> On 06/08/2012 03:31 PM, Andrew Cooper wrote:
>>> On 08/06/12 14:28, Wei Wang wrote:
>>>> I found that recent dom0 (e.g 3.4 pv_ops) disables iommu msi capability
>>>> for some reasons and iommu cannot generate any interrupts in this case.
>>>> Attached patch re-enables it in device assignment.
>>>
>>> Under which circumstances should dom0 able to play with the IOMMUs ?
>>> Surely the fact that dom0 can play with the IOMMUs is a bug in itself.
>>
>> It looks like some generic msi/pci codes disable it, not the Linux iommu
>> driver itself, which is only loaded on bare metal. AMD IOMMU expose
>> interrupt capability as a normal msi block. So the general pci/msi layer
>> of dom0 might touch it...
>
> In that case it wouldn't be on pv-ops alone (which your patch
> says).

Yes, probably upstream Linux also has this issue. I will check that. But 
I did not see this issue on 3.2 pv_ops & 2.6 pv_ops.

> Also, your patch using IOMMU_CONTROL_ENABLED instead of
> PCI_MSI_FLAGS_ENABLE is quite misleading (as it hides the fact
> the what you play with is not IOMMU-specific).

Thanks, I will use PCI_MSI_FLAGS_ENABLE in next post.

Wei

> Jan
>
>

Re: [PATCH] amd iommu: re-enable iommu msi if dom0 disabled it

[Jan Beulich]

>>> On 08.06.12 at 15:28, Wei Wang <wei.wang2@amd.com> wrote:
> I found that recent dom0 (e.g 3.4 pv_ops) disables iommu msi capability 
> for some reasons and iommu cannot generate any interrupts in this case.

That shouldn't happen in the first place, and hence should be fixed
in the kernel.

> Attached patch re-enables it in device assignment.

I'm not really keen on taking this - it's silently working around a
problem, and doesn't appear to cover the case where Dom0
would have its I/O got through the IOMMU as well.

If anything, I'd be much more in favor of hiding the entire config
space of the IOMMU device from Dom0 (or at least making it
read-only), which admittedly might be a little tricky. So first of
all let's understand whether this can't reasonably be fixed in
Linux.

Jan