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

Re: [PATCH 11/11] xen/arm: List static shared memory regions as /memory nodes


  • To: Luca Fancellu <Luca.Fancellu@xxxxxxx>
  • From: Michal Orzel <michal.orzel@xxxxxxx>
  • Date: Wed, 27 Mar 2024 09:27:01 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=arm.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; 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=8hdwUDH+LOMuz2G4XNxJl8/mIOZLll89oCdGwJEamxk=; b=nzQx/moVUBUrbIoEy/f5PuyDclOsoa4wwU9EFwgwqQCNj7YQgsiEnyImNKWxqRnoECN8viWsUcRMzrapEGYYWNlHBqQRNhobR2WGX/Gbe1BLGS4+HRrL8+rtbjbwzSltbPwzy5hT34lxM/KhqbfNbmNAHKauezLZWyLLddCfC7gAA3SKhpGbdqYHE3Fh0SDRigOabAy5E7Gl9e7gGIyfFwteRo9xQq1HRvHmTJJkGggJi3YM3+jS+Xiadx/Ovh/ggNMEZYU8ZY9EX7bzUoyc8MajvDHjWXCZP7HgBq1ke2w6SRES5AD7H6mAD2hXx33h38jkoXLc7ehIdaLJuENZsw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Owl561qJuf4oC8K58KA/OwuxUdZyW/Wp22TThoY3g9+T858f+WJWOmPbnUz7+M741nAS5D9aQsaJBSYKny1vW1FOd9eZVUEYrVhH5Tpbb5UURDwWULEzk0OZb4ARKwp685RRhi+5uCmzKcTDCNTJX8YsY+nLbsEeIqZUwZ47asRa+PR+F9Tk+S5kEZdcMql05BzbjEwccH/DFcNn2bJajlyFSF8QjUce2CX+ZHZXbvf0Rh7i0u9fioKzKabv/utpbs4GEe4Guwk+28hVcPTV3S5ZafoNKSXmmEgAv8HTM5RwF4X2tOvHZP3zAOVzCin1MiBCnsn8AMw2QaoUPHE7Gg==
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Wed, 27 Mar 2024 08:27:22 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Luca,

On 26/03/2024 15:19, Luca Fancellu wrote:
> 
> 
>> On 25 Mar 2024, at 08:58, Michal Orzel <michal.orzel@xxxxxxx> wrote:
>>
>> Hi Luca,
>>
> 
> Hi Michal,
> 
>> On 12/03/2024 14:03, Luca Fancellu wrote:
>>>
>>>
>>> Currently Xen is not exporting the static shared memory regions
>>> to the device tree as /memory node, this commit is fixing this
>>> issue.
>> Looking at the implementation, you will always call make_shm_memory_node() 
>> twice. For the first
>> time, to create /memory node and for the second time to create entry under 
>> /reserved-memory. Also,
>> you will create a separate /memory node for every single shmem region 
>> instead of combining them
>> in a single /memory region like make_memory_node() would do. Can't we reuse 
>> this function for simplicity?
> 
> You mean using make_memory_node() to populate /reserved-memory and /memory? I 
> feel they are very different
> In terms of properties to be created, so I don’t think we should create a 
> make_memory_node() that does both.
> 
> Otherwise if you were suggesting to modify make_memory_node() only for what 
> concerns the /memory nodes,
yes, this is what I meant

> it might be feasible, however there are some parts that need to be skipped 
> with some flags (all the code accessing .type
> member), if I understand correctly you like this function because it doesn’t 
> create one node for every bank, but it creates
> reg addresses instead, in that case I could modify the current 
> make_shm_memory_node() to do the same.
My concern is that we will have 2 functions to create memory nodes instead of 
one that can be reused. I know the issue is with
.type member. If skipping results in a worse code, then I'm ok with 
make_shm_memory_node() used for 2 purposes (would it be possible
to create /memory and entry under /reserved in the same call to a function?).

> 
>>
>> Also, afaict it is not forbidden to specify shmem range (correct me if I'm 
>> wrong), where guest address will be
>> within with RAM allocated by Xen (e.g. GPA RAM range 0x40000000 - 0x60000000 
>> and shmem is at 0x50000000). In this case,
>> you would create yet another /memory node that would result in overlap (i.e. 
>> more than one /memory node specifying the same range).
> 
> This is a good point I didn’t think about, yes currently the code is creating 
> overlapping nodes in that case, wow so it means I
> need to compute the non overlapping regions and emit a /memory node then! :) 
> ouch
> 
> Please let me know if I understood correctly your comments.
> 
> Cheers,
> Luca
> 
>>
>> ~Michal
> 

~Michal




 


Rackspace

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