Re: [PATCH 10 of 16] amd iommu: Enable FC bit in iommu host level PTE

[Wei Wang2 <wei.wang2@amd.com>]

On Monday 02 January 2012 12:36:08 Jan Beulich wrote:
> >>> On 23.12.11 at 12:29, Wei Wang <wei.wang2@amd.com> wrote:
> >
> > # HG changeset patch
> > # User Wei Wang <wei.wang2@amd.com>
> > # Date 1324569401 -3600
> > # Node ID 30b1f434160d989be5e0bb6c6956bb7e3985db59
> > # Parent  dd808bdd61c581b041d5b7e816b18674de51da6f
> > amd iommu: Enable FC bit in iommu host level PTE
> >
> > Signed-off-by: Wei Wang <wei.wang2@amd.com>
> >
> > diff -r dd808bdd61c5 -r 30b1f434160d
> > xen/drivers/passthrough/amd/iommu_map.c ---
> > a/xen/drivers/passthrough/amd/iommu_map.c	Thu Dec 22 16:56:38 2011 +0100
> > +++ b/xen/drivers/passthrough/amd/iommu_map.c	Thu Dec 22 16:56:41 2011
> > +0100 @@ -83,6 +83,11 @@ static bool_t set_iommu_pde_present(u32
> >      set_field_in_reg_u32(ir, entry,
> >                           IOMMU_PDE_IO_READ_PERMISSION_MASK,
> >                           IOMMU_PDE_IO_READ_PERMISSION_SHIFT, &entry);
> > +
> > +    /* IOMMUv2 needs FC bit enabled  */
>
> This comment suggests that the patches prior to that aren't consistent.
> Is this really a proper standalone patch, or is the word "needs" too
> strict, or should it really be moved ahead in the series?
>
> > +    if ( next_level == IOMMU_PAGING_MODE_LEVEL_0 )
> > +        set_field_in_reg_u32(IOMMU_CONTROL_ENABLED, entry,
> > +                             IOMMU_PTE_FC_MASK, IOMMU_PTE_FC_SHIFT,
> > &entry);
>
> This is being done no matter whether it actually is a v2 IOMMU that
> you deal with here - if that's correct, the comment above should be
> adjusted accordingly.

This bit forces pci-defined no snoop bit to be cleared. This helps to solve 
potential issues in ATS devices with early drivers. I did not see any breaks 
on legacy devices and iommuv1 with FC = 1. But if you like I could make this 
only for v2 or change the comment a bit.
Thanks,
Wei
  

> Jan
>
> >      pde[1] = entry;
> >
> >      /* mark next level as 'present' */