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

[Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx>]

On Wed, Jan 19, 2022 at 06:10:46PM -0800, Stefano Stabellini wrote:
> 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://urldefense.com/v3/__https://marc.info/?l=xen-devel&m=163997768108997__;!!GF_29dbcQIUBPA!kvNsu9pjqIwZ42N2q6aSQhTT_zA3OCEDkr7DwmAiuldEMwj2UiFReaPI8XlxsG-HOZ6v$
>  [marc[.]info]). 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.

Thank you for the answer.
I think it will be best to reserve some space and generate address for
SMCI. Hope Julien will advise how it can be done.