Re: [Xen-devel] [PATCH v2 4/8] xen: make grant resource limits per domain

[Juergen Gross <jgross@xxxxxxxx>]

On 06/09/17 11:10, Paul Durrant wrote:
>> -----Original Message-----
>> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of
>> Juergen Gross
>> Sent: 06 September 2017 09:26
>> To: xen-devel@xxxxxxxxxxxxx
>> Cc: Juergen Gross <jgross@xxxxxxxx>; sstabellini@xxxxxxxxxx; Wei Liu
>> <wei.liu2@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>;
>> Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Ian Jackson
>> <Ian.Jackson@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>;
>> jbeulich@xxxxxxxx
>> Subject: [Xen-devel] [PATCH v2 4/8] xen: make grant resource limits per
>> domain
>>
>> Instead of using the same global resource limits of grant tables (max.
>> number of grant frames, max. number of maptrack frames) for all domains
>> make these limits per domain. This will allow setting individual limits
>> in the future. For now initialize the per domain limits with the global
>> values.
>>
>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>> ---
>>  xen/common/grant_table.c | 83 +++++++++++++++++++++++++++----------
>> -----------
>>  1 file changed, 46 insertions(+), 37 deletions(-)
>>
>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
>> index b50c334f70..ae5f1262e8 100644
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -69,6 +69,9 @@ struct grant_table {
>>      /* The defined versions are 1 and 2.  Set to 0 if we don't know
>>         what version to use yet. */
>>      unsigned              gt_version;
>> +    /* Resource limits of the domain. */
>> +    unsigned int          max_grant_frames;
>> +    unsigned int          max_maptrack_frames;
>>  };
>>
>>  #ifndef DEFAULT_MAX_NR_GRANT_FRAMES /* to allow arch to override */
>> @@ -285,8 +288,8 @@ num_act_frames_from_sha_frames(const unsigned
>> int num)
>>      return DIV_ROUND_UP(num * sha_per_page, ACGNT_PER_PAGE);
>>  }
>>
>> -#define max_nr_active_grant_frames \
>> -    num_act_frames_from_sha_frames(max_grant_frames)
>> +#define max_nr_active_grant_frames(gt) \
>> +    num_act_frames_from_sha_frames(gt->max_grant_frames)
>>
>>  static inline unsigned int
>>  nr_active_grant_frames(struct grant_table *gt)
>> @@ -524,7 +527,7 @@ get_maptrack_handle(
>>       * out of memory, try stealing an entry from another VCPU (in case the
>>       * guest isn't mapping across its VCPUs evenly).
>>       */
>> -    if ( nr_maptrack_frames(lgt) < max_maptrack_frames )
>> +    if ( nr_maptrack_frames(lgt) < lgt->max_maptrack_frames )
>>          new_mt = alloc_xenheap_page();
>>
>>      if ( !new_mt )
>> @@ -1663,7 +1666,7 @@ gnttab_grow_table(struct domain *d, unsigned int
>> req_nr_frames)
>>      struct grant_table *gt = d->grant_table;
>>      unsigned int i, j;
>>
>> -    ASSERT(req_nr_frames <= max_grant_frames);
>> +    ASSERT(req_nr_frames <= gt->max_grant_frames);
>>
>>      gdprintk(XENLOG_INFO,
>>              "Expanding dom (%d) grant table from (%d) to (%d) frames.\n",
>> @@ -1728,14 +1731,15 @@ grant_table_init(struct domain *d)
>>      if ( gt->nr_grant_frames )
>>          return 0;
>>
>> -    gt->nr_grant_frames = INITIAL_NR_GRANT_FRAMES;
>> +    gt->nr_grant_frames = min_t(unsigned int,
>> INITIAL_NR_GRANT_FRAMES,
>> +                                              gt->max_grant_frames);
>>
>>      /* Active grant table. */
>>      if ( (gt->active = xzalloc_array(struct active_grant_entry *,
>> -                                     max_nr_active_grant_frames)) == NULL )
>> +                                     max_nr_active_grant_frames(gt))) == 
>> NULL )
>>          goto no_mem_1;
>>      for ( i = 0;
>> -          i < num_act_frames_from_sha_frames(INITIAL_NR_GRANT_FRAMES);
>> i++ )
>> +          i < num_act_frames_from_sha_frames(gt->nr_grant_frames); i++ )
>>      {
>>          if ( (gt->active[i] = alloc_xenheap_page()) == NULL )
>>              goto no_mem_2;
>> @@ -1745,14 +1749,14 @@ grant_table_init(struct domain *d)
>>      }
>>
>>      /* Tracking of mapped foreign frames table */
>> -    gt->maptrack = vzalloc(max_maptrack_frames * sizeof(*gt->maptrack));
>> +    gt->maptrack = vzalloc(gt->max_maptrack_frames * sizeof(*gt-
>>> maptrack));
>>      if ( gt->maptrack == NULL )
>>          goto no_mem_2;
>>
>>      /* Shared grant table. */
>> -    if ( (gt->shared_raw = xzalloc_array(void *, max_grant_frames)) == NULL 
>> )
>> +    if ( (gt->shared_raw = xzalloc_array(void *, gt->max_grant_frames)) ==
>> NULL )
>>          goto no_mem_3;
>> -    for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
>> +    for ( i = 0; i < gt->nr_grant_frames; i++ )
>>      {
>>          if ( (gt->shared_raw[i] = alloc_xenheap_page()) == NULL )
>>              goto no_mem_4;
>> @@ -1761,11 +1765,11 @@ grant_table_init(struct domain *d)
>>
>>      /* Status pages for grant table - for version 2 */
>>      gt->status = xzalloc_array(grant_status_t *,
>> -                               grant_to_status_frames(max_grant_frames));
>> +                               
>> grant_to_status_frames(gt->max_grant_frames));
>>      if ( gt->status == NULL )
>>          goto no_mem_4;
>>
>> -    for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
>> +    for ( i = 0; i < gt->nr_grant_frames; i++ )
>>          gnttab_create_shared_page(d, gt, i);
>>
>>      gt->nr_status_frames = 0;
>> @@ -1773,7 +1777,7 @@ grant_table_init(struct domain *d)
>>      return 0;
>>
>>   no_mem_4:
>> -    for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
>> +    for ( i = 0; i < gt->nr_grant_frames; i++ )
>>          free_xenheap_page(gt->shared_raw[i]);
>>      xfree(gt->shared_raw);
>>      gt->shared_raw = NULL;
>> @@ -1782,7 +1786,7 @@ grant_table_init(struct domain *d)
>>      gt->maptrack = NULL;
>>   no_mem_2:
>>      for ( i = 0;
>> -          i < num_act_frames_from_sha_frames(INITIAL_NR_GRANT_FRAMES);
>> i++ )
>> +          i < num_act_frames_from_sha_frames(gt->nr_grant_frames); i++ )
>>          free_xenheap_page(gt->active[i]);
>>      xfree(gt->active);
>>      gt->active = NULL;
>> @@ -1807,15 +1811,6 @@ gnttab_setup_table(
>>      if ( unlikely(copy_from_guest(&op, uop, 1)) )
>>          return -EFAULT;
>>
>> -    if ( unlikely(op.nr_frames > max_grant_frames) )
>> -    {
>> -        gdprintk(XENLOG_INFO, "Xen only supports up to %d grant-table
>> frames"
>> -                " per domain.\n",
>> -                max_grant_frames);
>> -        op.status = GNTST_general_error;
>> -        goto out;
>> -    }
>> -
>>      if ( !guest_handle_okay(op.frame_list, op.nr_frames) )
>>          return -EFAULT;
>>
>> @@ -1835,6 +1830,15 @@ gnttab_setup_table(
>>      gt = d->grant_table;
>>      grant_write_lock(gt);
>>
>> +    if ( unlikely(op.nr_frames > gt->max_grant_frames) )
>> +    {
>> +        gdprintk(XENLOG_INFO, "Xen only supports up to %d grant-table
>> frames"
>> +                " per domain.\n",
>> +                gt->max_grant_frames);
> 
> The message should probably now say that the domain has only been provisioned 
> with X grant frames, rather than implying that the limit is still global as 
> it does.

Aah, of course.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel