error messages when doing VT-d device passthrough

error messages when doing VT-d device passthrough

[Kay, Allen M]

[-- Attachment #1.1: Type: text/plain, Size: 418 bytes --]

I'm getting the following error messages when doing Vt-d device passthrough:

...
 (XEN) p2m.c:2626:d0 clear_mmio_p2m_entry: gfn_to_mfn failed! gfn=000f303b
...

This message is printed out for each MMIO page used by the device.  This seems to be cause by the code change from "if ( INVALID_MFN == mfn)" to "if (!mfn_valid(mfn) )" in p2m.c/clear_mmio_p2m_entry().

What is the reason for the change?

Allen

[-- Attachment #1.2: Type: text/html, Size: 5230 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: error messages when doing VT-d device passthrough

[Kay, Allen M]

[-- Attachment #1.1: Type: text/plain, Size: 845 bytes --]

Cc'ing Christoph Egger since this change was part of the "nested Virtualization: p2m infrastructure" in cs# 21940.

From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Kay, Allen M
Sent: Monday, August 30, 2010 4:57 PM
To: xen-devel@lists.xensource.com
Cc: tim.deegan@eu.citrix.com; Keir Fraser
Subject: [Xen-devel] error messages when doing VT-d device passthrough

I'm getting the following error messages when doing Vt-d device passthrough:

...
 (XEN) p2m.c:2626:d0 clear_mmio_p2m_entry: gfn_to_mfn failed! gfn=000f303b
...

This message is printed out for each MMIO page used by the device.  This seems to be cause by the code change from "if ( INVALID_MFN == mfn)" to "if (!mfn_valid(mfn) )" in p2m.c/clear_mmio_p2m_entry().

What is the reason for the change?

Allen

[-- Attachment #1.2: Type: text/html, Size: 6412 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH] re-enable HVM PCI passthrough functionality

[Kay, Allen M]

[-- Attachment #1: Type: text/plain, Size: 151 bytes --]

Attached patch reverts back to INVALID_MFN.  PCI passthrough functionality works with this patch.

Signed-off-by: Allen Kay allen.m.kay@intel.com


[-- Attachment #2: mm.patch --]
[-- Type: application/octet-stream, Size: 419 bytes --]

diff -r 32c9a19ac98b xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c	Mon Aug 30 14:59:12 2010 +0100
+++ b/xen/arch/x86/mm/p2m.c	Mon Aug 30 11:56:12 2010 -0700
@@ -2620,7 +2620,7 @@
         return 0;
 
     mfn = gfn_to_mfn(p2m, gfn, &t);
-    if ( !mfn_valid(mfn) )
+    if ( INVALID_MFN == mfn_x(mfn) )
     {
         gdprintk(XENLOG_ERR,
             "clear_mmio_p2m_entry: gfn_to_mfn failed! gfn=%08lx\n", gfn);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] re-enable HVM PCI passthrough functionality

[Tim Deegan]

At 02:58 +0100 on 31 Aug (1283223494), Kay, Allen M wrote:
> Attached patch reverts back to INVALID_MFN.  PCI passthrough functionality works with this patch.
> 

Apologies for missing this - I did ask for this enormous patch to
contain only what it said on the tin (and I did read it to check!) but
this one got through. 

> Signed-off-by: Allen Kay allen.m.kay@intel.com
> 

This is fine, but please add a comment explaining the rather subtle use
of INVALID_MFN here so the next tidy-minded person doesn't "fix" it
again. :)

In fact would it be better to check the returned type == p2m_mmio_direct
instead?  That seems like it would fit better with the intention.   Or
do other callers use this to clean non-mmio entries too?

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH] re-enable HVM PCI passthrough functionality

[Christoph Egger]

On Tuesday 31 August 2010 11:17:27 Tim Deegan wrote:
> At 02:58 +0100 on 31 Aug (1283223494), Kay, Allen M wrote:
> > Attached patch reverts back to INVALID_MFN.  PCI passthrough
> > functionality works with this patch.
>
> Apologies for missing this - I did ask for this enormous patch to
> contain only what it said on the tin (and I did read it to check!) but
> this one got through.

Also apologies from my side. I did what Tim requested for, but that
one slipped through.

> > Signed-off-by: Allen Kay allen.m.kay@intel.com
>
> This is fine, but please add a comment explaining the rather subtle use
> of INVALID_MFN here so the next tidy-minded person doesn't "fix" it
> again. :)
>
> In fact would it be better to check the returned type == p2m_mmio_direct
> instead?  That seems like it would fit better with the intention.   Or
> do other callers use this to clean non-mmio entries too?

I'm interested in if this patch fixes the SR-IOV issue. We are still about
investigation...

Christoph



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

RE: [PATCH] re-enable HVM PCI passthrough functionality

[Kay, Allen M]

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

I have updated the patch to indicate valid_mfn() cannot be used here as MMIO pages are always above max_mem.  Also added sanity checking for type == p2m_mmio_direct before clearing page table entry.  This should fix both VT-d and SR-IOV issues reported recently as there are no SR-IOV specific code in Xen.  As far as Xen is concern, SR-IOV virtual function is just another PCI device.

Signed-off-by: Allen Kay allen.m.kay@intel.com

-----Original Message-----
From: Christoph Egger [mailto:Christoph.Egger@amd.com] 
Sent: Tuesday, August 31, 2010 2:35 AM
To: Tim Deegan
Cc: Kay, Allen M; xen-devel@lists.xensource.com; Keir Fraser
Subject: Re: [PATCH] re-enable HVM PCI passthrough functionality

On Tuesday 31 August 2010 11:17:27 Tim Deegan wrote:
> At 02:58 +0100 on 31 Aug (1283223494), Kay, Allen M wrote:
> > Attached patch reverts back to INVALID_MFN.  PCI passthrough
> > functionality works with this patch.
>
> Apologies for missing this - I did ask for this enormous patch to
> contain only what it said on the tin (and I did read it to check!) but
> this one got through.

Also apologies from my side. I did what Tim requested for, but that
one slipped through.

> > Signed-off-by: Allen Kay allen.m.kay@intel.com
>
> This is fine, but please add a comment explaining the rather subtle use
> of INVALID_MFN here so the next tidy-minded person doesn't "fix" it
> again. :)
>
> In fact would it be better to check the returned type == p2m_mmio_direct
> instead?  That seems like it would fit better with the intention.   Or
> do other callers use this to clean non-mmio entries too?

I'm interested in if this patch fixes the SR-IOV issue. We are still about
investigation...

Christoph



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


[-- Attachment #2: mm.patch --]
[-- Type: application/octet-stream, Size: 528 bytes --]

diff -r 32c9a19ac98b xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c	Mon Aug 30 14:59:12 2010 +0100
+++ b/xen/arch/x86/mm/p2m.c	Tue Aug 31 02:08:24 2010 -0700
@@ -2620,7 +2620,9 @@
         return 0;
 
     mfn = gfn_to_mfn(p2m, gfn, &t);
-    if ( !mfn_valid(mfn) )
+
+    /* Do not use mfn_valid() here as MMIO pages are always above max_page */
+    if ( (INVALID_MFN == mfn_x(mfn)) || (t != p2m_mmio_direct) )
     {
         gdprintk(XENLOG_ERR,
             "clear_mmio_p2m_entry: gfn_to_mfn failed! gfn=%08lx\n", gfn);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] re-enable HVM PCI passthrough functionality

[Tim Deegan]

At 17:13 +0100 on 31 Aug (1283274786), Kay, Allen M wrote:
> I have updated the patch to indicate valid_mfn() cannot be used here
> as MMIO pages are always above max_mem.  Also added sanity checking
> for type == p2m_mmio_direct before clearing page table entry.  This
> should fix both VT-d and SR-IOV issues reported recently as there are
> no SR-IOV specific code in Xen.  As far as Xen is concern, SR-IOV
> virtual function is just another PCI device.

Applied; thanks for the fix.

Tim.

> Signed-off-by: Allen Kay allen.m.kay@intel.com
> 
> -----Original Message-----
> From: Christoph Egger [mailto:Christoph.Egger@amd.com] 
> Sent: Tuesday, August 31, 2010 2:35 AM
> To: Tim Deegan
> Cc: Kay, Allen M; xen-devel@lists.xensource.com; Keir Fraser
> Subject: Re: [PATCH] re-enable HVM PCI passthrough functionality
> 
> On Tuesday 31 August 2010 11:17:27 Tim Deegan wrote:
> > At 02:58 +0100 on 31 Aug (1283223494), Kay, Allen M wrote:
> > > Attached patch reverts back to INVALID_MFN.  PCI passthrough
> > > functionality works with this patch.
> >
> > Apologies for missing this - I did ask for this enormous patch to
> > contain only what it said on the tin (and I did read it to check!) but
> > this one got through.
> 
> Also apologies from my side. I did what Tim requested for, but that
> one slipped through.
> 
> > > Signed-off-by: Allen Kay allen.m.kay@intel.com
> >
> > This is fine, but please add a comment explaining the rather subtle use
> > of INVALID_MFN here so the next tidy-minded person doesn't "fix" it
> > again. :)
> >
> > In fact would it be better to check the returned type == p2m_mmio_direct
> > instead?  That seems like it would fit better with the intention.   Or
> > do other callers use this to clean non-mmio entries too?
> 
> I'm interested in if this patch fixes the SR-IOV issue. We are still about
> investigation...
> 
> Christoph
> 
> 
> 
> -- 
> ---to satisfy European Law for business letters:
> Advanced Micro Devices GmbH
> Einsteinring 24, 85609 Dornach b. Muenchen
> Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
> Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
> Registergericht Muenchen, HRB Nr. 43632
> 



-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

RE: Re: [PATCH] re-enable HVM PCI passthrough functionality

[Zhang, Yang Z]

With allen's patch, both vt-d and sr-iov can work now. Thanks allen.

best regards
yang


> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Tim Deegan
> Sent: Wednesday, September 01, 2010 4:57 PM
> To: Kay, Allen M
> Cc: Christoph Egger; xen-devel@lists.xensource.com; Keir Fraser
> Subject: [Xen-devel] Re: [PATCH] re-enable HVM PCI passthrough functionality
> 
> At 17:13 +0100 on 31 Aug (1283274786), Kay, Allen M wrote:
> > I have updated the patch to indicate valid_mfn() cannot be used here
> > as MMIO pages are always above max_mem.  Also added sanity checking
> > for type == p2m_mmio_direct before clearing page table entry.  This
> > should fix both VT-d and SR-IOV issues reported recently as there are
> > no SR-IOV specific code in Xen.  As far as Xen is concern, SR-IOV
> > virtual function is just another PCI device.
> 
> Applied; thanks for the fix.
> 
> Tim.
> 
> > Signed-off-by: Allen Kay allen.m.kay@intel.com
> >
> > -----Original Message-----
> > From: Christoph Egger [mailto:Christoph.Egger@amd.com]
> > Sent: Tuesday, August 31, 2010 2:35 AM
> > To: Tim Deegan
> > Cc: Kay, Allen M; xen-devel@lists.xensource.com; Keir Fraser
> > Subject: Re: [PATCH] re-enable HVM PCI passthrough functionality
> >
> > On Tuesday 31 August 2010 11:17:27 Tim Deegan wrote:
> > > At 02:58 +0100 on 31 Aug (1283223494), Kay, Allen M wrote:
> > > > Attached patch reverts back to INVALID_MFN.  PCI passthrough
> > > > functionality works with this patch.
> > >
> > > Apologies for missing this - I did ask for this enormous patch to
> > > contain only what it said on the tin (and I did read it to check!) but
> > > this one got through.
> >
> > Also apologies from my side. I did what Tim requested for, but that
> > one slipped through.
> >
> > > > Signed-off-by: Allen Kay allen.m.kay@intel.com
> > >
> > > This is fine, but please add a comment explaining the rather subtle use
> > > of INVALID_MFN here so the next tidy-minded person doesn't "fix" it
> > > again. :)
> > >
> > > In fact would it be better to check the returned type == p2m_mmio_direct
> > > instead?  That seems like it would fit better with the intention.   Or
> > > do other callers use this to clean non-mmio entries too?
> >
> > I'm interested in if this patch fixes the SR-IOV issue. We are still about
> > investigation...
> >
> > Christoph
> >
> >
> >
> > --
> > ---to satisfy European Law for business letters:
> > Advanced Micro Devices GmbH
> > Einsteinring 24, 85609 Dornach b. Muenchen
> > Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
> > Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
> > Registergericht Muenchen, HRB Nr. 43632
> >
> 
> 
> 
> --
> Tim Deegan <Tim.Deegan@citrix.com>
> Principal Software Engineer, XenServer Engineering
> Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: Re: [PATCH] re-enable HVM PCI passthrough functionality

[Christoph Egger]

On Thursday 02 September 2010 04:26:32 Zhang, Yang Z wrote:
> With allen's patch, both vt-d and sr-iov can work now. Thanks allen.

We were not able to reproduce SR-IOV failures with and without the fix
on AMD machines. I am wondering if there are differences in memory
mappings between AMD and Intel PCI passthrough.

Nonetheless, good to know this has been fixed.

Thanks,
Christoph

> best regards
> yang
>
> > -----Original Message-----
> > From: xen-devel-bounces@lists.xensource.com
> > [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Tim Deegan
> > Sent: Wednesday, September 01, 2010 4:57 PM
> > To: Kay, Allen M
> > Cc: Christoph Egger; xen-devel@lists.xensource.com; Keir Fraser
> > Subject: [Xen-devel] Re: [PATCH] re-enable HVM PCI passthrough
> > functionality
> >
> > At 17:13 +0100 on 31 Aug (1283274786), Kay, Allen M wrote:
> > > I have updated the patch to indicate valid_mfn() cannot be used here
> > > as MMIO pages are always above max_mem.  Also added sanity checking
> > > for type == p2m_mmio_direct before clearing page table entry.  This
> > > should fix both VT-d and SR-IOV issues reported recently as there are
> > > no SR-IOV specific code in Xen.  As far as Xen is concern, SR-IOV
> > > virtual function is just another PCI device.
> >
> > Applied; thanks for the fix.
> >
> > Tim.
> >
> > > Signed-off-by: Allen Kay allen.m.kay@intel.com
> > >
> > > -----Original Message-----
> > > From: Christoph Egger [mailto:Christoph.Egger@amd.com]
> > > Sent: Tuesday, August 31, 2010 2:35 AM
> > > To: Tim Deegan
> > > Cc: Kay, Allen M; xen-devel@lists.xensource.com; Keir Fraser
> > > Subject: Re: [PATCH] re-enable HVM PCI passthrough functionality
> > >
> > > On Tuesday 31 August 2010 11:17:27 Tim Deegan wrote:
> > > > At 02:58 +0100 on 31 Aug (1283223494), Kay, Allen M wrote:
> > > > > Attached patch reverts back to INVALID_MFN.  PCI passthrough
> > > > > functionality works with this patch.
> > > >
> > > > Apologies for missing this - I did ask for this enormous patch to
> > > > contain only what it said on the tin (and I did read it to check!)
> > > > but this one got through.
> > >
> > > Also apologies from my side. I did what Tim requested for, but that
> > > one slipped through.
> > >
> > > > > Signed-off-by: Allen Kay allen.m.kay@intel.com
> > > >
> > > > This is fine, but please add a comment explaining the rather subtle
> > > > use of INVALID_MFN here so the next tidy-minded person doesn't "fix"
> > > > it again. :)
> > > >
> > > > In fact would it be better to check the returned type ==
> > > > p2m_mmio_direct instead?  That seems like it would fit better with
> > > > the intention.   Or do other callers use this to clean non-mmio
> > > > entries too?
> > >
> > > I'm interested in if this patch fixes the SR-IOV issue. We are still
> > > about investigation...
> > >
> > > Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: Re: [PATCH] re-enable HVM PCI passthrough functionality

[Tim Deegan]

At 09:34 +0100 on 02 Sep (1283420058), Christoph Egger wrote:
> On Thursday 02 September 2010 04:26:32 Zhang, Yang Z wrote:
> > With allen's patch, both vt-d and sr-iov can work now. Thanks allen.
> 
> We were not able to reproduce SR-IOV failures with and without the fix
> on AMD machines. I am wondering if there are differences in memory
> mappings between AMD and Intel PCI passthrough.

I expect that you tested on machines with >4GiB of RAM, passing through
32-bit MMIO addresses (which would then pass mfn_valid()) and Intel
either had less RAM or higher BARs in their test (so their MMIO
addresses were !mfn_valid() but != INVALID_MFN).

Cheers,

Tim.

> Nonetheless, good to know this has been fixed.
> 
> Thanks,
> Christoph
> 
> > best regards
> > yang
> >
> > > -----Original Message-----
> > > From: xen-devel-bounces@lists.xensource.com
> > > [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Tim Deegan
> > > Sent: Wednesday, September 01, 2010 4:57 PM
> > > To: Kay, Allen M
> > > Cc: Christoph Egger; xen-devel@lists.xensource.com; Keir Fraser
> > > Subject: [Xen-devel] Re: [PATCH] re-enable HVM PCI passthrough
> > > functionality
> > >
> > > At 17:13 +0100 on 31 Aug (1283274786), Kay, Allen M wrote:
> > > > I have updated the patch to indicate valid_mfn() cannot be used here
> > > > as MMIO pages are always above max_mem.  Also added sanity checking
> > > > for type == p2m_mmio_direct before clearing page table entry.  This
> > > > should fix both VT-d and SR-IOV issues reported recently as there are
> > > > no SR-IOV specific code in Xen.  As far as Xen is concern, SR-IOV
> > > > virtual function is just another PCI device.
> > >
> > > Applied; thanks for the fix.
> > >
> > > Tim.
> > >
> > > > Signed-off-by: Allen Kay allen.m.kay@intel.com
> > > >
> > > > -----Original Message-----
> > > > From: Christoph Egger [mailto:Christoph.Egger@amd.com]
> > > > Sent: Tuesday, August 31, 2010 2:35 AM
> > > > To: Tim Deegan
> > > > Cc: Kay, Allen M; xen-devel@lists.xensource.com; Keir Fraser
> > > > Subject: Re: [PATCH] re-enable HVM PCI passthrough functionality
> > > >
> > > > On Tuesday 31 August 2010 11:17:27 Tim Deegan wrote:
> > > > > At 02:58 +0100 on 31 Aug (1283223494), Kay, Allen M wrote:
> > > > > > Attached patch reverts back to INVALID_MFN.  PCI passthrough
> > > > > > functionality works with this patch.
> > > > >
> > > > > Apologies for missing this - I did ask for this enormous patch to
> > > > > contain only what it said on the tin (and I did read it to check!)
> > > > > but this one got through.
> > > >
> > > > Also apologies from my side. I did what Tim requested for, but that
> > > > one slipped through.
> > > >
> > > > > > Signed-off-by: Allen Kay allen.m.kay@intel.com
> > > > >
> > > > > This is fine, but please add a comment explaining the rather subtle
> > > > > use of INVALID_MFN here so the next tidy-minded person doesn't "fix"
> > > > > it again. :)
> > > > >
> > > > > In fact would it be better to check the returned type ==
> > > > > p2m_mmio_direct instead?  That seems like it would fit better with
> > > > > the intention.   Or do other callers use this to clean non-mmio
> > > > > entries too?
> > > >
> > > > I'm interested in if this patch fixes the SR-IOV issue. We are still
> > > > about investigation...
> > > >
> > > > Christoph
> 
> 
> -- 
> ---to satisfy European Law for business letters:
> Advanced Micro Devices GmbH
> Einsteinring 24, 85609 Dornach b. Muenchen
> Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
> Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
> Registergericht Muenchen, HRB Nr. 43632
> 

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)