[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 1/2] xen/arm: exclude xen,reg from direct-map domU extended regions


  • To: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: "Orzel, Michal" <michal.orzel@xxxxxxx>
  • Date: Wed, 4 Jun 2025 09:00:55 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=RPgnn9uUfAEefzWq1OSr6YrAA1PzqHiV8NmfF8vMYGI=; b=aPTgAZY4iiZ3wjdb4OGgXSDmpVw4tIpYnpM5QJogsaPpV98P2wTwxzSXF0Nn3kcqjET7ul4Mpnzydtk44/lb2a/tdBEocaRZRPkzPU3EqlAjFKZ5Z14+uLyIIio+lz1ZYtQde2KUna/0Gj6ucrkN+NaEaJ4CDZQ7ilH26vFP6NzLoaevF5A4MSvxEhlYWyZwBg5pqHSCPmL1yI7w5zvVDNqYQmKujjf8gWyJJzl2LhTQinfbpPpwrXUfOsuWHXLa7wyu+IWALC4H17jPQaJdJZqyw+4gnFBfRTK2CaUt+gLBiDhTQC1yIMK4B4Fktz35FdaTUyooyzkBYktL/UTeFg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=KbZLmrMvWXDka6uT9OhlWbyWvlTPCQAYEoRQ8J3ctt6rskasqz8x0xIZfnpvnBF0WajI2XUPy0aTwUiPLyayHdWmLlnlVC4RJ2AP9u66rjFjYBtOjOuRPY7VJRRSdyh5CgfszvhRR3+iFGdtTSxT8pUnv9+H97QS/UhELMjoTKsxRfnDZLGMJM4AgR0E5Rh8MEKn4WJzgtvd9O4Rd5nWinbKSU2fd9v33tM3QiKno2oDdZxdYkEeEkcZcrFpZz/qTjKYuCnnmA8GFKBin6oBGtiLwFk98LfWOAkoq9+MalrpBcjr+UyaC9X93m1N0Bb5uDzBz1JXKo9Dha3LQJnrUA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Wed, 04 Jun 2025 07:01:29 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


On 03/06/2025 23:15, Stewart Hildebrand wrote:
> On 5/14/25 03:31, Orzel, Michal wrote:
>> On 13/05/2025 21:54, Stewart Hildebrand wrote:
>>> Similarly to fba1b0974dd8, when a device is passed through to a
>>> direct-map dom0less domU, the xen,reg ranges may overlap with the
>>> extended regions. Remove xen,reg from direct-map domU extended regions.
>>>
>>> Introduce rangeset_count_ranges().
>>>
>>> Take the opportunity to update the comment ahead of find_memory_holes().
>>>
>>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
>>> ---
>>> v2->v3:
>>> * new patch
>>> ---
>>>  xen/arch/arm/domain_build.c | 57 +++++++++++++++++++++++++++++++++----
>>>  xen/common/rangeset.c       | 14 +++++++++
>>>  xen/include/xen/rangeset.h  |  2 ++
>>>  3 files changed, 68 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>> index b189a7cfae9f..3cdf5839bc98 100644
>>> --- a/xen/arch/arm/domain_build.c
>>> +++ b/xen/arch/arm/domain_build.c
>>> @@ -824,15 +824,17 @@ static int __init handle_pci_range(const struct 
>>> dt_device_node *dev,
>>>  }
>>>  
>>>  /*
>>> - * Find the holes in the Host DT which can be exposed to Dom0 as extended
>>> - * regions for the special memory mappings. In order to calculate regions
>>> - * we exclude every addressable memory region described by "reg" and 
>>> "ranges"
>>> - * properties from the maximum possible addressable physical memory range:
>>> + * Find the holes in the Host DT which can be exposed to Dom0 or a 
>>> direct-map
>>> + * domU as extended regions for the special memory mappings. In order to
>>> + * calculate regions we exclude every addressable memory region described 
>>> by
>>> + * "reg" and "ranges" properties from the maximum possible addressable 
>>> physical
>>> + * memory range:
>>>   * - MMIO
>>>   * - Host RAM
>>>   * - PCI aperture
>>>   * - Static shared memory regions, which are described by special property
>>>   *   "xen,shared-mem"
>>> + * - xen,reg mappings
>>>   */
>>>  static int __init find_memory_holes(const struct kernel_info *kinfo,
>>>                                      struct membanks *ext_regions)
>>> @@ -914,6 +916,13 @@ static int __init find_memory_holes(const struct 
>>> kernel_info *kinfo,
>>>          }
>>>      }
>>>  
>>> +    if ( kinfo->xen_reg_assigned )
>>> +    {
>>> +        res = rangeset_subtract(mem_holes, kinfo->xen_reg_assigned);
>>> +        if ( res )
>>> +            goto out;
>>> +    }
>>> +
>>>      start = 0;
>>>      end = (1ULL << p2m_ipa_bits) - 1;
>>>      res = rangeset_report_ranges(mem_holes, PFN_DOWN(start), PFN_DOWN(end),
>>> @@ -994,11 +1003,30 @@ static int __init find_domU_holes(const struct 
>>> kernel_info *kinfo,
>>>      return res;
>>>  }
>>>  
>>> +static int __init rangeset_to_membank(unsigned long s_gfn, unsigned long 
>>> e_gfn,
>>> +                                      void *data)
>>> +{
>>> +    struct membanks *membank = data;
>>> +    paddr_t s = pfn_to_paddr(s_gfn);
>>> +    paddr_t e = pfn_to_paddr(e_gfn + 1) - 1;
>>> +
>>> +    if ( membank->nr_banks >= membank->max_banks )
>>> +        return 0;
>>> +
>>> +    membank->bank[membank->nr_banks].start = s;
>>> +    membank->bank[membank->nr_banks].size = e - s + 1;
>>> +    membank->nr_banks++;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>  static int __init find_host_extended_regions(const struct kernel_info 
>>> *kinfo,
>>>                                               struct membanks *ext_regions)
>>>  {
>>>      int res;
>>>      struct membanks *gnttab = membanks_xzalloc(1, MEMORY);
>>> +    struct membanks *xen_reg = membanks_xzalloc(
>>> +        max(1, rangeset_count_ranges(kinfo->xen_reg_assigned)), MEMORY);
>> You allocate at least 1 membank even though xen_reg_assigned may be empty 
>> because:
>>  - this function is called for hwdom - no xen,reg
>>  - there may be no xen,reg i.e. no passthrough
> 
> Ah, sorry, there's no need to allocate at least 1. This can just be:
> 
>     struct membanks *xen_reg = membanks_xzalloc(
>         rangeset_count_ranges(kinfo->arch.xen_reg_assigned), MEMORY);
No, it cannot. membanks_xzalloc() calls xzalloc_flex_struct(). If you pass 0
as size, the latter will calculate offset to FAM[0]. In other words, the
allocation will succeed but only for members up to FAM[0] (i.e. only for struct
membanks_hdr).

Also, even if you conditionally allocate for xen_reg_assigned or set NULL, in
latter case you will end up with mem_banks containing NULL member. AFAICT that's
not something expected by the users of mem_banks (+ it gives unneeded 
iteration).

~Michal




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.