* [Xen-devel] [PATCH for-4.13 0/2] Fixes to AMD IOMMU logging
@ 2019-11-26 15:01 Andrew Cooper
2019-11-26 15:01 ` [Xen-devel] [PATCH 1/2] AMD/IOMMU: Always print IOMMU errors Andrew Cooper
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Andrew Cooper @ 2019-11-26 15:01 UTC (permalink / raw)
To: Xen-devel; +Cc: Juergen Gross, Andrew Cooper, Jan Beulich
RFC for-4.13. These are diagnostic improvements/corrections, so are low risk
and high utility.
Andrew Cooper (2):
AMD/IOMMU: Always print IOMMU errors
AMD/IOMMU: Render IO_PAGE_FAULT errors in a more useful manner
xen/drivers/passthrough/amd/iommu_init.c | 48 +++++++++++++++------------
xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 3 --
2 files changed, 27 insertions(+), 24 deletions(-)
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [Xen-devel] [PATCH 1/2] AMD/IOMMU: Always print IOMMU errors 2019-11-26 15:01 [Xen-devel] [PATCH for-4.13 0/2] Fixes to AMD IOMMU logging Andrew Cooper @ 2019-11-26 15:01 ` Andrew Cooper 2019-11-27 9:19 ` Roger Pau Monné 2019-11-26 15:01 ` [Xen-devel] [PATCH 2/2] AMD/IOMMU: Render IO_PAGE_FAULT errors in a more useful manner Andrew Cooper 2019-11-26 15:36 ` [Xen-devel] [PATCH for-4.13 0/2] Fixes to AMD IOMMU logging Jürgen Groß 2 siblings, 1 reply; 10+ messages in thread From: Andrew Cooper @ 2019-11-26 15:01 UTC (permalink / raw) To: Xen-devel; +Cc: Juergen Gross, Andrew Cooper, Jan Beulich Unhandled IOMMU errors (i.e. not IO_PAGE_FAULT) should still be printed, and not hidden behind iommu=debug. While adjusting this, factor out the symbolic name handling to just one location exposing its off-by-one nature. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Juergen Gross <jgross@suse.com> --- xen/drivers/passthrough/amd/iommu_init.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c index 16e84d43d4..8aa8788797 100644 --- a/xen/drivers/passthrough/amd/iommu_init.c +++ b/xen/drivers/passthrough/amd/iommu_init.c @@ -530,6 +530,7 @@ static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[]) EVENT_STR(INVALID_DEV_REQUEST) #undef EVENT_STR }; + const char *code_str = "event"; code = get_field_from_reg_u32(entry[1], IOMMU_EVENT_CODE_MASK, IOMMU_EVENT_CODE_SHIFT); @@ -553,6 +554,10 @@ static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[]) IOMMU_EVENT_CODE_SHIFT); } + /* Look up the symbolic name for code. */ + if ( code <= ARRAY_SIZE(event_str) ) + code_str = event_str[code - 1]; + if ( code == IOMMU_EVENT_IO_PAGE_FAULT ) { device_id = iommu_get_devid_from_event(entry[0]); @@ -566,7 +571,7 @@ static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[]) printk(XENLOG_ERR "AMD-Vi: " "%s: domain = %d, device id = %#x, " "fault address = %#"PRIx64", flags = %#x\n", - event_str[code-1], domain_id, device_id, *addr, flags); + code_str, domain_id, device_id, *addr, flags); for ( bdf = 0; bdf < ivrs_bdf_entries; bdf++ ) if ( get_dma_requestor_id(iommu->seg, bdf) == device_id ) @@ -574,12 +579,8 @@ static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[]) PCI_DEVFN2(bdf)); } else - { - AMD_IOMMU_DEBUG("%s %08x %08x %08x %08x\n", - code <= ARRAY_SIZE(event_str) ? event_str[code - 1] - : "event", - entry[0], entry[1], entry[2], entry[3]); - } + printk(XENLOG_ERR "%s %08x %08x %08x %08x\n", + code_str, entry[0], entry[1], entry[2], entry[3]); memset(entry, 0, IOMMU_EVENT_LOG_ENTRY_SIZE); } -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH 1/2] AMD/IOMMU: Always print IOMMU errors 2019-11-26 15:01 ` [Xen-devel] [PATCH 1/2] AMD/IOMMU: Always print IOMMU errors Andrew Cooper @ 2019-11-27 9:19 ` Roger Pau Monné 2019-11-27 16:58 ` Jan Beulich 0 siblings, 1 reply; 10+ messages in thread From: Roger Pau Monné @ 2019-11-27 9:19 UTC (permalink / raw) To: Andrew Cooper; +Cc: Juergen Gross, Xen-devel, Jan Beulich On Tue, Nov 26, 2019 at 03:01:11PM +0000, Andrew Cooper wrote: > Unhandled IOMMU errors (i.e. not IO_PAGE_FAULT) should still be printed, and > not hidden behind iommu=debug. > > While adjusting this, factor out the symbolic name handling to just one > location exposing its off-by-one nature. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.comi> LGTM: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> I wonder however whether XENLOG_G_ERR should be used instead of XENLOG_ERR in order to rate limit IOMMU faults triggered by guests. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH 1/2] AMD/IOMMU: Always print IOMMU errors 2019-11-27 9:19 ` Roger Pau Monné @ 2019-11-27 16:58 ` Jan Beulich 0 siblings, 0 replies; 10+ messages in thread From: Jan Beulich @ 2019-11-27 16:58 UTC (permalink / raw) To: Andrew Cooper; +Cc: Juergen Gross, Xen-devel, Roger Pau Monné On 27.11.2019 10:19, Roger Pau Monné wrote: > On Tue, Nov 26, 2019 at 03:01:11PM +0000, Andrew Cooper wrote: >> Unhandled IOMMU errors (i.e. not IO_PAGE_FAULT) should still be printed, and >> not hidden behind iommu=debug. >> >> While adjusting this, factor out the symbolic name handling to just one >> location exposing its off-by-one nature. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.comi> > > LGTM: > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> > I wonder however whether XENLOG_G_ERR should be used instead of > XENLOG_ERR in order to rate limit IOMMU faults triggered by guests. IO_PAGE_FAULT uses XENLOG_ERR as well, so I'd stick to it. If there are really massive amounts of faults, log spam won't be our only problem, I think. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Xen-devel] [PATCH 2/2] AMD/IOMMU: Render IO_PAGE_FAULT errors in a more useful manner 2019-11-26 15:01 [Xen-devel] [PATCH for-4.13 0/2] Fixes to AMD IOMMU logging Andrew Cooper 2019-11-26 15:01 ` [Xen-devel] [PATCH 1/2] AMD/IOMMU: Always print IOMMU errors Andrew Cooper @ 2019-11-26 15:01 ` Andrew Cooper 2019-11-27 9:40 ` Roger Pau Monné 2019-11-27 17:02 ` Jan Beulich 2019-11-26 15:36 ` [Xen-devel] [PATCH for-4.13 0/2] Fixes to AMD IOMMU logging Jürgen Groß 2 siblings, 2 replies; 10+ messages in thread From: Andrew Cooper @ 2019-11-26 15:01 UTC (permalink / raw) To: Xen-devel; +Cc: Juergen Gross, Andrew Cooper, Jan Beulich Print the PCI coordinates in its common format and use d%u notation for the domain. As well as printing flags, decode them. IO_PAGE_FAULT is used for interrupt remapping errors as well as DMA remapping errors. Before: (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0xbf695000, flags = 0x10 (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0xbf695040, flags = 0x10 (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0xfffffff0, flags = 0x30 (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0x100000000, flags = 0x30 (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0x100000040, flags = 0x30 After: (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 00000000bf5fc000 flags 0x10 PR (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 00000000bf5fc040 flags 0x10 PR (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 00000000fffffff0 flags 0x30 RW PR (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 0000000100000000 flags 0x30 RW PR (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 0000000100000040 flags 0x30 RW PR No functional change. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Juergen Gross <jgross@suse.com> --- xen/drivers/passthrough/amd/iommu_init.c | 35 +++++++++++++++------------ xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 3 --- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c index 8aa8788797..cd4e6e16b8 100644 --- a/xen/drivers/passthrough/amd/iommu_init.c +++ b/xen/drivers/passthrough/amd/iommu_init.c @@ -513,10 +513,7 @@ static hw_irq_controller iommu_x2apic_type = { static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[]) { - u16 domain_id, device_id, flags; - unsigned int bdf; u32 code; - u64 *addr; int count = 0; static const char *const event_str[] = { #define EVENT_STR(name) [IOMMU_EVENT_##name - 1] = #name @@ -560,18 +557,26 @@ static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[]) if ( code == IOMMU_EVENT_IO_PAGE_FAULT ) { - device_id = iommu_get_devid_from_event(entry[0]); - domain_id = get_field_from_reg_u32(entry[1], - IOMMU_EVENT_DOMAIN_ID_MASK, - IOMMU_EVENT_DOMAIN_ID_SHIFT); - flags = get_field_from_reg_u32(entry[1], - IOMMU_EVENT_FLAGS_MASK, - IOMMU_EVENT_FLAGS_SHIFT); - addr= (u64*) (entry + 2); - printk(XENLOG_ERR "AMD-Vi: " - "%s: domain = %d, device id = %#x, " - "fault address = %#"PRIx64", flags = %#x\n", - code_str, domain_id, device_id, *addr, flags); + unsigned int bdf; + uint16_t device_id = MASK_EXTR(entry[0], IOMMU_CMD_DEVICE_ID_MASK); + uint16_t domain_id = MASK_EXTR(entry[1], IOMMU_EVENT_DOMAIN_ID_MASK); + uint16_t flags = MASK_EXTR(entry[1], IOMMU_EVENT_FLAGS_MASK); + uint64_t addr = *(uint64_t *)(entry + 2); + + printk(XENLOG_ERR "AMD-Vi: %s: %04x:%02x:%02x.%u d%d addr %016"PRIx64 + " flags %#x%s%s%s%s%s%s%s%s%s%s\n", + code_str, iommu->seg, PCI_BUS(device_id), PCI_SLOT(device_id), + PCI_FUNC(device_id), domain_id, addr, flags, + (flags & 0xe00) ? " ??" : "", + (flags & 0x100) ? " TR" : "", + (flags & 0x080) ? " RZ" : "", + (flags & 0x040) ? " PE" : "", + (flags & 0x020) ? " RW" : "", + (flags & 0x010) ? " PR" : "", + (flags & 0x008) ? " I" : "", + (flags & 0x004) ? " US" : "", + (flags & 0x002) ? " NX" : "", + (flags & 0x001) ? " GN" : ""); for ( bdf = 0; bdf < ivrs_bdf_entries; bdf++ ) if ( get_dma_requestor_id(iommu->seg, bdf) == device_id ) diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h index 8ed9482791..53900cd60c 100644 --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h @@ -265,9 +265,6 @@ static inline uint32_t iommu_get_addr_hi_from_cmd(uint32_t cmd) IOMMU_CMD_ADDR_HIGH_SHIFT); } -/* access address field from event log entry */ -#define iommu_get_devid_from_event iommu_get_devid_from_cmd - /* access iommu base addresses field from mmio regs */ static inline void iommu_set_addr_lo_to_reg(uint32_t *reg, uint32_t addr) { -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH 2/2] AMD/IOMMU: Render IO_PAGE_FAULT errors in a more useful manner 2019-11-26 15:01 ` [Xen-devel] [PATCH 2/2] AMD/IOMMU: Render IO_PAGE_FAULT errors in a more useful manner Andrew Cooper @ 2019-11-27 9:40 ` Roger Pau Monné 2019-11-27 17:38 ` Andrew Cooper 2019-11-27 17:02 ` Jan Beulich 1 sibling, 1 reply; 10+ messages in thread From: Roger Pau Monné @ 2019-11-27 9:40 UTC (permalink / raw) To: Andrew Cooper; +Cc: Juergen Gross, Xen-devel, Jan Beulich On Tue, Nov 26, 2019 at 03:01:12PM +0000, Andrew Cooper wrote: > Print the PCI coordinates in its common format and use d%u notation for the > domain. As well as printing flags, decode them. IO_PAGE_FAULT is used for > interrupt remapping errors as well as DMA remapping errors. > > Before: > (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0xbf695000, flags = 0x10 > (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0xbf695040, flags = 0x10 > (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0xfffffff0, flags = 0x30 > (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0x100000000, flags = 0x30 > (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0x100000040, flags = 0x30 > > After: > (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 00000000bf5fc000 flags 0x10 PR > (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 00000000bf5fc040 flags 0x10 PR > (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 00000000fffffff0 flags 0x30 RW PR > (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 0000000100000000 flags 0x30 RW PR > (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 0000000100000040 flags 0x30 RW PR Nit: I would place the domain id information at the beginning (since that's more similar to gprintk format), and maybe drop the AMD-Vi prefix, it's not very useful IMO: (XEN) d0 IO_PAGE_FAULT 0000:00:14.1 addr 0000000100000040 flags 0x30 RW PR But I'm not specially concerned. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> In it's current form or with some of the suggestions, in any case it's certainly an improvement. > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Juergen Gross <jgross@suse.com> > --- > xen/drivers/passthrough/amd/iommu_init.c | 35 +++++++++++++++------------ > xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 3 --- > 2 files changed, 20 insertions(+), 18 deletions(-) > > diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c > index 8aa8788797..cd4e6e16b8 100644 > --- a/xen/drivers/passthrough/amd/iommu_init.c > +++ b/xen/drivers/passthrough/amd/iommu_init.c > @@ -513,10 +513,7 @@ static hw_irq_controller iommu_x2apic_type = { > > static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[]) > { > - u16 domain_id, device_id, flags; > - unsigned int bdf; > u32 code; > - u64 *addr; > int count = 0; > static const char *const event_str[] = { > #define EVENT_STR(name) [IOMMU_EVENT_##name - 1] = #name > @@ -560,18 +557,26 @@ static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[]) > > if ( code == IOMMU_EVENT_IO_PAGE_FAULT ) > { > - device_id = iommu_get_devid_from_event(entry[0]); > - domain_id = get_field_from_reg_u32(entry[1], > - IOMMU_EVENT_DOMAIN_ID_MASK, > - IOMMU_EVENT_DOMAIN_ID_SHIFT); > - flags = get_field_from_reg_u32(entry[1], > - IOMMU_EVENT_FLAGS_MASK, > - IOMMU_EVENT_FLAGS_SHIFT); > - addr= (u64*) (entry + 2); > - printk(XENLOG_ERR "AMD-Vi: " > - "%s: domain = %d, device id = %#x, " > - "fault address = %#"PRIx64", flags = %#x\n", > - code_str, domain_id, device_id, *addr, flags); > + unsigned int bdf; > + uint16_t device_id = MASK_EXTR(entry[0], IOMMU_CMD_DEVICE_ID_MASK); > + uint16_t domain_id = MASK_EXTR(entry[1], IOMMU_EVENT_DOMAIN_ID_MASK); > + uint16_t flags = MASK_EXTR(entry[1], IOMMU_EVENT_FLAGS_MASK); I wouldn't mind using using unsigned int for the variables above. > + uint64_t addr = *(uint64_t *)(entry + 2); > + > + printk(XENLOG_ERR "AMD-Vi: %s: %04x:%02x:%02x.%u d%d addr %016"PRIx64 > + " flags %#x%s%s%s%s%s%s%s%s%s%s\n", > + code_str, iommu->seg, PCI_BUS(device_id), PCI_SLOT(device_id), > + PCI_FUNC(device_id), domain_id, addr, flags, > + (flags & 0xe00) ? " ??" : "", > + (flags & 0x100) ? " TR" : "", > + (flags & 0x080) ? " RZ" : "", > + (flags & 0x040) ? " PE" : "", > + (flags & 0x020) ? " RW" : "", > + (flags & 0x010) ? " PR" : "", > + (flags & 0x008) ? " I" : "", > + (flags & 0x004) ? " US" : "", > + (flags & 0x002) ? " NX" : "", > + (flags & 0x001) ? " GN" : ""); I wold rather have those added with proper defined names to amd-iommu-defs.h. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH 2/2] AMD/IOMMU: Render IO_PAGE_FAULT errors in a more useful manner 2019-11-27 9:40 ` Roger Pau Monné @ 2019-11-27 17:38 ` Andrew Cooper 0 siblings, 0 replies; 10+ messages in thread From: Andrew Cooper @ 2019-11-27 17:38 UTC (permalink / raw) To: Roger Pau Monné; +Cc: Juergen Gross, Xen-devel, Jan Beulich On 27/11/2019 09:40, Roger Pau Monné wrote: > On Tue, Nov 26, 2019 at 03:01:12PM +0000, Andrew Cooper wrote: >> Print the PCI coordinates in its common format and use d%u notation for the >> domain. As well as printing flags, decode them. IO_PAGE_FAULT is used for >> interrupt remapping errors as well as DMA remapping errors. >> >> Before: >> (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0xbf695000, flags = 0x10 >> (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0xbf695040, flags = 0x10 >> (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0xfffffff0, flags = 0x30 >> (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0x100000000, flags = 0x30 >> (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = 0xa1, fault address = 0x100000040, flags = 0x30 >> >> After: >> (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 00000000bf5fc000 flags 0x10 PR >> (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 00000000bf5fc040 flags 0x10 PR >> (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 00000000fffffff0 flags 0x30 RW PR >> (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 0000000100000000 flags 0x30 RW PR >> (XEN) AMD-Vi: IO_PAGE_FAULT: 0000:00:14.1 d0 addr 0000000100000040 flags 0x30 RW PR > Nit: I would place the domain id information at the beginning (since > that's more similar to gprintk format), and maybe drop the AMD-Vi > prefix, it's not very useful IMO: > > (XEN) d0 IO_PAGE_FAULT 0000:00:14.1 addr 0000000100000040 flags 0x30 RW PR > > But I'm not specially concerned. So I debated not using d%d format. This is the DTE's "domain_id" (a.k.a. Tag in the IO-TLB) field which by convention we set to the domid of the owning device, but isn't necessarily the best option. In particular, it might be wise to use domid + 1 and choke if we ever find 0 in use. > >> + uint64_t addr = *(uint64_t *)(entry + 2); >> + >> + printk(XENLOG_ERR "AMD-Vi: %s: %04x:%02x:%02x.%u d%d addr %016"PRIx64 >> + " flags %#x%s%s%s%s%s%s%s%s%s%s\n", >> + code_str, iommu->seg, PCI_BUS(device_id), PCI_SLOT(device_id), >> + PCI_FUNC(device_id), domain_id, addr, flags, >> + (flags & 0xe00) ? " ??" : "", >> + (flags & 0x100) ? " TR" : "", >> + (flags & 0x080) ? " RZ" : "", >> + (flags & 0x040) ? " PE" : "", >> + (flags & 0x020) ? " RW" : "", >> + (flags & 0x010) ? " PR" : "", >> + (flags & 0x008) ? " I" : "", >> + (flags & 0x004) ? " US" : "", >> + (flags & 0x002) ? " NX" : "", >> + (flags & 0x001) ? " GN" : ""); > I wold rather have those added with proper defined names to > amd-iommu-defs.h. All of this is in desperate need of turning into real C structs, rather than being opencoded in terms of u32[] and offsets/shifts/masks, but such a change definitely isn't appropriate for backport. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH 2/2] AMD/IOMMU: Render IO_PAGE_FAULT errors in a more useful manner 2019-11-26 15:01 ` [Xen-devel] [PATCH 2/2] AMD/IOMMU: Render IO_PAGE_FAULT errors in a more useful manner Andrew Cooper 2019-11-27 9:40 ` Roger Pau Monné @ 2019-11-27 17:02 ` Jan Beulich 2019-11-27 17:03 ` Andrew Cooper 1 sibling, 1 reply; 10+ messages in thread From: Jan Beulich @ 2019-11-27 17:02 UTC (permalink / raw) To: Andrew Cooper; +Cc: Juergen Gross, Xen-devel On 26.11.2019 16:01, Andrew Cooper wrote: > @@ -560,18 +557,26 @@ static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[]) > > if ( code == IOMMU_EVENT_IO_PAGE_FAULT ) > { > - device_id = iommu_get_devid_from_event(entry[0]); > - domain_id = get_field_from_reg_u32(entry[1], > - IOMMU_EVENT_DOMAIN_ID_MASK, > - IOMMU_EVENT_DOMAIN_ID_SHIFT); > - flags = get_field_from_reg_u32(entry[1], > - IOMMU_EVENT_FLAGS_MASK, > - IOMMU_EVENT_FLAGS_SHIFT); > - addr= (u64*) (entry + 2); > - printk(XENLOG_ERR "AMD-Vi: " > - "%s: domain = %d, device id = %#x, " > - "fault address = %#"PRIx64", flags = %#x\n", > - code_str, domain_id, device_id, *addr, flags); > + unsigned int bdf; > + uint16_t device_id = MASK_EXTR(entry[0], IOMMU_CMD_DEVICE_ID_MASK); s/CMD/EVENT/ and then Acked-by: Jan Beulich <jbeulich@suse.com> Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH 2/2] AMD/IOMMU: Render IO_PAGE_FAULT errors in a more useful manner 2019-11-27 17:02 ` Jan Beulich @ 2019-11-27 17:03 ` Andrew Cooper 0 siblings, 0 replies; 10+ messages in thread From: Andrew Cooper @ 2019-11-27 17:03 UTC (permalink / raw) To: Jan Beulich; +Cc: Juergen Gross, Xen-devel On 27/11/2019 17:02, Jan Beulich wrote: > On 26.11.2019 16:01, Andrew Cooper wrote: >> @@ -560,18 +557,26 @@ static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[]) >> >> if ( code == IOMMU_EVENT_IO_PAGE_FAULT ) >> { >> - device_id = iommu_get_devid_from_event(entry[0]); >> - domain_id = get_field_from_reg_u32(entry[1], >> - IOMMU_EVENT_DOMAIN_ID_MASK, >> - IOMMU_EVENT_DOMAIN_ID_SHIFT); >> - flags = get_field_from_reg_u32(entry[1], >> - IOMMU_EVENT_FLAGS_MASK, >> - IOMMU_EVENT_FLAGS_SHIFT); >> - addr= (u64*) (entry + 2); >> - printk(XENLOG_ERR "AMD-Vi: " >> - "%s: domain = %d, device id = %#x, " >> - "fault address = %#"PRIx64", flags = %#x\n", >> - code_str, domain_id, device_id, *addr, flags); >> + unsigned int bdf; >> + uint16_t device_id = MASK_EXTR(entry[0], IOMMU_CMD_DEVICE_ID_MASK); > s/CMD/EVENT/ and then > Acked-by: Jan Beulich <jbeulich@suse.com> Oops yes. That was a consequence of following #define iommu_get_devid_from_event iommu_get_devid_from_cmd to get the mask to use. These really need turning into structs, but that is a job for a different day. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH for-4.13 0/2] Fixes to AMD IOMMU logging 2019-11-26 15:01 [Xen-devel] [PATCH for-4.13 0/2] Fixes to AMD IOMMU logging Andrew Cooper 2019-11-26 15:01 ` [Xen-devel] [PATCH 1/2] AMD/IOMMU: Always print IOMMU errors Andrew Cooper 2019-11-26 15:01 ` [Xen-devel] [PATCH 2/2] AMD/IOMMU: Render IO_PAGE_FAULT errors in a more useful manner Andrew Cooper @ 2019-11-26 15:36 ` Jürgen Groß 2 siblings, 0 replies; 10+ messages in thread From: Jürgen Groß @ 2019-11-26 15:36 UTC (permalink / raw) To: Andrew Cooper, Xen-devel; +Cc: Jan Beulich On 26.11.19 16:01, Andrew Cooper wrote: > RFC for-4.13. These are diagnostic improvements/corrections, so are low risk > and high utility. Sorry, but the release is at least 1 month late now. As said before: I'll only take real bug fixes now. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-11-27 17:38 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-11-26 15:01 [Xen-devel] [PATCH for-4.13 0/2] Fixes to AMD IOMMU logging Andrew Cooper 2019-11-26 15:01 ` [Xen-devel] [PATCH 1/2] AMD/IOMMU: Always print IOMMU errors Andrew Cooper 2019-11-27 9:19 ` Roger Pau Monné 2019-11-27 16:58 ` Jan Beulich 2019-11-26 15:01 ` [Xen-devel] [PATCH 2/2] AMD/IOMMU: Render IO_PAGE_FAULT errors in a more useful manner Andrew Cooper 2019-11-27 9:40 ` Roger Pau Monné 2019-11-27 17:38 ` Andrew Cooper 2019-11-27 17:02 ` Jan Beulich 2019-11-27 17:03 ` Andrew Cooper 2019-11-26 15:36 ` [Xen-devel] [PATCH for-4.13 0/2] Fixes to AMD IOMMU logging Jürgen Groß
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.