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

Re: [Xen-devel] [PATCH RFC 25/35] arm: acpi add helper functions to map memory regions



Hi Stefano,

On 06/02/2015 22:12, Stefano Stabellini wrote:
On Fri, 6 Feb 2015, Julien Grall wrote:
On 06/02/2015 00:21, Stefano Stabellini wrote:
On Thu, 5 Feb 2015, Julien Grall wrote:
Hi parth,

Title: this is not acpi specific.

On 04/02/2015 14:02, parth.dixit@xxxxxxxxxx wrote:
From: Parth Dixit <parth.dixit@xxxxxxxxxx>

For passing ACPI tables to dom0, UEFI memory needs to be mapped
by xen in dom0 address space. This patch adds helper functions for
mapping.

I believe that this is not ACPI/RAM specific. Any cached MMIO regions will
have same issue.

This because Device memory is too strong and disallow unaligned access.

Signed-off-by: Parth Dixit <parth.dixit@xxxxxxxxxx>
---
    xen/arch/arm/p2m.c        | 24 ++++++++++++++++++++++++
    xen/include/asm-arm/p2m.h | 10 ++++++++++
    2 files changed, 34 insertions(+)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 8809f5a..5593a91 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -943,6 +943,30 @@ int p2m_populate_ram(struct domain *d,
                                 0, MATTR_MEM, p2m_ram_rw);
    }

+int map_ram_regions(struct domain *d,
+                     unsigned long start_gfn,
+                     unsigned long nr,
+                     unsigned long mfn)

I don't like the name of the function. It gives the impression that we map
any
RAM region to the guest via this function.

Which is obviously wrong and should never be done.

map_mem_regions?

It's still unclear, what mem mean? is an MMIO region? Is it cached, uncached?
Is it read-only/read-write.

We already have a function map_mmio_regions. Maybe it would make sense to
extend it to give more control about the mapping (cached/uncached, read-only,
read-write,....). But this function is common with x86. So I'm not sure about
what to do.

Usually mmio refers to device memory and not just from the caching
attributes point of view.

I didn't find anything which state ACPI is always living in the RAM. It could also be in the ROM.

> I would rather not use map_mmio_regions for
something that doesn't belong to a device.

The current MMIO code assume that the memory will always be non-cached (MATTR_DEVICE). This will lead to various issue in the guest when it's trying to access cached BARs.

It would be good to have a common helper handling this case because we will have to handle it for PCI sooner or later.

That said, it is difficult to come up with a good name.

Maybe we could introduce a function map_region which the following prototype

map_region(strut domain *d, unsigned long start_gfn,
           unsigned long nr, unsigned long mfn,
           int mattr, p2m_type_t type)

We could also drop add new types (either real or virtual) to differentiate the use cases. This is because we can infer the mattr (MATTR_DEVICE/MATTR_MEM).

At the same time, it would clean up a bit the current code by dropping one parameter.

Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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