Re: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 19 Jan 2022, Oleksii Moisieiev wrote:
> On Thu, Jan 06, 2022 at 04:04:34PM +0000, Julien Grall wrote:
> > On 06/01/2022 15:43, Oleksii Moisieiev wrote:
> > > On Thu, Jan 06, 2022 at 02:02:10PM +0000, Julien Grall wrote:
> > > > On 06/01/2022 13:53, Oleksii Moisieiev wrote:
> > > > > Hi Julien,
> > > > > 
> > > > > On Mon, Jan 03, 2022 at 01:14:17PM +0000, Julien Grall wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On 24/12/2021 17:02, Oleksii Moisieiev wrote:
> > > > > > > On Fri, Dec 24, 2021 at 03:42:42PM +0100, Julien Grall wrote:
> > > > > > > > On 20/12/2021 16:41, Oleksii Moisieiev wrote:
> > > > > > > > > >       2) What are the expected memory attribute for the 
> > > > > > > > > > regions?
> > > > > > > > > 
> > > > > > > > > xen uses iommu_permit_access to pass agent page to the guest. 
> > > > > > > > > So guest can access the page directly.
> > > > > > > > 
> > > > > > > > I think you misunderstood my comment. Memory can be mapped with 
> > > > > > > > various type
> > > > > > > > (e.g. Device, Memory) and attribute (cacheable, 
> > > > > > > > non-cacheable...). What will
> > > > > > > > the firmware expect? What will the guest OS usually?
> > > > > > > > 
> > > > > > > > The reason I am asking is the attributes have to matched to 
> > > > > > > > avoid any
> > > > > > > > coherency issues. At the moment, if you use 
> > > > > > > > XEN_DOMCTL_memory_mapping, Xen
> > > > > > > > will configure the stage-2 to use Device nGnRnE. As the result, 
> > > > > > > > the result
> > > > > > > > memory access will be Device nGnRnE which is very strict.
> > > > > > > > :w
> > > > > > > 
> > > > > > > Let me share with you the configuration example:
> > > > > > > scmi expects memory to be configured in the device-tree:
> > > > > > > 
> > > > > > > cpu_scp_shm: scp-shmem@0xXXXXXXX {
> > > > > > >   compatible = "arm,scmi-shmem";
> > > > > > >   reg = <0x0 0xXXXXXX 0x0 0x1000>;
> > > > > > > };
> > > > > > > 
> > > > > > > where XXXXXX address I allocate in alloc_magic_pages function.
> > > > > > 
> > > > > > The goal of alloc_magic_pages() is to allocate RAM. However, what 
> > > > > > you want
> > > > > > is a guest physical address and then map a part of the shared page.
> > > > > 
> > > > > Do you mean that I can't use alloc_magic_pages to allocate shared
> > > > > memory region for SCMI?
> > > > Correct. alloc_magic_pages() will allocate a RAM page and then assign 
> > > > to the
> > > > guest. From your description, this is not what you want because you will
> > > > call XEN_DOMCTL_memory_mapping (and therefore replace the mapping).
> > > > 
> > > 
> > > Ok thanks, I will refactor this part in v2.
> > > 
> > > > > 
> > > > > > 
> > > > > > I can see two options here:
> > > > > >     1) Hardcode the SCMI region in the memory map
> > > > > >     2) Create a new region in the memory map that can be used for 
> > > > > > reserving
> > > > > > memory for mapping.
> > > > > 
> > > > > Could you please explain what do you mean under the "new region in the
> > > > > memory map"?
> > > > 
> > > > I mean reserving some guest physical address that could be used for map 
> > > > host
> > > > physical address (e.g. SCMI region, GIC CPU interface...).
> > > > 
> > > > So rather than hardcoding the address, we have something more flexible.
> > > > 
> > > 
> > > Ok, I will fix that in v2.
> > 
> > Just for avoidance of doubt. I was clarify option 2 and not requesting to
> > implement. That said, if you want to implement option 2 I would be happy to
> > review it.
> > 
> 
> I'm thinking about the best way to reserve address for the domain.
> We have xen_pfn_t shared_info_pfn in struct xc_dom_image which is not
> used for Arm architecture. It can be set from shared_info_arm callback,
> defined in xg_dom_arm.c.
> I can use shared_info to store address of the SCMI and then use map_sci_page 
> to
> call XEN_DOMCTL_memory_mapping.
> 
> This will allow me to reuse existing functionality and do not allocate
> extra RAM.
> 
> What do you think about that?

I cannot speak for Julien but I think he meant something else (Julien
please correct me if I am wrong.) Exposing addresses via device tree is
not a problem.

Normally we pick a fixed address for guest resources, for instance
GUEST_GICD_BASE, see xen/include/public/arch-arm.h. We could do that for
SCMI as well and it is basically approach 1).

However, it is a bit inflexible and could cause issues with things like
direct-map (https://marc.info/?l=xen-devel&m=163997768108997). A more
flexible way would be for the SCMI guest address to be dynamically
generated somehow.

I am not sure how Julien envisioned the address to be generated exactly.

Thanks to Oleksandr's work we have a way to find large regions of "free"
address space. It is currently used for grant-table mappings. Maybe we
could use a subset of it for SCMI? It might be best to wait for Julien's
answer as he might have a better idea.