WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] Re: [PATCH 2 of 5] amd iommu: Fix iommu page size encodi

To: Jan Beulich <JBeulich@xxxxxxxx>
Subject: Re: [Xen-devel] Re: [PATCH 2 of 5] amd iommu: Fix iommu page size encoding when page order > 0
From: Wei Wang2 <wei.wang2@xxxxxxx>
Date: Thu, 3 Nov 2011 14:29:30 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Thu, 03 Nov 2011 06:27:10 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4EB29C92020000780005EB6A@xxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <patchbomb.1320315180@xxxxxxxxxxxx> <4EB29BF3020000780005EB5B@xxxxxxxxxxxxxxxxxxxx> <4EB29C92020000780005EB6A@xxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: KMail/1.9.6 (enterprise 20070904.708012)
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