[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] Re: [PATCH 2 of 5] amd iommu: Fix iommu page size encoding when page order > 0
On Thursday 03 November 2011 13:52:18 Jan Beulich wrote: > >>> On 03.11.11 at 13:49, "Jan Beulich" <JBeulich@xxxxxxxx> wrote: > >>>> On 03.11.11 at 11:13, Wei Wang <wei.wang2@xxxxxxx> wrote: > >> > >> # HG changeset patch > >> # User Wei Wang <wei.wang2@xxxxxxx> > >> # Date 1320314617 -3600 > >> # Node ID d0c38cb215cd96e01de27eadf5ec0a5e711de448 > >> # Parent d422e3cf7976c76c57fc2d455b784d0fcc24d06b > >> amd iommu: Fix iommu page size encoding when page order > 0 > >> > >> Signed-off-by: Wei Wang <wei.wang2@xxxxxxx> > >> > >> diff -r d422e3cf7976 -r d0c38cb215cd > >> xen/drivers/passthrough/amd/iommu_map.c --- > >> a/xen/drivers/passthrough/amd/iommu_map.c Thu Nov 03 11:02:17 2011 +0100 > >> +++ b/xen/drivers/passthrough/amd/iommu_map.c Thu Nov 03 11:03:37 2011 > >> +0100 @@ -77,23 +77,24 @@ static void invalidate_iommu_pages(struc > >> { > >> u64 addr_lo, addr_hi; > >> u32 cmd[4], entry; > >> - u64 mask = 0; > >> int sflag = 0, pde = 0; > >> > >> + ASSERT ( order == 0 || order == 9 || order == 18 ); > >> + > >> + /* All pages associated with the domainID are invalidated */ > >> + if ( order || (io_addr == INV_IOMMU_ALL_PAGES_ADDRESS ) ) > >> + { > >> + sflag = 1; > >> + pde = 1; > >> + } > >> + > >> /* If sflag == 1, the size of the invalidate command is determined > >> by the first zero bit in the address starting from Address[12] */ > >> - if ( order == 9 || order == 18 ) > >> + if ( order ) > >> { > >> - mask = ((1ULL << (order - 1)) - 1) << PAGE_SHIFT; > >> - io_addr |= mask; > >> - sflag = 1; > >> - } > >> - > >> - /* All pages associated with the domainID are invalidated */ > >> - else if ( io_addr == 0x7FFFFFFFFFFFF000ULL ) > > > > Note the difference between this and your adjusted definition > > of INV_IOMMU_ALL_PAGES_ADDRESS. I don't think this is > > correct (or else your patch description should say why). > > > >> - { > >> - sflag = 1; > >> - pde = 1; > >> + u64 mask = 1ULL << (order - 1 + PAGE_SHIFT); > >> + io_addr &= ~mask; > >> + io_addr |= mask - 1; > >> } > >> > >> addr_lo = io_addr & DMA_32BIT_MASK; > >> @@ -917,7 +918,7 @@ static void _amd_iommu_flush_pages(struc > >> > >> void amd_iommu_flush_all_pages(struct domain *d) > >> { > >> - _amd_iommu_flush_pages(d, 0x7FFFFFFFFFFFFULL, 0); > >> + _amd_iommu_flush_pages(d, INV_IOMMU_ALL_PAGES_ADDRESS, 0); > > > > Same here, for a slightly different reason. > > > > Jan > > > >> } > >> > >> void amd_iommu_flush_pages(struct domain *d, > >> diff -r d422e3cf7976 -r d0c38cb215cd > >> xen/include/asm-x86/hvm/svm/amd-iommu-defs.h --- > >> a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h Thu Nov 03 11:02:17 2011 > >> +0100 +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h Thu Nov 03 > >> 11:03:37 2011 +0100 > >> @@ -407,4 +407,6 @@ > >> #define INT_REMAP_ENTRY_VECTOR_MASK 0x00FF0000 > >> #define INT_REMAP_ENTRY_VECTOR_SHIFT 16 > >> > >> +#define INV_IOMMU_ALL_PAGES_ADDRESS 0x7FFFFFFFFFFFFFFFULL > >> + > > Oh, additionally, this still isn't being expressed by a shift expression, > which makes it still badly readable (one has to count F-s in order to > know what this actually represents). Well, this is a copy from the spec.. How about using (1<<63) -1? Thanks, Wei > Jan > > >> #endif /* _ASM_X86_64_AMD_IOMMU_DEFS_H */ > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@xxxxxxxxxxxxxxxxxxx > > http://lists.xensource.com/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |