Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Yu, Zhang]

On 1/27/2016 11:12 PM, Jan Beulich wrote:

On 27.01.16 at 15:56,  wrote:

On 1/27/2016 10:32 PM, Jan Beulich wrote:

On 27.01.16 at 15:13,  wrote:

About the truncation issue:
 I do not quite follow. Will this hurt if the value configured does
not exceed 4G? What about a type cast?


A typecast would not alter behavior in any way. And of course
a problem only arises if the value was above 4 billion. You either
need to refuse such values while the attempt is made to set it.
or you need to deal with the full range of possible values. Likely
the former is the better (and I wonder whether the upper
bound shouldn't be forced even lower than 4 billion).


Oh, I see. A check with the upper bound sounds better. Using 4G as the
upper bound is a little conservative, but I do not have any better
criteria right now. :)


But when making that decision keep security in mind: How much
memory would it take to populate 4G rangeset nodes?


Well, for XenGT, one extreme case I can imagine would be half of all
the guest ram is used as the GPU page table, and page frames containing
these page tables are discontinuous (rangeset can combine continuous
ranges). For other virtual devices to leverage the write-protected gfn
rangeset, I believe the same idea applies. :)
Is this logic OK?

Thanks
Yu

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

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Jan Beulich]

>>> On 27.01.16 at 16:23,  wrote:

> 
> On 1/27/2016 11:12 PM, Jan Beulich wrote:
> On 27.01.16 at 15:56,  wrote:
>>> On 1/27/2016 10:32 PM, Jan Beulich wrote:
>>> On 27.01.16 at 15:13,  wrote:
> About the truncation issue:
>  I do not quite follow. Will this hurt if the value configured does
> not exceed 4G? What about a type cast?

 A typecast would not alter behavior in any way. And of course
 a problem only arises if the value was above 4 billion. You either
 need to refuse such values while the attempt is made to set it.
 or you need to deal with the full range of possible values. Likely
 the former is the better (and I wonder whether the upper
 bound shouldn't be forced even lower than 4 billion).
>>>
>>> Oh, I see. A check with the upper bound sounds better. Using 4G as the
>>> upper bound is a little conservative, but I do not have any better
>>> criteria right now. :)
>>
>> But when making that decision keep security in mind: How much
>> memory would it take to populate 4G rangeset nodes?
>>
> Well, for XenGT, one extreme case I can imagine would be half of all
> the guest ram is used as the GPU page table, and page frames containing
> these page tables are discontinuous (rangeset can combine continuous
> ranges). For other virtual devices to leverage the write-protected gfn
> rangeset, I believe the same idea applies. :)
> Is this logic OK?

I can follow it, yes, but 4G ranges mean 16Tb of memory put
in page tables, which to be honest doesn't seem reasonable to
me.

Jan


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

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Yu, Zhang]

On 1/27/2016 11:58 PM, Jan Beulich wrote:

On 27.01.16 at 16:23,  wrote:




On 1/27/2016 11:12 PM, Jan Beulich wrote:

On 27.01.16 at 15:56,  wrote:

On 1/27/2016 10:32 PM, Jan Beulich wrote:

On 27.01.16 at 15:13,  wrote:

About the truncation issue:
  I do not quite follow. Will this hurt if the value configured does
not exceed 4G? What about a type cast?


A typecast would not alter behavior in any way. And of course
a problem only arises if the value was above 4 billion. You either
need to refuse such values while the attempt is made to set it.
or you need to deal with the full range of possible values. Likely
the former is the better (and I wonder whether the upper
bound shouldn't be forced even lower than 4 billion).


Oh, I see. A check with the upper bound sounds better. Using 4G as the
upper bound is a little conservative, but I do not have any better
criteria right now. :)


But when making that decision keep security in mind: How much
memory would it take to populate 4G rangeset nodes?


Well, for XenGT, one extreme case I can imagine would be half of all
the guest ram is used as the GPU page table, and page frames containing
these page tables are discontinuous (rangeset can combine continuous
ranges). For other virtual devices to leverage the write-protected gfn
rangeset, I believe the same idea applies. :)
Is this logic OK?


I can follow it, yes, but 4G ranges mean 16Tb of memory put
in page tables, which to be honest doesn't seem reasonable to
me.



Thanks for your reply, Jan.
In most cases max_memkb in configuration file will not be set to such a
big value. So I'd suggest we use a comparison between the one
calculated from max_memkb and 4G, and choose the smaller one as upper
bound. If some time in the future, it becomes common cases for VMs to
use huge rams, we should use uint64 for the rangeset limit other than a 
uint32.


Yu

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

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Jan Beulich]

>>> On 27.01.16 at 15:13,  wrote:
> About the default value:
>You are right. :) For XenGT, MAX_NR_IO_RANGES may only work under
> limited conditions. Having it default to zero means XenGT users must
> manually configure this option. Since we have plans to push other XenGT
> tool stack parameters(including a GVT-g flag), how about we set this
> max_wp_ram_ranges to a default one when GVT-g flag is detected, and
> till then, max_wp_ram_ranges is supposed to be configured explicitly for
> XenGT?

Sounds reasonable, and in line with what iirc was discussed on
the tool stack side.

> About the truncation issue:
>I do not quite follow. Will this hurt if the value configured does
> not exceed 4G? What about a type cast?

A typecast would not alter behavior in any way. And of course
a problem only arises if the value was above 4 billion. You either
need to refuse such values while the attempt is made to set it.
or you need to deal with the full range of possible values. Likely
the former is the better (and I wonder whether the upper
bound shouldn't be forced even lower than 4 billion).

Jan


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

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Yu, Zhang]

On 1/27/2016 6:27 PM, Jan Beulich wrote:

On 27.01.16 at 08:01,  wrote:




On 1/26/2016 7:00 PM, Jan Beulich wrote:

On 26.01.16 at 08:32,  wrote:

On 1/22/2016 4:01 PM, Jan Beulich wrote:

On 22.01.16 at 04:20,  wrote:

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -940,6 +940,10 @@ static int hvm_ioreq_server_alloc_rangesets(struct
hvm_ioreq_server *s,
{
unsigned int i;
int rc;
+unsigned int max_wp_ram_ranges =
+( s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] > 0 ) 
?
+s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] :
+MAX_NR_IO_RANGES;


Besides this having stray blanks inside the parentheses it truncates
the value from 64 to 32 bits and would benefit from using the gcc
extension of omitting the middle operand of ?:. But even better
would imo be if you avoided the local variable and ...


After second thought, how about we define a default value for this
parameter in libx.h, and initialize the parameter when creating the
domain with default value if it's not configured.


No, I don't think the tool stack should be determining the default
here (unless you want the default to be zero, and have zero
indeed mean zero).


Thank you, Jan.
If we do not provide a default value in tool stack, the code above
should be kept, to initialize the local variable with either the one
set in the configuration file, or with MAX_NR_IO_RANGES. Is this OK?


Well, not exactly: For one, the original comment (still present
above) regarding truncation holds. And then another question is:
Do you expect this resource type to be useful with its number of
ranges limited to MAX_NR_IO_RANGES? I ask because if the
answer is "no", having it default to zero might be as reasonable.



Thanks, Jan.

About the default value:
  You are right. :) For XenGT, MAX_NR_IO_RANGES may only work under
limited conditions. Having it default to zero means XenGT users must
manually configure this option. Since we have plans to push other XenGT
tool stack parameters(including a GVT-g flag), how about we set this
max_wp_ram_ranges to a default one when GVT-g flag is detected, and
till then, max_wp_ram_ranges is supposed to be configured explicitly for
XenGT?

About the truncation issue:
  I do not quite follow. Will this hurt if the value configured does
not exceed 4G? What about a type cast?

B.R.
Yu




Jan




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

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Jan Beulich]

>>> On 27.01.16 at 08:01,  wrote:

> 
> On 1/26/2016 7:00 PM, Jan Beulich wrote:
> On 26.01.16 at 08:32,  wrote:
>>> On 1/22/2016 4:01 PM, Jan Beulich wrote:
>>> On 22.01.16 at 04:20,  wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -940,6 +940,10 @@ static int hvm_ioreq_server_alloc_rangesets(struct
> hvm_ioreq_server *s,
>{
>unsigned int i;
>int rc;
> +unsigned int max_wp_ram_ranges =
> +( s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] 
> > 0 ) ?
> +s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] :
> +MAX_NR_IO_RANGES;

 Besides this having stray blanks inside the parentheses it truncates
 the value from 64 to 32 bits and would benefit from using the gcc
 extension of omitting the middle operand of ?:. But even better
 would imo be if you avoided the local variable and ...

>>> After second thought, how about we define a default value for this
>>> parameter in libx.h, and initialize the parameter when creating the
>>> domain with default value if it's not configured.
>>
>> No, I don't think the tool stack should be determining the default
>> here (unless you want the default to be zero, and have zero
>> indeed mean zero).
>>
> Thank you, Jan.
> If we do not provide a default value in tool stack, the code above
> should be kept, to initialize the local variable with either the one
> set in the configuration file, or with MAX_NR_IO_RANGES. Is this OK?

Well, not exactly: For one, the original comment (still present
above) regarding truncation holds. And then another question is:
Do you expect this resource type to be useful with its number of
ranges limited to MAX_NR_IO_RANGES? I ask because if the
answer is "no", having it default to zero might be as reasonable.

Jan


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

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Yu, Zhang]

On 1/27/2016 10:32 PM, Jan Beulich wrote:

On 27.01.16 at 15:13,  wrote:

About the default value:
You are right. :) For XenGT, MAX_NR_IO_RANGES may only work under
limited conditions. Having it default to zero means XenGT users must
manually configure this option. Since we have plans to push other XenGT
tool stack parameters(including a GVT-g flag), how about we set this
max_wp_ram_ranges to a default one when GVT-g flag is detected, and
till then, max_wp_ram_ranges is supposed to be configured explicitly for
XenGT?


Sounds reasonable, and in line with what iirc was discussed on
the tool stack side.



Great, and thanks.


About the truncation issue:
I do not quite follow. Will this hurt if the value configured does
not exceed 4G? What about a type cast?


A typecast would not alter behavior in any way. And of course
a problem only arises if the value was above 4 billion. You either
need to refuse such values while the attempt is made to set it.
or you need to deal with the full range of possible values. Likely
the former is the better (and I wonder whether the upper
bound shouldn't be forced even lower than 4 billion).



Oh, I see. A check with the upper bound sounds better. Using 4G as the
upper bound is a little conservative, but I do not have any better
criteria right now. :)


Jan




Thanks
Yu

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

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Jan Beulich]

>>> On 27.01.16 at 15:56,  wrote:
> On 1/27/2016 10:32 PM, Jan Beulich wrote:
> On 27.01.16 at 15:13,  wrote:
>>> About the truncation issue:
>>> I do not quite follow. Will this hurt if the value configured does
>>> not exceed 4G? What about a type cast?
>>
>> A typecast would not alter behavior in any way. And of course
>> a problem only arises if the value was above 4 billion. You either
>> need to refuse such values while the attempt is made to set it.
>> or you need to deal with the full range of possible values. Likely
>> the former is the better (and I wonder whether the upper
>> bound shouldn't be forced even lower than 4 billion).
> 
> Oh, I see. A check with the upper bound sounds better. Using 4G as the
> upper bound is a little conservative, but I do not have any better
> criteria right now. :)

But when making that decision keep security in mind: How much
memory would it take to populate 4G rangeset nodes?

Jan


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

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[David Vrabel]

On 22/01/16 03:20, Yu Zhang wrote:
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -962,6 +962,24 @@ FIFO-based event channel ABI support up to 131,071 event 
> channels.
>  Other guests are limited to 4095 (64-bit x86 and ARM) or 1023 (32-bit
>  x86).
>  
> +=item 

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Jan Beulich]

>>> On 26.01.16 at 08:32,  wrote:
> On 1/22/2016 4:01 PM, Jan Beulich wrote:
> On 22.01.16 at 04:20,  wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -940,6 +940,10 @@ static int hvm_ioreq_server_alloc_rangesets(struct
>>> hvm_ioreq_server *s,
>>>   {
>>>   unsigned int i;
>>>   int rc;
>>> +unsigned int max_wp_ram_ranges =
>>> +( s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] > 
>>> 0 ) ?
>>> +s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] :
>>> +MAX_NR_IO_RANGES;
>>
>> Besides this having stray blanks inside the parentheses it truncates
>> the value from 64 to 32 bits and would benefit from using the gcc
>> extension of omitting the middle operand of ?:. But even better
>> would imo be if you avoided the local variable and ...
>>
> After second thought, how about we define a default value for this
> parameter in libx.h, and initialize the parameter when creating the
> domain with default value if it's not configured.

No, I don't think the tool stack should be determining the default
here (unless you want the default to be zero, and have zero
indeed mean zero).

> About this local variable, we keep it, and ...
> 
>>> @@ -962,7 +966,10 @@ static int hvm_ioreq_server_alloc_rangesets(struct 
> hvm_ioreq_server *s,
>>>   if ( !s->range[i] )
>>>   goto fail;
>>>
>>> -rangeset_limit(s->range[i], MAX_NR_IO_RANGES);
>>> +if ( i == HVMOP_IO_RANGE_WP_MEM )
>>> +rangeset_limit(s->range[i], max_wp_ram_ranges);
>>> +else
>>> +rangeset_limit(s->range[i], MAX_NR_IO_RANGES);
>>
>> ... did the entire computation here, using ?: for the second argument
>> of the function invocation.
>>
> ... replace the if/else pair with sth. like:
>  rangeset_limit(s->range[i],
> ((i == HVMOP_IO_RANGE_WP_MEM)?
>  max_wp_ram_ranges:
>  MAX_NR_IO_RANGES));
> This 'max_wp_ram_ranges' has no particular usages, but the string
> "s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] "
> is too lengthy, and can easily break the 80 column limitation. :)
> Does this approach sounds OK? :)

Seems better than the original, so okay.

>>> @@ -6009,6 +6016,7 @@ static int hvm_allow_set_param(struct domain *d,
>>>   case HVM_PARAM_IOREQ_SERVER_PFN:
>>>   case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
>>>   case HVM_PARAM_ALTP2M:
>>> +case HVM_PARAM_MAX_WP_RAM_RANGES:
>>>   if ( value != 0 && a->value != value )
>>>   rc = -EEXIST;
>>>   break;
>>
>> Is there a particular reason you want this limit to be unchangeable
>> after having got set once?
>>
> Well, not exactly. :)
> I added this limit because by now we do not have any approach to
> change the max range numbers inside ioreq server during run-time.
> I can add another patch to introduce an xl command, which can change
> it dynamically. But I doubt the necessity of this new command and
> am also wonder if this new command would cause more confusion for
> the user...

And I didn't say you need to expose this to the user. All I asked
was whether you really mean the value to be a set-once one. If
yes, the code above is fine. If no, the code above should be
changed, but there's then still no need to expose a way to
"manually" adjust the value until a need for such arises.

Jan


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

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Yu, Zhang]

On 1/26/2016 7:16 PM, David Vrabel wrote:

On 22/01/16 03:20, Yu Zhang wrote:

--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -962,6 +962,24 @@ FIFO-based event channel ABI support up to 131,071 event 
channels.
  Other guests are limited to 4095 (64-bit x86 and ARM) or 1023 (32-bit
  x86).

+=item 

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Yu, Zhang]

On 1/26/2016 7:00 PM, Jan Beulich wrote:

On 26.01.16 at 08:32,  wrote:

On 1/22/2016 4:01 PM, Jan Beulich wrote:

On 22.01.16 at 04:20,  wrote:

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -940,6 +940,10 @@ static int hvm_ioreq_server_alloc_rangesets(struct
hvm_ioreq_server *s,
   {
   unsigned int i;
   int rc;
+unsigned int max_wp_ram_ranges =
+( s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] > 0 ) 
?
+s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] :
+MAX_NR_IO_RANGES;


Besides this having stray blanks inside the parentheses it truncates
the value from 64 to 32 bits and would benefit from using the gcc
extension of omitting the middle operand of ?:. But even better
would imo be if you avoided the local variable and ...


After second thought, how about we define a default value for this
parameter in libx.h, and initialize the parameter when creating the
domain with default value if it's not configured.


No, I don't think the tool stack should be determining the default
here (unless you want the default to be zero, and have zero
indeed mean zero).


Thank you, Jan.
If we do not provide a default value in tool stack, the code above
should be kept, to initialize the local variable with either the one
set in the configuration file, or with MAX_NR_IO_RANGES. Is this OK?


About this local variable, we keep it, and ...


@@ -962,7 +966,10 @@ static int hvm_ioreq_server_alloc_rangesets(struct

hvm_ioreq_server *s,

   if ( !s->range[i] )
   goto fail;

-rangeset_limit(s->range[i], MAX_NR_IO_RANGES);
+if ( i == HVMOP_IO_RANGE_WP_MEM )
+rangeset_limit(s->range[i], max_wp_ram_ranges);
+else
+rangeset_limit(s->range[i], MAX_NR_IO_RANGES);


... did the entire computation here, using ?: for the second argument
of the function invocation.


... replace the if/else pair with sth. like:
  rangeset_limit(s->range[i],
 ((i == HVMOP_IO_RANGE_WP_MEM)?
  max_wp_ram_ranges:
  MAX_NR_IO_RANGES));
This 'max_wp_ram_ranges' has no particular usages, but the string
"s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES]"
is too lengthy, and can easily break the 80 column limitation. :)
Does this approach sounds OK? :)


Seems better than the original, so okay.


@@ -6009,6 +6016,7 @@ static int hvm_allow_set_param(struct domain *d,
   case HVM_PARAM_IOREQ_SERVER_PFN:
   case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
   case HVM_PARAM_ALTP2M:
+case HVM_PARAM_MAX_WP_RAM_RANGES:
   if ( value != 0 && a->value != value )
   rc = -EEXIST;
   break;


Is there a particular reason you want this limit to be unchangeable
after having got set once?


Well, not exactly. :)
I added this limit because by now we do not have any approach to
change the max range numbers inside ioreq server during run-time.
I can add another patch to introduce an xl command, which can change
it dynamically. But I doubt the necessity of this new command and
am also wonder if this new command would cause more confusion for
the user...


And I didn't say you need to expose this to the user. All I asked
was whether you really mean the value to be a set-once one. If
yes, the code above is fine. If no, the code above should be
changed, but there's then still no need to expose a way to
"manually" adjust the value until a need for such arises.



I see. The constraint is not necessary. And I'll remove this code. :)


Jan




B.R.
Yu

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

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Yu, Zhang]

Thank you, Jan.

On 1/22/2016 4:01 PM, Jan Beulich wrote:

On 22.01.16 at 04:20,  wrote:

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -940,6 +940,10 @@ static int hvm_ioreq_server_alloc_rangesets(struct
hvm_ioreq_server *s,
  {
  unsigned int i;
  int rc;
+unsigned int max_wp_ram_ranges =
+( s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] > 0 ) 
?
+s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] :
+MAX_NR_IO_RANGES;


Besides this having stray blanks inside the parentheses it truncates
the value from 64 to 32 bits and would benefit from using the gcc
extension of omitting the middle operand of ?:. But even better
would imo be if you avoided the local variable and ...


After second thought, how about we define a default value for this
parameter in libx.h, and initialize the parameter when creating the
domain with default value if it's not configured.
About this local variable, we keep it, and ...


@@ -962,7 +966,10 @@ static int hvm_ioreq_server_alloc_rangesets(struct 
hvm_ioreq_server *s,
  if ( !s->range[i] )
  goto fail;

-rangeset_limit(s->range[i], MAX_NR_IO_RANGES);
+if ( i == HVMOP_IO_RANGE_WP_MEM )
+rangeset_limit(s->range[i], max_wp_ram_ranges);
+else
+rangeset_limit(s->range[i], MAX_NR_IO_RANGES);


... did the entire computation here, using ?: for the second argument
of the function invocation.


... replace the if/else pair with sth. like:
rangeset_limit(s->range[i],
   ((i == HVMOP_IO_RANGE_WP_MEM)?
max_wp_ram_ranges:
MAX_NR_IO_RANGES));
This 'max_wp_ram_ranges' has no particular usages, but the string
"s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] "
is too lengthy, and can easily break the 80 column limitation. :)
Does this approach sounds OK? :)


@@ -6009,6 +6016,7 @@ static int hvm_allow_set_param(struct domain *d,
  case HVM_PARAM_IOREQ_SERVER_PFN:
  case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
  case HVM_PARAM_ALTP2M:
+case HVM_PARAM_MAX_WP_RAM_RANGES:
  if ( value != 0 && a->value != value )
  rc = -EEXIST;
  break;


Is there a particular reason you want this limit to be unchangeable
after having got set once?


Well, not exactly. :)
I added this limit because by now we do not have any approach to
change the max range numbers inside ioreq server during run-time.
I can add another patch to introduce an xl command, which can change
it dynamically. But I doubt the necessity of this new command and
am also wonder if this new command would cause more confusion for
the user...

Jan



B.R.
Yu

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



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

Re: [Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Jan Beulich]

>>> On 22.01.16 at 04:20,  wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -940,6 +940,10 @@ static int hvm_ioreq_server_alloc_rangesets(struct 
> hvm_ioreq_server *s,
>  {
>  unsigned int i;
>  int rc;
> +unsigned int max_wp_ram_ranges =
> +( s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] > 0 
> ) ?
> +s->domain->arch.hvm_domain.params[HVM_PARAM_MAX_WP_RAM_RANGES] :
> +MAX_NR_IO_RANGES;

Besides this having stray blanks inside the parentheses it truncates
the value from 64 to 32 bits and would benefit from using the gcc
extension of omitting the middle operand of ?:. But even better
would imo be if you avoided the local variable and ...

> @@ -962,7 +966,10 @@ static int hvm_ioreq_server_alloc_rangesets(struct 
> hvm_ioreq_server *s,
>  if ( !s->range[i] )
>  goto fail;
>  
> -rangeset_limit(s->range[i], MAX_NR_IO_RANGES);
> +if ( i == HVMOP_IO_RANGE_WP_MEM )
> +rangeset_limit(s->range[i], max_wp_ram_ranges);
> +else
> +rangeset_limit(s->range[i], MAX_NR_IO_RANGES);

... did the entire computation here, using ?: for the second argument
of the function invocation.

> @@ -6009,6 +6016,7 @@ static int hvm_allow_set_param(struct domain *d,
>  case HVM_PARAM_IOREQ_SERVER_PFN:
>  case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
>  case HVM_PARAM_ALTP2M:
> +case HVM_PARAM_MAX_WP_RAM_RANGES:
>  if ( value != 0 && a->value != value )
>  rc = -EEXIST;
>  break;

Is there a particular reason you want this limit to be unchangeable
after having got set once?

Jan


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

[Xen-devel] [PATCH v2 3/3] tools: introduce parameter max_wp_ram_ranges.

[Yu Zhang]

A new parameter - max_wp_ram_ranges is added to set the upper limit
of write-protected ram ranges to be tracked inside one ioreq server
rangeset.

Ioreq server uses a group of rangesets to track the I/O or memory
resources to be emulated. Default limit of ranges that one rangeset
can allocate is set to a small value, due to the fact that these ranges
are allocated in xen heap. Yet for the write-protected ram ranges,
there are circumstances under which the upper limit inside one rangeset
should exceed the default one. E.g. in XenGT, when tracking the
per-process graphic translation tables on intel broadwell platforms,
the number of page tables concerned will be several thousand(normally
in this case, 8192 could be a big enough value). Users who set this
item explicitly are supposed to know the specific scenarios that
necessitate this configuration.

Signed-off-by: Yu Zhang 
---
 docs/man/xl.cfg.pod.5   | 18 ++
 tools/libxl/libxl.h |  5 +
 tools/libxl/libxl_dom.c |  3 +++
 tools/libxl/libxl_types.idl |  1 +
 tools/libxl/xl_cmdimpl.c|  4 
 xen/arch/x86/hvm/hvm.c  | 10 +-
 xen/include/public/hvm/params.h |  5 -
 7 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 8899f75..7634c42 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -962,6 +962,24 @@ FIFO-based event channel ABI support up to 131,071 event 
channels.
 Other guests are limited to 4095 (64-bit x86 and ARM) or 1023 (32-bit
 x86).
 
+=item