RE: RFC: vfio API changes needed for powerpc (v3)
> -Original Message-
> From: Wood Scott-B07421
> Sent: Friday, April 05, 2013 5:17 PM
> To: Yoder Stuart-B08248
> Cc: Alex Williamson; Wood Scott-B07421; ag...@suse.de; Bhushan Bharat-R65777;
> Sethi Varun-B16395;
> k...@vger.kernel.org; qemu-de...@nongnu.org; iommu@lists.linux-foundation.org
> Subject: Re: RFC: vfio API changes needed for powerpc (v3)
>
> On 04/04/2013 05:10:27 PM, Yoder Stuart-B08248 wrote:
> > /*
> > * VFIO_IOMMU_PAMU_UNMAP_MSI_BANK
> > *
> > * Unmaps the MSI bank at the specified iova.
> > * Caller provides struct vfio_pamu_msi_bank_unmap with all fields
> > set.
> > * Operates on VFIO file descriptor (/dev/vfio/vfio).
> > * Return: 0 on success, -errno on failure
> > */
> >
> > struct vfio_pamu_msi_bank_unmap {
> > __u32 argsz;
> > __u32 flags; /* no flags currently */
> > __u64 iova; /* the iova to be unmapped to */
> > };
> > #define VFIO_IOMMU_PAMU_UNMAP_MSI_BANK _IO(VFIO_TYPE, VFIO_BASE + x,
> > struct vfio_pamu_msi_bank_unmap )
>
> What happens if a normal unmap call is done on the MSI iova? Do we
> need a separate unmap?
I was thinking a normal unmap on an MSI windows would be an error...but
I'm not set on that. I put the msi unmap there to make things symmetric,
a normal unmap would work as well...and then we could drop the msi unmap.
Stuart
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V3] iommu/amd: Add logic to decode AMD IOMMU event flag
On Mon, Apr 08, 2013 at 10:43:45AM -0500, Suravee Suthikulanit wrote: > On 4/8/2013 9:50 AM, Borislav Petkov wrote: > >Independent from Joerg's feedback on this, I have only one question: > >you're not seriously considering on dumping this "Note:..." line above > >on*every* IO-PF, right? > If you think that is obvious, I can get rid of this also. I don't know whether it is obvious or not, but this thing doesn't belong there. I'm sure you can think of a much better fitting location to point to the documentation. And while you do that, you can simply add a link to the pdf version of the spec so that people don't have to look for it at all. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] documentation: iommu: add description of ARM System MMU binding
Hi Will, On 4/8/2013 2:25 AM, Will Deacon wrote: > Hi Olav, > > On Fri, Apr 05, 2013 at 09:44:49PM +0100, Olav Haugan wrote: >> On 4/4/2013 9:50 AM, Will Deacon wrote: >>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt >>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt >>> new file mode 100644 >>> index 000..938325f >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt >>> @@ -0,0 +1,61 @@ >>> +* ARM System MMU Architecture Implementation >>> + >>> +ARM SoCs may contain an implementation of the ARM System Memory >>> +Management Unit Architecture, which can be used to provide 1 or 2 stages >>> +of address translation to bus masters external to the CPU. >>> + >>> +The SMMU may also raise interrupts in response to various fault >>> +conditions. >>> + >>> +** System MMU required properties: >>> + >>> +- compatible: Should be one of "arm,smmu-v1" or "arm,smmu-v2" >>> + depending on the version of the architecture >>> + implemented. >>> + >>> +- reg : Base address and size of the SMMU. >>> + >>> +- #global-interrupts : The number of global interrupts exposed by the >>> + device. >>> + >>> +- interrupts: Interrupt list, with the first #global-irqs entries >>> + corresponding to the global interrupts and any >>> + following entries corresponding to context interrupts, >>> + specified in order of their indexing by the SMMU. >>> + >>> +- mmu-masters : A list of phandles to device nodes representing bus >>> + masters for which the SMMU can provide a translation. >> >> I am not sure I understand the use of mmu-masters. I would imagine that >> the bus masters themselves would have phandles to their respective SMMUs >> that provides the translation. > > The problem with that is when you have chained SMMUs. E.g., a separate SMMU > for stage 1 and stage 2, where the StreamIDs are not the same going into the > second SMMU on the chain. The set of masters and StreamIDs coming into a > system MMU is really a property of that system MMU, and is determined at > integration time. > >>> + >>> +- stream-ids: A list of 16-bit values corresponding to the StreamIDs >>> + for the devices listed in the mmu-masters property. >>> + This list must be same length as mmu-masters, so >>> + masters with multiple stream-ids will have multiple >>> + entries in mmu-masters. >>> + >> >> Why are the stream IDs (SIDs) coupled with the bus masters in this way? >> You are probably following a different pattern than we do. Our SMMU >> driver programs the SMMU SIDs and does not really know or care which >> mmu-master it belongs to. > > Generally, the StreamIDs are fixed in hardware (as a function of various AXI > bits -- see the SMMU integration guide) and cannot be set by software. > Furthermore, when the StreamIDs have an implicit effect on IOMMU domain > configuration, since stream-matching may not be always be able to identify a > master uniquely. Let me clarify what I mean by "our SMMU driver programs the SMMU SIDs". What I meant to say is that our SMMU driver programs the SID into the Stream Match Register (SMR) to route the transactions to the correct context bank based on the SID. >> Please see my comments above. I would like to work with you on defining >> the bindings for System MMU. We have had System MMU DT bindings for some >> time and I'd like to share what we are doing in hope that we can merge >> something that works for all of us. >> >> We use some of the same properties but we have a lot more. We also model >> the context banks as children of each SMMU in an object-oriented way. > > Hmm, what you have looks really... complicated. Why do you need so much stuff? Yes, some of the stuff is implementation specific. >> Here is our current binding for SMMUv1: >> >> * Qualcomm MSM IOMMU v1 >> >> Required properties: >> - compatible : one of: >> - "qcom,msm-smmu-v1" >> - reg : offset and length of the register set for the device. Optional >> offset and length for clock register for additional clock that >> needs to be turned on for access to this IOMMU. >> - reg-names: "iommu_base", "clk_base" (optional) > > Since I'm just working on a software model, I've not gone near clocks. > However, clocks are fairly well understood so we could easily add those > later if they're needed. > >> - label: name of this IOMMU instance. > > What? This is mostly used for debugging allowing us to log a name that is more user friendly instead of something like "fda64000.qcom,iommu" as the device name. We also use this to create debugfs directory with a user friendly name. > >> Optional properties: >> - qcom,iommu-secure-id : Secure identifier for the IOMMU block >> - vdd-supply : vdd-supply: phandle to GDSC regulator controlling this IOMMU. >> - qcom,alt-vdd-supp
Re: [PATCH V3] iommu/amd: Add logic to decode AMD IOMMU event flag
On 4/8/2013 9:50 AM, Borislav Petkov wrote: Independent from Joerg's feedback on this, I have only one question: you're not seriously considering on dumping this "Note:..." line above on*every* IO-PF, right? Boris, If you think that is obvious, I can get rid of this also. Suravee ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V3] iommu/amd: Add logic to decode AMD IOMMU event flag
On Mon, Apr 08, 2013 at 02:33:32PM +, Suthikulpanit, Suravee wrote: > Joerg, > > Do you have any more feedback about this patch? > > Thanks, > > Suravee > > From: suravee.suthikulpa...@amd.com [suravee.suthikulpa...@amd.com] > Sent: Tuesday, April 02, 2013 7:06 PM > To: iommu@lists.linux-foundation.org; j...@8bytes.org > Cc: linux-ker...@vger.kernel.org; Suthikulpanit, Suravee > Subject: [PATCH V3] iommu/amd: Add logic to decode AMD IOMMU event flag > > From: Suravee Suthikulpanit > > Add logic to decode AMD IOMMU event flag based on information from AMD IOMMU > specification. > This should simplify debugging IOMMU errors. Also, dump DTE information in > additional cases. > > This is an example: > AMD-Vi: Event logged [IO_PAGE_FAULT device=51:00.0 domain=0x > address=0x flags=0x0fff] > AMD-Vi: Flags details: Guest NX=1 User Intr Present Write No-Perm Rsrv-Bit > Translation > AMD-Vi: Type of error: (0x7) > AMD-Vi: (Note: Please refer to AMD IOMMU specification for details.) Independent from Joerg's feedback on this, I have only one question: you're not seriously considering on dumping this "Note:..." line above on *every* IO-PF, right? I very positively assume that people who stare at that output should, as a first prerequisite, know where to find those fields' descriptions. :-) Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH V3] iommu/amd: Add logic to decode AMD IOMMU event flag
Joerg,
Do you have any more feedback about this patch?
Thanks,
Suravee
From: suravee.suthikulpa...@amd.com [suravee.suthikulpa...@amd.com]
Sent: Tuesday, April 02, 2013 7:06 PM
To: iommu@lists.linux-foundation.org; j...@8bytes.org
Cc: linux-ker...@vger.kernel.org; Suthikulpanit, Suravee
Subject: [PATCH V3] iommu/amd: Add logic to decode AMD IOMMU event flag
From: Suravee Suthikulpanit
Add logic to decode AMD IOMMU event flag based on information from AMD IOMMU
specification.
This should simplify debugging IOMMU errors. Also, dump DTE information in
additional cases.
This is an example:
AMD-Vi: Event logged [IO_PAGE_FAULT device=51:00.0 domain=0x
address=0x flags=0x0fff]
AMD-Vi: Flags details: Guest NX=1 User Intr Present Write No-Perm Rsrv-Bit
Translation
AMD-Vi: Type of error: (0x7)
AMD-Vi: (Note: Please refer to AMD IOMMU specification for details.)
AMD-Vi: DTE[0]: 603fa75e2403
AMD-Vi: DTE[1]: 0014
AMD-Vi: DTE[2]: 203fa5e09011
AMD-Vi: DTE[3]:
Signed-off-by: Suravee Suthikulpanit
---
Changelog:
V3:
* Move comments to end of line
* Shorten the print out to be within one line
V2:
* Fix printing format to reduce noise
* Use string table instead of switch/case
* Use pr_cont instead of printk
drivers/iommu/amd_iommu.c | 171 -
1 file changed, 137 insertions(+), 34 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index b287ca3..593a1a3 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -601,6 +601,94 @@ static void amd_iommu_stats_init(void)
*
/
+struct _event_log_flags {
+ u32 gn:1, /* 16 */
+ nx:1, /* 17 */
+ us:1, /* 18 */
+i:1, /* 19 */
+ pr:1, /* 20 */
+ rw:1, /* 21 */
+ pe:1, /* 22 */
+ rz:1, /* 23 */
+ tr:1, /* 24 */
+ type:3, /* [27:25] */
+ _reserved_:20; /* Reserved */
+};
+
+static const char * const _type_field_encodings[] = {
+ "Reserved", /* 00 */
+ "Master Abort", /* 01 */
+ "Target Abort", /* 10 */
+ "Data Error", /* 11 */
+};
+
+static const char * const _invalid_transaction_desc[] = {
+ "Read request or non-posted write in the interrupt "
+"addres range",/* 000 */
+ "Pretranslated transaction received from an "
+ "I/O device that has I=0 or V=0 in DTE",/* 001 */
+ "Port I/O space transaction received from an "
+ "I/O device that has IoCtl=00b in DTE", /* 010 */
+ "Posted write to invalid address range",/* 011 */
+ "Invalid read request or non-posted write", /* 100 */
+ "Posted write to the interrupt/EOI range from an "
+ "I/O device that has IntCtl=00b in DTE",/* 101 */
+ "Posted write to a reserved interrupt address range", /* 110 */
+ "Invalid transaction to the system management "
+ "address range",/* 111 */
+};
+
+static const char * const _invalid_translation_desc[] = {
+ "Translation request received from an I/O device "
+ "that has I=0, or has V=0, or has V=1 and "
+ "TV=0 in DTE", /* 000 */
+ "Translation request in invalid address range", /* 001 */
+ "Invalid translation request", /* 010 */
+ "Reserved", /* 011 */
+ "Reserved", /* 100 */
+ "Reserved", /* 101 */
+ "Reserved", /* 110 */
+ "Reserved", /* 111 */
+};
+
+static void dump_flags(int flags, int ev_type)
+{
+ struct _event_log_flags *p = (struct _event_log_flags *) &flags;
+ u32 err_type = p->type;
+
+ pr_err("AMD-Vi: Flags details: %s NX=%u %s %s %s %s %s %s %s\n",
+ (p->gn ? "Guest" : "Nested"),
+ (p->nx),
+ (p->us ? "User" : "Super"),
+ (p->i ? "Intr" : "Mem"),
+ (p->pr ? "Present" : "Absent"),
+ (p->rw ? "Write" : "Read"),
+ (p->pe ? "No-Perm" : "Has-Perm"),
+ (p->rz ? "Rsrv-Bit" : "Ill-Level"),
+ (p->tr ? "Translation" : "Transaction"));
+
+ pr_err("AMD-Vi: Type of error: (0x%x) ", err_type);
+ if ((ev_type == EVENT_TYPE_DEV_TAB_ERR) ||
+
Re: [PATCH] iommu/amd: Add workaround to propery clearing IOMMU status register
Joerg,
Do you have any feedback about this patch?
Thanks,
Suravee
On 4/3/2013 6:19 PM, suravee.suthikulpa...@amd.com wrote:
From: Suravee Suthikulpanit
The IOMMU interrupt handling in bottom half must clear the PPR log interrupt
and event log interrupt bits to re-enable the interrupt. This is done by
writing 1 to the memory mapped register to clear the bit. Due to hardware bug,
if the driver tries to clear this bit while the IOMMU hardware also setting
this bit, the conflict will result with the bit being set. If the interrupt
handling code does not make sure to clear this bit, subsequent changes in the
event/PPR logs will no longer generating interrupts, and would result if
buffer overflow. After clearing the bits, the driver must read back
the register to verify.
In the current interrupt handling scheme, there are as many threads as
the number of IOMMUs. Each thread is created and assigned to an IOMMU at
the time of registering interrupt handlers (request_threaded_irq).
When an IOMMU HW generates an interrupt, the irq handler (top half) wakes up
the corresponding thread to process event and PPR logs of all IOMMUs
starting from the 1st IOMMU.
In the system with multiple IOMMU,this handling scheme complicates the
synchronization of the IOMMU data structures and status registers as
there could be multiple threads competing for the same IOMMU while
the other IOMMU could be left unhandled.
In order to simplify the implementation of the workaround, this patch is
proposing a different interrupt handling scheme by having each thread only
managing interrupts of the corresponding IOMMU. This can be achieved by passing
the struct amd_iommu when registering the interrupt handlers. This structure is
unique for each IOMMU and can be used by the bottom half thread to identify
the IOMMU to be handled instead of calling for_each_iommu. Besides this also
eliminate the needs to lock the IOMMU for processing event and PPR logs.
Signed-off-by: Suravee Suthikulpanit
---
drivers/iommu/amd_iommu.c | 59
drivers/iommu/amd_iommu_init.c |2 +-
2 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index e5db3e5..3548d63 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -803,12 +803,6 @@ retry:
static void iommu_poll_events(struct amd_iommu *iommu)
{
u32 head, tail;
- unsigned long flags;
-
- /* enable event interrupts again */
- writel(MMIO_STATUS_EVT_INT_MASK, iommu->mmio_base + MMIO_STATUS_OFFSET);
-
- spin_lock_irqsave(&iommu->lock, flags);
head = readl(iommu->mmio_base + MMIO_EVT_HEAD_OFFSET);
tail = readl(iommu->mmio_base + MMIO_EVT_TAIL_OFFSET);
@@ -819,8 +813,6 @@ static void iommu_poll_events(struct amd_iommu *iommu)
}
writel(head, iommu->mmio_base + MMIO_EVT_HEAD_OFFSET);
-
- spin_unlock_irqrestore(&iommu->lock, flags);
}
static void iommu_handle_ppr_entry(struct amd_iommu *iommu, u64 *raw)
@@ -845,17 +837,11 @@ static void iommu_handle_ppr_entry(struct amd_iommu
*iommu, u64 *raw)
static void iommu_poll_ppr_log(struct amd_iommu *iommu)
{
- unsigned long flags;
u32 head, tail;
if (iommu->ppr_log == NULL)
return;
- /* enable ppr interrupts again */
- writel(MMIO_STATUS_PPR_INT_MASK, iommu->mmio_base + MMIO_STATUS_OFFSET);
-
- spin_lock_irqsave(&iommu->lock, flags);
-
head = readl(iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
tail = readl(iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
@@ -891,34 +877,49 @@ static void iommu_poll_ppr_log(struct amd_iommu *iommu)
head = (head + PPR_ENTRY_SIZE) % PPR_LOG_SIZE;
writel(head, iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
- /*
-* Release iommu->lock because ppr-handling might need to
-* re-acquire it
-*/
- spin_unlock_irqrestore(&iommu->lock, flags);
-
/* Handle PPR entry */
iommu_handle_ppr_entry(iommu, entry);
- spin_lock_irqsave(&iommu->lock, flags);
-
/* Refresh ring-buffer information */
head = readl(iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
tail = readl(iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
}
-
- spin_unlock_irqrestore(&iommu->lock, flags);
}
irqreturn_t amd_iommu_int_thread(int irq, void *data)
{
- struct amd_iommu *iommu;
+ struct amd_iommu *iommu = (struct amd_iommu *) data;
+ u32 status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
- for_each_iommu(iommu) {
- iommu_poll_events(iommu);
- iommu_poll_ppr_log(iommu);
- }
+ while (status & (MMIO_STATUS_EVT_INT_MASK | MMIO_STATUS_PPR_INT_MASK)) {
+ if (status & MMIO_STATUS_EVT_INT_MASK) {
+ pr_devel("AMD-Vi: Processing I
Re: [PATCH] documentation: iommu: add description of ARM System MMU binding
Hi Olav, On Fri, Apr 05, 2013 at 09:44:49PM +0100, Olav Haugan wrote: > On 4/4/2013 9:50 AM, Will Deacon wrote: > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt > > b/Documentation/devicetree/bindings/iommu/arm,smmu.txt > > new file mode 100644 > > index 000..938325f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt > > @@ -0,0 +1,61 @@ > > +* ARM System MMU Architecture Implementation > > + > > +ARM SoCs may contain an implementation of the ARM System Memory > > +Management Unit Architecture, which can be used to provide 1 or 2 stages > > +of address translation to bus masters external to the CPU. > > + > > +The SMMU may also raise interrupts in response to various fault > > +conditions. > > + > > +** System MMU required properties: > > + > > +- compatible: Should be one of "arm,smmu-v1" or "arm,smmu-v2" > > + depending on the version of the architecture > > + implemented. > > + > > +- reg : Base address and size of the SMMU. > > + > > +- #global-interrupts : The number of global interrupts exposed by the > > + device. > > + > > +- interrupts: Interrupt list, with the first #global-irqs entries > > + corresponding to the global interrupts and any > > + following entries corresponding to context interrupts, > > + specified in order of their indexing by the SMMU. > > + > > +- mmu-masters : A list of phandles to device nodes representing bus > > + masters for which the SMMU can provide a translation. > > I am not sure I understand the use of mmu-masters. I would imagine that > the bus masters themselves would have phandles to their respective SMMUs > that provides the translation. The problem with that is when you have chained SMMUs. E.g., a separate SMMU for stage 1 and stage 2, where the StreamIDs are not the same going into the second SMMU on the chain. The set of masters and StreamIDs coming into a system MMU is really a property of that system MMU, and is determined at integration time. > > + > > +- stream-ids: A list of 16-bit values corresponding to the StreamIDs > > + for the devices listed in the mmu-masters property. > > + This list must be same length as mmu-masters, so > > + masters with multiple stream-ids will have multiple > > + entries in mmu-masters. > > + > > Why are the stream IDs (SIDs) coupled with the bus masters in this way? > You are probably following a different pattern than we do. Our SMMU > driver programs the SMMU SIDs and does not really know or care which > mmu-master it belongs to. Generally, the StreamIDs are fixed in hardware (as a function of various AXI bits -- see the SMMU integration guide) and cannot be set by software. Furthermore, when the StreamIDs have an implicit effect on IOMMU domain configuration, since stream-matching may not be always be able to identify a master uniquely. > Please see my comments above. I would like to work with you on defining > the bindings for System MMU. We have had System MMU DT bindings for some > time and I'd like to share what we are doing in hope that we can merge > something that works for all of us. > > We use some of the same properties but we have a lot more. We also model > the context banks as children of each SMMU in an object-oriented way. Hmm, what you have looks really... complicated. Why do you need so much stuff? > Here is our current binding for SMMUv1: > > * Qualcomm MSM IOMMU v1 > > Required properties: > - compatible : one of: > - "qcom,msm-smmu-v1" > - reg : offset and length of the register set for the device. Optional > offset and length for clock register for additional clock that > needs to be turned on for access to this IOMMU. > - reg-names: "iommu_base", "clk_base" (optional) Since I'm just working on a software model, I've not gone near clocks. However, clocks are fairly well understood so we could easily add those later if they're needed. > - label: name of this IOMMU instance. What? > Optional properties: > - qcom,iommu-secure-id : Secure identifier for the IOMMU block > - vdd-supply : vdd-supply: phandle to GDSC regulator controlling this IOMMU. > - qcom,alt-vdd-supply : Alternative regulator needed to access IOMMU > configuration registers. > - interrupts : should contain the performance monitor overflow interrupt > number. > - qcom,iommu-enable-halt : Enable halt of the IOMMU before programming > certain registers > - qcom,iommu-pmu-ngroups: Number of Performance Monitor Unit (PMU) groups. > - qcom,iommu-pmu-ncounters: Number of PMU counters per group. > - qcom,iommu-pmu-event-classes: List of event classes supported. > - qcom,needs-alt-core-clk : boolean to enable the secondary core clock > for access to the IOMMU configuration registers > - qcom,iommu-bfb-regs : An array of unsigned 32-bit int
Re: [PATCH] documentation: iommu: add description of ARM System MMU binding
On Fri, Apr 05, 2013 at 07:25:26PM +0100, Rob Herring wrote: > On 04/05/2013 11:57 AM, Will Deacon wrote: > > Hi Rob, > > > > On Fri, Apr 05, 2013 at 05:43:06PM +0100, Rob Herring wrote: > > [...] > > >>> +- compatible: Should be one of "arm,smmu-v1" or "arm,smmu-v2" > >>> + depending on the version of the architecture > >>> + implemented. > >> > >> We can keep these, but we should have specific models like arm,smmu-400, > >> etc. as well. > > > > Ok, if distinctions need to be between MMU-400 and a v1 implementation, then > > we can add those strings later. > > No, you want to have specific values in the dtb's up front. If there is > something needed later in the kernel, you don't want to require a dtb > update then. Adding future parts later is fine, but we already know what > blocks we have. Yes, you're right. I'll add those for v2. Cheers, Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] intel-iommu: Synchronize gcmd value with global command register
(2013/04/04 23:24), David Woodhouse wrote: > On Thu, 2013-04-04 at 14:48 +0900, Takao Indoh wrote: >> >> - DMAR fault messages floods and second kernel does not boot. Recently I >>saw similar report. https://lkml.org/lkml/2013/3/8/120 > > Right. So the fix for that is to make the subsequent errors silent, > until/unless we actually get a request to create a mapping for the given > device. > >> - igb driver detectes error on linkup and kdump via network fails. > > That's a driver bug, IIRC. It was failing to completely reset the > hardware. It's fixed now, isn't it? No, it can be reproduced with latest kernel(3.9.0-rc6). > >> - On a certain platform, though kdump itself works, PCIe error like >>Unexpected Completion is detected and it gets hardware degraded. > > More information required. When I tested intel_iommu on a certain machine, the following error message was logged in its firmware, and I/O board got abnormal status. 05:00.0 is igb, so I think this was caused by DMA error on igb. This occurs before igb driver loading, so this cannot be fixed in driver. PCI: Unexpected Completion Bus: 5 Device: 0x00 Function: 0x00 Anyway, I'm thinking we should introduce something framework to clean all devices to stop DMA at boot time rather than dealing with the problem in each driver. And one of the way I found is resetting devcies by PCIe layer. If DMAR is disabled in init_dmars(), we can have a chance to handle devices to stop DMA in PCI layer, like qci-quirk. This is one of the reason why I propose this patch. Thanks, Takao Indoh ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
