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

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

On Tuesday 03 January 2012 11:12:35 Jan Beulich wrote:
> >>> On 03.01.12 at 11:05, Wei Wang2 <wei.wang2@amd.com> wrote:
> >
> > 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.
>
> Whatever is the most appropriate thing to do here (you definitely
> know better than I do)

I intend to keep fc =1 for both v1 and v2. This setup also aligns with Linux 
iommu driver. I will change the comment.
Thanks,
wei

> Jan