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

Re: [PATCH v3 10/52] xen/arm: Move MMU related definitions from config.h to mmu/layout.h


  • To: Julien Grall <julien@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Penny Zheng <penny.zheng@xxxxxxx>
  • Date: Thu, 6 Jul 2023 15:36:47 +0800
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 40.67.248.234) smtp.rcpttodomain=xen.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=none (message not signed); arc=none
  • 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=t0Y95sPTVDAB8hXY0t0JqFTvngRZNVAQT03PkRKRyUw=; b=GjvAZBP63g7DR4s5hyUtVZi8JEe6tYpQJKyKekyOWd62dFstYqzmvk+rhy2ynKYTbzwxrQAIrtFeJeB2cU4+jRkYqqs3QDERUa+4zJRJh2wkn5qVLq1jiqAYWo3BWhSqmGjE+UCfYcMmgHYK8Flvl+pbEB4tSq4sWgbOs89mgSaTIq5WTT4Sz+mTPWa48RZxrotf3BAcXV67Ncguc0WN4dKy+dgfinZthCYJIZV8ellQ+hJtbiB8ncfxkvvKgHQJ2cAUA92RV8mXGFbZZZsk8b6Evs+9Y3FU/YufIRtbZ3OBELMbIB7s0+6lnzzZTHAaOUxbJIK48Q1UWiOW5BHZ4A==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ajfIj7711o3si50q3yIgfLxAbb+Gve4qEolrGdT3wh2qDZKW+YrMtWmq+BNte2e2AK2Yn5Oxpq2tK4EqlsXJcMuJqBCIzFjwbcMkXTdjzktOJBFA875b7vdoCXNK0A2AvsyYyHqJbZkZglgh8iExlEodLBJuv4LQz3vuZy5khkvHQMOdJv1Cerq+4BNnZcTdn5iog+mnH0mb1ucHDnuaXf9TGYgxuhsBz50YeIvEwZ1n+LlhGM7ChjLEIug70BbwYo11hOnhtyCPOgoErCcRcHPHSev8gr4jRGXt63qA8mv30dJtCWI1il+iuY/CAhXSnFwndcewtqezSqECwNKAlA==
  • Cc: Wei Chen <wei.chen@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Thu, 06 Jul 2023 07:37:18 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true

Hi Julien

On 2023/7/5 18:30, Julien Grall wrote:
Hi Penny,

On 05/07/2023 07:51, Penny Zheng wrote:
On 2023/7/5 05:54, Julien Grall wrote:
Hi Penny,

On 26/06/2023 04:34, Penny Zheng wrote:
From: Wei Chen <wei.chen@xxxxxxx>

Xen defines some global configuration macros for Arm in config.h.
We still want to use it for MMU systems, but there are some address

Did you mean MPU?


yes, typo

layout related definitions that are defined for MMU systems only.
These definitions could not be used by MPU systems, but adding
ifdefery with CONFIG_HAS_MPU to gate these definitions will result
in a messy and hard-to-read/maintain code.

So we keep some common definitions still in config.h, but move MMU
related definitions to a new file - mmu/layout.h to avoid spreading
"#ifdef" everywhere.

Just to ease the review, can you add some details which one are considered common?


Sure,
IMO, the only part left in common is as follows:
```
#ifdef CONFIG_ARM_64

#define FRAMETABLE_SIZE        GB(32)
#define FRAMETABLE_NR          (FRAMETABLE_SIZE / sizeof(*frame_table))

#endif
```

Hmmm... Looking at the result of the patch, you moved FRAMETABLE_SIZE and FRAMETABLE_NR in layout.h. Also, I can't find any layout specific define in config.h. So I think the paragraph could be dropped.


That's because I define this same snippet in both mmu/layout.h and
mpu/layout.h, see [PATCH v3 23/52] xen/arm: create mpu/layout.h for MPU related address definitions.
So it is common for both mmu/layout.h and mpu/layout.h.
We could leave it later for discussion.

I couldn't figure a proper way to remove the limit for MPU system.

when calculating variable "pdx_group_valid", which is defined as
```
unsigned long __read_mostly pdx_group_valid[BITS_TO_LONGS(
     (FRAMETABLE_NR + PDX_GROUP_COUNT - 1)/PDX_GROUP_COUNT)] = {[0] = 1}
'''

It relies on FRAMETABLE_NR to limit array length. If we are trying to
get rid of the limit for the MPU, it may bring a lot of changes in pdx common codes, like, variable "pdx_group_valid" needs to be allocated in runtime, according actual frametable size, at least for MPU case.

The main problem is that, at least on Arm, the PDX is initialized before you can allocate any memory. You should be able to re-order the code so we populate the boot allocator first.

But I don't think this is worth it right now as, if I am not mistaken, the pdx_group_valid is only 256 bytes on Arm64.

Cheers,




 


Rackspace

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