From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 62B27C433F5 for ; Wed, 6 Oct 2021 18:15:38 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0F6906109F for ; Wed, 6 Oct 2021 18:15:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 0F6906109F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.203114.358176 (Exim 4.92) (envelope-from ) id 1mYBRw-0007fB-Sa; Wed, 06 Oct 2021 18:15:28 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 203114.358176; Wed, 06 Oct 2021 18:15:28 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mYBRw-0007f4-PF; Wed, 06 Oct 2021 18:15:28 +0000 Received: by outflank-mailman (input) for mailman id 203114; Wed, 06 Oct 2021 18:15:27 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mYBRv-0007ey-7Z for xen-devel@lists.xenproject.org; Wed, 06 Oct 2021 18:15:27 +0000 Received: from mail-wr1-x42a.google.com (unknown [2a00:1450:4864:20::42a]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 6de185d7-75f8-4bb7-a08d-daead4cdc0d3; Wed, 06 Oct 2021 18:15:25 +0000 (UTC) Received: by mail-wr1-x42a.google.com with SMTP id s15so11450067wrv.11 for ; Wed, 06 Oct 2021 11:15:25 -0700 (PDT) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 6de185d7-75f8-4bb7-a08d-daead4cdc0d3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=In+CPk9Y9hRsGCu4EI/YiOGsZmiwADtJ3T86NdTx2nk=; b=GSMTYVd5VZ8fFfYwEiw9pSospUHXDci+OKQ9KLq/Sz5zYRPb1sDIcQRP+D2vMetb/u GVwPTnXKrzaXGIAVs3CYFXInRYKwJOal9+wXeMapy+If/0ygBfyYnet5z/ozO/p7ujSG B2whru24XPqNXsgaDsM4ON5pUaGNCoTjEA4MgHahx9OSvrub1wkZayRdgA+y9BBU4cEV avZPyp/LlOXKzmpTE3yxKuw+JoLeuwVe6++cMNXFGgP77CzasAmU0QODBgG/MQ7nZIz+ lJ/QqgEhR9T3H+weRoMukY3kyEte7kY4BRGWt/3kMnguCqDpRaVzeD36cStP8UhtcZnA 0Yrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=In+CPk9Y9hRsGCu4EI/YiOGsZmiwADtJ3T86NdTx2nk=; b=4dhkh2XSZEFCKSHRdMAyQ3pYtxIqyIZ5WfSSgaDa96vO/zSWPNgRogyn9o9tyGd4v9 DTJ9RbiJXyBbvc2cHaNdDouotiV3QqWpgBET7lf4VYlHP6ff398LI/GIHf8SH+JmCaXx jfaPSFuMEHT6jUb6pM1FWTy/MfugeH+Wdy9LDP10LTnd4EhOjf8+3h/6O978hu87QXiN TxT8ZKWwDKc5oTFVNW1zfigz4ilGz3mZW4n7LgPwzZKZz8QdDs/jcZkmKkXLa2Tt7CW3 OHs4po5PT+TdLKPlrtcBiYKuHT5vMiqJEQobVSCbUw6E8KdLT3pN4CuRvyBEv7uyyKIm bw9Q== X-Gm-Message-State: AOAM532ZzgkyCd/6DGYUjSGogkFVahAGVuysOtOiIcWtNN5mDv9Q5ofX 7h4g9XwT+eE4dj+D2VBiTqrRuwLCgcFPPyMWIqU= X-Google-Smtp-Source: ABdhPJwyyJ9AiSJwionp2qYKV6mUf2jdnVHRqPXQhaf/5/9tHXh31Q256mUAFonNtEYhWs2ftJpcP/keKmRMS6Kn0nU= X-Received: by 2002:a5d:4882:: with SMTP id g2mr29905801wrq.399.1633544124993; Wed, 06 Oct 2021 11:15:24 -0700 (PDT) MIME-Version: 1.0 References: <1632955927-27911-1-git-send-email-olekstysh@gmail.com> <1632955927-27911-3-git-send-email-olekstysh@gmail.com> <8318a7b0-80fa-ccd6-75c5-c3135b82235d@xen.org> <20f00d52-76c8-2afe-6544-6f1396e121e9@gmail.com> In-Reply-To: From: Oleksandr Tyshchenko Date: Wed, 6 Oct 2021 21:15:14 +0300 Message-ID: Subject: Re: [PATCH V4 2/3] xen/arm: Add handling of extended regions for Dom0 To: Julien Grall Cc: xen-devel , Oleksandr Tyshchenko , Andrew Cooper , George Dunlap , Ian Jackson , Jan Beulich , Stefano Stabellini , Wei Liu , Volodymyr Babchuk Content-Type: multipart/alternative; boundary="0000000000004b93a005cdb321ce" --0000000000004b93a005cdb321ce Content-Type: text/plain; charset="UTF-8" On Wed, Oct 6, 2021 at 9:11 PM Julien Grall wrote: > Hi Oleksandr, > Hi Julien [Sorry for the possible format issues] > > On 04/10/2021 14:08, Oleksandr wrote: > > > > On 04.10.21 09:59, Julien Grall wrote: > >> Hi Oleksandr, > > > > Hi Julien > > Hi Oleksandr, > > > > > > >> > >> I saw Stefano committed this patch on Friday. However, I didn't have a > >> chance go to through a second time and would like to request some > >> follow-up changes. > > > > ok, do you prefer the follow-up patch to be pushed separately or within > > the rest patches of this series (#1 and #3)? If the former, I will try > > to push it today to close this question. > > I don't mind. My main ask is they are addressed for 4.16. > > > > > > >> > >> > >> On 30/09/2021 00:52, Oleksandr Tyshchenko wrote: > >>> From: Oleksandr Tyshchenko > >>> > >>> The extended region (safe range) is a region of guest physical > >>> address space which is unused and could be safely used to create > >>> grant/foreign mappings instead of wasting real RAM pages from > >>> the domain memory for establishing these mappings. > >>> > >>> The extended regions are chosen at the domain creation time and > >>> advertised to it via "reg" property under hypervisor node in > >>> the guest device-tree. As region 0 is reserved for grant table > >>> space (always present), the indexes for extended regions are 1...N. > >>> If extended regions could not be allocated for some reason, > >>> Xen doesn't fail and behaves as usual, so only inserts region 0. > >>> > >>> Please note the following limitations: > >>> - The extended region feature is only supported for 64-bit domain > >>> currently. > >>> - The ACPI case is not covered. > >>> > >>> *** > >>> > >>> As Dom0 is direct mapped domain on Arm (e.g. MFN == GFN) > >>> the algorithm to choose extended regions for it is different > >>> in comparison with the algorithm for non-direct mapped DomU. > >>> What is more, that extended regions should be chosen differently > >>> whether IOMMU is enabled or not. > >>> > >>> Provide RAM not assigned to Dom0 if IOMMU is disabled or memory > >>> holes found in host device-tree if otherwise. Make sure that > >>> extended regions are 2MB-aligned and located within maximum possible > >>> addressable physical memory range. The minimum size of extended > >>> region is 64MB. > >> > >> You explained below why the 128 limits, but I don't see any > >> explanation on why 2MB and 64MB. > >> > >> IIRC, 2MB was to potentally allow superpage mapping. I am not entirely > >> sure for 64MB. > >> > >> Could you add an in-code comment explaining the two limits? > > > > Yes. There was a discussion at [1]. 64MB was chosen as a reasonable > > value to deal with between initial 2MB (we might end up having a lot of > > small ranges which are not quite useful but increase bookkeeping) and > > suggested 1GB (we might not be able find a suitable regions at all). > > Ok. Please document in the code. Note that I don't think this choice > should be advertised to the OS. This would give us some flexibility to > change the size in the future (e.g. if we have platform where chunk of > less than 64MB is beneficial). > > >> The code below looks like an open-coding version of > >> dt_for_each_range(). Can you try to re-use it please? This will help > >> to reduce the complexity of this function. > > > > You are right, it makes sense, will definitely reuse. If I was aware of > > that function before I would safe some time I spent on the investigation > > how to parse that) > > I have only skimmed through the diff below. This looks fine to me. > Please submit a formal patch. > Already submitted, please take a look at [1]. [1] https://lore.kernel.org/xen-devel/1633519346-3686-4-git-send-email-olekstysh@gmail.com/ -- Regards, Oleksandr Tyshchenko --0000000000004b93a005cdb321ce Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Wed, Oct 6, 2021 at 9:11 PM Julien= Grall <julien@xen.o= rg> wrote:

On 04/10/2021 14:08, Oleksandr wrote:
>
> On 04.10.21 09:59, Julien Grall wrote:
>> Hi Oleksandr,
>
> Hi Julien

Hi Oleksandr,

>
>
>>
>> I saw Stefano committed this patch on Friday. However, I didn'= t have a
>> chance go to through a second time and would like to request some =
>> follow-up changes.
>
> ok, do you prefer the follow-up patch to be pushed separately or withi= n
> the rest patches of this series (#1 and #3)?=C2=A0 If the former, I wi= ll try
> to push it today to close this question.

I don't mind. My main ask is they are addressed for 4.16.

>
>
>>
>>
>> On 30/09/2021 00:52, Oleksandr Tyshchenko wrote:
>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >>>
>>> The extended region (safe range) is a region of guest physical=
>>> address space which is unused and could be safely used to crea= te
>>> grant/foreign mappings instead of wasting real RAM pages from<= br> >>> the domain memory for establishing these mappings.
>>>
>>> The extended regions are chosen at the domain creation time an= d
>>> advertised to it via "reg" property under hypervisor= node in
>>> the guest device-tree. As region 0 is reserved for grant table=
>>> space (always present), the indexes for extended regions are 1= ...N.
>>> If extended regions could not be allocated for some reason, >>> Xen doesn't fail and behaves as usual, so only inserts reg= ion 0.
>>>
>>> Please note the following limitations:
>>> - The extended region feature is only supported for 64-bit dom= ain
>>> =C2=A0=C2=A0 currently.
>>> - The ACPI case is not covered.
>>>
>>> ***
>>>
>>> As Dom0 is direct mapped domain on Arm (e.g. MFN =3D=3D GFN) >>> the algorithm to choose extended regions for it is different >>> in comparison with the algorithm for non-direct mapped DomU. >>> What is more, that extended regions should be chosen different= ly
>>> whether IOMMU is enabled or not.
>>>
>>> Provide RAM not assigned to Dom0 if IOMMU is disabled or memor= y
>>> holes found in host device-tree if otherwise. Make sure that >>> extended regions are 2MB-aligned and located within maximum po= ssible
>>> addressable physical memory range. The minimum size of extende= d
>>> region is 64MB.
>>
>> You explained below why the 128 limits, but I don't see any >> explanation on why 2MB and 64MB.
>>
>> IIRC, 2MB was to potentally allow superpage mapping. I am not enti= rely
>> sure for 64MB.
>>
>> Could you add an in-code comment explaining the two limits?
>
> Yes. There was a discussion at [1]. 64MB was chosen as a reasonable > value to deal with between initial 2MB (we might end up having a lot o= f
> small ranges which are not quite useful but increase bookkeeping) and =
> suggested 1GB (we might not be able find a suitable regions at all).
Ok. Please document in the code. Note that I don't think this choice should be advertised to the OS. This would give us some flexibility to
change the size in the future (e.g. if we have platform where chunk of
less than 64MB is beneficial).

>> The code below looks like an open-coding version of
>> dt_for_each_range(). Can you try to re-use it please? This will he= lp
>> to reduce the complexity of this function.
>
> You are right, it makes sense, will definitely reuse. If I was aware o= f
> that function before I would safe some time I spent on the investigati= on
> how to parse that)

I have only skimmed through the diff below. This looks fine to me.
Please submit a formal patch.

Already s= ubmitted, please take a look at [1].


-- <= br>
Regards,

Oleksandr T= yshchenko
--0000000000004b93a005cdb321ce--