On 11/27/15 at 12:21pm, Joerg Roedel wrote:
> On Fri, Nov 06, 2015 at 08:10:47PM +0800, Baoquan He wrote:
> > +static void copy_dev_tables(void)
> > +{
> > +        u64 entry;
> > +        u32 lo, hi;
> > +        phys_addr_t old_devtb_phys;
> > +        struct dev_table_entry *old_devtb;
> > +        struct amd_iommu *iommu;
> > +        u16 dom_id;
> > +        u32 devid;
> > +
> > +        for_each_iommu(iommu) {
> 
> You don't need to do this for all IOMMUs in the system. Linux uses the
> same device table for all IOMMUs, so it is better to just check whether
> all IOMMUs in the system are enabled and also point to the same device
> table. If any of this is not the case we should just bail out of any
> tries to copy over data from the old kernel.

Make sense. Will do.

> 
> > +           /*
> > +            * This is used to prevent the WARNING in 
> > iommu_queue_command_sync()
> > +            * when call flush function
> > +            */
> > +           iommu->cmd_buf_size &= ~(CMD_BUFFER_UNINITIALIZED);
> 
> This is a hack. You need to add code to re-initialize the command buffer
> and the error and ppr logs in the new kernel.

Yeah, it's just a hack. I removed it directly since you have a commit to
remove cmd_buf_size in struct amd_iommu.

commit deba4bce168a87ef90211ba69850d3428b453765
Author: Joerg Roedel <jroe...@suse.de>
Date:   Tue Oct 20 17:33:41 2015 +0200

    iommu/amd: Remove cmd_buf_size and evt_buf_size from struct amd_iommu

> 
> > +
> > +                lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
> > +                hi = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET + 4);
> > +                entry = (((u64) hi) << 32) + lo;
> > +                old_devtb_phys = entry & PAGE_MASK;
> > +                old_devtb = ioremap_cache(old_devtb_phys, dev_table_size);
> 
> Please use memremap.

Will do.
> 
> > +                for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
> > +                        amd_iommu_dev_table[devid] = old_devtb[devid];
> > +                        dom_id = amd_iommu_dev_table[devid].data[1] & 
> > DEV_DOMID_MASK;
> > +                   if (!dom_id)
> > +                           continue;
> 
> Check the V and IV bits of the here instead of the domid.

Will do.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to