Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
WARNING: multiple messages refer to this Message-ID
From: Julien Grall <Julien.Grall@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	nd <nd@arm.com>, Stefano Stabellini <stefanos@xilinx.com>
Subject: Re: [PATCH v2 10/10] xen/arm: add reserved-memory regions to the dom0 memory node
Date: Fri, 10 May 2019 21:43:27 +0000
Message-ID: <0dc6d4db-7b83-0b8d-77c4-2f97f73a659e@arm.com> (raw)
In-Reply-To: <alpine.DEB.2.21.1905101326060.25766@sstabellini-ThinkPad-T480s>

Hi,

On 10/05/2019 21:51, Stefano Stabellini wrote:
> On Tue, 7 May 2019, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 4/30/19 10:02 PM, Stefano Stabellini wrote:
>>> Reserved memory regions are automatically remapped to dom0. Their device
>>> tree nodes are also added to dom0 device tree. However, the dom0 memory
>>> node is not currently extended to cover the reserved memory regions
>>> ranges as required by the spec.  This commit fixes it.
>>
>> AFAICT, this does not cover the problem mention by Amit in [1].
> 
> What do you think is required to fix Amit's problem?

I haven't fully investigated the problem to be able to answer the 
question here. Although I provided some insights in:

<b293d89c-9ed1-2033-44e5-227643ae1b0c@arm.com>

> 
> 
>> But I am still not happy with the approach taken for the reserved-memory
>> regions in this series. As I pointed out before, they are just normal memory
>> that was reserved for other purpose (CMA, framebuffer...).
>>
>> Treating them as "device" from Xen POV is a clear abuse of the meaning and I
>> don't believe it is a viable solution long term.
> 
> If we don't consider "reusable" memory regions as part of the
> discussion, the distinction becomes more philosophical than practical:
> 
> - Xen is not supposed to use them for anything
> - only given them to the VM configured for it
> 
> I don't see much of a difference with MMIO regions, except for the
> expected pagetable attributes: i.e. cacheable, not-cacheable. But even
> in that case, there could be reasonable use cases for non-cacheable
> mappings of reserved-memory regions, even if reserved-memory regions are
> "normal" memory.
> 
> Could you please help me understand why you see them so differently, as
> far as to say that "treating them as "device" from Xen POV is a clear
> abuse of the meaning"?

Obviously if you take half of the picture, then it makes things easier.
However, we are not here to discuss half of the picture but the full one 
(even if at the end you only implement half of it).

>> Indeed, some of the regions may have a property "reusable" allowing the the OS
>> to use them until they are claimed by the device driver owning the region. I
>> don't know how Linux (or any other OS) is using it today, but I don't see what
>> would prevent it to use them as hypercall buffer. This would obviously not
>> work because they are not actual RAM from Xen POV.
> 
> I haven't attempted at handling "reusable" reserved-memory regions
> because I don't have a test environment and/or a use-case for them. In
> other words, I don't have any "reusable" reserved-memory regions in any
> of the boards (Xilinx and not Xilinx) I have access to. I could add a
> warning if we find a "reusable" reserved-memory region at boot.

Don't get me wrong, I don't ask for the implementation now, so a warning 
would be fine here. However, you need at least to show me some ground 
that re-usable memory can be implemented with your solution or they are 
not a concern for Xen at all.

> 
> Nonetheless, if you have a concrete suggestion which doesn't require a
> complete rework of this series, I can try to put extra effort to handle
> this case even if it is not a benefit to my employer. I am also open to
> the possibility of dropping patches 6-10 from the series.
I don't think the series as it is would allow us to support re-usable 
memory. However as I haven't spent enough time to understand how this 
could be possibly dealt. So I am happy to be proved wrong.

> 
> There is also the option of going to devicetree.org to request a new
> binding different from reserved-memory. If reserved-memory regions are
> expected to be treated as normal memory for all intents and purposes
> except for being reserved sometimes, then they might not be the right
> bindings to describe Xilinx hardware, which requires fully dedicated
> memory regions with both cacheable and non-cacheable mappings for the
> purpose of communicating with foreign CPUs.
> 
> As a maintainer, even if the approach might considered not-ideal, my
> opinion is that this series is still an improvement over what we have
> today.

Well yes it is an improvement compare to what we have today. However, I 
don't think the problem of the reserved-memory region has been fully 
thought through so far. I am worry that your suggestion is going to put 
us into a corner make impossible to expand it (such as re-usable) in the 
future without breaking backward compatibility.

Maybe your solution is correct and we will be able to expand for 
re-usable or at least add it in a backward compatible way. But for that, 
I need solid explanation from your side that it would be possible.

>> On a similar topic, because they are normal memory, I don't think
>> XEN_DOMCTL_memory_mapping should be able to map reserved-regions. So the iomem
>> rangeset should not contain them.
> 
> What hypercall do you suggest should be used instead?

Let's talk about that once we agree on the overall approach for 
reserved-memory.

Cheers,

-- 
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

From: Julien Grall <Julien.Grall@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	nd <nd@arm.com>, Stefano Stabellini <stefanos@xilinx.com>
Subject: Re: [Xen-devel] [PATCH v2 10/10] xen/arm: add reserved-memory regions to the dom0 memory node
Date: Fri, 10 May 2019 21:43:27 +0000
Message-ID: <0dc6d4db-7b83-0b8d-77c4-2f97f73a659e@arm.com> (raw)
Message-ID: <20190510214327.gCrBRh1f7prKJhbp05vDpb_SeywidfoYc6YFUVlBzLA@z> (raw)
In-Reply-To: <alpine.DEB.2.21.1905101326060.25766@sstabellini-ThinkPad-T480s>

Hi,

On 10/05/2019 21:51, Stefano Stabellini wrote:
> On Tue, 7 May 2019, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 4/30/19 10:02 PM, Stefano Stabellini wrote:
>>> Reserved memory regions are automatically remapped to dom0. Their device
>>> tree nodes are also added to dom0 device tree. However, the dom0 memory
>>> node is not currently extended to cover the reserved memory regions
>>> ranges as required by the spec.  This commit fixes it.
>>
>> AFAICT, this does not cover the problem mention by Amit in [1].
> 
> What do you think is required to fix Amit's problem?

I haven't fully investigated the problem to be able to answer the 
question here. Although I provided some insights in:

<b293d89c-9ed1-2033-44e5-227643ae1b0c@arm.com>

> 
> 
>> But I am still not happy with the approach taken for the reserved-memory
>> regions in this series. As I pointed out before, they are just normal memory
>> that was reserved for other purpose (CMA, framebuffer...).
>>
>> Treating them as "device" from Xen POV is a clear abuse of the meaning and I
>> don't believe it is a viable solution long term.
> 
> If we don't consider "reusable" memory regions as part of the
> discussion, the distinction becomes more philosophical than practical:
> 
> - Xen is not supposed to use them for anything
> - only given them to the VM configured for it
> 
> I don't see much of a difference with MMIO regions, except for the
> expected pagetable attributes: i.e. cacheable, not-cacheable. But even
> in that case, there could be reasonable use cases for non-cacheable
> mappings of reserved-memory regions, even if reserved-memory regions are
> "normal" memory.
> 
> Could you please help me understand why you see them so differently, as
> far as to say that "treating them as "device" from Xen POV is a clear
> abuse of the meaning"?

Obviously if you take half of the picture, then it makes things easier.
However, we are not here to discuss half of the picture but the full one 
(even if at the end you only implement half of it).

>> Indeed, some of the regions may have a property "reusable" allowing the the OS
>> to use them until they are claimed by the device driver owning the region. I
>> don't know how Linux (or any other OS) is using it today, but I don't see what
>> would prevent it to use them as hypercall buffer. This would obviously not
>> work because they are not actual RAM from Xen POV.
> 
> I haven't attempted at handling "reusable" reserved-memory regions
> because I don't have a test environment and/or a use-case for them. In
> other words, I don't have any "reusable" reserved-memory regions in any
> of the boards (Xilinx and not Xilinx) I have access to. I could add a
> warning if we find a "reusable" reserved-memory region at boot.

Don't get me wrong, I don't ask for the implementation now, so a warning 
would be fine here. However, you need at least to show me some ground 
that re-usable memory can be implemented with your solution or they are 
not a concern for Xen at all.

> 
> Nonetheless, if you have a concrete suggestion which doesn't require a
> complete rework of this series, I can try to put extra effort to handle
> this case even if it is not a benefit to my employer. I am also open to
> the possibility of dropping patches 6-10 from the series.
I don't think the series as it is would allow us to support re-usable 
memory. However as I haven't spent enough time to understand how this 
could be possibly dealt. So I am happy to be proved wrong.

> 
> There is also the option of going to devicetree.org to request a new
> binding different from reserved-memory. If reserved-memory regions are
> expected to be treated as normal memory for all intents and purposes
> except for being reserved sometimes, then they might not be the right
> bindings to describe Xilinx hardware, which requires fully dedicated
> memory regions with both cacheable and non-cacheable mappings for the
> purpose of communicating with foreign CPUs.
> 
> As a maintainer, even if the approach might considered not-ideal, my
> opinion is that this series is still an improvement over what we have
> today.

Well yes it is an improvement compare to what we have today. However, I 
don't think the problem of the reserved-memory region has been fully 
thought through so far. I am worry that your suggestion is going to put 
us into a corner make impossible to expand it (such as re-usable) in the 
future without breaking backward compatibility.

Maybe your solution is correct and we will be able to expand for 
re-usable or at least add it in a backward compatible way. But for that, 
I need solid explanation from your side that it would be possible.

>> On a similar topic, because they are normal memory, I don't think
>> XEN_DOMCTL_memory_mapping should be able to map reserved-regions. So the iomem
>> rangeset should not contain them.
> 
> What hypercall do you suggest should be used instead?

Let's talk about that once we agree on the overall approach for 
reserved-memory.

Cheers,

-- 
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  parent reply index

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30 21:02 [PATCH v2 0/10] iomem memory policy Stefano Stabellini
2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini
2019-04-30 21:02 ` [PATCH v2 01/10] xen: add a p2mt parameter to map_mmio_regions Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-02 14:59   ` Jan Beulich
2019-05-02 14:59     ` [Xen-devel] " Jan Beulich
2019-05-02 18:49     ` Stefano Stabellini
2019-05-02 18:49       ` [Xen-devel] " Stefano Stabellini
2019-05-15 13:39   ` Oleksandr
2019-05-15 13:39     ` [Xen-devel] " Oleksandr
2019-04-30 21:02 ` [PATCH v2 02/10] xen: rename un/map_mmio_regions to un/map_regions Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-01  9:22   ` Julien Grall
2019-05-01  9:22     ` [Xen-devel] " Julien Grall
2019-06-17 21:24     ` Stefano Stabellini
2019-06-18 11:05       ` Julien Grall
2019-06-18 20:19         ` Stefano Stabellini
2019-05-02 15:03   ` Jan Beulich
2019-05-02 15:03     ` [Xen-devel] " Jan Beulich
2019-05-02 18:55     ` Stefano Stabellini
2019-05-02 18:55       ` [Xen-devel] " Stefano Stabellini
2019-04-30 21:02 ` [PATCH v2 03/10] xen: extend XEN_DOMCTL_memory_mapping to handle memory policy Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-02 15:12   ` Jan Beulich
2019-05-02 15:12     ` [Xen-devel] " Jan Beulich
2019-06-17 21:28     ` Stefano Stabellini
2019-06-18  8:59       ` Jan Beulich
2019-06-18 20:32         ` Stefano Stabellini
2019-06-18 23:15           ` Stefano Stabellini
2019-06-19  6:53             ` Jan Beulich
2019-05-07 16:41   ` Julien Grall
2019-05-07 16:41     ` [Xen-devel] " Julien Grall
2019-06-17 22:43     ` Stefano Stabellini
2019-06-18 11:13       ` Julien Grall
2019-05-15 14:40   ` Oleksandr
2019-05-15 14:40     ` [Xen-devel] " Oleksandr
2019-04-30 21:02 ` [PATCH v2 04/10] libxc: introduce xc_domain_mem_map_policy Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-04-30 21:02 ` [PATCH v2 05/10] libxl/xl: add memory policy option to iomem Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-01  9:42   ` Julien Grall
2019-05-01  9:42     ` [Xen-devel] " Julien Grall
2019-06-17 22:32     ` Stefano Stabellini
2019-06-18 11:09       ` Julien Grall
2019-06-18 11:15   ` Julien Grall
2019-06-18 22:07     ` Stefano Stabellini
2019-06-18 22:20       ` Julien Grall
2019-06-18 22:46         ` Stefano Stabellini
2019-04-30 21:02 ` [PATCH v2 06/10] xen/arm: extend device_tree_for_each_node Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-07 17:12   ` Julien Grall
2019-05-07 17:12     ` [Xen-devel] " Julien Grall
2019-04-30 21:02 ` [PATCH v2 07/10] xen/arm: make process_memory_node a device_tree_node_func Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-01  9:47   ` Julien Grall
2019-05-01  9:47     ` [Xen-devel] " Julien Grall
2019-04-30 21:02 ` [PATCH v2 08/10] xen/arm: keep track of reserved-memory regions Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-01 10:03   ` Julien Grall
2019-05-01 10:03     ` [Xen-devel] " Julien Grall
2019-06-21 23:47     ` Stefano Stabellini
2019-05-07 17:21   ` Julien Grall
2019-05-07 17:21     ` [Xen-devel] " Julien Grall
2019-04-30 21:02 ` [PATCH v2 09/10] xen/arm: map reserved-memory regions as normal memory in dom0 Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-07 19:52   ` Julien Grall
2019-05-07 19:52     ` [Xen-devel] " Julien Grall
2019-04-30 21:02 ` [PATCH v2 10/10] xen/arm: add reserved-memory regions to the dom0 memory node Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-07 20:15   ` Julien Grall
2019-05-07 20:15     ` [Xen-devel] " Julien Grall
2019-05-10 20:51     ` Stefano Stabellini
2019-05-10 20:51       ` [Xen-devel] " Stefano Stabellini
2019-05-10 21:43       ` Julien Grall [this message]
2019-05-10 21:43         ` Julien Grall
2019-05-11 12:40         ` Julien Grall
2019-05-11 12:40           ` [Xen-devel] " Julien Grall
2019-05-20 21:26           ` Stefano Stabellini
2019-05-20 21:26             ` [Xen-devel] " Stefano Stabellini
2019-05-20 22:38             ` Julien Grall
2019-05-20 22:38               ` [Xen-devel] " Julien Grall
2019-06-05 16:30               ` Julien Grall
2019-06-21 23:47                 ` Stefano Stabellini
2019-05-16 16:52 ` [PATCH v2 0/10] iomem memory policy Oleksandr
2019-05-16 16:52   ` [Xen-devel] " Oleksandr
2019-06-21 23:48   ` Stefano Stabellini

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0dc6d4db-7b83-0b8d-77c4-2f97f73a659e@arm.com \
    --to=julien.grall@arm.com \
    --cc=nd@arm.com \
    --cc=sstabellini@kernel.org \
    --cc=stefanos@xilinx.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@archiver.kernel.org
	public-inbox-index xen-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox