Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Jan Beulich
>>> On 25.04.16 at 12:35,  wrote:
> Previously p2m type p2m_mmio_write_dm was introduced for write-
> protected memory pages whose write operations are supposed to be
> forwarded to and emulated by an ioreq server. Yet limitations of
> rangeset restrict the number of guest pages to be write-protected.
> 
> This patch replaces the p2m type p2m_mmio_write_dm with a new name:
> p2m_ioreq_server, which means this p2m type can be claimed by one
> ioreq server, instead of being tracked inside the rangeset of ioreq
> server. Patches following up will add the related hvmop handling
> code which map/unmap type p2m_ioreq_server to/from an ioreq server.
> 
> changes in v3:
>   - According to Jan & George's comments, keep HVMMEM_mmio_write_dm
> for old xen interface versions, and replace it with HVMMEM_unused
> for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
> enum - HVMMEM_ioreq_server is introduced for the get/set mem type
> interfaces;
>   - Add George's Reviewed-by and Acked-by from Tim & Andrew.
> 
> changes in v2:
>   - According to George Dunlap's comments, only rename the p2m type,
> with no behavior changes.
> 
> Signed-off-by: Paul Durrant 
> Signed-off-by: Yu Zhang 
> Acked-by: Tim Deegan 
> Acked-by: Andrew Cooper 
> Reviewed-by: George Dunlap 

Reviewed-by: Jan Beulich 

I hope the other three tags above are still being considered
applicable by their originators.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Wei Liu
On Mon, Apr 25, 2016 at 06:12:34AM -0600, Jan Beulich wrote:
> >>> On 25.04.16 at 12:35,  wrote:
> > Previously p2m type p2m_mmio_write_dm was introduced for write-
> > protected memory pages whose write operations are supposed to be
> > forwarded to and emulated by an ioreq server. Yet limitations of
> > rangeset restrict the number of guest pages to be write-protected.
> > 
> > This patch replaces the p2m type p2m_mmio_write_dm with a new name:
> > p2m_ioreq_server, which means this p2m type can be claimed by one
> > ioreq server, instead of being tracked inside the rangeset of ioreq
> > server. Patches following up will add the related hvmop handling
> > code which map/unmap type p2m_ioreq_server to/from an ioreq server.
> > 
> > changes in v3:
> >   - According to Jan & George's comments, keep HVMMEM_mmio_write_dm
> > for old xen interface versions, and replace it with HVMMEM_unused
> > for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
> > enum - HVMMEM_ioreq_server is introduced for the get/set mem type
> > interfaces;
> >   - Add George's Reviewed-by and Acked-by from Tim & Andrew.
> > 
> > changes in v2:
> >   - According to George Dunlap's comments, only rename the p2m type,
> > with no behavior changes.
> > 
> > Signed-off-by: Paul Durrant 
> > Signed-off-by: Yu Zhang 
> > Acked-by: Tim Deegan 
> > Acked-by: Andrew Cooper 
> > Reviewed-by: George Dunlap 
> 
> Reviewed-by: Jan Beulich 
> 
> I hope the other three tags above are still being considered
> applicable by their originators.
> 

Subject to confirmation from the originators:

Release-acked-by: Wei Liu 

> Jan
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread George Dunlap
On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang  wrote:
> Previously p2m type p2m_mmio_write_dm was introduced for write-
> protected memory pages whose write operations are supposed to be
> forwarded to and emulated by an ioreq server. Yet limitations of
> rangeset restrict the number of guest pages to be write-protected.
>
> This patch replaces the p2m type p2m_mmio_write_dm with a new name:
> p2m_ioreq_server, which means this p2m type can be claimed by one
> ioreq server, instead of being tracked inside the rangeset of ioreq
> server. Patches following up will add the related hvmop handling
> code which map/unmap type p2m_ioreq_server to/from an ioreq server.
>
> changes in v3:
>   - According to Jan & George's comments, keep HVMMEM_mmio_write_dm
> for old xen interface versions, and replace it with HVMMEM_unused
> for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
> enum - HVMMEM_ioreq_server is introduced for the get/set mem type
> interfaces;
>   - Add George's Reviewed-by and Acked-by from Tim & Andrew.

Unfortunately these rather contradict each other -- I consider
Reviewed-by to only stick if the code I've specified hasn't changed
(or has only changed trivially).

Also...

>
> changes in v2:
>   - According to George Dunlap's comments, only rename the p2m type,
> with no behavior changes.
>
> Signed-off-by: Paul Durrant 
> Signed-off-by: Yu Zhang 
> Acked-by: Tim Deegan 
> Acked-by: Andrew Cooper 
> Reviewed-by: George Dunlap 
> Cc: Keir Fraser 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Jun Nakajima 
> Cc: Kevin Tian 
> Cc: George Dunlap 
> Cc: Tim Deegan 
> ---
>  xen/arch/x86/hvm/hvm.c  | 14 --
>  xen/arch/x86/mm/p2m-ept.c   |  2 +-
>  xen/arch/x86/mm/p2m-pt.c|  2 +-
>  xen/arch/x86/mm/shadow/multi.c  |  2 +-
>  xen/include/asm-x86/p2m.h   |  4 ++--
>  xen/include/public/hvm/hvm_op.h |  8 +++-
>  6 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index f24126d..874cb0f 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1857,7 +1857,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned 
> long gla,
>   */
>  if ( (p2mt == p2m_mmio_dm) ||
>   (npfec.write_access &&
> -  (p2m_is_discard_write(p2mt) || (p2mt == p2m_mmio_write_dm))) )
> +  (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
>  {
>  __put_gfn(p2m, gfn);
>  if ( ap2m_active )
> @@ -5499,8 +5499,8 @@ long do_hvm_op(unsigned long op, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  get_gfn_query_unlocked(d, a.pfn, &t);
>  if ( p2m_is_mmio(t) )
>  a.mem_type =  HVMMEM_mmio_dm;
> -else if ( t == p2m_mmio_write_dm )
> -a.mem_type = HVMMEM_mmio_write_dm;
> +else if ( t == p2m_ioreq_server )
> +a.mem_type = HVMMEM_ioreq_server;
>  else if ( p2m_is_readonly(t) )
>  a.mem_type =  HVMMEM_ram_ro;
>  else if ( p2m_is_ram(t) )
> @@ -5531,7 +5531,8 @@ long do_hvm_op(unsigned long op, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  [HVMMEM_ram_rw]  = p2m_ram_rw,
>  [HVMMEM_ram_ro]  = p2m_ram_ro,
>  [HVMMEM_mmio_dm] = p2m_mmio_dm,
> -[HVMMEM_mmio_write_dm] = p2m_mmio_write_dm
> +[HVMMEM_unused] = p2m_invalid,
> +[HVMMEM_ioreq_server] = p2m_ioreq_server
>  };
>
>  if ( copy_from_guest(&a, arg, 1) )
> @@ -,7 +5556,8 @@ long do_hvm_op(unsigned long op, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>   ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
>  goto setmemtype_fail;
>
> -if ( a.hvmmem_type >= ARRAY_SIZE(memtype) )
> +if ( a.hvmmem_type >= ARRAY_SIZE(memtype) ||
> + unlikely(a.hvmmem_type == HVMMEM_unused) )
>  goto setmemtype_fail;
>
>  while ( a.nr > start_iter )
> @@ -5579,7 +5581,7 @@ long do_hvm_op(unsigned long op, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  }
>  if ( !p2m_is_ram(t) &&
>   (!p2m_is_hole(t) || a.hvmmem_type != HVMMEM_mmio_dm) &&
> - (t != p2m_mmio_write_dm || a.hvmmem_type != HVMMEM_ram_rw) )
> + (t != p2m_ioreq_server || a.hvmmem_type != HVMMEM_ram_rw) )
>  {
>  put_gfn(d, pfn);
>  goto setmemtype_fail;
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index 3cb6868..380ec25 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -171,7 +171,7 @@ static void ept_p2m_type_to_flags(struct p2m_domain *p2m, 
> ept_entry_t *entry,
>  entry->a = entry->d = !!cpu_has_vmx_ept_ad;
>  break;
>  case p2m_grant_map_ro:
> -case p2m_mmio_write_dm:
> +case p2m_ioreq_server:
>  entry->r = 1;
>  entry->w = entry->

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Paul Durrant
> -Original Message-
> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of
> George Dunlap
> Sent: 25 April 2016 14:39
> To: Yu Zhang
> Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
> Andrew Cooper; Tim (Xen.org); Paul Durrant; Lv, Zhiyuan; Jan Beulich; Wei Liu
> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> 
> On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang 
> wrote:
> > Previously p2m type p2m_mmio_write_dm was introduced for write-
> > protected memory pages whose write operations are supposed to be
> > forwarded to and emulated by an ioreq server. Yet limitations of
> > rangeset restrict the number of guest pages to be write-protected.
> >
> > This patch replaces the p2m type p2m_mmio_write_dm with a new name:
> > p2m_ioreq_server, which means this p2m type can be claimed by one
> > ioreq server, instead of being tracked inside the rangeset of ioreq
> > server. Patches following up will add the related hvmop handling
> > code which map/unmap type p2m_ioreq_server to/from an ioreq server.
> >
> > changes in v3:
> >   - According to Jan & George's comments, keep
> HVMMEM_mmio_write_dm
> > for old xen interface versions, and replace it with HVMMEM_unused
> > for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
> > enum - HVMMEM_ioreq_server is introduced for the get/set mem type
> > interfaces;
> >   - Add George's Reviewed-by and Acked-by from Tim & Andrew.
> 
> Unfortunately these rather contradict each other -- I consider
> Reviewed-by to only stick if the code I've specified hasn't changed
> (or has only changed trivially).
> 
> Also...
> 
> >
> > changes in v2:
> >   - According to George Dunlap's comments, only rename the p2m type,
> > with no behavior changes.
> >
> > Signed-off-by: Paul Durrant 
> > Signed-off-by: Yu Zhang 
> > Acked-by: Tim Deegan 
> > Acked-by: Andrew Cooper 
> > Reviewed-by: George Dunlap 
> > Cc: Keir Fraser 
> > Cc: Jan Beulich 
> > Cc: Andrew Cooper 
> > Cc: Jun Nakajima 
> > Cc: Kevin Tian 
> > Cc: George Dunlap 
> > Cc: Tim Deegan 
> > ---
> >  xen/arch/x86/hvm/hvm.c  | 14 --
> >  xen/arch/x86/mm/p2m-ept.c   |  2 +-
> >  xen/arch/x86/mm/p2m-pt.c|  2 +-
> >  xen/arch/x86/mm/shadow/multi.c  |  2 +-
> >  xen/include/asm-x86/p2m.h   |  4 ++--
> >  xen/include/public/hvm/hvm_op.h |  8 +++-
> >  6 files changed, 20 insertions(+), 12 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > index f24126d..874cb0f 100644
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -1857,7 +1857,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
> unsigned long gla,
> >   */
> >  if ( (p2mt == p2m_mmio_dm) ||
> >   (npfec.write_access &&
> > -  (p2m_is_discard_write(p2mt) || (p2mt == p2m_mmio_write_dm))) )
> > +  (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
> >  {
> >  __put_gfn(p2m, gfn);
> >  if ( ap2m_active )
> > @@ -5499,8 +5499,8 @@ long do_hvm_op(unsigned long op,
> XEN_GUEST_HANDLE_PARAM(void) arg)
> >  get_gfn_query_unlocked(d, a.pfn, &t);
> >  if ( p2m_is_mmio(t) )
> >  a.mem_type =  HVMMEM_mmio_dm;
> > -else if ( t == p2m_mmio_write_dm )
> > -a.mem_type = HVMMEM_mmio_write_dm;
> > +else if ( t == p2m_ioreq_server )
> > +a.mem_type = HVMMEM_ioreq_server;
> >  else if ( p2m_is_readonly(t) )
> >  a.mem_type =  HVMMEM_ram_ro;
> >  else if ( p2m_is_ram(t) )
> > @@ -5531,7 +5531,8 @@ long do_hvm_op(unsigned long op,
> XEN_GUEST_HANDLE_PARAM(void) arg)
> >  [HVMMEM_ram_rw]  = p2m_ram_rw,
> >  [HVMMEM_ram_ro]  = p2m_ram_ro,
> >  [HVMMEM_mmio_dm] = p2m_mmio_dm,
> > -[HVMMEM_mmio_write_dm] = p2m_mmio_write_dm
> > +[HVMMEM_unused] = p2m_invalid,
> > +[HVMMEM_ioreq_server] = p2m_ioreq_server
> >  };
> >
> >  if ( copy_from_guest(&a, arg, 1) )
> > @@ -,7 +5556,8 @@ long do_hvm_op(unsigned long op,
> XEN_GUEST_HANDLE_PARAM(void) arg)
> >   ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
> >  goto setmemtype_fail;
> >
> &g

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Jan Beulich
>>> On 25.04.16 at 15:39,  wrote:
> On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang  wrote:
>> --- a/xen/include/public/hvm/hvm_op.h
>> +++ b/xen/include/public/hvm/hvm_op.h
>> @@ -83,7 +83,13 @@ typedef enum {
>>  HVMMEM_ram_rw, /* Normal read/write guest RAM */
>>  HVMMEM_ram_ro, /* Read-only; writes are discarded */
>>  HVMMEM_mmio_dm,/* Reads and write go to the device model */
>> -HVMMEM_mmio_write_dm   /* Read-only; writes go to the device model 
>> */
>> +#if __XEN_INTERFACE_VERSION__ < 0x00040700
>> +HVMMEM_mmio_write_dm,  /* Read-only; writes go to the device model 
>> */
>> +#else
>> +HVMMEM_unused, /* Placeholder; setting memory to this type
>> +  will fail for code after 4.7.0 */
>> +#endif
>> +HVMMEM_ioreq_server
> 
> Also, I don't think we've had a convincing argument for why this patch
> needs to be in 4.7.  The p2m name changes are internal only, and so
> don't need to be made at all; and the old functionality will work as
> well as it ever did.  Furthermore, the whole reason we're in this
> situation is that we checked in a publicly-visible change to the
> interface before it was properly ready; I think we should avoid making
> the same mistake this time.

Good point.

> So personally I'd just leave this patch entirely for 4.8; but if Paul
> and/or Jan have strong opinions, then I would say check in only a
> patch to do the #if/#else/#endif, and leave both the p2m type changes
> and the new HVMMEM_ioreq_server enum for when the 4.8 development
> window opens.

Doing that alone would break the build - it would need to be a
little more than that.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread George Dunlap
On 25/04/16 15:01, Paul Durrant wrote:
>> -Original Message-
>> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of
>> George Dunlap
>> Sent: 25 April 2016 14:39
>> To: Yu Zhang
>> Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
>> Andrew Cooper; Tim (Xen.org); Paul Durrant; Lv, Zhiyuan; Jan Beulich; Wei Liu
>> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
>> Rename p2m_mmio_write_dm to p2m_ioreq_server.
>>
>> On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang 
>> wrote:
>>> Previously p2m type p2m_mmio_write_dm was introduced for write-
>>> protected memory pages whose write operations are supposed to be
>>> forwarded to and emulated by an ioreq server. Yet limitations of
>>> rangeset restrict the number of guest pages to be write-protected.
>>>
>>> This patch replaces the p2m type p2m_mmio_write_dm with a new name:
>>> p2m_ioreq_server, which means this p2m type can be claimed by one
>>> ioreq server, instead of being tracked inside the rangeset of ioreq
>>> server. Patches following up will add the related hvmop handling
>>> code which map/unmap type p2m_ioreq_server to/from an ioreq server.
>>>
>>> changes in v3:
>>>   - According to Jan & George's comments, keep
>> HVMMEM_mmio_write_dm
>>> for old xen interface versions, and replace it with HVMMEM_unused
>>> for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
>>> enum - HVMMEM_ioreq_server is introduced for the get/set mem type
>>> interfaces;
>>>   - Add George's Reviewed-by and Acked-by from Tim & Andrew.
>>
>> Unfortunately these rather contradict each other -- I consider
>> Reviewed-by to only stick if the code I've specified hasn't changed
>> (or has only changed trivially).
>>
>> Also...
>>
>>>
>>> changes in v2:
>>>   - According to George Dunlap's comments, only rename the p2m type,
>>> with no behavior changes.
>>>
>>> Signed-off-by: Paul Durrant 
>>> Signed-off-by: Yu Zhang 
>>> Acked-by: Tim Deegan 
>>> Acked-by: Andrew Cooper 
>>> Reviewed-by: George Dunlap 
>>> Cc: Keir Fraser 
>>> Cc: Jan Beulich 
>>> Cc: Andrew Cooper 
>>> Cc: Jun Nakajima 
>>> Cc: Kevin Tian 
>>> Cc: George Dunlap 
>>> Cc: Tim Deegan 
>>> ---
>>>  xen/arch/x86/hvm/hvm.c  | 14 --
>>>  xen/arch/x86/mm/p2m-ept.c   |  2 +-
>>>  xen/arch/x86/mm/p2m-pt.c|  2 +-
>>>  xen/arch/x86/mm/shadow/multi.c  |  2 +-
>>>  xen/include/asm-x86/p2m.h   |  4 ++--
>>>  xen/include/public/hvm/hvm_op.h |  8 +++-
>>>  6 files changed, 20 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>> index f24126d..874cb0f 100644
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -1857,7 +1857,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>> unsigned long gla,
>>>   */
>>>  if ( (p2mt == p2m_mmio_dm) ||
>>>   (npfec.write_access &&
>>> -  (p2m_is_discard_write(p2mt) || (p2mt == p2m_mmio_write_dm))) )
>>> +  (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
>>>  {
>>>  __put_gfn(p2m, gfn);
>>>  if ( ap2m_active )
>>> @@ -5499,8 +5499,8 @@ long do_hvm_op(unsigned long op,
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>  get_gfn_query_unlocked(d, a.pfn, &t);
>>>  if ( p2m_is_mmio(t) )
>>>  a.mem_type =  HVMMEM_mmio_dm;
>>> -else if ( t == p2m_mmio_write_dm )
>>> -a.mem_type = HVMMEM_mmio_write_dm;
>>> +else if ( t == p2m_ioreq_server )
>>> +a.mem_type = HVMMEM_ioreq_server;
>>>  else if ( p2m_is_readonly(t) )
>>>  a.mem_type =  HVMMEM_ram_ro;
>>>  else if ( p2m_is_ram(t) )
>>> @@ -5531,7 +5531,8 @@ long do_hvm_op(unsigned long op,
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>  [HVMMEM_ram_rw]  = p2m_ram_rw,
>>>  [HVMMEM_ram_ro]  = p2m_ram_ro,
>>>  [HVMMEM_mmio_dm] = p2m_mmio_dm,
>>> -[HVMMEM_mmio_write_dm] = p2m_mmio_write_dm
>>> +[HVMMEM_unused] = p2m_invalid,
>>> +[HVMMEM_ioreq_server] = 

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Jan Beulich
>>> On 25.04.16 at 16:01,  wrote:
> The p2m type changes are also wrong. That type needs to be left alone, 
> presumably, so that anything using HVMMEM_mmio_write_dm and compiled to the 
> old interface version continues to function. I think HVMMEM_ioreq_server 
> needs to map to a new p2m type which should be introduced in patch #3.

I don't understand this part: I thought it was agreed that the old
p2m type needs to go away (not the least because we're tight on
these), and use of the old HVMMEM_* type needs to result in
errors.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Paul Durrant
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 25 April 2016 15:16
> To: Paul Durrant
> Cc: Andrew Cooper; George Dunlap; Wei Liu; Jun Nakajima; Kevin Tian;
> Zhiyuan Lv; Yu Zhang; xen-devel@lists.xen.org; Keir (Xen.org); Tim (Xen.org)
> Subject: RE: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> 
> >>> On 25.04.16 at 16:01,  wrote:
> > The p2m type changes are also wrong. That type needs to be left alone,
> > presumably, so that anything using HVMMEM_mmio_write_dm and
> compiled to the
> > old interface version continues to function. I think HVMMEM_ioreq_server
> > needs to map to a new p2m type which should be introduced in patch #3.
> 
> I don't understand this part: I thought it was agreed that the old
> p2m type needs to go away (not the least because we're tight on
> these), and use of the old HVMMEM_* type needs to result in
> errors.
> 

I may have misunderstood. I thought we'd back-tracked on that because there's a 
concern that we also need to keep anything compiled against the old header 
working. If not then this patch should also remove that p2m type, not rename it.

  Paul

> Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread George Dunlap
On Mon, Apr 25, 2016 at 3:19 PM, Paul Durrant  wrote:
>> -Original Message-
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 25 April 2016 15:16
>> To: Paul Durrant
>> Cc: Andrew Cooper; George Dunlap; Wei Liu; Jun Nakajima; Kevin Tian;
>> Zhiyuan Lv; Yu Zhang; xen-devel@lists.xen.org; Keir (Xen.org); Tim (Xen.org)
>> Subject: RE: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
>> Rename p2m_mmio_write_dm to p2m_ioreq_server.
>>
>> >>> On 25.04.16 at 16:01,  wrote:
>> > The p2m type changes are also wrong. That type needs to be left alone,
>> > presumably, so that anything using HVMMEM_mmio_write_dm and
>> compiled to the
>> > old interface version continues to function. I think HVMMEM_ioreq_server
>> > needs to map to a new p2m type which should be introduced in patch #3.
>>
>> I don't understand this part: I thought it was agreed that the old
>> p2m type needs to go away (not the least because we're tight on
>> these), and use of the old HVMMEM_* type needs to result in
>> errors.
>>
>
> I may have misunderstood. I thought we'd back-tracked on that because there's 
> a concern that we also need to keep anything compiled against the old header 
> working. If not then this patch should also remove that p2m type, not rename 
> it.

You mean remove the old HVMMEM type?

There are two issues:
1. Whether old code should continue to compile
2. How old code should act on new hypervisors

I think we've determined that we definitely cannot allow code compiled
against old hypervisors to accidentally use a different p2m type; so
we certainly need to "burn" an enum here.

I'd personally prefer we just straight-up rename it to HVMMEM_unused,
so nobody continues to think that the HVMMEM_mmio_write_dm
functionality might still actually work; I think Jan thinks that's not
allowed.

Honestly I don't see the point of letting it compile and then return
-EINVAL when we run it.  If people complain that it doesn't work
anymore we should either make it compile *and* maintain the
functionality, or say "Sorry, just use an older version" and make it
neither compile nor maintain the functionality.

But I sort of assumed this discussion on what support looked like had
already been had and Jan was just enforcing it.

(Maybe we should have had a talk about this in person at the Hackathon...)

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Paul Durrant
> -Original Message-
> From: George Dunlap
> Sent: 25 April 2016 15:28
> To: Paul Durrant
> Cc: Jan Beulich; Kevin Tian; Wei Liu; Andrew Cooper; Tim (Xen.org); xen-
> de...@lists.xen.org; Yu Zhang; Zhiyuan Lv; Jun Nakajima; Keir (Xen.org)
> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> 
> On Mon, Apr 25, 2016 at 3:19 PM, Paul Durrant 
> wrote:
> >> -Original Message-
> >> From: Jan Beulich [mailto:jbeul...@suse.com]
> >> Sent: 25 April 2016 15:16
> >> To: Paul Durrant
> >> Cc: Andrew Cooper; George Dunlap; Wei Liu; Jun Nakajima; Kevin Tian;
> >> Zhiyuan Lv; Yu Zhang; xen-devel@lists.xen.org; Keir (Xen.org); Tim
> (Xen.org)
> >> Subject: RE: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> >> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> >>
> >> >>> On 25.04.16 at 16:01,  wrote:
> >> > The p2m type changes are also wrong. That type needs to be left alone,
> >> > presumably, so that anything using HVMMEM_mmio_write_dm and
> >> compiled to the
> >> > old interface version continues to function. I think
> HVMMEM_ioreq_server
> >> > needs to map to a new p2m type which should be introduced in patch
> #3.
> >>
> >> I don't understand this part: I thought it was agreed that the old
> >> p2m type needs to go away (not the least because we're tight on
> >> these), and use of the old HVMMEM_* type needs to result in
> >> errors.
> >>
> >
> > I may have misunderstood. I thought we'd back-tracked on that because
> there's a concern that we also need to keep anything compiled against the
> old header working. If not then this patch should also remove that p2m type,
> not rename it.
> 
> You mean remove the old HVMMEM type?
> 
> There are two issues:
> 1. Whether old code should continue to compile
> 2. How old code should act on new hypervisors
> 
> I think we've determined that we definitely cannot allow code compiled
> against old hypervisors to accidentally use a different p2m type; so
> we certainly need to "burn" an enum here.
> 
> I'd personally prefer we just straight-up rename it to HVMMEM_unused,
> so nobody continues to think that the HVMMEM_mmio_write_dm
> functionality might still actually work; I think Jan thinks that's not
> allowed.
> 
> Honestly I don't see the point of letting it compile and then return
> -EINVAL when we run it.  If people complain that it doesn't work
> anymore we should either make it compile *and* maintain the
> functionality, or say "Sorry, just use an older version" and make it
> neither compile nor maintain the functionality.
> 
> But I sort of assumed this discussion on what support looked like had
> already been had and Jan was just enforcing it.
> 
> (Maybe we should have had a talk about this in person at the Hackathon...)
> 

I'm now confused as to what was agreed, if anything.

The fact of the matter is though that the old type escaped into the wild. I 
wanted it to go away but because it escaped I guess that may just not be 
possible.

  Paul

>  -George
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Yu, Zhang



On 4/25/2016 10:01 PM, Paul Durrant wrote:

-Original Message-
From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of
George Dunlap
Sent: 25 April 2016 14:39
To: Yu Zhang
Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
Andrew Cooper; Tim (Xen.org); Paul Durrant; Lv, Zhiyuan; Jan Beulich; Wei Liu
Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.

On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang 
wrote:

Previously p2m type p2m_mmio_write_dm was introduced for write-
protected memory pages whose write operations are supposed to be
forwarded to and emulated by an ioreq server. Yet limitations of
rangeset restrict the number of guest pages to be write-protected.

This patch replaces the p2m type p2m_mmio_write_dm with a new name:
p2m_ioreq_server, which means this p2m type can be claimed by one
ioreq server, instead of being tracked inside the rangeset of ioreq
server. Patches following up will add the related hvmop handling
code which map/unmap type p2m_ioreq_server to/from an ioreq server.

changes in v3:
  - According to Jan & George's comments, keep

HVMMEM_mmio_write_dm

for old xen interface versions, and replace it with HVMMEM_unused
for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
enum - HVMMEM_ioreq_server is introduced for the get/set mem type
interfaces;
  - Add George's Reviewed-by and Acked-by from Tim & Andrew.


Unfortunately these rather contradict each other -- I consider
Reviewed-by to only stick if the code I've specified hasn't changed
(or has only changed trivially).

Also...



changes in v2:
  - According to George Dunlap's comments, only rename the p2m type,
with no behavior changes.

Signed-off-by: Paul Durrant 
Signed-off-by: Yu Zhang 
Acked-by: Tim Deegan 
Acked-by: Andrew Cooper 
Reviewed-by: George Dunlap 
Cc: Keir Fraser 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Jun Nakajima 
Cc: Kevin Tian 
Cc: George Dunlap 
Cc: Tim Deegan 
---
 xen/arch/x86/hvm/hvm.c  | 14 --
 xen/arch/x86/mm/p2m-ept.c   |  2 +-
 xen/arch/x86/mm/p2m-pt.c|  2 +-
 xen/arch/x86/mm/shadow/multi.c  |  2 +-
 xen/include/asm-x86/p2m.h   |  4 ++--
 xen/include/public/hvm/hvm_op.h |  8 +++-
 6 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index f24126d..874cb0f 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1857,7 +1857,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa,

unsigned long gla,

  */
 if ( (p2mt == p2m_mmio_dm) ||
  (npfec.write_access &&
-  (p2m_is_discard_write(p2mt) || (p2mt == p2m_mmio_write_dm))) )
+  (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
 {
 __put_gfn(p2m, gfn);
 if ( ap2m_active )
@@ -5499,8 +5499,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 get_gfn_query_unlocked(d, a.pfn, &t);
 if ( p2m_is_mmio(t) )
 a.mem_type =  HVMMEM_mmio_dm;
-else if ( t == p2m_mmio_write_dm )
-a.mem_type = HVMMEM_mmio_write_dm;
+else if ( t == p2m_ioreq_server )
+a.mem_type = HVMMEM_ioreq_server;
 else if ( p2m_is_readonly(t) )
 a.mem_type =  HVMMEM_ram_ro;
 else if ( p2m_is_ram(t) )
@@ -5531,7 +5531,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 [HVMMEM_ram_rw]  = p2m_ram_rw,
 [HVMMEM_ram_ro]  = p2m_ram_ro,
 [HVMMEM_mmio_dm] = p2m_mmio_dm,
-[HVMMEM_mmio_write_dm] = p2m_mmio_write_dm
+[HVMMEM_unused] = p2m_invalid,
+[HVMMEM_ioreq_server] = p2m_ioreq_server
 };

 if ( copy_from_guest(&a, arg, 1) )
@@ -,7 +5556,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

  ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
 goto setmemtype_fail;

-if ( a.hvmmem_type >= ARRAY_SIZE(memtype) )
+if ( a.hvmmem_type >= ARRAY_SIZE(memtype) ||
+ unlikely(a.hvmmem_type == HVMMEM_unused) )
 goto setmemtype_fail;

 while ( a.nr > start_iter )
@@ -5579,7 +5581,7 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 }
 if ( !p2m_is_ram(t) &&
  (!p2m_is_hole(t) || a.hvmmem_type != HVMMEM_mmio_dm)

&&

- (t != p2m_mmio_write_dm || a.hvmmem_type !=

HVMMEM_ram_rw) )

+ (t != p2m_ioreq_server || a.hvmmem_type !=

HVMMEM_ram_rw) )

 {
 put_gfn(d, pfn);
 goto setmemtype_fail;
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 3cb6868..380ec25 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-e

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Paul Durrant
> -Original Message-
> From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
> Sent: 25 April 2016 16:22
> To: Paul Durrant; George Dunlap
> Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
> Andrew Cooper; Tim (Xen.org); Lv, Zhiyuan; Jan Beulich; Wei Liu
> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> 
> 
> 
> On 4/25/2016 10:01 PM, Paul Durrant wrote:
> >> -Original Message-
> >> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of
> >> George Dunlap
> >> Sent: 25 April 2016 14:39
> >> To: Yu Zhang
> >> Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
> >> Andrew Cooper; Tim (Xen.org); Paul Durrant; Lv, Zhiyuan; Jan Beulich; Wei
> Liu
> >> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> >> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> >>
> >> On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang
> 
> >> wrote:
> >>> Previously p2m type p2m_mmio_write_dm was introduced for write-
> >>> protected memory pages whose write operations are supposed to be
> >>> forwarded to and emulated by an ioreq server. Yet limitations of
> >>> rangeset restrict the number of guest pages to be write-protected.
> >>>
> >>> This patch replaces the p2m type p2m_mmio_write_dm with a new
> name:
> >>> p2m_ioreq_server, which means this p2m type can be claimed by one
> >>> ioreq server, instead of being tracked inside the rangeset of ioreq
> >>> server. Patches following up will add the related hvmop handling
> >>> code which map/unmap type p2m_ioreq_server to/from an ioreq
> server.
> >>>
> >>> changes in v3:
> >>>   - According to Jan & George's comments, keep
> >> HVMMEM_mmio_write_dm
> >>> for old xen interface versions, and replace it with HVMMEM_unused
> >>> for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
> >>> enum - HVMMEM_ioreq_server is introduced for the get/set mem
> type
> >>> interfaces;
> >>>   - Add George's Reviewed-by and Acked-by from Tim & Andrew.
> >>
> >> Unfortunately these rather contradict each other -- I consider
> >> Reviewed-by to only stick if the code I've specified hasn't changed
> >> (or has only changed trivially).
> >>
> >> Also...
> >>
> >>>
> >>> changes in v2:
> >>>   - According to George Dunlap's comments, only rename the p2m type,
> >>> with no behavior changes.
> >>>
> >>> Signed-off-by: Paul Durrant 
> >>> Signed-off-by: Yu Zhang 
> >>> Acked-by: Tim Deegan 
> >>> Acked-by: Andrew Cooper 
> >>> Reviewed-by: George Dunlap 
> >>> Cc: Keir Fraser 
> >>> Cc: Jan Beulich 
> >>> Cc: Andrew Cooper 
> >>> Cc: Jun Nakajima 
> >>> Cc: Kevin Tian 
> >>> Cc: George Dunlap 
> >>> Cc: Tim Deegan 
> >>> ---
> >>>  xen/arch/x86/hvm/hvm.c  | 14 --
> >>>  xen/arch/x86/mm/p2m-ept.c   |  2 +-
> >>>  xen/arch/x86/mm/p2m-pt.c|  2 +-
> >>>  xen/arch/x86/mm/shadow/multi.c  |  2 +-
> >>>  xen/include/asm-x86/p2m.h   |  4 ++--
> >>>  xen/include/public/hvm/hvm_op.h |  8 +++-
> >>>  6 files changed, 20 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >>> index f24126d..874cb0f 100644
> >>> --- a/xen/arch/x86/hvm/hvm.c
> >>> +++ b/xen/arch/x86/hvm/hvm.c
> >>> @@ -1857,7 +1857,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
> >> unsigned long gla,
> >>>   */
> >>>  if ( (p2mt == p2m_mmio_dm) ||
> >>>   (npfec.write_access &&
> >>> -  (p2m_is_discard_write(p2mt) || (p2mt ==
> p2m_mmio_write_dm))) )
> >>> +  (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
> >>>  {
> >>>  __put_gfn(p2m, gfn);
> >>>  if ( ap2m_active )
> >>> @@ -5499,8 +5499,8 @@ long do_hvm_op(unsigned long op,
> >> XEN_GUEST_HANDLE_PARAM(void) arg)
> >>>  get_gfn_query_unlocked(d, a.pfn, &t);
> >>>  if ( p2m_is_mmio(t) )
> >>>  a.mem_type 

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Jan Beulich
>>> On 25.04.16 at 17:29,  wrote:
>>  -Original Message-
>> From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
>> Sent: 25 April 2016 16:22
>> To: Paul Durrant; George Dunlap
>> Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
>> Andrew Cooper; Tim (Xen.org); Lv, Zhiyuan; Jan Beulich; Wei Liu
>> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
>> Rename p2m_mmio_write_dm to p2m_ioreq_server.
>> 
>> 
>> 
>> On 4/25/2016 10:01 PM, Paul Durrant wrote:
>> >> -Original Message-
>> >> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of
>> >> George Dunlap
>> >> Sent: 25 April 2016 14:39
>> >> To: Yu Zhang
>> >> Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
>> >> Andrew Cooper; Tim (Xen.org); Paul Durrant; Lv, Zhiyuan; Jan Beulich; Wei
>> Liu
>> >> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
>> >> Rename p2m_mmio_write_dm to p2m_ioreq_server.
>> >>
>> >> On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang
>> 
>> >> wrote:
>> >>> Previously p2m type p2m_mmio_write_dm was introduced for write-
>> >>> protected memory pages whose write operations are supposed to be
>> >>> forwarded to and emulated by an ioreq server. Yet limitations of
>> >>> rangeset restrict the number of guest pages to be write-protected.
>> >>>
>> >>> This patch replaces the p2m type p2m_mmio_write_dm with a new
>> name:
>> >>> p2m_ioreq_server, which means this p2m type can be claimed by one
>> >>> ioreq server, instead of being tracked inside the rangeset of ioreq
>> >>> server. Patches following up will add the related hvmop handling
>> >>> code which map/unmap type p2m_ioreq_server to/from an ioreq
>> server.
>> >>>
>> >>> changes in v3:
>> >>>   - According to Jan & George's comments, keep
>> >> HVMMEM_mmio_write_dm
>> >>> for old xen interface versions, and replace it with HVMMEM_unused
>> >>> for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
>> >>> enum - HVMMEM_ioreq_server is introduced for the get/set mem
>> type
>> >>> interfaces;
>> >>>   - Add George's Reviewed-by and Acked-by from Tim & Andrew.
>> >>
>> >> Unfortunately these rather contradict each other -- I consider
>> >> Reviewed-by to only stick if the code I've specified hasn't changed
>> >> (or has only changed trivially).
>> >>
>> >> Also...
>> >>
>> >>>
>> >>> changes in v2:
>> >>>   - According to George Dunlap's comments, only rename the p2m type,
>> >>> with no behavior changes.
>> >>>
>> >>> Signed-off-by: Paul Durrant 
>> >>> Signed-off-by: Yu Zhang 
>> >>> Acked-by: Tim Deegan 
>> >>> Acked-by: Andrew Cooper 
>> >>> Reviewed-by: George Dunlap 
>> >>> Cc: Keir Fraser 
>> >>> Cc: Jan Beulich 
>> >>> Cc: Andrew Cooper 
>> >>> Cc: Jun Nakajima 
>> >>> Cc: Kevin Tian 
>> >>> Cc: George Dunlap 
>> >>> Cc: Tim Deegan 
>> >>> ---
>> >>>  xen/arch/x86/hvm/hvm.c  | 14 --
>> >>>  xen/arch/x86/mm/p2m-ept.c   |  2 +-
>> >>>  xen/arch/x86/mm/p2m-pt.c|  2 +-
>> >>>  xen/arch/x86/mm/shadow/multi.c  |  2 +-
>> >>>  xen/include/asm-x86/p2m.h   |  4 ++--
>> >>>  xen/include/public/hvm/hvm_op.h |  8 +++-
>> >>>  6 files changed, 20 insertions(+), 12 deletions(-)
>> >>>
>> >>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> >>> index f24126d..874cb0f 100644
>> >>> --- a/xen/arch/x86/hvm/hvm.c
>> >>> +++ b/xen/arch/x86/hvm/hvm.c
>> >>> @@ -1857,7 +1857,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>> >> unsigned long gla,
>> >>>   */
>> >>>  if ( (p2mt == p2m_mmio_dm) ||
>> >>>   (npfec.write_access &&
>> >>> -  (p2m_is_discard_write(p2mt) || (p2mt ==
>> p2m_mmio_write_dm))) )
>> >>> +  (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
&g

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Yu, Zhang



On 4/25/2016 11:29 PM, Paul Durrant wrote:

-Original Message-
From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
Sent: 25 April 2016 16:22
To: Paul Durrant; George Dunlap
Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
Andrew Cooper; Tim (Xen.org); Lv, Zhiyuan; Jan Beulich; Wei Liu
Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.



On 4/25/2016 10:01 PM, Paul Durrant wrote:

-Original Message-
From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of
George Dunlap
Sent: 25 April 2016 14:39
To: Yu Zhang
Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
Andrew Cooper; Tim (Xen.org); Paul Durrant; Lv, Zhiyuan; Jan Beulich; Wei

Liu

Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.

On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang



wrote:

Previously p2m type p2m_mmio_write_dm was introduced for write-
protected memory pages whose write operations are supposed to be
forwarded to and emulated by an ioreq server. Yet limitations of
rangeset restrict the number of guest pages to be write-protected.

This patch replaces the p2m type p2m_mmio_write_dm with a new

name:

p2m_ioreq_server, which means this p2m type can be claimed by one
ioreq server, instead of being tracked inside the rangeset of ioreq
server. Patches following up will add the related hvmop handling
code which map/unmap type p2m_ioreq_server to/from an ioreq

server.


changes in v3:
  - According to Jan & George's comments, keep

HVMMEM_mmio_write_dm

for old xen interface versions, and replace it with HVMMEM_unused
for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
enum - HVMMEM_ioreq_server is introduced for the get/set mem

type

interfaces;
  - Add George's Reviewed-by and Acked-by from Tim & Andrew.


Unfortunately these rather contradict each other -- I consider
Reviewed-by to only stick if the code I've specified hasn't changed
(or has only changed trivially).

Also...



changes in v2:
  - According to George Dunlap's comments, only rename the p2m type,
with no behavior changes.

Signed-off-by: Paul Durrant 
Signed-off-by: Yu Zhang 
Acked-by: Tim Deegan 
Acked-by: Andrew Cooper 
Reviewed-by: George Dunlap 
Cc: Keir Fraser 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Jun Nakajima 
Cc: Kevin Tian 
Cc: George Dunlap 
Cc: Tim Deegan 
---
 xen/arch/x86/hvm/hvm.c  | 14 --
 xen/arch/x86/mm/p2m-ept.c   |  2 +-
 xen/arch/x86/mm/p2m-pt.c|  2 +-
 xen/arch/x86/mm/shadow/multi.c  |  2 +-
 xen/include/asm-x86/p2m.h   |  4 ++--
 xen/include/public/hvm/hvm_op.h |  8 +++-
 6 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index f24126d..874cb0f 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1857,7 +1857,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa,

unsigned long gla,

  */
 if ( (p2mt == p2m_mmio_dm) ||
  (npfec.write_access &&
-  (p2m_is_discard_write(p2mt) || (p2mt ==

p2m_mmio_write_dm))) )

+  (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
 {
 __put_gfn(p2m, gfn);
 if ( ap2m_active )
@@ -5499,8 +5499,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 get_gfn_query_unlocked(d, a.pfn, &t);
 if ( p2m_is_mmio(t) )
 a.mem_type =  HVMMEM_mmio_dm;
-else if ( t == p2m_mmio_write_dm )
-a.mem_type = HVMMEM_mmio_write_dm;
+else if ( t == p2m_ioreq_server )
+a.mem_type = HVMMEM_ioreq_server;
 else if ( p2m_is_readonly(t) )
 a.mem_type =  HVMMEM_ram_ro;
 else if ( p2m_is_ram(t) )
@@ -5531,7 +5531,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 [HVMMEM_ram_rw]  = p2m_ram_rw,
 [HVMMEM_ram_ro]  = p2m_ram_ro,
 [HVMMEM_mmio_dm] = p2m_mmio_dm,
-[HVMMEM_mmio_write_dm] = p2m_mmio_write_dm
+[HVMMEM_unused] = p2m_invalid,
+[HVMMEM_ioreq_server] = p2m_ioreq_server
 };

 if ( copy_from_guest(&a, arg, 1) )
@@ -,7 +5556,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

  ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
 goto setmemtype_fail;

-if ( a.hvmmem_type >= ARRAY_SIZE(memtype) )
+if ( a.hvmmem_type >= ARRAY_SIZE(memtype) ||
+ unlikely(a.hvmmem_type == HVMMEM_unused) )
 goto setmemtype_fail;

 while ( a.nr > start_iter )
@@ -5579,7 +5581,7 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 }
 if ( !p2m_is_ram(t) &&
  (!p2m_is_

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Yu, Zhang



On 4/25/2016 11:38 PM, Jan Beulich wrote:

On 25.04.16 at 17:29,  wrote:

 -Original Message-
From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
Sent: 25 April 2016 16:22
To: Paul Durrant; George Dunlap
Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
Andrew Cooper; Tim (Xen.org); Lv, Zhiyuan; Jan Beulich; Wei Liu
Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.



On 4/25/2016 10:01 PM, Paul Durrant wrote:

-Original Message-
From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of
George Dunlap
Sent: 25 April 2016 14:39
To: Yu Zhang
Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
Andrew Cooper; Tim (Xen.org); Paul Durrant; Lv, Zhiyuan; Jan Beulich; Wei

Liu

Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.

On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang



wrote:

Previously p2m type p2m_mmio_write_dm was introduced for write-
protected memory pages whose write operations are supposed to be
forwarded to and emulated by an ioreq server. Yet limitations of
rangeset restrict the number of guest pages to be write-protected.

This patch replaces the p2m type p2m_mmio_write_dm with a new

name:

p2m_ioreq_server, which means this p2m type can be claimed by one
ioreq server, instead of being tracked inside the rangeset of ioreq
server. Patches following up will add the related hvmop handling
code which map/unmap type p2m_ioreq_server to/from an ioreq

server.


changes in v3:
  - According to Jan & George's comments, keep

HVMMEM_mmio_write_dm

for old xen interface versions, and replace it with HVMMEM_unused
for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
enum - HVMMEM_ioreq_server is introduced for the get/set mem

type

interfaces;
  - Add George's Reviewed-by and Acked-by from Tim & Andrew.


Unfortunately these rather contradict each other -- I consider
Reviewed-by to only stick if the code I've specified hasn't changed
(or has only changed trivially).

Also...



changes in v2:
  - According to George Dunlap's comments, only rename the p2m type,
with no behavior changes.

Signed-off-by: Paul Durrant 
Signed-off-by: Yu Zhang 
Acked-by: Tim Deegan 
Acked-by: Andrew Cooper 
Reviewed-by: George Dunlap 
Cc: Keir Fraser 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Jun Nakajima 
Cc: Kevin Tian 
Cc: George Dunlap 
Cc: Tim Deegan 
---
 xen/arch/x86/hvm/hvm.c  | 14 --
 xen/arch/x86/mm/p2m-ept.c   |  2 +-
 xen/arch/x86/mm/p2m-pt.c|  2 +-
 xen/arch/x86/mm/shadow/multi.c  |  2 +-
 xen/include/asm-x86/p2m.h   |  4 ++--
 xen/include/public/hvm/hvm_op.h |  8 +++-
 6 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index f24126d..874cb0f 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1857,7 +1857,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa,

unsigned long gla,

  */
 if ( (p2mt == p2m_mmio_dm) ||
  (npfec.write_access &&
-  (p2m_is_discard_write(p2mt) || (p2mt ==

p2m_mmio_write_dm))) )

+  (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
 {
 __put_gfn(p2m, gfn);
 if ( ap2m_active )
@@ -5499,8 +5499,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 get_gfn_query_unlocked(d, a.pfn, &t);
 if ( p2m_is_mmio(t) )
 a.mem_type =  HVMMEM_mmio_dm;
-else if ( t == p2m_mmio_write_dm )
-a.mem_type = HVMMEM_mmio_write_dm;
+else if ( t == p2m_ioreq_server )
+a.mem_type = HVMMEM_ioreq_server;
 else if ( p2m_is_readonly(t) )
 a.mem_type =  HVMMEM_ram_ro;
 else if ( p2m_is_ram(t) )
@@ -5531,7 +5531,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 [HVMMEM_ram_rw]  = p2m_ram_rw,
 [HVMMEM_ram_ro]  = p2m_ram_ro,
 [HVMMEM_mmio_dm] = p2m_mmio_dm,
-[HVMMEM_mmio_write_dm] = p2m_mmio_write_dm
+[HVMMEM_unused] = p2m_invalid,
+[HVMMEM_ioreq_server] = p2m_ioreq_server
 };

 if ( copy_from_guest(&a, arg, 1) )
@@ -,7 +5556,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

  ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
 goto setmemtype_fail;

-if ( a.hvmmem_type >= ARRAY_SIZE(memtype) )
+if ( a.hvmmem_type >= ARRAY_SIZE(memtype) ||
+ unlikely(a.hvmmem_type == HVMMEM_unused) )
 goto setmemtype_fail;

 while ( a.nr > start_iter )
@@ -5579,7 +5581,7 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 }
 if ( !p2m_is_ram(t) &&

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread George Dunlap
On 25/04/16 16:53, Yu, Zhang wrote:
> 
> 
> On 4/25/2016 11:38 PM, Jan Beulich wrote:
>>>>> On 25.04.16 at 17:29,  wrote:
>>>>  -Original Message-
>>>> From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
>>>> Sent: 25 April 2016 16:22
>>>> To: Paul Durrant; George Dunlap
>>>> Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
>>>> Andrew Cooper; Tim (Xen.org); Lv, Zhiyuan; Jan Beulich; Wei Liu
>>>> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for
>>>> 4.7):
>>>> Rename p2m_mmio_write_dm to p2m_ioreq_server.
>>>>
>>>>
>>>>
>>>> On 4/25/2016 10:01 PM, Paul Durrant wrote:
>>>>>> -Original Message-
>>>>>> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of
>>>>>> George Dunlap
>>>>>> Sent: 25 April 2016 14:39
>>>>>> To: Yu Zhang
>>>>>> Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun
>>>>>> Nakajima;
>>>>>> Andrew Cooper; Tim (Xen.org); Paul Durrant; Lv, Zhiyuan; Jan
>>>>>> Beulich; Wei
>>>> Liu
>>>>>> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for
>>>>>> 4.7):
>>>>>> Rename p2m_mmio_write_dm to p2m_ioreq_server.
>>>>>>
>>>>>> On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang
>>>> 
>>>>>> wrote:
>>>>>>> Previously p2m type p2m_mmio_write_dm was introduced for write-
>>>>>>> protected memory pages whose write operations are supposed to be
>>>>>>> forwarded to and emulated by an ioreq server. Yet limitations of
>>>>>>> rangeset restrict the number of guest pages to be write-protected.
>>>>>>>
>>>>>>> This patch replaces the p2m type p2m_mmio_write_dm with a new
>>>> name:
>>>>>>> p2m_ioreq_server, which means this p2m type can be claimed by one
>>>>>>> ioreq server, instead of being tracked inside the rangeset of ioreq
>>>>>>> server. Patches following up will add the related hvmop handling
>>>>>>> code which map/unmap type p2m_ioreq_server to/from an ioreq
>>>> server.
>>>>>>>
>>>>>>> changes in v3:
>>>>>>>   - According to Jan & George's comments, keep
>>>>>> HVMMEM_mmio_write_dm
>>>>>>> for old xen interface versions, and replace it with
>>>>>>> HVMMEM_unused
>>>>>>> for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
>>>>>>> enum - HVMMEM_ioreq_server is introduced for the get/set mem
>>>> type
>>>>>>> interfaces;
>>>>>>>   - Add George's Reviewed-by and Acked-by from Tim & Andrew.
>>>>>>
>>>>>> Unfortunately these rather contradict each other -- I consider
>>>>>> Reviewed-by to only stick if the code I've specified hasn't changed
>>>>>> (or has only changed trivially).
>>>>>>
>>>>>> Also...
>>>>>>
>>>>>>>
>>>>>>> changes in v2:
>>>>>>>   - According to George Dunlap's comments, only rename the p2m type,
>>>>>>> with no behavior changes.
>>>>>>>
>>>>>>> Signed-off-by: Paul Durrant 
>>>>>>> Signed-off-by: Yu Zhang 
>>>>>>> Acked-by: Tim Deegan 
>>>>>>> Acked-by: Andrew Cooper 
>>>>>>> Reviewed-by: George Dunlap 
>>>>>>> Cc: Keir Fraser 
>>>>>>> Cc: Jan Beulich 
>>>>>>> Cc: Andrew Cooper 
>>>>>>> Cc: Jun Nakajima 
>>>>>>> Cc: Kevin Tian 
>>>>>>> Cc: George Dunlap 
>>>>>>> Cc: Tim Deegan 
>>>>>>> ---
>>>>>>>  xen/arch/x86/hvm/hvm.c  | 14 --
>>>>>>>  xen/arch/x86/mm/p2m-ept.c   |  2 +-
>>>>>>>  xen/arch/x86/mm/p2m-pt.c|  2 +-
>>>>>>>  xen/arch/x86/mm/shadow/multi.c  |  2 +-
>>>>>>>  xen/include/asm-x86/p2m.h   |  4 ++--
>>>>>>>  xen/include/public/hvm/hvm_op.h |  8 +++-
>>>

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Yu, Zhang



On 4/26/2016 12:15 AM, George Dunlap wrote:

On 25/04/16 16:53, Yu, Zhang wrote:



On 4/25/2016 11:38 PM, Jan Beulich wrote:

On 25.04.16 at 17:29,  wrote:

 -Original Message-
From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
Sent: 25 April 2016 16:22
To: Paul Durrant; George Dunlap
Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
Andrew Cooper; Tim (Xen.org); Lv, Zhiyuan; Jan Beulich; Wei Liu
Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for
4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.



On 4/25/2016 10:01 PM, Paul Durrant wrote:

-Original Message-
From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of
George Dunlap
Sent: 25 April 2016 14:39
To: Yu Zhang
Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun
Nakajima;
Andrew Cooper; Tim (Xen.org); Paul Durrant; Lv, Zhiyuan; Jan
Beulich; Wei

Liu

Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for
4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.

On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang



wrote:

Previously p2m type p2m_mmio_write_dm was introduced for write-
protected memory pages whose write operations are supposed to be
forwarded to and emulated by an ioreq server. Yet limitations of
rangeset restrict the number of guest pages to be write-protected.

This patch replaces the p2m type p2m_mmio_write_dm with a new

name:

p2m_ioreq_server, which means this p2m type can be claimed by one
ioreq server, instead of being tracked inside the rangeset of ioreq
server. Patches following up will add the related hvmop handling
code which map/unmap type p2m_ioreq_server to/from an ioreq

server.


changes in v3:
  - According to Jan & George's comments, keep

HVMMEM_mmio_write_dm

for old xen interface versions, and replace it with
HVMMEM_unused
for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a new
enum - HVMMEM_ioreq_server is introduced for the get/set mem

type

interfaces;
  - Add George's Reviewed-by and Acked-by from Tim & Andrew.


Unfortunately these rather contradict each other -- I consider
Reviewed-by to only stick if the code I've specified hasn't changed
(or has only changed trivially).

Also...



changes in v2:
  - According to George Dunlap's comments, only rename the p2m type,
with no behavior changes.

Signed-off-by: Paul Durrant 
Signed-off-by: Yu Zhang 
Acked-by: Tim Deegan 
Acked-by: Andrew Cooper 
Reviewed-by: George Dunlap 
Cc: Keir Fraser 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Jun Nakajima 
Cc: Kevin Tian 
Cc: George Dunlap 
Cc: Tim Deegan 
---
 xen/arch/x86/hvm/hvm.c  | 14 --
 xen/arch/x86/mm/p2m-ept.c   |  2 +-
 xen/arch/x86/mm/p2m-pt.c|  2 +-
 xen/arch/x86/mm/shadow/multi.c  |  2 +-
 xen/include/asm-x86/p2m.h   |  4 ++--
 xen/include/public/hvm/hvm_op.h |  8 +++-
 6 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index f24126d..874cb0f 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1857,7 +1857,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa,

unsigned long gla,

  */
 if ( (p2mt == p2m_mmio_dm) ||
  (npfec.write_access &&
-  (p2m_is_discard_write(p2mt) || (p2mt ==

p2m_mmio_write_dm))) )

+  (p2m_is_discard_write(p2mt) || (p2mt ==
p2m_ioreq_server))) )
 {
 __put_gfn(p2m, gfn);
 if ( ap2m_active )
@@ -5499,8 +5499,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 get_gfn_query_unlocked(d, a.pfn, &t);
 if ( p2m_is_mmio(t) )
 a.mem_type =  HVMMEM_mmio_dm;
-else if ( t == p2m_mmio_write_dm )
-a.mem_type = HVMMEM_mmio_write_dm;
+else if ( t == p2m_ioreq_server )
+a.mem_type = HVMMEM_ioreq_server;
 else if ( p2m_is_readonly(t) )
 a.mem_type =  HVMMEM_ram_ro;
 else if ( p2m_is_ram(t) )
@@ -5531,7 +5531,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 [HVMMEM_ram_rw]  = p2m_ram_rw,
 [HVMMEM_ram_ro]  = p2m_ram_ro,
 [HVMMEM_mmio_dm] = p2m_mmio_dm,
-[HVMMEM_mmio_write_dm] = p2m_mmio_write_dm
+[HVMMEM_unused] = p2m_invalid,
+[HVMMEM_ioreq_server] = p2m_ioreq_server
 };

 if ( copy_from_guest(&a, arg, 1) )
@@ -,7 +5556,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

  ((a.first_pfn + a.nr - 1) >
domain_get_maximum_gpfn(d)) )
 goto setmemtype_fail;

-if ( a.hvmmem_type >= ARRAY_SIZE(memtype) )
+if ( a.hvmmem_type >= ARRAY_SIZE(memtype) ||
+ unlikely(a.hvmmem_type == HVMMEM_unused) )
 goto setmemtype_fail;

 while ( a.nr > start_iter )
@@ -5579,7 +5581,7 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PAR

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-25 Thread Paul Durrant
> -Original Message-
> From: George Dunlap [mailto:george.dun...@citrix.com]
> Sent: 25 April 2016 17:16
> To: Yu, Zhang; Jan Beulich; Paul Durrant
> Cc: Andrew Cooper; Wei Liu; Jun Nakajima; Kevin Tian; Zhiyuan Lv; xen-
> de...@lists.xen.org; Keir (Xen.org); Tim (Xen.org)
> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> 
> On 25/04/16 16:53, Yu, Zhang wrote:
> >
> >
> > On 4/25/2016 11:38 PM, Jan Beulich wrote:
> >>>>> On 25.04.16 at 17:29,  wrote:
> >>>>  -Original Message-
> >>>> From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
> >>>> Sent: 25 April 2016 16:22
> >>>> To: Paul Durrant; George Dunlap
> >>>> Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
> >>>> Andrew Cooper; Tim (Xen.org); Lv, Zhiyuan; Jan Beulich; Wei Liu
> >>>> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for
> >>>> 4.7):
> >>>> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> >>>>
> >>>>
> >>>>
> >>>> On 4/25/2016 10:01 PM, Paul Durrant wrote:
> >>>>>> -Original Message-
> >>>>>> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf
> Of
> >>>>>> George Dunlap
> >>>>>> Sent: 25 April 2016 14:39
> >>>>>> To: Yu Zhang
> >>>>>> Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun
> >>>>>> Nakajima;
> >>>>>> Andrew Cooper; Tim (Xen.org); Paul Durrant; Lv, Zhiyuan; Jan
> >>>>>> Beulich; Wei
> >>>> Liu
> >>>>>> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for
> >>>>>> 4.7):
> >>>>>> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> >>>>>>
> >>>>>> On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang
> >>>> 
> >>>>>> wrote:
> >>>>>>> Previously p2m type p2m_mmio_write_dm was introduced for
> write-
> >>>>>>> protected memory pages whose write operations are supposed to
> be
> >>>>>>> forwarded to and emulated by an ioreq server. Yet limitations of
> >>>>>>> rangeset restrict the number of guest pages to be write-protected.
> >>>>>>>
> >>>>>>> This patch replaces the p2m type p2m_mmio_write_dm with a new
> >>>> name:
> >>>>>>> p2m_ioreq_server, which means this p2m type can be claimed by
> one
> >>>>>>> ioreq server, instead of being tracked inside the rangeset of ioreq
> >>>>>>> server. Patches following up will add the related hvmop handling
> >>>>>>> code which map/unmap type p2m_ioreq_server to/from an ioreq
> >>>> server.
> >>>>>>>
> >>>>>>> changes in v3:
> >>>>>>>   - According to Jan & George's comments, keep
> >>>>>> HVMMEM_mmio_write_dm
> >>>>>>> for old xen interface versions, and replace it with
> >>>>>>> HVMMEM_unused
> >>>>>>> for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a
> new
> >>>>>>> enum - HVMMEM_ioreq_server is introduced for the get/set
> mem
> >>>> type
> >>>>>>> interfaces;
> >>>>>>>   - Add George's Reviewed-by and Acked-by from Tim & Andrew.
> >>>>>>
> >>>>>> Unfortunately these rather contradict each other -- I consider
> >>>>>> Reviewed-by to only stick if the code I've specified hasn't changed
> >>>>>> (or has only changed trivially).
> >>>>>>
> >>>>>> Also...
> >>>>>>
> >>>>>>>
> >>>>>>> changes in v2:
> >>>>>>>   - According to George Dunlap's comments, only rename the p2m
> type,
> >>>>>>> with no behavior changes.
> >>>>>>>
> >>>>>>> Signed-off-by: Paul Durrant 
> >>>>>>> Signed-off-by: Yu Zhang 
> >>>>>>> Acked-by: Tim Deegan 
> >>>>>>> Acked-by: Andrew Cooper 
> >>>>>>

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-26 Thread Yu, Zhang



On 4/26/2016 1:01 AM, Paul Durrant wrote:

-Original Message-
From: George Dunlap [mailto:george.dun...@citrix.com]
Sent: 25 April 2016 17:16
To: Yu, Zhang; Jan Beulich; Paul Durrant
Cc: Andrew Cooper; Wei Liu; Jun Nakajima; Kevin Tian; Zhiyuan Lv; xen-
de...@lists.xen.org; Keir (Xen.org); Tim (Xen.org)
Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.

On 25/04/16 16:53, Yu, Zhang wrote:



On 4/25/2016 11:38 PM, Jan Beulich wrote:

On 25.04.16 at 17:29,  wrote:

 -Original Message-
From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
Sent: 25 April 2016 16:22
To: Paul Durrant; George Dunlap
Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun Nakajima;
Andrew Cooper; Tim (Xen.org); Lv, Zhiyuan; Jan Beulich; Wei Liu
Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for
4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.



On 4/25/2016 10:01 PM, Paul Durrant wrote:

-Original Message-
From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf

Of

George Dunlap
Sent: 25 April 2016 14:39
To: Yu Zhang
Cc: xen-devel@lists.xen.org; Kevin Tian; Keir (Xen.org); Jun
Nakajima;
Andrew Cooper; Tim (Xen.org); Paul Durrant; Lv, Zhiyuan; Jan
Beulich; Wei

Liu

Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for
4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.

On Mon, Apr 25, 2016 at 11:35 AM, Yu Zhang



wrote:

Previously p2m type p2m_mmio_write_dm was introduced for

write-

protected memory pages whose write operations are supposed to

be

forwarded to and emulated by an ioreq server. Yet limitations of
rangeset restrict the number of guest pages to be write-protected.

This patch replaces the p2m type p2m_mmio_write_dm with a new

name:

p2m_ioreq_server, which means this p2m type can be claimed by

one

ioreq server, instead of being tracked inside the rangeset of ioreq
server. Patches following up will add the related hvmop handling
code which map/unmap type p2m_ioreq_server to/from an ioreq

server.


changes in v3:
  - According to Jan & George's comments, keep

HVMMEM_mmio_write_dm

for old xen interface versions, and replace it with
HVMMEM_unused
for xen interfaces newer than 4.7.0; For p2m_ioreq_server, a

new

enum - HVMMEM_ioreq_server is introduced for the get/set

mem

type

interfaces;
  - Add George's Reviewed-by and Acked-by from Tim & Andrew.


Unfortunately these rather contradict each other -- I consider
Reviewed-by to only stick if the code I've specified hasn't changed
(or has only changed trivially).

Also...



changes in v2:
  - According to George Dunlap's comments, only rename the p2m

type,

with no behavior changes.

Signed-off-by: Paul Durrant 
Signed-off-by: Yu Zhang 
Acked-by: Tim Deegan 
Acked-by: Andrew Cooper 
Reviewed-by: George Dunlap 
Cc: Keir Fraser 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Jun Nakajima 
Cc: Kevin Tian 
Cc: George Dunlap 
Cc: Tim Deegan 
---
 xen/arch/x86/hvm/hvm.c  | 14 --
 xen/arch/x86/mm/p2m-ept.c   |  2 +-
 xen/arch/x86/mm/p2m-pt.c|  2 +-
 xen/arch/x86/mm/shadow/multi.c  |  2 +-
 xen/include/asm-x86/p2m.h   |  4 ++--
 xen/include/public/hvm/hvm_op.h |  8 +++-
 6 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index f24126d..874cb0f 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1857,7 +1857,7 @@ int hvm_hap_nested_page_fault(paddr_t

gpa,

unsigned long gla,

  */
 if ( (p2mt == p2m_mmio_dm) ||
  (npfec.write_access &&
-  (p2m_is_discard_write(p2mt) || (p2mt ==

p2m_mmio_write_dm))) )

+  (p2m_is_discard_write(p2mt) || (p2mt ==
p2m_ioreq_server))) )
 {
 __put_gfn(p2m, gfn);
 if ( ap2m_active )
@@ -5499,8 +5499,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 get_gfn_query_unlocked(d, a.pfn, &t);
 if ( p2m_is_mmio(t) )
 a.mem_type =  HVMMEM_mmio_dm;
-else if ( t == p2m_mmio_write_dm )
-a.mem_type = HVMMEM_mmio_write_dm;
+else if ( t == p2m_ioreq_server )
+a.mem_type = HVMMEM_ioreq_server;
 else if ( p2m_is_readonly(t) )
 a.mem_type =  HVMMEM_ram_ro;
 else if ( p2m_is_ram(t) )
@@ -5531,7 +5531,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

 [HVMMEM_ram_rw]  = p2m_ram_rw,
 [HVMMEM_ram_ro]  = p2m_ram_ro,
 [HVMMEM_mmio_dm] = p2m_mmio_dm,
-[HVMMEM_mmio_write_dm] = p2m_mmio_write_dm
+[HVMMEM_unused] = p2m_invalid,
+[HVMMEM_ioreq_server] = p2m_ioreq_server
 };

 if ( copy_from_guest(&a, arg, 1) )
@@ -,7 +5556,8 @@ long do_hvm_op(unsigned long op,

XEN_GUEST_HANDLE_PARAM(void) arg)

  ((a.

Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-26 Thread Paul Durrant
> -Original Message-
[snip]
> >
> > For clarity, do you expect any existing use of HVMMEM_mmio_write_dm
> to continue to *function*? I agree that things should continue to build, but 
> if
> they don't need to function then the now redundant p2m type should be
> removed IMO and any attempt to set a page to HVMMEM_mmio_write_dm
> (or the new HVMMEM_unused) name should result in -EINVAL. What is your
> position on this?
> >
> 
> Thanks, Paul.
> My expectation is that HVMMEM_mmio_write_dm shall fail in new xen
> version, but I do not think we need to remove the p2m type, just
> rename it could be OK.
> 

I think we need George's response before we can proceed.

  Paul
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-27 Thread George Dunlap
On Mon, Apr 25, 2016 at 6:01 PM, Paul Durrant  wrote:
> For clarity, do you expect any existing use of HVMMEM_mmio_write_dm to 
> continue to *function*? I agree that things should continue to build, but if 
> they don't need to function then the now redundant p2m type should be removed 
> IMO and any attempt to set a page to HVMMEM_mmio_write_dm (or the new 
> HVMMEM_unused) name should result in -EINVAL. What is your position on this?

I sort of feel like we're playing some strange guessing game with the
color of this bike shed, where all 4 of us give a random combination
of constrants and then we have to figure out what the solution is. :-)

There are two issues: the interface (HVMMEM_*) and the internals.(p2m_*).

Jan says that code that calls HVMOP_get_mem_type has to continue to
compile and function.  "Functioning" is easy, as you just don't return
that value and you're done.  Compiling just means having the #ifdef.

It sounds like we all agree that HVMOP_set_mem_type with the current
HVMMEM_mmio_write_dm value should return -EINVAL.

Regarding the p2m type which now should be impossible to set -- I
don't think it's critical to remove from the release, since it's just
internal.  I'd normally say just leave it for now to reduce code
churn.

But mostly I think we just want to get this bike shed painted, so if
anyone thinks we should really remove the p2m type from this release,
then that's fine with me too (assuming it's OK with Wei).

Does this cover everything?

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-27 Thread Paul Durrant
> -Original Message-
> From: George Dunlap
> Sent: 27 April 2016 15:13
> To: Paul Durrant
> Cc: Yu, Zhang; Jan Beulich; Kevin Tian; Wei Liu; Andrew Cooper; Tim
> (Xen.org); xen-devel@lists.xen.org; Zhiyuan Lv; Jun Nakajima; Keir (Xen.org)
> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> 
> On Mon, Apr 25, 2016 at 6:01 PM, Paul Durrant 
> wrote:
> > For clarity, do you expect any existing use of HVMMEM_mmio_write_dm
> to continue to *function*? I agree that things should continue to build, but 
> if
> they don't need to function then the now redundant p2m type should be
> removed IMO and any attempt to set a page to HVMMEM_mmio_write_dm
> (or the new HVMMEM_unused) name should result in -EINVAL. What is your
> position on this?
> 
> I sort of feel like we're playing some strange guessing game with the
> color of this bike shed, where all 4 of us give a random combination
> of constrants and then we have to figure out what the solution is. :-)
> 
> There are two issues: the interface (HVMMEM_*) and the
> internals.(p2m_*).
> 
> Jan says that code that calls HVMOP_get_mem_type has to continue to
> compile and function.  "Functioning" is easy, as you just don't return
> that value and you're done.  Compiling just means having the #ifdef.
> 
> It sounds like we all agree that HVMOP_set_mem_type with the current
> HVMMEM_mmio_write_dm value should return -EINVAL.
> 
> Regarding the p2m type which now should be impossible to set -- I
> don't think it's critical to remove from the release, since it's just
> internal.  I'd normally say just leave it for now to reduce code
> churn.
> 
> But mostly I think we just want to get this bike shed painted, so if
> anyone thinks we should really remove the p2m type from this release,
> then that's fine with me too (assuming it's OK with Wei).
> 
> Does this cover everything?
> 

I think so. Thanks for the clarification. 

Yu, are you happy to submit a patch with the #ifdef in the header, and that 
removes any ability to set the old type?

I guess leaving the p2m type in place to avoid code churn is reasonable at this 
stage, but anyone looking at the p2m code is probably going to question why 
it's there in 4.7.

  Paul

>  -George
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-27 Thread Wei Liu
On Wed, Apr 27, 2016 at 03:12:46PM +0100, George Dunlap wrote:
> On Mon, Apr 25, 2016 at 6:01 PM, Paul Durrant  wrote:
> > For clarity, do you expect any existing use of HVMMEM_mmio_write_dm to 
> > continue to *function*? I agree that things should continue to build, but 
> > if they don't need to function then the now redundant p2m type should be 
> > removed IMO and any attempt to set a page to HVMMEM_mmio_write_dm (or the 
> > new HVMMEM_unused) name should result in -EINVAL. What is your position on 
> > this?
> 
> I sort of feel like we're playing some strange guessing game with the
> color of this bike shed, where all 4 of us give a random combination
> of constrants and then we have to figure out what the solution is. :-)
> 
> There are two issues: the interface (HVMMEM_*) and the internals.(p2m_*).
> 
> Jan says that code that calls HVMOP_get_mem_type has to continue to
> compile and function.  "Functioning" is easy, as you just don't return
> that value and you're done.  Compiling just means having the #ifdef.
> 
> It sounds like we all agree that HVMOP_set_mem_type with the current
> HVMMEM_mmio_write_dm value should return -EINVAL.
> 

This, is the most urgent issue at the moment AIUI.

> Regarding the p2m type which now should be impossible to set -- I
> don't think it's critical to remove from the release, since it's just
> internal.  I'd normally say just leave it for now to reduce code
> churn.
> 
> But mostly I think we just want to get this bike shed painted, so if
> anyone thinks we should really remove the p2m type from this release,
> then that's fine with me too (assuming it's OK with Wei).
> 

I would prefer as few churns as possible.

Wei.

> Does this cover everything?
> 
>  -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-27 Thread Yu, Zhang



On 4/27/2016 10:42 PM, Paul Durrant wrote:

-Original Message-
From: George Dunlap
Sent: 27 April 2016 15:13
To: Paul Durrant
Cc: Yu, Zhang; Jan Beulich; Kevin Tian; Wei Liu; Andrew Cooper; Tim
(Xen.org); xen-devel@lists.xen.org; Zhiyuan Lv; Jun Nakajima; Keir (Xen.org)
Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.

On Mon, Apr 25, 2016 at 6:01 PM, Paul Durrant 
wrote:

For clarity, do you expect any existing use of HVMMEM_mmio_write_dm

to continue to *function*? I agree that things should continue to build, but if
they don't need to function then the now redundant p2m type should be
removed IMO and any attempt to set a page to HVMMEM_mmio_write_dm
(or the new HVMMEM_unused) name should result in -EINVAL. What is your
position on this?

I sort of feel like we're playing some strange guessing game with the
color of this bike shed, where all 4 of us give a random combination
of constrants and then we have to figure out what the solution is. :-)

There are two issues: the interface (HVMMEM_*) and the
internals.(p2m_*).

Jan says that code that calls HVMOP_get_mem_type has to continue to
compile and function.  "Functioning" is easy, as you just don't return
that value and you're done.  Compiling just means having the #ifdef.

It sounds like we all agree that HVMOP_set_mem_type with the current
HVMMEM_mmio_write_dm value should return -EINVAL.

Regarding the p2m type which now should be impossible to set -- I
don't think it's critical to remove from the release, since it's just
internal.  I'd normally say just leave it for now to reduce code
churn.

But mostly I think we just want to get this bike shed painted, so if
anyone thinks we should really remove the p2m type from this release,
then that's fine with me too (assuming it's OK with Wei).

Does this cover everything?



I think so. Thanks for the clarification.

Yu, are you happy to submit a patch with the #ifdef in the header, and that 
removes any ability to set the old type?



I'm fine with this, and thanks. :)
So my understanding is that the only difference between the new
patch and this current one is we do not replace p2m_mmio_write_dm
with p2m_ioreq_server, hence no need to introduce HVMMEM_ioreq_server.
Is this understanding correct?

Besides, do you think it acceptable we just replace p2m_mmio_write_dm
with p2m_ioreq_server in next version, or should we remove this p2m
type and define p2m_ioreq_server with a different value? :)



I guess leaving the p2m type in place to avoid code churn is reasonable at this 
stage, but anyone looking at the p2m code is probably going to question why 
it's there in 4.7.

  Paul


 -George


B.R.
Yu


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-28 Thread Paul Durrant
> -Original Message-
> From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
> Sent: 28 April 2016 03:47
> To: Paul Durrant; George Dunlap
> Cc: Kevin Tian; Wei Liu; Jun Nakajima; Andrew Cooper; Tim (Xen.org); xen-
> de...@lists.xen.org; Zhiyuan Lv; Jan Beulich; Keir (Xen.org)
> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> 
> 
> 
> On 4/27/2016 10:42 PM, Paul Durrant wrote:
> >> -Original Message-
> >> From: George Dunlap
> >> Sent: 27 April 2016 15:13
> >> To: Paul Durrant
> >> Cc: Yu, Zhang; Jan Beulich; Kevin Tian; Wei Liu; Andrew Cooper; Tim
> >> (Xen.org); xen-devel@lists.xen.org; Zhiyuan Lv; Jun Nakajima; Keir
> (Xen.org)
> >> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> >> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> >>
> >> On Mon, Apr 25, 2016 at 6:01 PM, Paul Durrant 
> >> wrote:
> >>> For clarity, do you expect any existing use of
> HVMMEM_mmio_write_dm
> >> to continue to *function*? I agree that things should continue to build,
> but if
> >> they don't need to function then the now redundant p2m type should be
> >> removed IMO and any attempt to set a page to
> HVMMEM_mmio_write_dm
> >> (or the new HVMMEM_unused) name should result in -EINVAL. What is
> your
> >> position on this?
> >>
> >> I sort of feel like we're playing some strange guessing game with the
> >> color of this bike shed, where all 4 of us give a random combination
> >> of constrants and then we have to figure out what the solution is. :-)
> >>
> >> There are two issues: the interface (HVMMEM_*) and the
> >> internals.(p2m_*).
> >>
> >> Jan says that code that calls HVMOP_get_mem_type has to continue to
> >> compile and function.  "Functioning" is easy, as you just don't return
> >> that value and you're done.  Compiling just means having the #ifdef.
> >>
> >> It sounds like we all agree that HVMOP_set_mem_type with the current
> >> HVMMEM_mmio_write_dm value should return -EINVAL.
> >>
> >> Regarding the p2m type which now should be impossible to set -- I
> >> don't think it's critical to remove from the release, since it's just
> >> internal.  I'd normally say just leave it for now to reduce code
> >> churn.
> >>
> >> But mostly I think we just want to get this bike shed painted, so if
> >> anyone thinks we should really remove the p2m type from this release,
> >> then that's fine with me too (assuming it's OK with Wei).
> >>
> >> Does this cover everything?
> >>
> >
> > I think so. Thanks for the clarification.
> >
> > Yu, are you happy to submit a patch with the #ifdef in the header, and that
> removes any ability to set the old type?
> >
> 
> I'm fine with this, and thanks. :)
> So my understanding is that the only difference between the new
> patch and this current one is we do not replace p2m_mmio_write_dm
> with p2m_ioreq_server, hence no need to introduce
> HVMMEM_ioreq_server.
> Is this understanding correct?
> 

I believe so. The main points are that:

a) HVMMEM_mmio_write_dm no longer exists with the new interface version and is 
replaced by HVMMEM_unused
b) Any attempt to set a page to type HVMMEM_unused results in -EINVAL

> Besides, do you think it acceptable we just replace p2m_mmio_write_dm
> with p2m_ioreq_server in next version, or should we remove this p2m
> type and define p2m_ioreq_server with a different value? :)
> 

p2m_ioreq_server becomes defunct after the aforementioned patch. With that 
patch applied there will be no way to set that type on a p2m entry so it should 
be ok to re-use the type.

  Paul

> 
> > I guess leaving the p2m type in place to avoid code churn is reasonable at
> this stage, but anyone looking at the p2m code is probably going to question
> why it's there in 4.7.
> >
> >   Paul
> >
> >>  -George
> 
> B.R.
> Yu

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-28 Thread Yu, Zhang



On 4/28/2016 3:14 PM, Paul Durrant wrote:

-Original Message-
From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
Sent: 28 April 2016 03:47
To: Paul Durrant; George Dunlap
Cc: Kevin Tian; Wei Liu; Jun Nakajima; Andrew Cooper; Tim (Xen.org); xen-
de...@lists.xen.org; Zhiyuan Lv; Jan Beulich; Keir (Xen.org)
Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.



On 4/27/2016 10:42 PM, Paul Durrant wrote:

-Original Message-
From: George Dunlap
Sent: 27 April 2016 15:13
To: Paul Durrant
Cc: Yu, Zhang; Jan Beulich; Kevin Tian; Wei Liu; Andrew Cooper; Tim
(Xen.org); xen-devel@lists.xen.org; Zhiyuan Lv; Jun Nakajima; Keir

(Xen.org)

Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
Rename p2m_mmio_write_dm to p2m_ioreq_server.

On Mon, Apr 25, 2016 at 6:01 PM, Paul Durrant 
wrote:

For clarity, do you expect any existing use of

HVMMEM_mmio_write_dm

to continue to *function*? I agree that things should continue to build,

but if

they don't need to function then the now redundant p2m type should be
removed IMO and any attempt to set a page to

HVMMEM_mmio_write_dm

(or the new HVMMEM_unused) name should result in -EINVAL. What is

your

position on this?

I sort of feel like we're playing some strange guessing game with the
color of this bike shed, where all 4 of us give a random combination
of constrants and then we have to figure out what the solution is. :-)

There are two issues: the interface (HVMMEM_*) and the
internals.(p2m_*).

Jan says that code that calls HVMOP_get_mem_type has to continue to
compile and function.  "Functioning" is easy, as you just don't return
that value and you're done.  Compiling just means having the #ifdef.

It sounds like we all agree that HVMOP_set_mem_type with the current
HVMMEM_mmio_write_dm value should return -EINVAL.

Regarding the p2m type which now should be impossible to set -- I
don't think it's critical to remove from the release, since it's just
internal.  I'd normally say just leave it for now to reduce code
churn.

But mostly I think we just want to get this bike shed painted, so if
anyone thinks we should really remove the p2m type from this release,
then that's fine with me too (assuming it's OK with Wei).

Does this cover everything?



I think so. Thanks for the clarification.

Yu, are you happy to submit a patch with the #ifdef in the header, and that

removes any ability to set the old type?




I'm fine with this, and thanks. :)
So my understanding is that the only difference between the new
patch and this current one is we do not replace p2m_mmio_write_dm
with p2m_ioreq_server, hence no need to introduce
HVMMEM_ioreq_server.
Is this understanding correct?



I believe so. The main points are that:

a) HVMMEM_mmio_write_dm no longer exists with the new interface version and is 
replaced by HVMMEM_unused
b) Any attempt to set a page to type HVMMEM_unused results in -EINVAL



Yep.


Besides, do you think it acceptable we just replace p2m_mmio_write_dm
with p2m_ioreq_server in next version, or should we remove this p2m
type and define p2m_ioreq_server with a different value? :)



p2m_ioreq_server becomes defunct after the aforementioned patch. With that 
patch applied there will be no way to set that type on a p2m entry so it should 
be ok to re-use the type.



Got it. Thank you, Paul.
Will send the patch out later. :)


  Paul




I guess leaving the p2m type in place to avoid code churn is reasonable at

this stage, but anyone looking at the p2m code is probably going to question
why it's there in 4.7.


  Paul


 -George


B.R.
Yu




Yu

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-28 Thread Jan Beulich
>>> On 28.04.16 at 09:14,  wrote:
>> From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
>> Sent: 28 April 2016 03:47
>> Besides, do you think it acceptable we just replace p2m_mmio_write_dm
>> with p2m_ioreq_server in next version, or should we remove this p2m
>> type and define p2m_ioreq_server with a different value? :)
>> 
> 
> p2m_ioreq_server becomes defunct after the aforementioned patch. With that 
> patch applied there will be no way to set that type on a p2m entry so it 
> should be ok to re-use the type.

s/p2m_ioreq_server/p2m_mmio_write_dm/ ?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

2016-04-28 Thread Paul Durrant
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 28 April 2016 11:03
> To: Paul Durrant
> Cc: Andrew Cooper; George Dunlap; Wei Liu; JunNakajima; Kevin Tian;
> Zhiyuan Lv; Zhang Yu; xen-devel@lists.xen.org; Keir (Xen.org); Tim (Xen.org)
> Subject: RE: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> 
> >>> On 28.04.16 at 09:14,  wrote:
> >> From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
> >> Sent: 28 April 2016 03:47
> >> Besides, do you think it acceptable we just replace p2m_mmio_write_dm
> >> with p2m_ioreq_server in next version, or should we remove this p2m
> >> type and define p2m_ioreq_server with a different value? :)
> >>
> >
> > p2m_ioreq_server becomes defunct after the aforementioned patch. With
> that
> > patch applied there will be no way to set that type on a p2m entry so it
> > should be ok to re-use the type.
> 
> s/p2m_ioreq_server/p2m_mmio_write_dm/ ?
> 

Yes, that's what I meant.

  Paul

> Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel