From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Wang Subject: Re: [PATCH] amd iommu: Add workaround for erratum 732 & 733 Date: Wed, 23 May 2012 11:19:19 +0200 Message-ID: <4FBCAB97.2000402@amd.com> References: <4FBB9B06.1030200@amd.com> <4FBBBA870200007800085296@nat28.tlf.novell.com> <4FBBB11E.5070709@amd.com> <4FBCA65C02000078000855A3@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4FBCA65C02000078000855A3@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Keir Fraser , xen-devel List-Id: xen-devel@lists.xenproject.org On 05/23/2012 08:57 AM, Jan Beulich wrote: >>>> On 22.05.12 at 17:30, Wei Wang wrote: >> Thanks for review it. New version has been attached. It should have >> fixed issues you mentioned. We don't have a particular number for loop >> count, so I cut it to 1000, it should be enough. Please have a look. > > I adjusted it further to address a few things I noticed while > pulling it into my local tree. Please let me know if any of the > adjustments I did are in error. > > I'll also send a half-way related (and dependent in terms of > being able to cleanly apply) follow-on patch in a minute. Both look great to me. Acked. Thanks, Wei > Jan > > ************************************************** > > amd iommu: Add workaround for erratum 732& 733 > > Signed-off-by: Wei Wang > > Add missing barriers. Fix early return from parse_ppr_log_entry(). > Slightly adjust comments. Strip trailing blanks. > > Signed-off-by: Jan Beulich > > --- a/xen/drivers/passthrough/amd/iommu_init.c > +++ b/xen/drivers/passthrough/amd/iommu_init.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > > static int __initdata nr_amd_iommus; > > @@ -566,6 +567,7 @@ static void parse_event_log_entry(struct > u16 domain_id, device_id, bdf, cword; > u32 code; > u64 *addr; > + int count = 0; > char * event_str[] = {"ILLEGAL_DEV_TABLE_ENTRY", > "IO_PAGE_FAULT", > "DEV_TABLE_HW_ERROR", > @@ -578,6 +580,25 @@ static void parse_event_log_entry(struct > code = get_field_from_reg_u32(entry[1], IOMMU_EVENT_CODE_MASK, > IOMMU_EVENT_CODE_SHIFT); > > + /* > + * Workaround for erratum 732: > + * It can happen that the tail pointer is updated before the actual entry > + * got written. As suggested by RevGuide, we initialize the event log > + * buffer to all zeros and clear event log entries after processing them. > + */ > + while ( code == 0 ) > + { > + if ( unlikely(++count == IOMMU_LOG_ENTRY_TIMEOUT) ) > + { > + AMD_IOMMU_DEBUG("AMD-Vi: No event written to log\n"); > + return; > + } > + udelay(1); > + rmb(); > + code = get_field_from_reg_u32(entry[1], IOMMU_EVENT_CODE_MASK, > + IOMMU_EVENT_CODE_SHIFT); > + } > + > if ( (code> IOMMU_EVENT_INVALID_DEV_REQUEST) || > (code< IOMMU_EVENT_ILLEGAL_DEV_TABLE_ENTRY) ) > { > @@ -615,6 +636,8 @@ static void parse_event_log_entry(struct > AMD_IOMMU_DEBUG("event 0x%08x 0x%08x 0x%08x 0x%08x\n", entry[0], > entry[1], entry[2], entry[3]); > } > + > + memset(entry, 0, IOMMU_EVENT_LOG_ENTRY_SIZE); > } > > static void iommu_check_event_log(struct amd_iommu *iommu) > @@ -646,9 +669,31 @@ void parse_ppr_log_entry(struct amd_iomm > { > > u16 device_id; > - u8 bus, devfn; > + u8 bus, devfn, code; > struct pci_dev *pdev; > - struct domain *d; > + int count = 0; > + > + code = get_field_from_reg_u32(entry[1], IOMMU_PPR_LOG_CODE_MASK, > + IOMMU_PPR_LOG_CODE_SHIFT); > + > + /* > + * Workaround for erratum 733: > + * It can happen that the tail pointer is updated before the actual entry > + * got written. As suggested by RevGuide, we initialize the event log > + * buffer to all zeros and clear ppr log entries after processing them. > + */ > + while ( code == 0 ) > + { > + if ( unlikely(++count == IOMMU_LOG_ENTRY_TIMEOUT) ) > + { > + AMD_IOMMU_DEBUG("AMD-Vi: No ppr written to log\n"); > + return; > + } > + udelay(1); > + rmb(); > + code = get_field_from_reg_u32(entry[1], IOMMU_PPR_LOG_CODE_MASK, > + IOMMU_PPR_LOG_CODE_SHIFT); > + } > > /* here device_id is physical value */ > device_id = iommu_get_devid_from_cmd(entry[0]); > @@ -659,12 +704,10 @@ void parse_ppr_log_entry(struct amd_iomm > pdev = pci_get_pdev(iommu->seg, bus, devfn); > spin_unlock(&pcidevs_lock); > > - if ( pdev == NULL ) > - return; > - > - d = pdev->domain; > + if ( pdev ) > + guest_iommu_add_ppr_log(pdev->domain, entry); > > - guest_iommu_add_ppr_log(d, entry); > + memset(entry, 0, IOMMU_PPR_LOG_ENTRY_SIZE); > } > > static void iommu_check_ppr_log(struct amd_iommu *iommu) > --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h > +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h > @@ -301,6 +301,10 @@ > #define IOMMU_PPR_LOG_TAIL_OFFSET 0x2038 > #define IOMMU_PPR_LOG_DEVICE_ID_MASK 0x0000FFFF > #define IOMMU_PPR_LOG_DEVICE_ID_SHIFT 0 > +#define IOMMU_PPR_LOG_CODE_MASK 0xF0000000 > +#define IOMMU_PPR_LOG_CODE_SHIFT 28 > + > +#define IOMMU_LOG_ENTRY_TIMEOUT 1000 > > /* Control Register */ > #define IOMMU_CONTROL_MMIO_OFFSET 0x18 > > >