Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-31 Thread Liu, Yi L
> -Original Message-
> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Saturday, April 1, 2017 1:01 PM
> To: Liu, Yi L ; 'Peter Xu' 
> Cc: Lan, Tianyu ; Tian, Kevin ;
> 'm...@redhat.com' ; 'jan.kis...@siemens.com'
> ; 'bd.a...@gmail.com' ; 'qemu-
> de...@nongnu.org' ; 'alex.william...@redhat.com'
> ; 'David Gibson' 
> Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> MemoryRegionIOMMUOps.replay() callback
> 
> 
> 
> On 2017年03月31日 15:30, Liu, Yi L wrote:
> >> -Original Message-
> >> From: Jason Wang [mailto:jasow...@redhat.com]
> >> Sent: Friday, March 31, 2017 3:17 PM
> >> To: Liu, Yi L ; 'Peter Xu' 
> >> Cc: Lan, Tianyu ; Tian, Kevin
> >> ; 'm...@redhat.com' ;
> 'jan.kis...@siemens.com'
> >> ; 'bd.a...@gmail.com' ;
> >> 'qemu- de...@nongnu.org' ;
> 'alex.william...@redhat.com'
> >> ; 'David Gibson'
> >> 
> >> Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> >> MemoryRegionIOMMUOps.replay() callback
> >>
> >>
> >>
> >> On 2017年03月31日 13:34, Liu, Yi L wrote:
> >>>> -Original Message-
> >>>> From: Jason Wang [mailto:jasow...@redhat.com]
> >>>> Sent: Thursday, March 30, 2017 7:58 PM
> >>>> To: Liu, Yi L ; 'Peter Xu' 
> >>>> Cc: 'alex.william...@redhat.com' ; Lan,
> >>>> Tianyu ; Tian, Kevin ;
> >> 'm...@redhat.com'
> >>>> ; 'jan.kis...@siemens.com'
> >>>> ; 'bd.a...@gmail.com' ;
> 'David Gibson'
> >>>> ; 'qemu-devel@nongnu.org'  >>>> de...@nongnu.org>
> >>>> Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>>> MemoryRegionIOMMUOps.replay() callback
> >>>>
> >>>>
> >>>>
> >>>> On 2017年03月30日 19:06, Liu, Yi L wrote:
> >>>>>> -Original Message-
> >>>>>> From: Liu, Yi L
> >>>>>> Sent: Monday, March 27, 2017 5:22 PM
> >>>>>> To: Peter Xu 
> >>>>>> Cc: alex.william...@redhat.com; Lan, Tianyu
> >>>>>> ; Tian, Kevin ;
> >>>>>> m...@redhat.com; jan.kis...@siemens.com; jasow...@redhat.com;
> >>>>>> bd.a...@gmail.com; David Gibson ;
> >>>>>> qemu-devel@nongnu.org
> >>>>>> Subject: RE: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>>>>> MemoryRegionIOMMUOps.replay() callback
> >>>>>>
> >>>>>>> -Original Message-
> >>>>>>> From: Peter Xu [mailto:pet...@redhat.com]
> >>>>>>> Sent: Monday, March 27, 2017 5:12 PM
> >>>>>>> To: Liu, Yi L 
> >>>>>>> Cc: alex.william...@redhat.com; Lan, Tianyu
> >>>>>>> ; Tian, Kevin ;
> >>>>>>> m...@redhat.com; jan.kis...@siemens.com; jasow...@redhat.com;
> >>>>>>> bd.a...@gmail.com; David Gibson ;
> >>>>>>> qemu-devel@nongnu.org
> >>>>>>> Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>>>>>> MemoryRegionIOMMUOps.replay() callback
> >>>>>>>
> >>>>>>> On Mon, Mar 27, 2017 at 08:35:05AM +, Liu, Yi L wrote:
> >>>>>>>>> -Original Message-
> >>>>>>>>> From: Qemu-devel
> >>>>>>>>> [mailto:qemu-devel-bounces+yi.l.liu=intel@nongnu.org] On
> >>>>>>>>> Behalf Of Peter Xu
> >>>>>>>>> Sent: Tuesday, February 7, 2017 4:28 PM
> >>>>>>>>> To: qemu-devel@nongnu.org
> >>>>>>>>> Cc: Lan, Tianyu ; Tian, Kevin
> >>>>>>>>> ; m...@redhat.com;
> >>>>>>>>> jan.kis...@siemens.com; jasow...@redhat.com;
> >>>>>>>>> pet...@redhat.com; alex.william...@redhat.com;
> >>>>>>>>> bd.a...@gmail.com; David Gibson 
> >>>>>>>>> Subject: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>>>>>>>> MemoryRegionIOMMUOps.replay() callback
> >>>>>>>>>
> >>>>>>>>> Originally we have one memory_re

Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-31 Thread Jason Wang



On 2017年03月31日 15:30, Liu, Yi L wrote:

-Original Message-
From: Jason Wang [mailto:jasow...@redhat.com]
Sent: Friday, March 31, 2017 3:17 PM
To: Liu, Yi L ; 'Peter Xu' 
Cc: Lan, Tianyu ; Tian, Kevin ;
'm...@redhat.com' ; 'jan.kis...@siemens.com'
; 'bd.a...@gmail.com' ; 'qemu-
de...@nongnu.org' ; 'alex.william...@redhat.com'
; 'David Gibson' 
Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback



On 2017年03月31日 13:34, Liu, Yi L wrote:

-Original Message-
From: Jason Wang [mailto:jasow...@redhat.com]
Sent: Thursday, March 30, 2017 7:58 PM
To: Liu, Yi L ; 'Peter Xu' 
Cc: 'alex.william...@redhat.com' ; Lan,
Tianyu ; Tian, Kevin ;

'm...@redhat.com'

; 'jan.kis...@siemens.com' ;
'bd.a...@gmail.com' ; 'David Gibson'
; 'qemu-devel@nongnu.org' 
Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback



On 2017年03月30日 19:06, Liu, Yi L wrote:

-Original Message-
From: Liu, Yi L
Sent: Monday, March 27, 2017 5:22 PM
To: Peter Xu 
Cc: alex.william...@redhat.com; Lan, Tianyu ;
Tian, Kevin ; m...@redhat.com;
jan.kis...@siemens.com; jasow...@redhat.com; bd.a...@gmail.com;
David Gibson ; qemu-devel@nongnu.org
Subject: RE: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback


-Original Message-
From: Peter Xu [mailto:pet...@redhat.com]
Sent: Monday, March 27, 2017 5:12 PM
To: Liu, Yi L 
Cc: alex.william...@redhat.com; Lan, Tianyu
; Tian, Kevin ;
m...@redhat.com; jan.kis...@siemens.com; jasow...@redhat.com;
bd.a...@gmail.com; David Gibson ;
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback

On Mon, Mar 27, 2017 at 08:35:05AM +, Liu, Yi L wrote:

-Original Message-
From: Qemu-devel
[mailto:qemu-devel-bounces+yi.l.liu=intel@nongnu.org] On
Behalf Of Peter Xu
Sent: Tuesday, February 7, 2017 4:28 PM
To: qemu-devel@nongnu.org
Cc: Lan, Tianyu ; Tian, Kevin
; m...@redhat.com; jan.kis...@siemens.com;
jasow...@redhat.com; pet...@redhat.com;
alex.william...@redhat.com; bd.a...@gmail.com; David Gibson

Subject: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback

Originally we have one memory_region_iommu_replay() function,
which is the default behavior to replay the translations of the
whole IOMMU region. However, on some platform like x86, we may
want our own

replay logic for IOMMU regions.

This patch add one more hook for IOMMUOps for the callback, and
it'll override the default if set.

Signed-off-by: Peter Xu 
---
include/exec/memory.h | 2 ++
memory.c  | 6 ++
2 files changed, 8 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h index
0767888..30b2a74 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
void (*notify_flag_changed)(MemoryRegion *iommu,
IOMMUNotifierFlag old_flags,
IOMMUNotifierFlag new_flags);
+/* Set this up to provide customized IOMMU replay function */
+void (*replay)(MemoryRegion *iommu, IOMMUNotifier
+ *notifier);
};

typedef struct CoalescedMemoryRange CoalescedMemoryRange;
diff --git a/memory.c b/memory.c index 7a4f2f9..9c253cc 100644
--- a/memory.c
+++ b/memory.c
@@ -1630,6 +1630,12 @@ void
memory_region_iommu_replay(MemoryRegion
*mr, IOMMUNotifier *n,
hwaddr addr, granularity;
IOMMUTLBEntry iotlb;
+/* If the IOMMU has its own replay callback, override */
+if (mr->iommu_ops->replay) {
+mr->iommu_ops->replay(mr, n);
+return;
+}

Hi Alex, Peter,

Will all the other vendors(e.g. PPC, s390, ARM) add their own
replay callback as well? I guess it depends on whether the
original replay algorithm work well for them? Do you have such knowledge?

I guess so. At least for VT-d we had this callback since the
default replay mechanism did not work well on x86 due to its
extremely large memory region size. Thanks,

thx. that would make sense.

Peter,

Just come to mind that there may be a corner case here.

Intel VT-d actually has a "pt" mode which allows device use physical
address even when VT-d is enabled. In kernel, there is a

iommu_identity_mapping.

If a device is in this map, then it would use "pt" mode. So that
IOMMU driver would not build second-level page table for it.

Yes, but qemu does not support ECAP_PT now, so guest will still have
a page table in this case.

That's true. Without ECAP_PT, IOMMU driver would create a 1:1 map. So
this solution can work well even a device is in identify_map.


Back to the virtual IOVA implementation, if an assigned device is in
the iommu_identity_mapping(e.g. VGA controller), it uses GPA directly

Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-31 Thread Liu, Yi L
> -Original Message-
> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Friday, March 31, 2017 3:17 PM
> To: Liu, Yi L ; 'Peter Xu' 
> Cc: Lan, Tianyu ; Tian, Kevin ;
> 'm...@redhat.com' ; 'jan.kis...@siemens.com'
> ; 'bd.a...@gmail.com' ; 'qemu-
> de...@nongnu.org' ; 'alex.william...@redhat.com'
> ; 'David Gibson' 
> Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> MemoryRegionIOMMUOps.replay() callback
> 
> 
> 
> On 2017年03月31日 13:34, Liu, Yi L wrote:
> >> -Original Message-
> >> From: Jason Wang [mailto:jasow...@redhat.com]
> >> Sent: Thursday, March 30, 2017 7:58 PM
> >> To: Liu, Yi L ; 'Peter Xu' 
> >> Cc: 'alex.william...@redhat.com' ; Lan,
> >> Tianyu ; Tian, Kevin ;
> 'm...@redhat.com'
> >> ; 'jan.kis...@siemens.com' ;
> >> 'bd.a...@gmail.com' ; 'David Gibson'
> >> ; 'qemu-devel@nongnu.org'  >> de...@nongnu.org>
> >> Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> >> MemoryRegionIOMMUOps.replay() callback
> >>
> >>
> >>
> >> On 2017年03月30日 19:06, Liu, Yi L wrote:
> >>>> -Original Message-----
> >>>> From: Liu, Yi L
> >>>> Sent: Monday, March 27, 2017 5:22 PM
> >>>> To: Peter Xu 
> >>>> Cc: alex.william...@redhat.com; Lan, Tianyu ;
> >>>> Tian, Kevin ; m...@redhat.com;
> >>>> jan.kis...@siemens.com; jasow...@redhat.com; bd.a...@gmail.com;
> >>>> David Gibson ; qemu-devel@nongnu.org
> >>>> Subject: RE: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>>> MemoryRegionIOMMUOps.replay() callback
> >>>>
> >>>>> -Original Message-
> >>>>> From: Peter Xu [mailto:pet...@redhat.com]
> >>>>> Sent: Monday, March 27, 2017 5:12 PM
> >>>>> To: Liu, Yi L 
> >>>>> Cc: alex.william...@redhat.com; Lan, Tianyu
> >>>>> ; Tian, Kevin ;
> >>>>> m...@redhat.com; jan.kis...@siemens.com; jasow...@redhat.com;
> >>>>> bd.a...@gmail.com; David Gibson ;
> >>>>> qemu-devel@nongnu.org
> >>>>> Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>>>> MemoryRegionIOMMUOps.replay() callback
> >>>>>
> >>>>> On Mon, Mar 27, 2017 at 08:35:05AM +, Liu, Yi L wrote:
> >>>>>>> -Original Message-
> >>>>>>> From: Qemu-devel
> >>>>>>> [mailto:qemu-devel-bounces+yi.l.liu=intel@nongnu.org] On
> >>>>>>> Behalf Of Peter Xu
> >>>>>>> Sent: Tuesday, February 7, 2017 4:28 PM
> >>>>>>> To: qemu-devel@nongnu.org
> >>>>>>> Cc: Lan, Tianyu ; Tian, Kevin
> >>>>>>> ; m...@redhat.com; jan.kis...@siemens.com;
> >>>>>>> jasow...@redhat.com; pet...@redhat.com;
> >>>>>>> alex.william...@redhat.com; bd.a...@gmail.com; David Gibson
> >>>>>>> 
> >>>>>>> Subject: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>>>>>> MemoryRegionIOMMUOps.replay() callback
> >>>>>>>
> >>>>>>> Originally we have one memory_region_iommu_replay() function,
> >>>>>>> which is the default behavior to replay the translations of the
> >>>>>>> whole IOMMU region. However, on some platform like x86, we may
> >>>>>>> want our own
> >>>>> replay logic for IOMMU regions.
> >>>>>>> This patch add one more hook for IOMMUOps for the callback, and
> >>>>>>> it'll override the default if set.
> >>>>>>>
> >>>>>>> Signed-off-by: Peter Xu 
> >>>>>>> ---
> >>>>>>>include/exec/memory.h | 2 ++
> >>>>>>>memory.c  | 6 ++
> >>>>>>>2 files changed, 8 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/include/exec/memory.h b/include/exec/memory.h index
> >>>>>>> 0767888..30b2a74 100644
> >>>>>>> --- a/include/exec/memory.h
> >>>>>>> +++ b/include/exec/memory.h
> >>>>>>> @@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
> >&

Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-31 Thread Jason Wang



On 2017年03月31日 13:34, Liu, Yi L wrote:

-Original Message-
From: Jason Wang [mailto:jasow...@redhat.com]
Sent: Thursday, March 30, 2017 7:58 PM
To: Liu, Yi L ; 'Peter Xu' 
Cc: 'alex.william...@redhat.com' ; Lan, Tianyu
; Tian, Kevin ; 'm...@redhat.com'
; 'jan.kis...@siemens.com' ;
'bd.a...@gmail.com' ; 'David Gibson'
; 'qemu-devel@nongnu.org' 
Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback



On 2017年03月30日 19:06, Liu, Yi L wrote:

-Original Message-
From: Liu, Yi L
Sent: Monday, March 27, 2017 5:22 PM
To: Peter Xu 
Cc: alex.william...@redhat.com; Lan, Tianyu ;
Tian, Kevin ; m...@redhat.com;
jan.kis...@siemens.com; jasow...@redhat.com; bd.a...@gmail.com; David
Gibson ; qemu-devel@nongnu.org
Subject: RE: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback


-Original Message-
From: Peter Xu [mailto:pet...@redhat.com]
Sent: Monday, March 27, 2017 5:12 PM
To: Liu, Yi L 
Cc: alex.william...@redhat.com; Lan, Tianyu ;
Tian, Kevin ; m...@redhat.com;
jan.kis...@siemens.com; jasow...@redhat.com; bd.a...@gmail.com;
David Gibson ; qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback

On Mon, Mar 27, 2017 at 08:35:05AM +, Liu, Yi L wrote:

-Original Message-
From: Qemu-devel
[mailto:qemu-devel-bounces+yi.l.liu=intel@nongnu.org] On
Behalf Of Peter Xu
Sent: Tuesday, February 7, 2017 4:28 PM
To: qemu-devel@nongnu.org
Cc: Lan, Tianyu ; Tian, Kevin
; m...@redhat.com; jan.kis...@siemens.com;
jasow...@redhat.com; pet...@redhat.com;
alex.william...@redhat.com; bd.a...@gmail.com; David Gibson

Subject: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback

Originally we have one memory_region_iommu_replay() function,
which is the default behavior to replay the translations of the
whole IOMMU region. However, on some platform like x86, we may
want our own

replay logic for IOMMU regions.

This patch add one more hook for IOMMUOps for the callback, and
it'll override the default if set.

Signed-off-by: Peter Xu 
---
   include/exec/memory.h | 2 ++
   memory.c  | 6 ++
   2 files changed, 8 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h index
0767888..30b2a74 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
   void (*notify_flag_changed)(MemoryRegion *iommu,
   IOMMUNotifierFlag old_flags,
   IOMMUNotifierFlag new_flags);
+/* Set this up to provide customized IOMMU replay function */
+void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
   };

   typedef struct CoalescedMemoryRange CoalescedMemoryRange; diff
--git a/memory.c b/memory.c index 7a4f2f9..9c253cc 100644
--- a/memory.c
+++ b/memory.c
@@ -1630,6 +1630,12 @@ void
memory_region_iommu_replay(MemoryRegion
*mr, IOMMUNotifier *n,
   hwaddr addr, granularity;
   IOMMUTLBEntry iotlb;
+/* If the IOMMU has its own replay callback, override */
+if (mr->iommu_ops->replay) {
+mr->iommu_ops->replay(mr, n);
+return;
+}

Hi Alex, Peter,

Will all the other vendors(e.g. PPC, s390, ARM) add their own
replay callback as well? I guess it depends on whether the original
replay algorithm work well for them? Do you have such knowledge?

I guess so. At least for VT-d we had this callback since the default
replay mechanism did not work well on x86 due to its extremely large
memory region size. Thanks,

thx. that would make sense.

Peter,

Just come to mind that there may be a corner case here.

Intel VT-d actually has a "pt" mode which allows device use physical
address even when VT-d is enabled. In kernel, there is a iommu_identity_mapping.
If a device is in this map, then it would use "pt" mode. So that IOMMU
driver would not build second-level page table for it.

Yes, but qemu does not support ECAP_PT now, so guest will still have a page 
table in
this case.

That's true. Without ECAP_PT, IOMMU driver would create a 1:1 map. So this 
solution
can work well even a device is in identify_map.


Back to the virtual IOVA implementation, if an assigned device is in
the iommu_identity_mapping(e.g. VGA controller), it uses GPA directly to do DMA.
So it demands a GPA->HPA mapping in host. However, the
iommu->ops.replay is not able to build it when guest SL page table is empty.

So I think building an entire guest PA->HPA mapping before guest
kernel boot would be recommended. Any thoughts?

We plan to add PT in 2.10, a possible rough idea is disabled iommu dmar region 
and
use another region without iommu_ops. Then
vfio_listener_region_add() will just do the correct mappings.

Good to know it. Actually, I also need to expose ECAP_PT for vSVM. So just 

Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-30 Thread Liu, Yi L
> -Original Message-
> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Thursday, March 30, 2017 7:58 PM
> To: Liu, Yi L ; 'Peter Xu' 
> Cc: 'alex.william...@redhat.com' ; Lan, Tianyu
> ; Tian, Kevin ; 'm...@redhat.com'
> ; 'jan.kis...@siemens.com' ;
> 'bd.a...@gmail.com' ; 'David Gibson'
> ; 'qemu-devel@nongnu.org'  de...@nongnu.org>
> Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> MemoryRegionIOMMUOps.replay() callback
> 
> 
> 
> On 2017年03月30日 19:06, Liu, Yi L wrote:
> >> -Original Message-
> >> From: Liu, Yi L
> >> Sent: Monday, March 27, 2017 5:22 PM
> >> To: Peter Xu 
> >> Cc: alex.william...@redhat.com; Lan, Tianyu ;
> >> Tian, Kevin ; m...@redhat.com;
> >> jan.kis...@siemens.com; jasow...@redhat.com; bd.a...@gmail.com; David
> >> Gibson ; qemu-devel@nongnu.org
> >> Subject: RE: [Qemu-devel] [PATCH v7 14/17] memory: add
> >> MemoryRegionIOMMUOps.replay() callback
> >>
> >>> -Original Message-
> >>> From: Peter Xu [mailto:pet...@redhat.com]
> >>> Sent: Monday, March 27, 2017 5:12 PM
> >>> To: Liu, Yi L 
> >>> Cc: alex.william...@redhat.com; Lan, Tianyu ;
> >>> Tian, Kevin ; m...@redhat.com;
> >>> jan.kis...@siemens.com; jasow...@redhat.com; bd.a...@gmail.com;
> >>> David Gibson ; qemu-devel@nongnu.org
> >>> Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>> MemoryRegionIOMMUOps.replay() callback
> >>>
> >>> On Mon, Mar 27, 2017 at 08:35:05AM +, Liu, Yi L wrote:
> >>>>> -Original Message-
> >>>>> From: Qemu-devel
> >>>>> [mailto:qemu-devel-bounces+yi.l.liu=intel@nongnu.org] On
> >>>>> Behalf Of Peter Xu
> >>>>> Sent: Tuesday, February 7, 2017 4:28 PM
> >>>>> To: qemu-devel@nongnu.org
> >>>>> Cc: Lan, Tianyu ; Tian, Kevin
> >>>>> ; m...@redhat.com; jan.kis...@siemens.com;
> >>>>> jasow...@redhat.com; pet...@redhat.com;
> >>>>> alex.william...@redhat.com; bd.a...@gmail.com; David Gibson
> >>>>> 
> >>>>> Subject: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>>>> MemoryRegionIOMMUOps.replay() callback
> >>>>>
> >>>>> Originally we have one memory_region_iommu_replay() function,
> >>>>> which is the default behavior to replay the translations of the
> >>>>> whole IOMMU region. However, on some platform like x86, we may
> >>>>> want our own
> >>> replay logic for IOMMU regions.
> >>>>> This patch add one more hook for IOMMUOps for the callback, and
> >>>>> it'll override the default if set.
> >>>>>
> >>>>> Signed-off-by: Peter Xu 
> >>>>> ---
> >>>>>   include/exec/memory.h | 2 ++
> >>>>>   memory.c  | 6 ++
> >>>>>   2 files changed, 8 insertions(+)
> >>>>>
> >>>>> diff --git a/include/exec/memory.h b/include/exec/memory.h index
> >>>>> 0767888..30b2a74 100644
> >>>>> --- a/include/exec/memory.h
> >>>>> +++ b/include/exec/memory.h
> >>>>> @@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
> >>>>>   void (*notify_flag_changed)(MemoryRegion *iommu,
> >>>>>   IOMMUNotifierFlag old_flags,
> >>>>>   IOMMUNotifierFlag new_flags);
> >>>>> +/* Set this up to provide customized IOMMU replay function */
> >>>>> +void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
> >>>>>   };
> >>>>>
> >>>>>   typedef struct CoalescedMemoryRange CoalescedMemoryRange; diff
> >>>>> --git a/memory.c b/memory.c index 7a4f2f9..9c253cc 100644
> >>>>> --- a/memory.c
> >>>>> +++ b/memory.c
> >>>>> @@ -1630,6 +1630,12 @@ void
> >>>>> memory_region_iommu_replay(MemoryRegion
> >>>>> *mr, IOMMUNotifier *n,
> >>>>>   hwaddr addr, granularity;
> >>>>>   IOMMUTLBEntry iotlb;
> >>>>> +/* If the IOMMU has its own replay callback, override */
> >>>>> +if (mr->iommu_ops->replay) {
> >>>>> +

Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-30 Thread Peter Xu
On Fri, Mar 31, 2017 at 01:12:56PM +0800, Jason Wang wrote:
> 
> 
> On 2017年03月31日 13:01, Peter Xu wrote:
> >On Fri, Mar 31, 2017 at 12:21:23PM +0800, Jason Wang wrote:
> >>
> >>On 2017年03月31日 10:56, Peter Xu wrote:
> >Just come to mind that there may be a corner case here.
> >
> >Intel VT-d actually has a "pt" mode which allows device use physical 
> >address
> >even when VT-d is enabled. In kernel, there is a iommu_identity_mapping.
> >If a device is in this map, then it would use "pt" mode. So that IOMMU 
> >driver
> >would not build second-level page table for it.
> Yes, but qemu does not support ECAP_PT now, so guest will still have a 
> page
> table in this case.
> 
> >Back to the virtual IOVA implementation, if an assigned device is in the
> >iommu_identity_mapping(e.g. VGA controller), it uses GPA directly to do 
> >DMA.
> >So it demands a GPA->HPA mapping in host. However, the iommu->ops.replay
> >is not able to build it when guest SL page table is empty.
> >
> >So I think building an entire guest PA->HPA mapping before guest kernel 
> >boot
> >would be recommended. Any thoughts?
> We plan to add PT in 2.10, a possible rough idea is disabled iommu dmar
> region and use another region without iommu_ops. Then
> vfio_listener_region_add() will just do the correct mappings.
> >>>Even without any new region. With the patch 16/17 ("intel_iommu: allow
> >>>dynamic switch of IOMMU region"), we can just turn the IOMMU region
> >>>on/off, following the device's PT bit, maybe using the new
> >>>vtd_switch_address_space() interface. That should be enough.
> >>Right. For vhost it was probably need more works, e.g setting up static
> >>mappings during region_add().
> >Do we need to?
> 
> Not a must if we don't care about performance.
> 
> >
> >VFIO will need it for building up shadow page table, even without a
> >vIOMMU. But imho that should not be needed by vhost, right?
> 
> Device IOTLB will be enabled unconditionally if iommu_platform is specified.
> If we don't set static mappings, vhost will send IOTLB miss request. The
> performance will be horrible in this case.

I see, thanks. So looks like we will need one more patch for PT
support now. :)

-- peterx



Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-30 Thread Jason Wang



On 2017年03月31日 13:01, Peter Xu wrote:

On Fri, Mar 31, 2017 at 12:21:23PM +0800, Jason Wang wrote:


On 2017年03月31日 10:56, Peter Xu wrote:

Just come to mind that there may be a corner case here.

Intel VT-d actually has a "pt" mode which allows device use physical address
even when VT-d is enabled. In kernel, there is a iommu_identity_mapping.
If a device is in this map, then it would use "pt" mode. So that IOMMU driver
would not build second-level page table for it.

Yes, but qemu does not support ECAP_PT now, so guest will still have a page
table in this case.


Back to the virtual IOVA implementation, if an assigned device is in the
iommu_identity_mapping(e.g. VGA controller), it uses GPA directly to do DMA.
So it demands a GPA->HPA mapping in host. However, the iommu->ops.replay
is not able to build it when guest SL page table is empty.

So I think building an entire guest PA->HPA mapping before guest kernel boot
would be recommended. Any thoughts?

We plan to add PT in 2.10, a possible rough idea is disabled iommu dmar
region and use another region without iommu_ops. Then
vfio_listener_region_add() will just do the correct mappings.

Even without any new region. With the patch 16/17 ("intel_iommu: allow
dynamic switch of IOMMU region"), we can just turn the IOMMU region
on/off, following the device's PT bit, maybe using the new
vtd_switch_address_space() interface. That should be enough.

Right. For vhost it was probably need more works, e.g setting up static
mappings during region_add().

Do we need to?


Not a must if we don't care about performance.



VFIO will need it for building up shadow page table, even without a
vIOMMU. But imho that should not be needed by vhost, right?


Device IOTLB will be enabled unconditionally if iommu_platform is 
specified. If we don't set static mappings, vhost will send IOTLB miss 
request. The performance will be horrible in this case.


Thanks



-- peterx





Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-30 Thread Peter Xu
On Fri, Mar 31, 2017 at 12:21:23PM +0800, Jason Wang wrote:
> 
> 
> On 2017年03月31日 10:56, Peter Xu wrote:
> >>>Just come to mind that there may be a corner case here.
> >>>
> >>>Intel VT-d actually has a "pt" mode which allows device use physical 
> >>>address
> >>>even when VT-d is enabled. In kernel, there is a iommu_identity_mapping.
> >>>If a device is in this map, then it would use "pt" mode. So that IOMMU 
> >>>driver
> >>>would not build second-level page table for it.
> >>Yes, but qemu does not support ECAP_PT now, so guest will still have a page
> >>table in this case.
> >>
> >>>Back to the virtual IOVA implementation, if an assigned device is in the
> >>>iommu_identity_mapping(e.g. VGA controller), it uses GPA directly to do 
> >>>DMA.
> >>>So it demands a GPA->HPA mapping in host. However, the iommu->ops.replay
> >>>is not able to build it when guest SL page table is empty.
> >>>
> >>>So I think building an entire guest PA->HPA mapping before guest kernel 
> >>>boot
> >>>would be recommended. Any thoughts?
> >>We plan to add PT in 2.10, a possible rough idea is disabled iommu dmar
> >>region and use another region without iommu_ops. Then
> >>vfio_listener_region_add() will just do the correct mappings.
> >Even without any new region. With the patch 16/17 ("intel_iommu: allow
> >dynamic switch of IOMMU region"), we can just turn the IOMMU region
> >on/off, following the device's PT bit, maybe using the new
> >vtd_switch_address_space() interface. That should be enough.
> 
> Right. For vhost it was probably need more works, e.g setting up static
> mappings during region_add().

Do we need to?

VFIO will need it for building up shadow page table, even without a
vIOMMU. But imho that should not be needed by vhost, right?

-- peterx



Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-30 Thread Jason Wang



On 2017年03月31日 10:56, Peter Xu wrote:

Just come to mind that there may be a corner case here.

Intel VT-d actually has a "pt" mode which allows device use physical address
even when VT-d is enabled. In kernel, there is a iommu_identity_mapping.
If a device is in this map, then it would use "pt" mode. So that IOMMU driver
would not build second-level page table for it.

Yes, but qemu does not support ECAP_PT now, so guest will still have a page
table in this case.


Back to the virtual IOVA implementation, if an assigned device is in the
iommu_identity_mapping(e.g. VGA controller), it uses GPA directly to do DMA.
So it demands a GPA->HPA mapping in host. However, the iommu->ops.replay
is not able to build it when guest SL page table is empty.

So I think building an entire guest PA->HPA mapping before guest kernel boot
would be recommended. Any thoughts?

We plan to add PT in 2.10, a possible rough idea is disabled iommu dmar
region and use another region without iommu_ops. Then
vfio_listener_region_add() will just do the correct mappings.

Even without any new region. With the patch 16/17 ("intel_iommu: allow
dynamic switch of IOMMU region"), we can just turn the IOMMU region
on/off, following the device's PT bit, maybe using the new
vtd_switch_address_space() interface. That should be enough.


Right. For vhost it was probably need more works, e.g setting up static 
mappings during region_add().




Again, we just need to wait until current series merged.

(Oh, then I found why I had an extra "on/off" parameter in previous
  versions in vtd_switch_address_space(), but it was removed.)


Good to know this.

Thanks



Thanks,

-- peterx





Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-30 Thread Peter Xu
On Thu, Mar 30, 2017 at 07:57:38PM +0800, Jason Wang wrote:
> 
> 
> On 2017年03月30日 19:06, Liu, Yi L wrote:
> >>-Original Message-
> >>From: Liu, Yi L
> >>Sent: Monday, March 27, 2017 5:22 PM
> >>To: Peter Xu 
> >>Cc: alex.william...@redhat.com; Lan, Tianyu ; Tian, 
> >>Kevin
> >>; m...@redhat.com; jan.kis...@siemens.com;
> >>jasow...@redhat.com; bd.a...@gmail.com; David Gibson
> >>; qemu-devel@nongnu.org
> >>Subject: RE: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>MemoryRegionIOMMUOps.replay() callback
> >>
> >>>-Original Message-
> >>>From: Peter Xu [mailto:pet...@redhat.com]
> >>>Sent: Monday, March 27, 2017 5:12 PM
> >>>To: Liu, Yi L 
> >>>Cc: alex.william...@redhat.com; Lan, Tianyu ;
> >>>Tian, Kevin ; m...@redhat.com;
> >>>jan.kis...@siemens.com; jasow...@redhat.com; bd.a...@gmail.com; David
> >>>Gibson ; qemu-devel@nongnu.org
> >>>Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>>MemoryRegionIOMMUOps.replay() callback
> >>>
> >>>On Mon, Mar 27, 2017 at 08:35:05AM +, Liu, Yi L wrote:
> >>>>>-Original Message-
> >>>>>From: Qemu-devel
> >>>>>[mailto:qemu-devel-bounces+yi.l.liu=intel@nongnu.org] On
> >>>>>Behalf Of Peter Xu
> >>>>>Sent: Tuesday, February 7, 2017 4:28 PM
> >>>>>To: qemu-devel@nongnu.org
> >>>>>Cc: Lan, Tianyu ; Tian, Kevin
> >>>>>; m...@redhat.com; jan.kis...@siemens.com;
> >>>>>jasow...@redhat.com; pet...@redhat.com;
> >>>>>alex.william...@redhat.com; bd.a...@gmail.com; David Gibson
> >>>>>
> >>>>>Subject: [Qemu-devel] [PATCH v7 14/17] memory: add
> >>>>>MemoryRegionIOMMUOps.replay() callback
> >>>>>
> >>>>>Originally we have one memory_region_iommu_replay() function,
> >>>>>which is the default behavior to replay the translations of the
> >>>>>whole IOMMU region. However, on some platform like x86, we may
> >>>>>want our own
> >>>replay logic for IOMMU regions.
> >>>>>This patch add one more hook for IOMMUOps for the callback, and
> >>>>>it'll override the default if set.
> >>>>>
> >>>>>Signed-off-by: Peter Xu 
> >>>>>---
> >>>>>  include/exec/memory.h | 2 ++
> >>>>>  memory.c  | 6 ++
> >>>>>  2 files changed, 8 insertions(+)
> >>>>>
> >>>>>diff --git a/include/exec/memory.h b/include/exec/memory.h index
> >>>>>0767888..30b2a74 100644
> >>>>>--- a/include/exec/memory.h
> >>>>>+++ b/include/exec/memory.h
> >>>>>@@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
> >>>>>  void (*notify_flag_changed)(MemoryRegion *iommu,
> >>>>>  IOMMUNotifierFlag old_flags,
> >>>>>  IOMMUNotifierFlag new_flags);
> >>>>>+/* Set this up to provide customized IOMMU replay function */
> >>>>>+void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
> >>>>>  };
> >>>>>
> >>>>>  typedef struct CoalescedMemoryRange CoalescedMemoryRange; diff
> >>>>>--git a/memory.c b/memory.c index 7a4f2f9..9c253cc 100644
> >>>>>--- a/memory.c
> >>>>>+++ b/memory.c
> >>>>>@@ -1630,6 +1630,12 @@ void
> >>>>>memory_region_iommu_replay(MemoryRegion
> >>>>>*mr, IOMMUNotifier *n,
> >>>>>  hwaddr addr, granularity;
> >>>>>  IOMMUTLBEntry iotlb;
> >>>>>+/* If the IOMMU has its own replay callback, override */
> >>>>>+if (mr->iommu_ops->replay) {
> >>>>>+mr->iommu_ops->replay(mr, n);
> >>>>>+return;
> >>>>>+}
> >>>>Hi Alex, Peter,
> >>>>
> >>>>Will all the other vendors(e.g. PPC, s390, ARM) add their own replay
> >>>>callback as well? I guess it depends on whether the original replay
> >>>>algorithm work well for them? Do you have such knowledge?
> >>>I guess so. At least for VT-d we had this callback since the default
> >>>replay mechanism did n

Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-30 Thread Jason Wang



On 2017年03月30日 19:06, Liu, Yi L wrote:

-Original Message-
From: Liu, Yi L
Sent: Monday, March 27, 2017 5:22 PM
To: Peter Xu 
Cc: alex.william...@redhat.com; Lan, Tianyu ; Tian, Kevin
; m...@redhat.com; jan.kis...@siemens.com;
jasow...@redhat.com; bd.a...@gmail.com; David Gibson
; qemu-devel@nongnu.org
Subject: RE: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback


-Original Message-
From: Peter Xu [mailto:pet...@redhat.com]
Sent: Monday, March 27, 2017 5:12 PM
To: Liu, Yi L 
Cc: alex.william...@redhat.com; Lan, Tianyu ;
Tian, Kevin ; m...@redhat.com;
jan.kis...@siemens.com; jasow...@redhat.com; bd.a...@gmail.com; David
Gibson ; qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback

On Mon, Mar 27, 2017 at 08:35:05AM +, Liu, Yi L wrote:

-Original Message-
From: Qemu-devel
[mailto:qemu-devel-bounces+yi.l.liu=intel@nongnu.org] On
Behalf Of Peter Xu
Sent: Tuesday, February 7, 2017 4:28 PM
To: qemu-devel@nongnu.org
Cc: Lan, Tianyu ; Tian, Kevin
; m...@redhat.com; jan.kis...@siemens.com;
jasow...@redhat.com; pet...@redhat.com;
alex.william...@redhat.com; bd.a...@gmail.com; David Gibson

Subject: [Qemu-devel] [PATCH v7 14/17] memory: add
MemoryRegionIOMMUOps.replay() callback

Originally we have one memory_region_iommu_replay() function,
which is the default behavior to replay the translations of the
whole IOMMU region. However, on some platform like x86, we may
want our own

replay logic for IOMMU regions.

This patch add one more hook for IOMMUOps for the callback, and
it'll override the default if set.

Signed-off-by: Peter Xu 
---
  include/exec/memory.h | 2 ++
  memory.c  | 6 ++
  2 files changed, 8 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h index
0767888..30b2a74 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
  void (*notify_flag_changed)(MemoryRegion *iommu,
  IOMMUNotifierFlag old_flags,
  IOMMUNotifierFlag new_flags);
+/* Set this up to provide customized IOMMU replay function */
+void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
  };

  typedef struct CoalescedMemoryRange CoalescedMemoryRange; diff
--git a/memory.c b/memory.c index 7a4f2f9..9c253cc 100644
--- a/memory.c
+++ b/memory.c
@@ -1630,6 +1630,12 @@ void
memory_region_iommu_replay(MemoryRegion
*mr, IOMMUNotifier *n,
  hwaddr addr, granularity;
  IOMMUTLBEntry iotlb;
+/* If the IOMMU has its own replay callback, override */
+if (mr->iommu_ops->replay) {
+mr->iommu_ops->replay(mr, n);
+return;
+}

Hi Alex, Peter,

Will all the other vendors(e.g. PPC, s390, ARM) add their own replay
callback as well? I guess it depends on whether the original replay
algorithm work well for them? Do you have such knowledge?

I guess so. At least for VT-d we had this callback since the default
replay mechanism did not work well on x86 due to its extremely large
memory region size. Thanks,

thx. that would make sense.

Peter,

Just come to mind that there may be a corner case here.

Intel VT-d actually has a "pt" mode which allows device use physical address
even when VT-d is enabled. In kernel, there is a iommu_identity_mapping.
If a device is in this map, then it would use "pt" mode. So that IOMMU driver
would not build second-level page table for it.


Yes, but qemu does not support ECAP_PT now, so guest will still have a 
page table in this case.




Back to the virtual IOVA implementation, if an assigned device is in the
iommu_identity_mapping(e.g. VGA controller), it uses GPA directly to do DMA.
So it demands a GPA->HPA mapping in host. However, the iommu->ops.replay
is not able to build it when guest SL page table is empty.

So I think building an entire guest PA->HPA mapping before guest kernel boot
would be recommended. Any thoughts?


We plan to add PT in 2.10, a possible rough idea is disabled iommu dmar 
region and use another region without iommu_ops. Then 
vfio_listener_region_add() will just do the correct mappings.


Thanks



Regards,
Yi L





Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-30 Thread Liu, Yi L
> -Original Message-
> From: Liu, Yi L
> Sent: Monday, March 27, 2017 5:22 PM
> To: Peter Xu 
> Cc: alex.william...@redhat.com; Lan, Tianyu ; Tian, 
> Kevin
> ; m...@redhat.com; jan.kis...@siemens.com;
> jasow...@redhat.com; bd.a...@gmail.com; David Gibson
> ; qemu-devel@nongnu.org
> Subject: RE: [Qemu-devel] [PATCH v7 14/17] memory: add
> MemoryRegionIOMMUOps.replay() callback
> 
> > -Original Message-
> > From: Peter Xu [mailto:pet...@redhat.com]
> > Sent: Monday, March 27, 2017 5:12 PM
> > To: Liu, Yi L 
> > Cc: alex.william...@redhat.com; Lan, Tianyu ;
> > Tian, Kevin ; m...@redhat.com;
> > jan.kis...@siemens.com; jasow...@redhat.com; bd.a...@gmail.com; David
> > Gibson ; qemu-devel@nongnu.org
> > Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> > MemoryRegionIOMMUOps.replay() callback
> >
> > On Mon, Mar 27, 2017 at 08:35:05AM +, Liu, Yi L wrote:
> > > > -Original Message-
> > > > From: Qemu-devel
> > > > [mailto:qemu-devel-bounces+yi.l.liu=intel@nongnu.org] On
> > > > Behalf Of Peter Xu
> > > > Sent: Tuesday, February 7, 2017 4:28 PM
> > > > To: qemu-devel@nongnu.org
> > > > Cc: Lan, Tianyu ; Tian, Kevin
> > > > ; m...@redhat.com; jan.kis...@siemens.com;
> > > > jasow...@redhat.com; pet...@redhat.com;
> > > > alex.william...@redhat.com; bd.a...@gmail.com; David Gibson
> > > > 
> > > > Subject: [Qemu-devel] [PATCH v7 14/17] memory: add
> > > > MemoryRegionIOMMUOps.replay() callback
> > > >
> > > > Originally we have one memory_region_iommu_replay() function,
> > > > which is the default behavior to replay the translations of the
> > > > whole IOMMU region. However, on some platform like x86, we may
> > > > want our own
> > replay logic for IOMMU regions.
> > > > This patch add one more hook for IOMMUOps for the callback, and
> > > > it'll override the default if set.
> > > >
> > > > Signed-off-by: Peter Xu 
> > > > ---
> > > >  include/exec/memory.h | 2 ++
> > > >  memory.c  | 6 ++
> > > >  2 files changed, 8 insertions(+)
> > > >
> > > > diff --git a/include/exec/memory.h b/include/exec/memory.h index
> > > > 0767888..30b2a74 100644
> > > > --- a/include/exec/memory.h
> > > > +++ b/include/exec/memory.h
> > > > @@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
> > > >  void (*notify_flag_changed)(MemoryRegion *iommu,
> > > >  IOMMUNotifierFlag old_flags,
> > > >  IOMMUNotifierFlag new_flags);
> > > > +/* Set this up to provide customized IOMMU replay function */
> > > > +void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
> > > >  };
> > > >
> > > >  typedef struct CoalescedMemoryRange CoalescedMemoryRange; diff
> > > > --git a/memory.c b/memory.c index 7a4f2f9..9c253cc 100644
> > > > --- a/memory.c
> > > > +++ b/memory.c
> > > > @@ -1630,6 +1630,12 @@ void
> > > > memory_region_iommu_replay(MemoryRegion
> > > > *mr, IOMMUNotifier *n,
> > > >  hwaddr addr, granularity;
> > > >  IOMMUTLBEntry iotlb;
> > > > +/* If the IOMMU has its own replay callback, override */
> > > > +if (mr->iommu_ops->replay) {
> > > > +mr->iommu_ops->replay(mr, n);
> > > > +return;
> > > > +}
> > >
> > > Hi Alex, Peter,
> > >
> > > Will all the other vendors(e.g. PPC, s390, ARM) add their own replay
> > > callback as well? I guess it depends on whether the original replay
> > > algorithm work well for them? Do you have such knowledge?
> >
> > I guess so. At least for VT-d we had this callback since the default
> > replay mechanism did not work well on x86 due to its extremely large
> > memory region size. Thanks,
> 
> thx. that would make sense.

Peter,

Just come to mind that there may be a corner case here.

Intel VT-d actually has a "pt" mode which allows device use physical address
even when VT-d is enabled. In kernel, there is a iommu_identity_mapping. 
If a device is in this map, then it would use "pt" mode. So that IOMMU driver
would not build second-level page table for it.

Back to the virtual IOVA implementation, if an assigned device is in the 
iommu_identity_mapping(e.g. VGA controller), it uses GPA directly to do DMA.
So it demands a GPA->HPA mapping in host. However, the iommu->ops.replay
is not able to build it when guest SL page table is empty.

So I think building an entire guest PA->HPA mapping before guest kernel boot
would be recommended. Any thoughts?

Regards,
Yi L


Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-27 Thread Liu, Yi L
> -Original Message-
> From: Peter Xu [mailto:pet...@redhat.com]
> Sent: Monday, March 27, 2017 5:12 PM
> To: Liu, Yi L 
> Cc: alex.william...@redhat.com; Lan, Tianyu ; Tian, 
> Kevin
> ; m...@redhat.com; jan.kis...@siemens.com;
> jasow...@redhat.com; bd.a...@gmail.com; David Gibson
> ; qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add
> MemoryRegionIOMMUOps.replay() callback
> 
> On Mon, Mar 27, 2017 at 08:35:05AM +, Liu, Yi L wrote:
> > > -Original Message-
> > > From: Qemu-devel
> > > [mailto:qemu-devel-bounces+yi.l.liu=intel@nongnu.org] On Behalf
> > > Of Peter Xu
> > > Sent: Tuesday, February 7, 2017 4:28 PM
> > > To: qemu-devel@nongnu.org
> > > Cc: Lan, Tianyu ; Tian, Kevin
> > > ; m...@redhat.com; jan.kis...@siemens.com;
> > > jasow...@redhat.com; pet...@redhat.com; alex.william...@redhat.com;
> > > bd.a...@gmail.com; David Gibson 
> > > Subject: [Qemu-devel] [PATCH v7 14/17] memory: add
> > > MemoryRegionIOMMUOps.replay() callback
> > >
> > > Originally we have one memory_region_iommu_replay() function, which
> > > is the default behavior to replay the translations of the whole
> > > IOMMU region. However, on some platform like x86, we may want our own
> replay logic for IOMMU regions.
> > > This patch add one more hook for IOMMUOps for the callback, and
> > > it'll override the default if set.
> > >
> > > Signed-off-by: Peter Xu 
> > > ---
> > >  include/exec/memory.h | 2 ++
> > >  memory.c  | 6 ++
> > >  2 files changed, 8 insertions(+)
> > >
> > > diff --git a/include/exec/memory.h b/include/exec/memory.h index
> > > 0767888..30b2a74 100644
> > > --- a/include/exec/memory.h
> > > +++ b/include/exec/memory.h
> > > @@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
> > >  void (*notify_flag_changed)(MemoryRegion *iommu,
> > >  IOMMUNotifierFlag old_flags,
> > >  IOMMUNotifierFlag new_flags);
> > > +/* Set this up to provide customized IOMMU replay function */
> > > +void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
> > >  };
> > >
> > >  typedef struct CoalescedMemoryRange CoalescedMemoryRange; diff
> > > --git a/memory.c b/memory.c index 7a4f2f9..9c253cc 100644
> > > --- a/memory.c
> > > +++ b/memory.c
> > > @@ -1630,6 +1630,12 @@ void memory_region_iommu_replay(MemoryRegion
> > > *mr, IOMMUNotifier *n,
> > >  hwaddr addr, granularity;
> > >  IOMMUTLBEntry iotlb;
> > > +/* If the IOMMU has its own replay callback, override */
> > > +if (mr->iommu_ops->replay) {
> > > +mr->iommu_ops->replay(mr, n);
> > > +return;
> > > +}
> >
> > Hi Alex, Peter,
> >
> > Will all the other vendors(e.g. PPC, s390, ARM) add their own replay
> > callback as well? I guess it depends on whether the original replay
> > algorithm work well for them? Do you have such knowledge?
> 
> I guess so. At least for VT-d we had this callback since the default replay 
> mechanism
> did not work well on x86 due to its extremely large memory region size. 
> Thanks,

thx. that would make sense. 


Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-27 Thread Peter Xu
On Mon, Mar 27, 2017 at 08:35:05AM +, Liu, Yi L wrote:
> > -Original Message-
> > From: Qemu-devel [mailto:qemu-devel-bounces+yi.l.liu=intel@nongnu.org] 
> > On
> > Behalf Of Peter Xu
> > Sent: Tuesday, February 7, 2017 4:28 PM
> > To: qemu-devel@nongnu.org
> > Cc: Lan, Tianyu ; Tian, Kevin ;
> > m...@redhat.com; jan.kis...@siemens.com; jasow...@redhat.com;
> > pet...@redhat.com; alex.william...@redhat.com; bd.a...@gmail.com; David
> > Gibson 
> > Subject: [Qemu-devel] [PATCH v7 14/17] memory: add
> > MemoryRegionIOMMUOps.replay() callback
> > 
> > Originally we have one memory_region_iommu_replay() function, which is the
> > default behavior to replay the translations of the whole IOMMU region. 
> > However,
> > on some platform like x86, we may want our own replay logic for IOMMU 
> > regions.
> > This patch add one more hook for IOMMUOps for the callback, and it'll 
> > override the
> > default if set.
> > 
> > Signed-off-by: Peter Xu 
> > ---
> >  include/exec/memory.h | 2 ++
> >  memory.c  | 6 ++
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/include/exec/memory.h b/include/exec/memory.h index
> > 0767888..30b2a74 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
> >  void (*notify_flag_changed)(MemoryRegion *iommu,
> >  IOMMUNotifierFlag old_flags,
> >  IOMMUNotifierFlag new_flags);
> > +/* Set this up to provide customized IOMMU replay function */
> > +void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
> >  };
> > 
> >  typedef struct CoalescedMemoryRange CoalescedMemoryRange; diff --git
> > a/memory.c b/memory.c index 7a4f2f9..9c253cc 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -1630,6 +1630,12 @@ void memory_region_iommu_replay(MemoryRegion
> > *mr, IOMMUNotifier *n,
> >  hwaddr addr, granularity;
> >  IOMMUTLBEntry iotlb;
> > +/* If the IOMMU has its own replay callback, override */
> > +if (mr->iommu_ops->replay) {
> > +mr->iommu_ops->replay(mr, n);
> > +return;
> > +}
> 
> Hi Alex, Peter,
> 
> Will all the other vendors(e.g. PPC, s390, ARM) add their own replay callback
> as well? I guess it depends on whether the original replay algorithm work well
> for them? Do you have such knowledge?

I guess so. At least for VT-d we had this callback since the default
replay mechanism did not work well on x86 due to its extremely large
memory region size. Thanks,

-- peterx



Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-03-27 Thread Liu, Yi L
> -Original Message-
> From: Qemu-devel [mailto:qemu-devel-bounces+yi.l.liu=intel@nongnu.org] On
> Behalf Of Peter Xu
> Sent: Tuesday, February 7, 2017 4:28 PM
> To: qemu-devel@nongnu.org
> Cc: Lan, Tianyu ; Tian, Kevin ;
> m...@redhat.com; jan.kis...@siemens.com; jasow...@redhat.com;
> pet...@redhat.com; alex.william...@redhat.com; bd.a...@gmail.com; David
> Gibson 
> Subject: [Qemu-devel] [PATCH v7 14/17] memory: add
> MemoryRegionIOMMUOps.replay() callback
> 
> Originally we have one memory_region_iommu_replay() function, which is the
> default behavior to replay the translations of the whole IOMMU region. 
> However,
> on some platform like x86, we may want our own replay logic for IOMMU regions.
> This patch add one more hook for IOMMUOps for the callback, and it'll 
> override the
> default if set.
> 
> Signed-off-by: Peter Xu 
> ---
>  include/exec/memory.h | 2 ++
>  memory.c  | 6 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h index
> 0767888..30b2a74 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
>  void (*notify_flag_changed)(MemoryRegion *iommu,
>  IOMMUNotifierFlag old_flags,
>  IOMMUNotifierFlag new_flags);
> +/* Set this up to provide customized IOMMU replay function */
> +void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
>  };
> 
>  typedef struct CoalescedMemoryRange CoalescedMemoryRange; diff --git
> a/memory.c b/memory.c index 7a4f2f9..9c253cc 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1630,6 +1630,12 @@ void memory_region_iommu_replay(MemoryRegion
> *mr, IOMMUNotifier *n,
>  hwaddr addr, granularity;
>  IOMMUTLBEntry iotlb;
> +/* If the IOMMU has its own replay callback, override */
> +if (mr->iommu_ops->replay) {
> +mr->iommu_ops->replay(mr, n);
> +return;
> +}

Hi Alex, Peter,

Will all the other vendors(e.g. PPC, s390, ARM) add their own replay callback
as well? I guess it depends on whether the original replay algorithm work well
for them? Do you have such knowledge?

Regards,
Yi L

> +
>  granularity = memory_region_iommu_get_min_page_size(mr);
> 
>  for (addr = 0; addr < memory_region_size(mr); addr += granularity) {
> --
> 2.7.4
> 




Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-02-09 Thread David Gibson
On Tue, Feb 07, 2017 at 04:28:16PM +0800, Peter Xu wrote:
> Originally we have one memory_region_iommu_replay() function, which is
> the default behavior to replay the translations of the whole IOMMU
> region. However, on some platform like x86, we may want our own replay
> logic for IOMMU regions. This patch add one more hook for IOMMUOps for
> the callback, and it'll override the default if set.
> 
> Signed-off-by: Peter Xu 

Reviewed-by: David Gibson 

> ---
>  include/exec/memory.h | 2 ++
>  memory.c  | 6 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 0767888..30b2a74 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
>  void (*notify_flag_changed)(MemoryRegion *iommu,
>  IOMMUNotifierFlag old_flags,
>  IOMMUNotifierFlag new_flags);
> +/* Set this up to provide customized IOMMU replay function */
> +void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
>  };
>  
>  typedef struct CoalescedMemoryRange CoalescedMemoryRange;
> diff --git a/memory.c b/memory.c
> index 7a4f2f9..9c253cc 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1630,6 +1630,12 @@ void memory_region_iommu_replay(MemoryRegion *mr, 
> IOMMUNotifier *n,
>  hwaddr addr, granularity;
>  IOMMUTLBEntry iotlb;
>  
> +/* If the IOMMU has its own replay callback, override */
> +if (mr->iommu_ops->replay) {
> +mr->iommu_ops->replay(mr, n);
> +return;
> +}
> +
>  granularity = memory_region_iommu_get_min_page_size(mr);
>  
>  for (addr = 0; addr < memory_region_size(mr); addr += granularity) {

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


[Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

2017-02-07 Thread Peter Xu
Originally we have one memory_region_iommu_replay() function, which is
the default behavior to replay the translations of the whole IOMMU
region. However, on some platform like x86, we may want our own replay
logic for IOMMU regions. This patch add one more hook for IOMMUOps for
the callback, and it'll override the default if set.

Signed-off-by: Peter Xu 
---
 include/exec/memory.h | 2 ++
 memory.c  | 6 ++
 2 files changed, 8 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 0767888..30b2a74 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -191,6 +191,8 @@ struct MemoryRegionIOMMUOps {
 void (*notify_flag_changed)(MemoryRegion *iommu,
 IOMMUNotifierFlag old_flags,
 IOMMUNotifierFlag new_flags);
+/* Set this up to provide customized IOMMU replay function */
+void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
 };
 
 typedef struct CoalescedMemoryRange CoalescedMemoryRange;
diff --git a/memory.c b/memory.c
index 7a4f2f9..9c253cc 100644
--- a/memory.c
+++ b/memory.c
@@ -1630,6 +1630,12 @@ void memory_region_iommu_replay(MemoryRegion *mr, 
IOMMUNotifier *n,
 hwaddr addr, granularity;
 IOMMUTLBEntry iotlb;
 
+/* If the IOMMU has its own replay callback, override */
+if (mr->iommu_ops->replay) {
+mr->iommu_ops->replay(mr, n);
+return;
+}
+
 granularity = memory_region_iommu_get_min_page_size(mr);
 
 for (addr = 0; addr < memory_region_size(mr); addr += granularity) {
-- 
2.7.4